On Fri, 12 May 2017 12:58:12 +0000 David Laight <david.lai...@aculab.com> wrote:
> From: Linus Torvalds > > Sent: 11 May 2017 19:48 > ... > > The one question I have is about "spin_on_cond()": since you > > explicitly document that the "no spinning" case is expected to be the > > default, I really think that the default implementation should be > > along the lines if > > > > #define spin_on_cond(cond) do { \ > > if (unlikely(!(cond))) { \ > > spin_begin(); do spin_cpu_relax(); while (!(cond)); spin_end(); \ > > } \ > > } while (0) > > > > which will actually result in better code generation even if > > spin_begin/end() are no-ops, and just generally optimizes for the > > right behavior (ie do the spinning out-of-line, since by definition it > > cannot be performance-critical after the first iteration). > > At least some versions of gcc convert while (cond) do {body} > into if (cond) do {body} while (cond) even when 'cond' > is a non-trivial expression and 'body' is trivial. > The code-bloat is silly. > No point enforcing the 'optimisation' here. The point is for something like this: static inline unsigned __read_seqcount_begin(const seqcount_t *s) { unsigned ret; repeat: ret = READ_ONCE(s->sequence); if (unlikely(ret & 1)) { cpu_relax(); goto repeat; } return ret; } to be coded as: static inline unsigned __read_seqcount_begin(const seqcount_t *s) { unsigned ret; spin_on_cond( !((ret = READ_ONCE(s->sequence)) & 1) ); return ret; } That's about as complex as you'd want to go with this, but I think it's a reasonable case. Now for x86, you would want these to fall out to the same code generated. For powerpc, you do not want those spin_begin(); spin_end(); You are right there's a bit of code bloat there. It gets moved out of line, but gcc still isn't all that smart about it though, and it doesn't fold the tests back nicely if I go with Linus's suggestion, so it doesn't work so well as generic implementation. For powerpc we have to live with it I think. Thanks, Nick