This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH 1/6] Optimize number of accesses to *adapt_count in lock elision code.
- From: Torvald Riegel <triegel at redhat dot com>
- To: Andi Kleen <andi at firstfloor dot org>
- Cc: libc-alpha at sourceware dot org
- Date: Wed, 11 Sep 2013 17:19:18 +0200
- Subject: Re: [PATCH 1/6] Optimize number of accesses to *adapt_count in lock elision code.
- Authentication-results: sourceware.org; auth=none
- References: <20130902075228 dot GA4792 at linux dot vnet dot ibm dot com> <20130902075808 dot GA4997 at linux dot vnet dot ibm dot com> <874na3djaq dot fsf at tassilo dot jf dot intel dot com> <20130903080224 dot GD3368 at linux dot vnet dot ibm dot com> <8761uhpl0m dot fsf at tassilo dot jf dot intel dot com>
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.