On Mon, Jul 18, 2016 at 05:47:39PM +0800, Pan Xinhui wrote: > From: pan xinhui <[email protected]> > > This patch aims to get rid of endianness in queued_write_unlock(). We > want to set __qrwlock->wmode to NULL, however the address is not > &lock->cnts in big endian machine. That causes queued_write_unlock() > write NULL to the wrong field of __qrwlock. > > So implement __qrwlock_write_byte() which returns the correct > __qrwlock->wmode address. > > Suggested-by: Peter Zijlstra (Intel) <[email protected]> > Signed-off-by: Pan Xinhui <[email protected]> > --- > change from v3: > rewrite totally. :) > change from v2: > change macro's name, add comments. > change from v1: > A typo fix which is really bad... > thanks Will for the carefull review. > --- > include/asm-generic/qrwlock.h | 27 +++++++++++++++++++++++++-- > 1 file changed, 25 insertions(+), 2 deletions(-) > > diff --git a/include/asm-generic/qrwlock.h b/include/asm-generic/qrwlock.h > index 54a8e65..7d026bf 100644 > --- a/include/asm-generic/qrwlock.h > +++ b/include/asm-generic/qrwlock.h > @@ -25,7 +25,20 @@ > #include <asm-generic/qrwlock_types.h> > > /* > - * Writer states & reader shift and bias > + * Writer states & reader shift and bias. > + * > + * | +0 | +1 | +2 | +3 | > + * ----+----+----+----+----+ > + * LE | 78 | 56 | 34 | 12 | 0x12345678 > + * ----+----+----+----+----+ > + * | wr | rd | > + * +----+----+----+----+ > + * > + * ----+----+----+----+----+ > + * BE | 12 | 34 | 56 | 78 | 0x12345678 > + * ----+----+----+----+----+ > + * | rd | wr | > + * +----+----+----+----+ > */ > #define _QW_WAITING 1 /* A writer is waiting */ > #define _QW_LOCKED 0xff /* A writer holds the lock */ > @@ -134,12 +147,22 @@ static inline void queued_read_unlock(struct qrwlock > *lock) > } > > /** > + * __qrwlock_write_byte - retrieve the write byte address of a queue rwlock > + * @lock : Pointer to queue rwlock structure > + * Return: the write byte address of a queue rwlock > + */ > +static inline u8 *__qrwlock_write_byte(struct qrwlock *lock) > +{ > + return (u8 *)lock + 3 * IS_BUILTIN(CONFIG_CPU_BIG_ENDIAN); > +}
Might be worth a comment here, saying that IS_BUILTIN expands to 1 or 0. Either way, the patch looks correct to me: Acked-by: Will Deacon <[email protected]> Will
