This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 1/6] Optimize number of accesses to *adapt_count in lock elision code.


On Tue, 2013-09-03 at 16:01 -0700, Andi Kleen wrote:
> Dominik Vogt <vogt@linux.vnet.ibm.com> writes:
> 
> > On Mon, Sep 02, 2013 at 02:07:25PM -0700, Andi Kleen wrote:
> >> Dominik Vogt <vogt@linux.vnet.ibm.com> writes:
> >> 
> >> > The changes to the original patches before 2.18 led to semantical changes of
> >> > the adapt_count argument.  In the current code, *adapt_count cannot fall below
> >> > zero.  When elision is attempted, *adapt_count is always exactly zero.  This
> >> > fact can be exploited to reduce the number of accesses to *adapt_count and to
> >> > simplify updating *adapt_count.
> >> 
> >> First in hardware <= is exactly the same performance as == 
> >
> > Yes.  The patch changes that to "==" because it's clearer to the
> > reader that *adapt_count should not fall below 0 (and if it does,
> > it's a bug).  
> 
> I don't see it as a bug,  as the access to these fields is racy.
> So I was very conservative.

Conversations like this are another reason to properly document code in
the first place.  Obviously, there are assumptions in there that aren't
quite obvious when reading the code -- neither to Dominik, nor to *you*
after a while.

If you had documented the intent of this, the limitations of the
mechanism, where things actually race and why this doesn't matter, we
wouldn't have needed to have to look at this again.  It would have been
certainly easier for Dominik, and it would have been safer.

So PLEASE document the nontrivial bits of your code contributions.
Anything less is a disservice to anyone else working with your code,
IMO.

> It's better to keep it.

And this indicates to me that you don't quite remember why it's better
to keep it, which is not a position we want to be in wrt. the code base.
To be clear, I'm not blaming you for not remembering the details (I
forget a lot of things too, for example) -- but if your best answer is
"be conservative, it's safer", how can anyone else have an easier time
understanding your code?  If you had documented it, you'd just have to
point to the comments, and it would be more efficient for everyone
involved.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]