Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1422812 > unrolled thread
| Started by | Pan Xinhui <xinhui.pan@linux.vnet.ibm.com> |
|---|---|
| First post | 2016-06-15 11:20 +0200 |
| Last post | 2016-06-15 12:00 +0200 |
| Articles | 4 — 4 participants |
Back to article view | Back to linux.kernel
[PATCH] locking/qrwlock: Let qrwlock has same layout regardless of the endian Pan Xinhui <xinhui.pan@linux.vnet.ibm.com> - 2016-06-15 11:20 +0200
Re: [PATCH] locking/qrwlock: Let qrwlock has same layout regardless of the endian xinhui <xinhui.pan@linux.vnet.ibm.com> - 2016-06-15 11:30 +0200
Re: [PATCH] locking/qrwlock: Let qrwlock has same layout regardless of the endian Will Deacon <will.deacon@arm.com> - 2016-06-15 11:30 +0200
Re: [PATCH] locking/qrwlock: Let qrwlock has same layout regardless of the endian kbuild test robot <lkp@intel.com> - 2016-06-15 12:00 +0200
| From | Pan Xinhui <xinhui.pan@linux.vnet.ibm.com> |
|---|---|
| Date | 2016-06-15 11:20 +0200 |
| Subject | [PATCH] locking/qrwlock: Let qrwlock has same layout regardless of the endian |
| Message-ID | <rKf9o-F4-19@gated-at.bofh.it> |
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.
Actually qrwlock can have same layout, IOW we can remove the #if
__little_endian in struct __qrwlock. With such modification, we only
need define some _QW* and _QR* with corresponding values in different
endian systems.
Suggested-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Pan Xinhui <xinhui.pan@linux.vnet.ibm.com>
---
include/asm-generic/qrwlock.h | 15 +++++++++++----
kernel/locking/qrwlock.c | 10 ++++------
2 files changed, 15 insertions(+), 10 deletions(-)
diff --git a/include/asm-generic/qrwlock.h b/include/asm-generic/qrwlock.h
index 54a8e65..b135c11 100644
--- a/include/asm-generic/qrwlock.h
+++ b/include/asm-generic/qrwlock.h
@@ -27,11 +27,18 @@
/*
* Writer states & reader shift and bias
*/
-#define _QW_WAITING 1 /* A writer is waiting */
-#define _QW_LOCKED 0xff /* A writer holds the lock */
-#define _QW_WMASK 0xff /* Writer mask */
+#ifdef __LITTLE_ENDIAN
#define _QR_SHIFT 8 /* Reader count shift */
-#define _QR_BIAS (1U << _QR_SHIFT)
+#define _QW_SHIFF 0 /* Writer mode shift */
+#else
+#define _QR_SHIFT 0 /* Reader count shift */
+#define _QW_SHIFT 24 /* Writer mode shift */
+#endif
+
+#define _QW_WAITING (1U << _QW_SHIFT) /* A writer is waiting */
+#define _QW_LOCKED (0xffU << _QW_SHIFT) /* A writer holds the lock */
+#define _QW_WMASK (0xffU << _QW_SHIFT) /* Writer mask */
+#define _QR_BIAS (1U << _QR_SHIFT)
/*
* External function declarations
diff --git a/kernel/locking/qrwlock.c b/kernel/locking/qrwlock.c
index fec0823..57d66cf 100644
--- a/kernel/locking/qrwlock.c
+++ b/kernel/locking/qrwlock.c
@@ -30,18 +30,15 @@ struct __qrwlock {
union {
atomic_t cnts;
struct {
-#ifdef __LITTLE_ENDIAN
u8 wmode; /* Writer mode */
u8 rcnts[3]; /* Reader counts */
-#else
- u8 rcnts[3]; /* Reader counts */
- u8 wmode; /* Writer mode */
-#endif
};
};
arch_spinlock_t lock;
};
+#define _QW_MODEVAL(v) ((v) >> _QW_SHIFT)
+
/**
* rspin_until_writer_unlock - inc reader count & spin until writer is gone
* @lock : Pointer to queue rwlock structure
@@ -127,7 +124,8 @@ void queued_write_lock_slowpath(struct qrwlock *lock)
struct __qrwlock *l = (struct __qrwlock *)lock;
if (!READ_ONCE(l->wmode) &&
- (cmpxchg_relaxed(&l->wmode, 0, _QW_WAITING) == 0))
+ (cmpxchg_relaxed(&l->wmode, 0,
+ _QW_MODEVAL(_QW_WAITING)) == 0))
break;
cpu_relax_lowlatency();
--
1.9.1
[toc] | [next] | [standalone]
| From | xinhui <xinhui.pan@linux.vnet.ibm.com> |
|---|---|
| Date | 2016-06-15 11:30 +0200 |
| Subject | Re: [PATCH] locking/qrwlock: Let qrwlock has same layout regardless of the endian |
| Message-ID | <rKfj3-Ig-5@gated-at.bofh.it> |
| In reply to | #1422812 |
On 2016年06月15日 17:20, Will Deacon wrote: > On Wed, Jun 15, 2016 at 05:16:17PM +0800, Pan Xinhui wrote: >> 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. >> >> Actually qrwlock can have same layout, IOW we can remove the #if >> __little_endian in struct __qrwlock. With such modification, we only >> need define some _QW* and _QR* with corresponding values in different >> endian systems. >> >> Suggested-by: Will Deacon <will.deacon@arm.com> >> Signed-off-by: Pan Xinhui <xinhui.pan@linux.vnet.ibm.com> >> --- >> include/asm-generic/qrwlock.h | 15 +++++++++++---- >> kernel/locking/qrwlock.c | 10 ++++------ >> 2 files changed, 15 insertions(+), 10 deletions(-) >> >> diff --git a/include/asm-generic/qrwlock.h b/include/asm-generic/qrwlock.h >> index 54a8e65..b135c11 100644 >> --- a/include/asm-generic/qrwlock.h >> +++ b/include/asm-generic/qrwlock.h >> @@ -27,11 +27,18 @@ >> /* >> * Writer states & reader shift and bias >> */ >> -#define _QW_WAITING 1 /* A writer is waiting */ >> -#define _QW_LOCKED 0xff /* A writer holds the lock */ >> -#define _QW_WMASK 0xff /* Writer mask */ >> +#ifdef __LITTLE_ENDIAN >> #define _QR_SHIFT 8 /* Reader count shift */ >> -#define _QR_BIAS (1U << _QR_SHIFT) >> +#define _QW_SHIFF 0 /* Writer mode shift */ > > Well, there are other typos that could've been worse, but you probably > want to fix this... > oh... I am really in rush. sorry. will fix it and seed patch v2 soon. thank you for pointing it out. > Will >
[toc] | [prev] | [next] | [standalone]
| From | Will Deacon <will.deacon@arm.com> |
|---|---|
| Date | 2016-06-15 11:30 +0200 |
| Subject | Re: [PATCH] locking/qrwlock: Let qrwlock has same layout regardless of the endian |
| Message-ID | <rKfj3-Ig-7@gated-at.bofh.it> |
| In reply to | #1422812 |
On Wed, Jun 15, 2016 at 05:16:17PM +0800, Pan Xinhui wrote: > 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. > > Actually qrwlock can have same layout, IOW we can remove the #if > __little_endian in struct __qrwlock. With such modification, we only > need define some _QW* and _QR* with corresponding values in different > endian systems. > > Suggested-by: Will Deacon <will.deacon@arm.com> > Signed-off-by: Pan Xinhui <xinhui.pan@linux.vnet.ibm.com> > --- > include/asm-generic/qrwlock.h | 15 +++++++++++---- > kernel/locking/qrwlock.c | 10 ++++------ > 2 files changed, 15 insertions(+), 10 deletions(-) > > diff --git a/include/asm-generic/qrwlock.h b/include/asm-generic/qrwlock.h > index 54a8e65..b135c11 100644 > --- a/include/asm-generic/qrwlock.h > +++ b/include/asm-generic/qrwlock.h > @@ -27,11 +27,18 @@ > /* > * Writer states & reader shift and bias > */ > -#define _QW_WAITING 1 /* A writer is waiting */ > -#define _QW_LOCKED 0xff /* A writer holds the lock */ > -#define _QW_WMASK 0xff /* Writer mask */ > +#ifdef __LITTLE_ENDIAN > #define _QR_SHIFT 8 /* Reader count shift */ > -#define _QR_BIAS (1U << _QR_SHIFT) > +#define _QW_SHIFF 0 /* Writer mode shift */ Well, there are other typos that could've been worse, but you probably want to fix this... Will
[toc] | [prev] | [next] | [standalone]
| From | kbuild test robot <lkp@intel.com> |
|---|---|
| Date | 2016-06-15 12:00 +0200 |
| Subject | Re: [PATCH] locking/qrwlock: Let qrwlock has same layout regardless of the endian |
| Message-ID | <rKfM5-Te-5@gated-at.bofh.it> |
| In reply to | #1422812 |
[Multipart message — attachments visible in raw view] — view raw
Hi,
[auto build test WARNING on asm-generic/master]
[also build test WARNING on v4.7-rc3 next-20160615]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Pan-Xinhui/locking-qrwlock-Let-qrwlock-has-same-layout-regardless-of-the-endian/20160615-172044
base: https://git.kernel.org/pub/scm/linux/kernel/git/arnd/asm-generic master
config: x86_64-randconfig-x016-201624 (attached as .config)
compiler: gcc-6 (Debian 6.1.1-1) 6.1.1 20160430
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All warnings (new ones prefixed by >>):
In file included from arch/x86/include/asm/qrwlock.h:5:0,
from arch/x86/include/asm/spinlock.h:219,
from include/linux/spinlock.h:87,
from include/linux/mmzone.h:7,
from include/linux/gfp.h:5,
from include/linux/slab.h:14,
from include/linux/crypto.h:24,
from arch/x86/kernel/asm-offsets.c:8:
include/asm-generic/qrwlock.h: In function 'queued_read_can_lock':
include/asm-generic/qrwlock.h:40:29: error: '_QW_SHIFT' undeclared (first use in this function)
#define _QW_WMASK (0xffU << _QW_SHIFT) /* Writer mask */
^
include/asm-generic/qrwlock.h:55:38: note: in expansion of macro '_QW_WMASK'
return !(atomic_read(&lock->cnts) & _QW_WMASK);
^~~~~~~~~
include/asm-generic/qrwlock.h:40:29: note: each undeclared identifier is reported only once for each function it appears in
#define _QW_WMASK (0xffU << _QW_SHIFT) /* Writer mask */
^
include/asm-generic/qrwlock.h:55:38: note: in expansion of macro '_QW_WMASK'
return !(atomic_read(&lock->cnts) & _QW_WMASK);
^~~~~~~~~
In file included from arch/x86/include/asm/atomic.h:4:0,
from include/linux/atomic.h:4,
from include/linux/crypto.h:20,
from arch/x86/kernel/asm-offsets.c:8:
include/asm-generic/qrwlock.h: In function 'queued_read_trylock':
include/asm-generic/qrwlock.h:40:29: error: '_QW_SHIFT' undeclared (first use in this function)
#define _QW_WMASK (0xffU << _QW_SHIFT) /* Writer mask */
^
include/linux/compiler.h:151:30: note: in definition of macro '__trace_if'
if (__builtin_constant_p(!!(cond)) ? !!(cond) : \
^~~~
>> include/asm-generic/qrwlock.h:77:2: note: in expansion of macro 'if'
if (likely(!(cnts & _QW_WMASK))) {
^~
>> include/asm-generic/qrwlock.h:77:6: note: in expansion of macro 'likely'
if (likely(!(cnts & _QW_WMASK))) {
^~~~~~
include/asm-generic/qrwlock.h:77:22: note: in expansion of macro '_QW_WMASK'
if (likely(!(cnts & _QW_WMASK))) {
^~~~~~~~~
include/asm-generic/qrwlock.h: In function 'queued_write_trylock':
include/asm-generic/qrwlock.h:39:30: error: '_QW_SHIFT' undeclared (first use in this function)
#define _QW_LOCKED (0xffU << _QW_SHIFT) /* A writer holds the lock */
^
include/linux/compiler.h:138:43: note: in definition of macro 'likely'
# define likely(x) (__builtin_constant_p(x) ? !!(x) : __branch_check__(x, 1))
^
include/asm-generic/qrwlock.h:100:24: note: in expansion of macro '_QW_LOCKED'
cnts, cnts | _QW_LOCKED) == cnts);
^~~~~~~~~~
include/asm-generic/qrwlock.h: In function 'queued_read_lock':
include/asm-generic/qrwlock.h:40:29: error: '_QW_SHIFT' undeclared (first use in this function)
#define _QW_WMASK (0xffU << _QW_SHIFT) /* Writer mask */
^
include/linux/compiler.h:151:30: note: in definition of macro '__trace_if'
if (__builtin_constant_p(!!(cond)) ? !!(cond) : \
^~~~
include/asm-generic/qrwlock.h:111:2: note: in expansion of macro 'if'
if (likely(!(cnts & _QW_WMASK)))
^~
include/asm-generic/qrwlock.h:111:6: note: in expansion of macro 'likely'
if (likely(!(cnts & _QW_WMASK)))
^~~~~~
include/asm-generic/qrwlock.h:111:22: note: in expansion of macro '_QW_WMASK'
if (likely(!(cnts & _QW_WMASK)))
^~~~~~~~~
include/asm-generic/qrwlock.h: In function 'queued_write_lock':
include/asm-generic/qrwlock.h:39:30: error: '_QW_SHIFT' undeclared (first use in this function)
#define _QW_LOCKED (0xffU << _QW_SHIFT) /* A writer holds the lock */
^
include/linux/compiler.h:151:30: note: in definition of macro '__trace_if'
if (__builtin_constant_p(!!(cond)) ? !!(cond) : \
^~~~
include/asm-generic/qrwlock.h:125:2: note: in expansion of macro 'if'
if (atomic_cmpxchg_acquire(&lock->cnts, 0, _QW_LOCKED) == 0)
^~
include/asm-generic/qrwlock.h:125:45: note: in expansion of macro '_QW_LOCKED'
if (atomic_cmpxchg_acquire(&lock->cnts, 0, _QW_LOCKED) == 0)
^~~~~~~~~~
make[2]: *** [arch/x86/kernel/asm-offsets.s] Error 1
make[2]: Target '__build' not remade because of errors.
make[1]: *** [prepare0] Error 2
make[1]: Target 'prepare' not remade because of errors.
make: *** [sub-make] Error 2
vim +/if +77 include/asm-generic/qrwlock.h
c85626c3 Pan Xinhui 2016-06-15 34 #define _QR_SHIFT 0 /* Reader count shift */
c85626c3 Pan Xinhui 2016-06-15 35 #define _QW_SHIFT 24 /* Writer mode shift */
c85626c3 Pan Xinhui 2016-06-15 36 #endif
c85626c3 Pan Xinhui 2016-06-15 37
c85626c3 Pan Xinhui 2016-06-15 38 #define _QW_WAITING (1U << _QW_SHIFT) /* A writer is waiting */
c85626c3 Pan Xinhui 2016-06-15 39 #define _QW_LOCKED (0xffU << _QW_SHIFT) /* A writer holds the lock */
c85626c3 Pan Xinhui 2016-06-15 @40 #define _QW_WMASK (0xffU << _QW_SHIFT) /* Writer mask */
70af2f8a Waiman Long 2014-02-03 41 #define _QR_BIAS (1U << _QR_SHIFT)
70af2f8a Waiman Long 2014-02-03 42
70af2f8a Waiman Long 2014-02-03 43 /*
70af2f8a Waiman Long 2014-02-03 44 * External function declarations
70af2f8a Waiman Long 2014-02-03 45 */
0e06e5be Waiman Long 2015-06-19 46 extern void queued_read_lock_slowpath(struct qrwlock *lock, u32 cnts);
f7d71f20 Waiman Long 2015-06-19 47 extern void queued_write_lock_slowpath(struct qrwlock *lock);
70af2f8a Waiman Long 2014-02-03 48
70af2f8a Waiman Long 2014-02-03 49 /**
f7d71f20 Waiman Long 2015-06-19 50 * queued_read_can_lock- would read_trylock() succeed?
70af2f8a Waiman Long 2014-02-03 51 * @lock: Pointer to queue rwlock structure
70af2f8a Waiman Long 2014-02-03 52 */
f7d71f20 Waiman Long 2015-06-19 53 static inline int queued_read_can_lock(struct qrwlock *lock)
70af2f8a Waiman Long 2014-02-03 54 {
70af2f8a Waiman Long 2014-02-03 55 return !(atomic_read(&lock->cnts) & _QW_WMASK);
70af2f8a Waiman Long 2014-02-03 56 }
70af2f8a Waiman Long 2014-02-03 57
70af2f8a Waiman Long 2014-02-03 58 /**
f7d71f20 Waiman Long 2015-06-19 59 * queued_write_can_lock- would write_trylock() succeed?
70af2f8a Waiman Long 2014-02-03 60 * @lock: Pointer to queue rwlock structure
70af2f8a Waiman Long 2014-02-03 61 */
f7d71f20 Waiman Long 2015-06-19 62 static inline int queued_write_can_lock(struct qrwlock *lock)
70af2f8a Waiman Long 2014-02-03 63 {
70af2f8a Waiman Long 2014-02-03 64 return !atomic_read(&lock->cnts);
70af2f8a Waiman Long 2014-02-03 65 }
70af2f8a Waiman Long 2014-02-03 66
70af2f8a Waiman Long 2014-02-03 67 /**
f7d71f20 Waiman Long 2015-06-19 68 * queued_read_trylock - try to acquire read lock of a queue rwlock
70af2f8a Waiman Long 2014-02-03 69 * @lock : Pointer to queue rwlock structure
70af2f8a Waiman Long 2014-02-03 70 * Return: 1 if lock acquired, 0 if failed
70af2f8a Waiman Long 2014-02-03 71 */
f7d71f20 Waiman Long 2015-06-19 72 static inline int queued_read_trylock(struct qrwlock *lock)
70af2f8a Waiman Long 2014-02-03 73 {
70af2f8a Waiman Long 2014-02-03 74 u32 cnts;
70af2f8a Waiman Long 2014-02-03 75
70af2f8a Waiman Long 2014-02-03 76 cnts = atomic_read(&lock->cnts);
70af2f8a Waiman Long 2014-02-03 @77 if (likely(!(cnts & _QW_WMASK))) {
77e430e3 Will Deacon 2015-08-06 78 cnts = (u32)atomic_add_return_acquire(_QR_BIAS, &lock->cnts);
70af2f8a Waiman Long 2014-02-03 79 if (likely(!(cnts & _QW_WMASK)))
70af2f8a Waiman Long 2014-02-03 80 return 1;
:::::: The code at line 77 was first introduced by commit
:::::: 70af2f8a4f48d6cebdf92d533d3aef37853ce6de locking/rwlocks: Introduce 'qrwlocks' - fair, queued rwlocks
:::::: TO: Waiman Long <Waiman.Long@hp.com>
:::::: CC: Ingo Molnar <mingo@kernel.org>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web