Re: [PATCH 0/4] rwsem: Implement writer lock-stealing
On Wed, Feb 20, 2013 at 4:50 PM, Alex Shi wrote: > I did a quick review on the patchset and tested the patches 1~3, and 1~3 > plus 4th, my patch plus 4th. > > The patch looks much complicated, and also goes writing slow path to > steal locking. My patch looks quite straight and simple. > > This 1~3 patch has very very similar performance effect with mine. > > The highlight patch is the 4th, seems it can provide about ~2% aim7 > performance gain(base on both of mine or patches 1~3) on my 4S NHM EX > machine. Thanks for doing these measurements. I think we can keep patches 1-2 out of this discussion as they don't have much complexity and don't have much to do with write stealing either. Patch 4 is where fast path write stealing is implemented; however there are some parts of patch 3 that I think are required - the one I have in mind right now is when waking readers, the existing code assumes that writers can't get the lock while it counts how many reader locks need to be granted. There are several solutions to that - for example you could grant the first reader lock before counting how many more you need, but that would result in longer code than in my proposal. Either way, I believe if you go into fixing lib/rwsem.c to account for the possibility of fast path lock stealing you'll end up with something very similar to my proposed patch 3... (This is kinda how I proceeded with this series, I wrote the fastpath lock stealing first, then I thought about what's required to support it in the slow path) -- Michel "Walken" Lespinasse A program is never fully debugged until the last user dies. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/4] rwsem: Implement writer lock-stealing
* Alex Shi wrote: > > Alex, could you go through my patch and see if there is > > anything you find objectionable ? (if not about the details, > > at least about the general approach of enabling writer lock > > stealing on the fast path) > > I did a quick review on the patchset and tested the patches > 1~3, and 1~3 plus 4th, my patch plus 4th. > > The patch looks much complicated, and also goes writing slow > path to steal locking. My patch looks quite straight and > simple. > > This 1~3 patch has very very similar performance effect with > mine. > > The highlight patch is the 4th, seems it can provide about ~2% > aim7 performance gain(base on both of mine or patches 1~3) on > my 4S NHM EX machine. That speedup would be nice to have. Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/4] rwsem: Implement writer lock-stealing
> > Alex, could you go through my patch and see if there is anything you > find objectionable ? (if not about the details, at least about the > general approach of enabling writer lock stealing on the fast path) > I did a quick review on the patchset and tested the patches 1~3, and 1~3 plus 4th, my patch plus 4th. The patch looks much complicated, and also goes writing slow path to steal locking. My patch looks quite straight and simple. This 1~3 patch has very very similar performance effect with mine. The highlight patch is the 4th, seems it can provide about ~2% aim7 performance gain(base on both of mine or patches 1~3) on my 4S NHM EX machine. -- Thanks Alex -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/4] rwsem: Implement writer lock-stealing
Alex, could you go through my patch and see if there is anything you find objectionable ? (if not about the details, at least about the general approach of enabling writer lock stealing on the fast path) I did a quick review on the patchset and tested the patches 1~3, and 1~3 plus 4th, my patch plus 4th. The patch looks much complicated, and also goes writing slow path to steal locking. My patch looks quite straight and simple. This 1~3 patch has very very similar performance effect with mine. The highlight patch is the 4th, seems it can provide about ~2% aim7 performance gain(base on both of mine or patches 1~3) on my 4S NHM EX machine. -- Thanks Alex -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/4] rwsem: Implement writer lock-stealing
* Alex Shi alex@intel.com wrote: Alex, could you go through my patch and see if there is anything you find objectionable ? (if not about the details, at least about the general approach of enabling writer lock stealing on the fast path) I did a quick review on the patchset and tested the patches 1~3, and 1~3 plus 4th, my patch plus 4th. The patch looks much complicated, and also goes writing slow path to steal locking. My patch looks quite straight and simple. This 1~3 patch has very very similar performance effect with mine. The highlight patch is the 4th, seems it can provide about ~2% aim7 performance gain(base on both of mine or patches 1~3) on my 4S NHM EX machine. That speedup would be nice to have. Thanks, Ingo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/4] rwsem: Implement writer lock-stealing
On Wed, Feb 20, 2013 at 4:50 PM, Alex Shi alex@intel.com wrote: I did a quick review on the patchset and tested the patches 1~3, and 1~3 plus 4th, my patch plus 4th. The patch looks much complicated, and also goes writing slow path to steal locking. My patch looks quite straight and simple. This 1~3 patch has very very similar performance effect with mine. The highlight patch is the 4th, seems it can provide about ~2% aim7 performance gain(base on both of mine or patches 1~3) on my 4S NHM EX machine. Thanks for doing these measurements. I think we can keep patches 1-2 out of this discussion as they don't have much complexity and don't have much to do with write stealing either. Patch 4 is where fast path write stealing is implemented; however there are some parts of patch 3 that I think are required - the one I have in mind right now is when waking readers, the existing code assumes that writers can't get the lock while it counts how many reader locks need to be granted. There are several solutions to that - for example you could grant the first reader lock before counting how many more you need, but that would result in longer code than in my proposal. Either way, I believe if you go into fixing lib/rwsem.c to account for the possibility of fast path lock stealing you'll end up with something very similar to my proposed patch 3... (This is kinda how I proceeded with this series, I wrote the fastpath lock stealing first, then I thought about what's required to support it in the slow path) -- Michel Walken Lespinasse A program is never fully debugged until the last user dies. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/4] rwsem: Implement writer lock-stealing
On 02/14/2013 09:31 AM, Michel Lespinasse wrote: > Alex, could you go through my patch and see if there is anything you > find objectionable ? (if not about the details, at least about the > general approach of enabling writer lock stealing on the fast path) I am still in my lunar new year vacation. I may do a aim7 test for you after back to office. But as to fast path stealing, I didn't see extra benefit on fast path. -- Thanks Alex -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/4] rwsem: Implement writer lock-stealing
On 02/14/2013 09:31 AM, Michel Lespinasse wrote: Alex, could you go through my patch and see if there is anything you find objectionable ? (if not about the details, at least about the general approach of enabling writer lock stealing on the fast path) I am still in my lunar new year vacation. I may do a aim7 test for you after back to office. But as to fast path stealing, I didn't see extra benefit on fast path. -- Thanks Alex -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/4] rwsem: Implement writer lock-stealing
On Wed, Feb 13, 2013 at 6:49 AM, Ingo Molnar wrote: > > * Alex Shi wrote: > >> On 02/09/2013 10:45 AM, Michel Lespinasse wrote: >> > This proposal implements writer lock stealing in lib/rwsem.c, just as >> > Alex Shi's earlier proposal did for the simpler lib/rwsem-spinlock.c >> >> Ops, my patch in tip/urgent is for rwsem. Yuanhan's patch is >> for rwsem-spinlock. Sorry, my bad. Looks like an email snafu on my side - when I wanted to save your patch to look through it with more context, I ended up saving Yuanhan's instead. > Your rwsem patch is queued up for v3.9, in the tip:core/locking > tree: > > 3a15e0e0cdda rwsem: Implement writer lock-stealing for better scalability > > Michel, mind having a look at that and possibly generate a delta > patch, if your patch has changes we should apply? The main difference I can see is that my approach makes it possible to steal the rwsem in a fast path, without going through the lib/rwsem.c slow path. A few things fall out from that; most notably I had to change the readers_only side of __rwsem_do_wake() to account with the possibility of write lock stealing on the fast path, while Alex doesn't since he forces write lock stealing to use the slow path. Overall, my changes are more extensive but they also reduce the total line count, while Alex's proposal goes the other way. For these reasons, I think I still prefer my approach. However as mentioned, this is not the best time for me to push it as I'll be away for a little while. I should be able to get back to this in a couple weeks, though. Alex, could you go through my patch and see if there is anything you find objectionable ? (if not about the details, at least about the general approach of enabling writer lock stealing on the fast path) Thanks, -- Michel "Walken" Lespinasse A program is never fully debugged until the last user dies. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/4] rwsem: Implement writer lock-stealing
* Alex Shi wrote: > On 02/09/2013 10:45 AM, Michel Lespinasse wrote: > > This proposal implements writer lock stealing in lib/rwsem.c, just as > > Alex Shi's earlier proposal did for the simpler lib/rwsem-spinlock.c > > Ops, my patch in tip/urgent is for rwsem. Yuanhan's patch is > for rwsem-spinlock. Your rwsem patch is queued up for v3.9, in the tip:core/locking tree: 3a15e0e0cdda rwsem: Implement writer lock-stealing for better scalability Michel, mind having a look at that and possibly generate a delta patch, if your patch has changes we should apply? Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/4] rwsem: Implement writer lock-stealing
On 02/09/2013 10:45 AM, Michel Lespinasse wrote: > This proposal implements writer lock stealing in lib/rwsem.c, just as > Alex Shi's earlier proposal did for the simpler lib/rwsem-spinlock.c Ops, my patch in tip/urgent is for rwsem. Yuanhan's patch is for rwsem-spinlock. Thanks Alex -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/4] rwsem: Implement writer lock-stealing
On 02/09/2013 10:45 AM, Michel Lespinasse wrote: This proposal implements writer lock stealing in lib/rwsem.c, just as Alex Shi's earlier proposal did for the simpler lib/rwsem-spinlock.c Ops, my patch in tip/urgent is for rwsem. Yuanhan's patch is for rwsem-spinlock. Thanks Alex -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/4] rwsem: Implement writer lock-stealing
* Alex Shi alex@intel.com wrote: On 02/09/2013 10:45 AM, Michel Lespinasse wrote: This proposal implements writer lock stealing in lib/rwsem.c, just as Alex Shi's earlier proposal did for the simpler lib/rwsem-spinlock.c Ops, my patch in tip/urgent is for rwsem. Yuanhan's patch is for rwsem-spinlock. Your rwsem patch is queued up for v3.9, in the tip:core/locking tree: 3a15e0e0cdda rwsem: Implement writer lock-stealing for better scalability Michel, mind having a look at that and possibly generate a delta patch, if your patch has changes we should apply? Thanks, Ingo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/4] rwsem: Implement writer lock-stealing
On Wed, Feb 13, 2013 at 6:49 AM, Ingo Molnar mi...@kernel.org wrote: * Alex Shi alex@intel.com wrote: On 02/09/2013 10:45 AM, Michel Lespinasse wrote: This proposal implements writer lock stealing in lib/rwsem.c, just as Alex Shi's earlier proposal did for the simpler lib/rwsem-spinlock.c Ops, my patch in tip/urgent is for rwsem. Yuanhan's patch is for rwsem-spinlock. Sorry, my bad. Looks like an email snafu on my side - when I wanted to save your patch to look through it with more context, I ended up saving Yuanhan's instead. Your rwsem patch is queued up for v3.9, in the tip:core/locking tree: 3a15e0e0cdda rwsem: Implement writer lock-stealing for better scalability Michel, mind having a look at that and possibly generate a delta patch, if your patch has changes we should apply? The main difference I can see is that my approach makes it possible to steal the rwsem in a fast path, without going through the lib/rwsem.c slow path. A few things fall out from that; most notably I had to change the readers_only side of __rwsem_do_wake() to account with the possibility of write lock stealing on the fast path, while Alex doesn't since he forces write lock stealing to use the slow path. Overall, my changes are more extensive but they also reduce the total line count, while Alex's proposal goes the other way. For these reasons, I think I still prefer my approach. However as mentioned, this is not the best time for me to push it as I'll be away for a little while. I should be able to get back to this in a couple weeks, though. Alex, could you go through my patch and see if there is anything you find objectionable ? (if not about the details, at least about the general approach of enabling writer lock stealing on the fast path) Thanks, -- Michel Walken Lespinasse A program is never fully debugged until the last user dies. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 0/4] rwsem: Implement writer lock-stealing
This proposal implements writer lock stealing in lib/rwsem.c, just as Alex Shi's earlier proposal did for the simpler lib/rwsem-spinlock.c Patches 1 and 2 are small cleanups that I thought I should separate from the bulk of the changes; Patch 3 is the generic support for write lock stealing; Patch 4 is the x86 support (optional, only required for higher performance). I am not in an ideal position to push this as I will be out of town starting from the middle of next week. However, I hope that this proposal will at least get the ball rolling. I did run some basic testing including David Howell's synchro-test module (as found in Andrew's -mm tree). Michel Lespinasse (4): rwsem: make the waiter type an enumeration rather than a bitmask rwsem: shorter spinlocked section in rwsem_down_failed_common() rwsem: implement write lock stealing x86 rwsem: avoid taking slow path when stealing write lock arch/x86/include/asm/rwsem.h | 28 +++-- include/linux/rwsem.h| 2 + lib/rwsem.c | 252 --- 3 files changed, 139 insertions(+), 143 deletions(-) -- 1.8.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 0/4] rwsem: Implement writer lock-stealing
This proposal implements writer lock stealing in lib/rwsem.c, just as Alex Shi's earlier proposal did for the simpler lib/rwsem-spinlock.c Patches 1 and 2 are small cleanups that I thought I should separate from the bulk of the changes; Patch 3 is the generic support for write lock stealing; Patch 4 is the x86 support (optional, only required for higher performance). I am not in an ideal position to push this as I will be out of town starting from the middle of next week. However, I hope that this proposal will at least get the ball rolling. I did run some basic testing including David Howell's synchro-test module (as found in Andrew's -mm tree). Michel Lespinasse (4): rwsem: make the waiter type an enumeration rather than a bitmask rwsem: shorter spinlocked section in rwsem_down_failed_common() rwsem: implement write lock stealing x86 rwsem: avoid taking slow path when stealing write lock arch/x86/include/asm/rwsem.h | 28 +++-- include/linux/rwsem.h| 2 + lib/rwsem.c | 252 --- 3 files changed, 139 insertions(+), 143 deletions(-) -- 1.8.1 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
