Re: [rfc] lock bitops
On Wed, May 09, 2007 at 04:08:41PM +0400, Nikita Danilov wrote: > Nick Piggin writes: > > Hi, > > [...] > > > > > /** > > + * clear_bit_unlock - Clears a bit in memory with release > > + * @nr: Bit to clear > > + * @addr: Address to start counting from > > + * > > + * clear_bit() is atomic and may not be reordered. It does > > s/clear_bit/clear_bit_unlock/ ? Yes :) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [rfc] lock bitops
Nick Piggin writes:
> Hi,
[...]
>
> /**
> + * clear_bit_unlock - Clears a bit in memory with release
> + * @nr: Bit to clear
> + * @addr: Address to start counting from
> + *
> + * clear_bit() is atomic and may not be reordered. It does
s/clear_bit/clear_bit_unlock/ ?
> + * contain a memory barrier suitable for unlock type operations.
> + */
> +static __inline__ void
> +clear_bit_unlock (int nr, volatile void *addr)
> +{
Nikita.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [rfc] lock bitops
On Tue, May 08, 2007 at 01:22:56PM +0100, David Howells wrote:
> Nick Piggin <[EMAIL PROTECTED]> wrote:
>
> > +There are two special bitops with lock barrier semantics (acquire/release,
> > +same as spinlocks).
>
> You should update Documentation/memory-barriers.txt also.
>
> > #define TestSetPageLocked(page)\
> > test_and_set_bit(PG_locked, &(page)->flags)
> > +#define TestSetPageLocked_Lock(page) \
> > + test_and_set_bit_lock(PG_locked, &(page)->flags)
>
> Can we get away with just moving TestSetPageLocked() to the new function
> rather than adding another accessor? Or how about LockPageLocked() and
> UnlockPageLocked() rather than SetPageLocked_Lock() that last looks wrong
Updated memory-barriers.txt, made the generic routines #defines because
smp_mb isn't always included early enough, and culled some of the page-
flags.h macros in favour of coding the lock operations directly.
--
Introduce test_and_set_bit_lock / clear_bit_unlock bitops with lock semantics.
Add non-trivial implementations for powerpc and ia64. Convert page lock, buffer
lock, bit_spin_lock, tasklet locks to use the new locks.
---
Documentation/atomic_ops.txt |9 +++
Documentation/memory-barriers.txt | 14 ++--
drivers/scsi/sg.c |2 -
fs/buffer.c |7 ++
fs/cifs/file.c|2 -
fs/jbd/commit.c |4 +--
fs/jbd2/commit.c |4 +--
fs/ntfs/aops.c|2 -
fs/ntfs/compress.c|2 -
fs/ntfs/mft.c |4 +--
fs/reiserfs/inode.c |2 -
fs/reiserfs/journal.c |4 +--
fs/xfs/linux-2.6/xfs_aops.c |4 +--
include/asm-alpha/bitops.h|1
include/asm-arm/bitops.h |1
include/asm-arm26/bitops.h|1
include/asm-avr32/bitops.h|1
include/asm-cris/bitops.h |1
include/asm-frv/bitops.h |1
include/asm-generic/bitops.h |1
include/asm-generic/bitops/lock.h | 13 +++
include/asm-h8300/bitops.h|1
include/asm-i386/bitops.h |1
include/asm-ia64/bitops.h | 33
include/asm-m32r/bitops.h |1
include/asm-m68k/bitops.h |1
include/asm-m68knommu/bitops.h|1
include/asm-mips/bitops.h |1
include/asm-parisc/bitops.h |1
include/asm-powerpc/bitops.h | 44 ++
include/asm-s390/bitops.h |1
include/asm-sh/bitops.h |1
include/asm-sh64/bitops.h |1
include/asm-sparc/bitops.h|1
include/asm-sparc64/bitops.h |1
include/asm-v850/bitops.h |1
include/asm-x86_64/bitops.h |1
include/asm-xtensa/bitops.h |1
include/linux/bit_spinlock.h | 11 +
include/linux/buffer_head.h |8 +-
include/linux/interrupt.h |5 +---
include/linux/page-flags.h|6 -
include/linux/pagemap.h |9 ++-
kernel/wait.c |2 -
mm/filemap.c | 17 ++
mm/memory.c |2 -
mm/migrate.c |4 +--
mm/rmap.c |2 -
mm/shmem.c|4 +--
mm/swap.c |2 -
mm/swap_state.c |2 -
mm/swapfile.c |2 -
mm/truncate.c |4 +--
mm/vmscan.c |4 +--
54 files changed, 193 insertions(+), 63 deletions(-)
Index: linux-2.6/include/asm-powerpc/bitops.h
===
--- linux-2.6.orig/include/asm-powerpc/bitops.h 2007-05-09 13:56:56.0
+1000
+++ linux-2.6/include/asm-powerpc/bitops.h 2007-05-09 14:00:42.0
+1000
@@ -87,6 +87,24 @@
: "cc" );
}
+static __inline__ void clear_bit_unlock(int nr, volatile unsigned long *addr)
+{
+ unsigned long old;
+ unsigned long mask = BITOP_MASK(nr);
+ unsigned long *p = ((unsigned long *)addr) + BITOP_WORD(nr);
+
+ __asm__ __volatile__(
+ LWSYNC_ON_SMP
+"1:" PPC_LLARX "%0,0,%3 # clear_bit_unlock\n"
+ "andc %0,%0,%2\n"
+ PPC405_ERR77(0,%3)
+ PPC_STLCX "%0,0,%3\n"
+ "bne- 1b"
+ : "=" (old), "+m" (*p)
+ : "r" (mask), "r" (p)
+ : "cc" );
+}
+
static __inline__ void change_bit(int nr, volatile unsigned long *addr)
{
unsigned long old;
@@ -126,6 +144,27 @@
return (old & mask) != 0;
}
+static __inline__ int test_and_set_bit_lock(unsigned long nr,
+ volatile unsigned long *addr)
+{
+ unsigned long old, t;
+ unsigned long mask = BITOP_MASK(nr);
+ unsigned long *p = ((unsigned long
Re: [rfc] lock bitops
On Tue, May 08, 2007 at 01:22:56PM +0100, David Howells wrote:
Nick Piggin [EMAIL PROTECTED] wrote:
+There are two special bitops with lock barrier semantics (acquire/release,
+same as spinlocks).
You should update Documentation/memory-barriers.txt also.
#define TestSetPageLocked(page)\
test_and_set_bit(PG_locked, (page)-flags)
+#define TestSetPageLocked_Lock(page) \
+ test_and_set_bit_lock(PG_locked, (page)-flags)
Can we get away with just moving TestSetPageLocked() to the new function
rather than adding another accessor? Or how about LockPageLocked() and
UnlockPageLocked() rather than SetPageLocked_Lock() that last looks wrong
Updated memory-barriers.txt, made the generic routines #defines because
smp_mb isn't always included early enough, and culled some of the page-
flags.h macros in favour of coding the lock operations directly.
--
Introduce test_and_set_bit_lock / clear_bit_unlock bitops with lock semantics.
Add non-trivial implementations for powerpc and ia64. Convert page lock, buffer
lock, bit_spin_lock, tasklet locks to use the new locks.
---
Documentation/atomic_ops.txt |9 +++
Documentation/memory-barriers.txt | 14 ++--
drivers/scsi/sg.c |2 -
fs/buffer.c |7 ++
fs/cifs/file.c|2 -
fs/jbd/commit.c |4 +--
fs/jbd2/commit.c |4 +--
fs/ntfs/aops.c|2 -
fs/ntfs/compress.c|2 -
fs/ntfs/mft.c |4 +--
fs/reiserfs/inode.c |2 -
fs/reiserfs/journal.c |4 +--
fs/xfs/linux-2.6/xfs_aops.c |4 +--
include/asm-alpha/bitops.h|1
include/asm-arm/bitops.h |1
include/asm-arm26/bitops.h|1
include/asm-avr32/bitops.h|1
include/asm-cris/bitops.h |1
include/asm-frv/bitops.h |1
include/asm-generic/bitops.h |1
include/asm-generic/bitops/lock.h | 13 +++
include/asm-h8300/bitops.h|1
include/asm-i386/bitops.h |1
include/asm-ia64/bitops.h | 33
include/asm-m32r/bitops.h |1
include/asm-m68k/bitops.h |1
include/asm-m68knommu/bitops.h|1
include/asm-mips/bitops.h |1
include/asm-parisc/bitops.h |1
include/asm-powerpc/bitops.h | 44 ++
include/asm-s390/bitops.h |1
include/asm-sh/bitops.h |1
include/asm-sh64/bitops.h |1
include/asm-sparc/bitops.h|1
include/asm-sparc64/bitops.h |1
include/asm-v850/bitops.h |1
include/asm-x86_64/bitops.h |1
include/asm-xtensa/bitops.h |1
include/linux/bit_spinlock.h | 11 +
include/linux/buffer_head.h |8 +-
include/linux/interrupt.h |5 +---
include/linux/page-flags.h|6 -
include/linux/pagemap.h |9 ++-
kernel/wait.c |2 -
mm/filemap.c | 17 ++
mm/memory.c |2 -
mm/migrate.c |4 +--
mm/rmap.c |2 -
mm/shmem.c|4 +--
mm/swap.c |2 -
mm/swap_state.c |2 -
mm/swapfile.c |2 -
mm/truncate.c |4 +--
mm/vmscan.c |4 +--
54 files changed, 193 insertions(+), 63 deletions(-)
Index: linux-2.6/include/asm-powerpc/bitops.h
===
--- linux-2.6.orig/include/asm-powerpc/bitops.h 2007-05-09 13:56:56.0
+1000
+++ linux-2.6/include/asm-powerpc/bitops.h 2007-05-09 14:00:42.0
+1000
@@ -87,6 +87,24 @@
: cc );
}
+static __inline__ void clear_bit_unlock(int nr, volatile unsigned long *addr)
+{
+ unsigned long old;
+ unsigned long mask = BITOP_MASK(nr);
+ unsigned long *p = ((unsigned long *)addr) + BITOP_WORD(nr);
+
+ __asm__ __volatile__(
+ LWSYNC_ON_SMP
+1: PPC_LLARX %0,0,%3 # clear_bit_unlock\n
+ andc %0,%0,%2\n
+ PPC405_ERR77(0,%3)
+ PPC_STLCX %0,0,%3\n
+ bne- 1b
+ : =r (old), +m (*p)
+ : r (mask), r (p)
+ : cc );
+}
+
static __inline__ void change_bit(int nr, volatile unsigned long *addr)
{
unsigned long old;
@@ -126,6 +144,27 @@
return (old mask) != 0;
}
+static __inline__ int test_and_set_bit_lock(unsigned long nr,
+ volatile unsigned long *addr)
+{
+ unsigned long old, t;
+ unsigned long mask = BITOP_MASK(nr);
+ unsigned long *p = ((unsigned long *)addr) + BITOP_WORD(nr);
+
+ __asm__
Re: [rfc] lock bitops
Nick Piggin writes:
Hi,
[...]
/**
+ * clear_bit_unlock - Clears a bit in memory with release
+ * @nr: Bit to clear
+ * @addr: Address to start counting from
+ *
+ * clear_bit() is atomic and may not be reordered. It does
s/clear_bit/clear_bit_unlock/ ?
+ * contain a memory barrier suitable for unlock type operations.
+ */
+static __inline__ void
+clear_bit_unlock (int nr, volatile void *addr)
+{
Nikita.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [rfc] lock bitops
On Wed, May 09, 2007 at 04:08:41PM +0400, Nikita Danilov wrote: Nick Piggin writes: Hi, [...] /** + * clear_bit_unlock - Clears a bit in memory with release + * @nr: Bit to clear + * @addr: Address to start counting from + * + * clear_bit() is atomic and may not be reordered. It does s/clear_bit/clear_bit_unlock/ ? Yes :) - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [rfc] lock bitops
On Tue, May 08, 2007 at 04:40:36PM -0600, Matthew Wilcox wrote: > On Wed, May 09, 2007 at 12:29:27AM +0200, Nick Piggin wrote: > > On Tue, May 08, 2007 at 09:06:32AM -0600, Matthew Wilcox wrote: > > > On Tue, May 08, 2007 at 01:37:09PM +0200, Nick Piggin wrote: > > > > -- > > > > Introduce test_and_set_bit_lock / clear_bit_unlock bitops with lock > > > > semantics. > > > > Add non-trivial for powerpc and ia64. Convert page lock, buffer lock, > > > > bit_spin_lock, tasklet locks to use the new locks. > > > > > > The names are a bit clumsy. How about naming them after the effect, > > > rather than the implementation? It struck me that really these things > > > are bit mutexes -- you can sleep while holding the lock. How about > > > calling them bit_mutex_trylock() and bit_mutex_unlock()? > > > > bit_spin_trylock / bit_spin_unlock be OK? ;) > > We already have a bit_spin_trylock -- it keeps preempt disabled until > you bit_spin_unlock. Oh, and it only actually sets a bit if you've got > SMP or lock debugging on. Nice try though ;-) OK, I'll be blunt then. I think s/test_and_set_bit_lock/bit_mutex_trylock is much worse ;) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [rfc] lock bitops
On Wed, May 09, 2007 at 12:29:27AM +0200, Nick Piggin wrote: > On Tue, May 08, 2007 at 09:06:32AM -0600, Matthew Wilcox wrote: > > On Tue, May 08, 2007 at 01:37:09PM +0200, Nick Piggin wrote: > > > -- > > > Introduce test_and_set_bit_lock / clear_bit_unlock bitops with lock > > > semantics. > > > Add non-trivial for powerpc and ia64. Convert page lock, buffer lock, > > > bit_spin_lock, tasklet locks to use the new locks. > > > > The names are a bit clumsy. How about naming them after the effect, > > rather than the implementation? It struck me that really these things > > are bit mutexes -- you can sleep while holding the lock. How about > > calling them bit_mutex_trylock() and bit_mutex_unlock()? > > bit_spin_trylock / bit_spin_unlock be OK? ;) We already have a bit_spin_trylock -- it keeps preempt disabled until you bit_spin_unlock. Oh, and it only actually sets a bit if you've got SMP or lock debugging on. Nice try though ;-) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [rfc] lock bitops
On Tue, May 08, 2007 at 01:22:56PM +0100, David Howells wrote: > Nick Piggin <[EMAIL PROTECTED]> wrote: > > > This patch (along with the subsequent one to optimise unlock_page) reduces > > the overhead of lock_page/unlock_page (measured with page faults and a patch > > to lock the page in the fault handler) by about 425 cycles on my 2-way G5. > > Seems reasonable, though test_and_set_lock_bit() might be a better name. The postfix attaches lock semantics to the test_and_set_bit operation, so I don't think it is necessarily wrong to have this name. But it doesn't matter too much I guess. > > +There are two special bitops with lock barrier semantics (acquire/release, > > +same as spinlocks). > > You should update Documentation/memory-barriers.txt also. Will do. > > #define TestSetPageLocked(page)\ > > test_and_set_bit(PG_locked, &(page)->flags) > > +#define TestSetPageLocked_Lock(page) \ > > + test_and_set_bit_lock(PG_locked, &(page)->flags) > > Can we get away with just moving TestSetPageLocked() to the new function > rather than adding another accessor? Or how about LockPageLocked() and > UnlockPageLocked() rather than SetPageLocked_Lock() that last looks wrong > somehow. The problem is that it implies some semantics that it may not have. Possibly better is to just remove them all and use only trylock/lock/unlock_page. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [rfc] lock bitops
On Tue, May 08, 2007 at 09:06:32AM -0600, Matthew Wilcox wrote: > On Tue, May 08, 2007 at 01:37:09PM +0200, Nick Piggin wrote: > > -- > > Introduce test_and_set_bit_lock / clear_bit_unlock bitops with lock > > semantics. > > Add non-trivial for powerpc and ia64. Convert page lock, buffer lock, > > bit_spin_lock, tasklet locks to use the new locks. > > The names are a bit clumsy. How about naming them after the effect, > rather than the implementation? It struck me that really these things > are bit mutexes -- you can sleep while holding the lock. How about > calling them bit_mutex_trylock() and bit_mutex_unlock()? bit_spin_trylock / bit_spin_unlock be OK? ;) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [rfc] lock bitops
On Tue, 2007-05-08 at 09:06 -0600, Matthew Wilcox wrote: > On Tue, May 08, 2007 at 01:37:09PM +0200, Nick Piggin wrote: > > -- > > Introduce test_and_set_bit_lock / clear_bit_unlock bitops with lock > > semantics. > > Add non-trivial for powerpc and ia64. Convert page lock, buffer lock, > > bit_spin_lock, tasklet locks to use the new locks. > > The names are a bit clumsy. How about naming them after the effect, > rather than the implementation? It struck me that really these things > are bit mutexes -- you can sleep while holding the lock. How about > calling them bit_mutex_trylock() and bit_mutex_unlock()? Hrm... spin_trylock vs. mutex_trylock ... what difference ? :-) Note that if we're gonna generalize the usage as a mutex, we might want to extend the unlock semantic to return the first word flag atomically so that the caller can test for other bits without having to read the word back (which might be a performance hit on CPUs without store forwarding). That's already what Nick's new unlock_page() does (testing the flag again right after unlock). At first, I though it would make clear_bit_unlock() semantics a bit too clumsy but maybe it's worth it. Ben. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [rfc] lock bitops
On Tue, May 08, 2007 at 01:37:09PM +0200, Nick Piggin wrote: > -- > Introduce test_and_set_bit_lock / clear_bit_unlock bitops with lock semantics. > Add non-trivial for powerpc and ia64. Convert page lock, buffer lock, > bit_spin_lock, tasklet locks to use the new locks. The names are a bit clumsy. How about naming them after the effect, rather than the implementation? It struck me that really these things are bit mutexes -- you can sleep while holding the lock. How about calling them bit_mutex_trylock() and bit_mutex_unlock()? - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [rfc] lock bitops
Nick Piggin <[EMAIL PROTECTED]> wrote: > This patch (along with the subsequent one to optimise unlock_page) reduces > the overhead of lock_page/unlock_page (measured with page faults and a patch > to lock the page in the fault handler) by about 425 cycles on my 2-way G5. Seems reasonable, though test_and_set_lock_bit() might be a better name. > +There are two special bitops with lock barrier semantics (acquire/release, > +same as spinlocks). You should update Documentation/memory-barriers.txt also. > #define TestSetPageLocked(page) \ > test_and_set_bit(PG_locked, &(page)->flags) > +#define TestSetPageLocked_Lock(page) \ > + test_and_set_bit_lock(PG_locked, &(page)->flags) Can we get away with just moving TestSetPageLocked() to the new function rather than adding another accessor? Or how about LockPageLocked() and UnlockPageLocked() rather than SetPageLocked_Lock() that last looks wrong somehow. The FRV changes look reasonable, btw. David - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [rfc] lock bitops
Nick Piggin [EMAIL PROTECTED] wrote: This patch (along with the subsequent one to optimise unlock_page) reduces the overhead of lock_page/unlock_page (measured with page faults and a patch to lock the page in the fault handler) by about 425 cycles on my 2-way G5. Seems reasonable, though test_and_set_lock_bit() might be a better name. +There are two special bitops with lock barrier semantics (acquire/release, +same as spinlocks). You should update Documentation/memory-barriers.txt also. #define TestSetPageLocked(page) \ test_and_set_bit(PG_locked, (page)-flags) +#define TestSetPageLocked_Lock(page) \ + test_and_set_bit_lock(PG_locked, (page)-flags) Can we get away with just moving TestSetPageLocked() to the new function rather than adding another accessor? Or how about LockPageLocked() and UnlockPageLocked() rather than SetPageLocked_Lock() that last looks wrong somehow. The FRV changes look reasonable, btw. David - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [rfc] lock bitops
On Tue, May 08, 2007 at 01:37:09PM +0200, Nick Piggin wrote: -- Introduce test_and_set_bit_lock / clear_bit_unlock bitops with lock semantics. Add non-trivial for powerpc and ia64. Convert page lock, buffer lock, bit_spin_lock, tasklet locks to use the new locks. The names are a bit clumsy. How about naming them after the effect, rather than the implementation? It struck me that really these things are bit mutexes -- you can sleep while holding the lock. How about calling them bit_mutex_trylock() and bit_mutex_unlock()? - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [rfc] lock bitops
On Tue, 2007-05-08 at 09:06 -0600, Matthew Wilcox wrote: On Tue, May 08, 2007 at 01:37:09PM +0200, Nick Piggin wrote: -- Introduce test_and_set_bit_lock / clear_bit_unlock bitops with lock semantics. Add non-trivial for powerpc and ia64. Convert page lock, buffer lock, bit_spin_lock, tasklet locks to use the new locks. The names are a bit clumsy. How about naming them after the effect, rather than the implementation? It struck me that really these things are bit mutexes -- you can sleep while holding the lock. How about calling them bit_mutex_trylock() and bit_mutex_unlock()? Hrm... spin_trylock vs. mutex_trylock ... what difference ? :-) Note that if we're gonna generalize the usage as a mutex, we might want to extend the unlock semantic to return the first word flag atomically so that the caller can test for other bits without having to read the word back (which might be a performance hit on CPUs without store forwarding). That's already what Nick's new unlock_page() does (testing the flag again right after unlock). At first, I though it would make clear_bit_unlock() semantics a bit too clumsy but maybe it's worth it. Ben. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [rfc] lock bitops
On Tue, May 08, 2007 at 09:06:32AM -0600, Matthew Wilcox wrote: On Tue, May 08, 2007 at 01:37:09PM +0200, Nick Piggin wrote: -- Introduce test_and_set_bit_lock / clear_bit_unlock bitops with lock semantics. Add non-trivial for powerpc and ia64. Convert page lock, buffer lock, bit_spin_lock, tasklet locks to use the new locks. The names are a bit clumsy. How about naming them after the effect, rather than the implementation? It struck me that really these things are bit mutexes -- you can sleep while holding the lock. How about calling them bit_mutex_trylock() and bit_mutex_unlock()? bit_spin_trylock / bit_spin_unlock be OK? ;) - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [rfc] lock bitops
On Tue, May 08, 2007 at 01:22:56PM +0100, David Howells wrote: Nick Piggin [EMAIL PROTECTED] wrote: This patch (along with the subsequent one to optimise unlock_page) reduces the overhead of lock_page/unlock_page (measured with page faults and a patch to lock the page in the fault handler) by about 425 cycles on my 2-way G5. Seems reasonable, though test_and_set_lock_bit() might be a better name. The postfix attaches lock semantics to the test_and_set_bit operation, so I don't think it is necessarily wrong to have this name. But it doesn't matter too much I guess. +There are two special bitops with lock barrier semantics (acquire/release, +same as spinlocks). You should update Documentation/memory-barriers.txt also. Will do. #define TestSetPageLocked(page)\ test_and_set_bit(PG_locked, (page)-flags) +#define TestSetPageLocked_Lock(page) \ + test_and_set_bit_lock(PG_locked, (page)-flags) Can we get away with just moving TestSetPageLocked() to the new function rather than adding another accessor? Or how about LockPageLocked() and UnlockPageLocked() rather than SetPageLocked_Lock() that last looks wrong somehow. The problem is that it implies some semantics that it may not have. Possibly better is to just remove them all and use only trylock/lock/unlock_page. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [rfc] lock bitops
On Wed, May 09, 2007 at 12:29:27AM +0200, Nick Piggin wrote: On Tue, May 08, 2007 at 09:06:32AM -0600, Matthew Wilcox wrote: On Tue, May 08, 2007 at 01:37:09PM +0200, Nick Piggin wrote: -- Introduce test_and_set_bit_lock / clear_bit_unlock bitops with lock semantics. Add non-trivial for powerpc and ia64. Convert page lock, buffer lock, bit_spin_lock, tasklet locks to use the new locks. The names are a bit clumsy. How about naming them after the effect, rather than the implementation? It struck me that really these things are bit mutexes -- you can sleep while holding the lock. How about calling them bit_mutex_trylock() and bit_mutex_unlock()? bit_spin_trylock / bit_spin_unlock be OK? ;) We already have a bit_spin_trylock -- it keeps preempt disabled until you bit_spin_unlock. Oh, and it only actually sets a bit if you've got SMP or lock debugging on. Nice try though ;-) - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [rfc] lock bitops
On Tue, May 08, 2007 at 04:40:36PM -0600, Matthew Wilcox wrote: On Wed, May 09, 2007 at 12:29:27AM +0200, Nick Piggin wrote: On Tue, May 08, 2007 at 09:06:32AM -0600, Matthew Wilcox wrote: On Tue, May 08, 2007 at 01:37:09PM +0200, Nick Piggin wrote: -- Introduce test_and_set_bit_lock / clear_bit_unlock bitops with lock semantics. Add non-trivial for powerpc and ia64. Convert page lock, buffer lock, bit_spin_lock, tasklet locks to use the new locks. The names are a bit clumsy. How about naming them after the effect, rather than the implementation? It struck me that really these things are bit mutexes -- you can sleep while holding the lock. How about calling them bit_mutex_trylock() and bit_mutex_unlock()? bit_spin_trylock / bit_spin_unlock be OK? ;) We already have a bit_spin_trylock -- it keeps preempt disabled until you bit_spin_unlock. Oh, and it only actually sets a bit if you've got SMP or lock debugging on. Nice try though ;-) OK, I'll be blunt then. I think s/test_and_set_bit_lock/bit_mutex_trylock is much worse ;) - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
