From: Peter Zijlstra <peterz@infradead.org> To: Michal Hocko <mhocko@kernel.org> Cc: LKML <linux-kernel@vger.kernel.org>, Ingo Molnar <mingo@redhat.com>, Thomas Gleixner <tglx@linutronix.de>, "H. Peter Anvin" <hpa@zytor.com>, "David S. Miller" <davem@davemloft.net>, Tony Luck <tony.luck@intel.com>, Andrew Morton <akpm@linux-foundation.org>, Chris Zankel <chris@zankel.net>, Max Filippov <jcmvbkbc@gmail.com>, x86@kernel.org, linux-alpha@vger.kernel.org, linux-ia64@vger.kernel.org, linux-s390@vger.kernel.org, linux-sh@vger.kernel.org, sparclinux@vger.kernel.org, linux-xtensa@linux-xtensa.org, linux-arch@vger.kernel.org, Michal Hocko <mhocko@suse.com> Subject: Re: [PATCH 03/11] locking, rwsem: introduce basis for down_write_killable Date: Wed, 30 Mar 2016 13:25:49 +0000 [thread overview] Message-ID: <20160330132549.GU3408@twins.programming.kicks-ass.net> (raw) In-Reply-To: <1456750705-7141-4-git-send-email-mhocko@kernel.org> On Mon, Feb 29, 2016 at 01:58:17PM +0100, Michal Hocko wrote: > @@ -215,16 +216,34 @@ void __sched __down_write(struct rw_semaphore *sem) > */ > if (sem->count = 0) > break; > - set_task_state(tsk, TASK_UNINTERRUPTIBLE); > + set_task_state(tsk, state); > raw_spin_unlock_irqrestore(&sem->wait_lock, flags); > schedule(); > + if (signal_pending_state(state, current)) { > + ret = -EINTR; > + raw_spin_lock_irqsave(&sem->wait_lock, flags); > + goto out; > + } > raw_spin_lock_irqsave(&sem->wait_lock, flags); > } > /* got the lock */ > sem->count = -1; > @@ -487,20 +488,38 @@ struct rw_semaphore __sched *rwsem_down_write_failed(struct rw_semaphore *sem) > /* Block until there are no active lockers. */ > do { > schedule(); > - set_current_state(TASK_UNINTERRUPTIBLE); > + if (signal_pending_state(state, current)) { > + raw_spin_lock_irq(&sem->wait_lock); > + ret = ERR_PTR(-EINTR); > + goto out; > + } > + set_current_state(state); > } while ((count = sem->count) & RWSEM_ACTIVE_MASK); > > raw_spin_lock_irq(&sem->wait_lock); > } > __set_current_state(TASK_RUNNING); Why is the signal_pending_state() test _after_ the call to schedule() and before the 'trylock'. __mutex_lock_common() has it before the call to schedule and after the 'trylock'. The difference is that rwsem will now respond to the KILL and return -EINTR even if the lock is available, whereas mutex will acquire it and ignore the signal (for a little while longer). Neither is wrong per se, but I feel all the locking primitives should behave in a consistent manner in this regard.
WARNING: multiple messages have this Message-ID (diff)
From: Peter Zijlstra <peterz@infradead.org> To: Michal Hocko <mhocko@kernel.org> Cc: LKML <linux-kernel@vger.kernel.org>, Ingo Molnar <mingo@redhat.com>, Thomas Gleixner <tglx@linutronix.de>, "H. Peter Anvin" <hpa@zytor.com>, "David S. Miller" <davem@davemloft.net>, Tony Luck <tony.luck@intel.com>, Andrew Morton <akpm@linux-foundation.org>, Chris Zankel <chris@zankel.net>, Max Filippov <jcmvbkbc@gmail.com>, x86@kernel.org, linux-alpha@vger.kernel.org, linux-ia64@vger.kernel.org, linux-s390@vger.kernel.org, linux-sh@vger.kernel.org, sparclinux@vger.kernel.org, linux-xtensa@linux-xtensa.org, linux-arch@vger.kernel.org, Michal Hocko <mhocko@suse.com> Subject: Re: [PATCH 03/11] locking, rwsem: introduce basis for down_write_killable Date: Wed, 30 Mar 2016 15:25:49 +0200 [thread overview] Message-ID: <20160330132549.GU3408@twins.programming.kicks-ass.net> (raw) In-Reply-To: <1456750705-7141-4-git-send-email-mhocko@kernel.org> On Mon, Feb 29, 2016 at 01:58:17PM +0100, Michal Hocko wrote: > @@ -215,16 +216,34 @@ void __sched __down_write(struct rw_semaphore *sem) > */ > if (sem->count == 0) > break; > - set_task_state(tsk, TASK_UNINTERRUPTIBLE); > + set_task_state(tsk, state); > raw_spin_unlock_irqrestore(&sem->wait_lock, flags); > schedule(); > + if (signal_pending_state(state, current)) { > + ret = -EINTR; > + raw_spin_lock_irqsave(&sem->wait_lock, flags); > + goto out; > + } > raw_spin_lock_irqsave(&sem->wait_lock, flags); > } > /* got the lock */ > sem->count = -1; > @@ -487,20 +488,38 @@ struct rw_semaphore __sched *rwsem_down_write_failed(struct rw_semaphore *sem) > /* Block until there are no active lockers. */ > do { > schedule(); > - set_current_state(TASK_UNINTERRUPTIBLE); > + if (signal_pending_state(state, current)) { > + raw_spin_lock_irq(&sem->wait_lock); > + ret = ERR_PTR(-EINTR); > + goto out; > + } > + set_current_state(state); > } while ((count = sem->count) & RWSEM_ACTIVE_MASK); > > raw_spin_lock_irq(&sem->wait_lock); > } > __set_current_state(TASK_RUNNING); Why is the signal_pending_state() test _after_ the call to schedule() and before the 'trylock'. __mutex_lock_common() has it before the call to schedule and after the 'trylock'. The difference is that rwsem will now respond to the KILL and return -EINTR even if the lock is available, whereas mutex will acquire it and ignore the signal (for a little while longer). Neither is wrong per se, but I feel all the locking primitives should behave in a consistent manner in this regard.
next prev parent reply other threads:[~2016-03-30 13:25 UTC|newest] Thread overview: 226+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-02-29 12:58 [PATCH 0/11] introduce down_write_killable for rw_semaphore Michal Hocko 2016-02-29 12:58 ` Michal Hocko 2016-02-29 12:58 ` [PATCH 01/11] locking, rwsem: get rid of __down_write_nested Michal Hocko 2016-02-29 12:58 ` Michal Hocko 2016-02-29 12:58 ` [PATCH 02/11] locking, rwsem: drop explicit memory barriers Michal Hocko 2016-02-29 12:58 ` Michal Hocko 2016-02-29 12:58 ` [PATCH 03/11] locking, rwsem: introduce basis for down_write_killable Michal Hocko 2016-02-29 12:58 ` Michal Hocko 2016-03-30 13:25 ` Peter Zijlstra [this message] 2016-03-30 13:25 ` Peter Zijlstra 2016-03-31 8:33 ` Michal Hocko 2016-03-31 8:33 ` Michal Hocko 2016-03-31 8:44 ` Peter Zijlstra 2016-03-31 8:44 ` Peter Zijlstra 2016-03-31 8:55 ` [PATCH] " Michal Hocko 2016-03-31 8:55 ` Michal Hocko 2016-02-29 12:58 ` [PATCH 04/11] alpha, rwsem: provide __down_write_killable Michal Hocko 2016-02-29 12:58 ` Michal Hocko 2016-02-29 12:58 ` [PATCH 05/11] ia64, " Michal Hocko 2016-02-29 12:58 ` Michal Hocko 2016-02-29 12:58 ` [PATCH 06/11] s390, " Michal Hocko 2016-02-29 12:58 ` Michal Hocko 2016-02-29 12:58 ` [PATCH 07/11] sh, " Michal Hocko 2016-02-29 12:58 ` Michal Hocko 2016-02-29 12:58 ` [PATCH 08/11] sparc, " Michal Hocko 2016-02-29 12:58 ` Michal Hocko 2016-02-29 12:58 ` [PATCH 09/11] xtensa, " Michal Hocko 2016-02-29 12:58 ` Michal Hocko 2016-02-29 12:58 ` [PATCH 10/11] x86, " Michal Hocko 2016-02-29 12:58 ` Michal Hocko 2016-02-29 12:58 ` [PATCH 11/11] locking, rwsem: provide down_write_killable Michal Hocko 2016-02-29 12:58 ` Michal Hocko 2016-03-30 13:32 ` [PATCH 0/11] introduce down_write_killable for rw_semaphore Peter Zijlstra 2016-03-30 13:32 ` Peter Zijlstra 2016-03-31 8:59 ` Michal Hocko 2016-03-31 8:59 ` Michal Hocko 2016-03-31 9:20 ` Ingo Molnar 2016-03-31 9:20 ` Ingo Molnar 2016-03-31 10:58 ` Michal Hocko 2016-03-31 10:58 ` Michal Hocko 2016-03-31 17:03 ` Andrew Morton 2016-03-31 17:03 ` Andrew Morton 2016-04-01 6:33 ` Ingo Molnar 2016-04-01 6:33 ` Ingo Molnar 2016-04-01 9:21 ` Michal Hocko 2016-04-01 9:21 ` Michal Hocko 2016-04-01 9:50 ` Ingo Molnar 2016-04-01 9:50 ` Ingo Molnar 2016-04-01 10:52 ` Michal Hocko 2016-04-01 10:52 ` Michal Hocko 2016-04-01 7:26 ` Michal Hocko 2016-04-01 7:26 ` Michal Hocko 2016-04-01 9:11 ` Andrew Morton 2016-04-01 9:11 ` Andrew Morton 2016-04-01 11:04 ` [PATCH 0/11] introduce down_write_killable for rw_semaphore v2 Michal Hocko 2016-04-01 11:04 ` Michal Hocko 2016-04-01 11:04 ` [PATCH 01/11] locking, rwsem: get rid of __down_write_nested Michal Hocko 2016-04-01 11:04 ` Michal Hocko 2016-04-02 0:28 ` Davidlohr Bueso 2016-04-02 0:28 ` Davidlohr Bueso 2016-04-01 11:04 ` [PATCH 02/11] locking, rwsem: drop explicit memory barriers Michal Hocko 2016-04-01 11:04 ` Michal Hocko 2016-04-02 1:17 ` Davidlohr Bueso 2016-04-02 1:17 ` Davidlohr Bueso 2016-04-04 9:03 ` Michal Hocko 2016-04-04 9:03 ` Michal Hocko 2016-04-04 9:06 ` [PATCH 1/2] xtensa, rwsem: drop superfluous arch specific implementation Michal Hocko 2016-04-04 9:06 ` Michal Hocko 2016-04-04 9:06 ` [PATCH 2/2] sh, " Michal Hocko 2016-04-04 9:06 ` Michal Hocko 2016-04-06 9:26 ` Peter Zijlstra 2016-04-06 9:26 ` Peter Zijlstra 2016-04-06 9:50 ` Geert Uytterhoeven 2016-04-06 9:50 ` Geert Uytterhoeven 2016-04-06 10:27 ` Peter Zijlstra 2016-04-06 10:27 ` Peter Zijlstra 2016-04-04 10:23 ` [PATCH 1/2] xtensa, " Max Filippov 2016-04-04 10:23 ` Max Filippov 2016-04-06 9:06 ` [PATCH] sparc, " Michal Hocko 2016-04-06 9:06 ` Michal Hocko 2016-04-06 9:06 ` Michal Hocko 2016-04-06 9:06 ` Michal Hocko 2016-04-01 11:04 ` [PATCH 03/11] locking, rwsem: introduce basis for down_write_killable Michal Hocko 2016-04-01 11:04 ` Michal Hocko 2016-04-02 4:41 ` Davidlohr Bueso 2016-04-02 4:41 ` Davidlohr Bueso 2016-04-04 9:17 ` Michal Hocko 2016-04-04 9:17 ` Michal Hocko 2016-04-04 9:21 ` Peter Zijlstra 2016-04-04 9:21 ` Peter Zijlstra 2016-04-07 6:58 ` Davidlohr Bueso 2016-04-07 6:58 ` Davidlohr Bueso 2016-04-07 7:38 ` Michal Hocko 2016-04-07 7:38 ` Michal Hocko 2016-05-10 10:43 ` Tetsuo Handa 2016-05-10 11:53 ` Michal Hocko 2016-05-10 12:38 ` Peter Zijlstra 2016-05-10 13:57 ` Tetsuo Handa 2016-05-11 7:23 ` Michal Hocko 2016-05-11 8:28 ` Michal Hocko 2016-05-11 8:44 ` Peter Zijlstra 2016-05-11 9:04 ` Michal Hocko 2016-05-11 9:17 ` Peter Zijlstra 2016-05-11 9:31 ` Michal Hocko 2016-05-11 9:41 ` Peter Zijlstra 2016-05-11 13:59 ` Michal Hocko 2016-05-11 18:03 ` Michal Hocko 2016-05-12 11:57 ` [PATCH] locking, rwsem: Fix down_write_killable() Peter Zijlstra 2016-05-12 12:15 ` [tip:locking/rwsem] locking/rwsem: " tip-bot for Peter Zijlstra 2016-05-12 16:59 ` [PATCH] locking, rwsem: " Michal Hocko 2016-05-15 20:57 ` [tip:locking/rwsem] locking/rwsem: " tip-bot for Peter Zijlstra 2016-05-12 12:12 ` [PATCH 03/11] locking, rwsem: introduce basis for down_write_killable Peter Zijlstra 2016-05-12 12:19 ` Michal Hocko 2016-05-12 13:58 ` Peter Zijlstra 2016-05-12 19:42 ` Waiman Long 2016-05-11 8:35 ` Peter Zijlstra 2016-05-11 9:02 ` Michal Hocko 2016-04-01 11:04 ` [PATCH 04/11] alpha, rwsem: provide __down_write_killable Michal Hocko 2016-04-01 11:04 ` Michal Hocko 2016-04-01 11:04 ` [PATCH 05/11] ia64, " Michal Hocko 2016-04-01 11:04 ` Michal Hocko 2016-04-01 11:04 ` [PATCH 06/11] s390, " Michal Hocko 2016-04-01 11:04 ` Michal Hocko 2016-04-01 11:04 ` [PATCH 07/11] sh, " Michal Hocko 2016-04-01 11:04 ` Michal Hocko 2016-04-01 11:04 ` [PATCH 08/11] sparc, " Michal Hocko 2016-04-01 11:04 ` Michal Hocko 2016-04-01 11:04 ` [PATCH 09/11] xtensa, " Michal Hocko 2016-04-01 11:04 ` Michal Hocko 2016-04-01 11:04 ` [PATCH 10/11] x86, " Michal Hocko 2016-04-01 11:04 ` Michal Hocko 2016-04-06 18:31 ` Peter Zijlstra 2016-04-06 18:31 ` Peter Zijlstra 2016-04-01 11:04 ` [PATCH 11/11] locking, rwsem: provide down_write_killable Michal Hocko 2016-04-01 11:04 ` Michal Hocko 2016-04-07 15:12 ` [PATCH 0/11] introduce down_write_killable for rw_semaphore v3 Michal Hocko 2016-04-07 15:12 ` Michal Hocko 2016-04-07 15:12 ` [PATCH 01/11] locking, rwsem: get rid of __down_write_nested Michal Hocko 2016-04-07 15:12 ` Michal Hocko 2016-04-13 11:32 ` [tip:locking/rwsem] locking/rwsem: Get rid of __down_write_nested() tip-bot for Michal Hocko 2016-04-07 15:12 ` [PATCH 02/11] locking, rwsem: drop explicit memory barriers Michal Hocko 2016-04-07 15:12 ` Michal Hocko 2016-04-13 11:32 ` [tip:locking/rwsem] locking/rwsem: Drop " tip-bot for Michal Hocko 2016-04-07 15:12 ` [PATCH 03/11] xtensa, rwsem: drop superfluous arch specific implementation Michal Hocko 2016-04-07 15:12 ` Michal Hocko 2016-04-13 11:33 ` [tip:locking/rwsem] locking/rwsem, xtensa: Drop " tip-bot for Michal Hocko 2016-04-07 15:12 ` [PATCH 04/11] sh, rwsem: drop " Michal Hocko 2016-04-07 15:12 ` Michal Hocko 2016-04-13 11:33 ` [tip:locking/rwsem] locking/rwsem, sh: Drop " tip-bot for Michal Hocko 2016-04-07 15:12 ` [PATCH 05/11] sparc, rwsem: drop " Michal Hocko 2016-04-07 15:12 ` Michal Hocko 2016-04-13 11:33 ` [tip:locking/rwsem] locking/rwsem, sparc: Drop " tip-bot for Michal Hocko 2016-04-07 15:12 ` [PATCH 06/11] locking, rwsem: introduce basis for down_write_killable Michal Hocko 2016-04-07 15:12 ` Michal Hocko 2016-04-13 11:34 ` [tip:locking/rwsem] locking/rwsem: Introduce basis for down_write_killable() tip-bot for Michal Hocko 2016-04-07 15:12 ` [PATCH 07/11] alpha, rwsem: provide __down_write_killable Michal Hocko 2016-04-07 15:12 ` Michal Hocko 2016-04-22 9:42 ` [tip:locking/rwsem] locking/rwsem, alpha: Provide __down_write_killable() tip-bot for Michal Hocko 2016-04-07 15:12 ` [PATCH 08/11] ia64, rwsem: provide __down_write_killable Michal Hocko 2016-04-07 15:12 ` Michal Hocko 2016-04-07 17:28 ` Sergei Shtylyov 2016-04-07 17:28 ` Sergei Shtylyov 2016-04-08 6:22 ` Michal Hocko 2016-04-08 6:22 ` Michal Hocko 2016-04-22 9:43 ` [tip:locking/rwsem] locking/rwsem, ia64: Provide __down_write_killable() tip-bot for Michal Hocko 2016-04-07 15:12 ` [PATCH 09/11] s390, rwsem: provide __down_write_killable Michal Hocko 2016-04-07 15:12 ` Michal Hocko 2016-04-22 9:43 ` [tip:locking/rwsem] locking/rwsem, s390: Provide __down_write_killable() tip-bot for Michal Hocko 2016-04-07 15:12 ` [PATCH 10/11] x86, rwsem: provide __down_write_killable Michal Hocko 2016-04-07 15:12 ` Michal Hocko 2016-04-13 9:08 ` Ingo Molnar 2016-04-13 9:08 ` Ingo Molnar 2016-04-13 9:16 ` Michal Hocko 2016-04-13 9:16 ` Michal Hocko 2016-04-13 9:19 ` Ingo Molnar 2016-04-13 9:19 ` Ingo Molnar 2016-04-13 10:27 ` Ingo Molnar 2016-04-13 10:27 ` Ingo Molnar 2016-04-13 12:49 ` Michal Hocko 2016-04-13 12:49 ` Michal Hocko 2016-04-13 12:49 ` Michal Hocko 2016-04-13 12:49 ` Michal Hocko 2016-04-17 16:59 ` Michal Hocko 2016-04-17 16:59 ` Michal Hocko 2016-04-17 16:59 ` Michal Hocko 2016-04-20 13:40 ` Peter Zijlstra 2016-04-20 13:40 ` Peter Zijlstra 2016-04-20 13:40 ` Peter Zijlstra 2016-04-20 18:04 ` H. Peter Anvin 2016-04-20 18:04 ` H. Peter Anvin 2016-04-20 20:45 ` Borislav Petkov 2016-04-20 20:45 ` Borislav Petkov 2016-04-20 20:58 ` Michal Hocko 2016-04-20 20:58 ` Michal Hocko 2016-04-20 21:06 ` H. Peter Anvin 2016-04-20 21:06 ` H. Peter Anvin 2016-04-20 21:36 ` Borislav Petkov 2016-04-20 21:36 ` Borislav Petkov 2016-04-20 22:29 ` H. Peter Anvin 2016-04-20 22:29 ` H. Peter Anvin 2016-04-21 11:35 ` Borislav Petkov 2016-04-21 11:35 ` Borislav Petkov 2016-04-21 13:09 ` Michal Hocko 2016-04-21 13:09 ` Michal Hocko 2016-04-21 13:21 ` Borislav Petkov 2016-04-21 13:21 ` Borislav Petkov 2016-04-27 12:02 ` [PATCH] x86/locking/rwsem: Cleanup ____down_write() Borislav Petkov 2016-04-27 12:02 ` Borislav Petkov 2016-04-28 10:27 ` [tip:locking/rwsem] locking/rwsem, x86: Clean up ____down_write() tip-bot for Borislav Petkov 2016-04-22 6:53 ` [PATCH 10/11] x86, rwsem: provide __down_write_killable Ingo Molnar 2016-04-22 6:53 ` Ingo Molnar 2016-04-22 6:53 ` Ingo Molnar 2016-04-13 9:57 ` [PATCH] x86: add frame annotation for call_rwsem_down_write_failed_killable Michal Hocko 2016-04-13 9:57 ` Michal Hocko 2016-04-13 9:57 ` Michal Hocko 2016-04-22 9:44 ` [tip:locking/rwsem] locking/rwsem, x86: Add frame annotation for call_rwsem_down_write_failed_killable() tip-bot for Michal Hocko 2016-04-22 9:43 ` [tip:locking/rwsem] locking/rwsem, x86: Provide __down_write_killable() tip-bot for Michal Hocko 2016-04-07 15:12 ` [PATCH 11/11] locking, rwsem: provide down_write_killable Michal Hocko 2016-04-07 15:12 ` Michal Hocko 2016-04-22 9:44 ` [tip:locking/rwsem] locking/rwsem: Provide down_write_killable() tip-bot for Michal Hocko 2016-04-12 9:37 ` [PATCH 0/11] introduce down_write_killable for rw_semaphore v3 Michal Hocko 2016-04-12 9:37 ` Michal Hocko 2016-04-12 15:40 ` Peter Zijlstra 2016-04-12 15:40 ` Peter Zijlstra 2016-04-12 18:01 ` Michal Hocko 2016-04-12 18:01 ` Michal Hocko
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20160330132549.GU3408@twins.programming.kicks-ass.net \ --to=peterz@infradead.org \ --cc=akpm@linux-foundation.org \ --cc=chris@zankel.net \ --cc=davem@davemloft.net \ --cc=hpa@zytor.com \ --cc=jcmvbkbc@gmail.com \ --cc=linux-alpha@vger.kernel.org \ --cc=linux-arch@vger.kernel.org \ --cc=linux-ia64@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-s390@vger.kernel.org \ --cc=linux-sh@vger.kernel.org \ --cc=linux-xtensa@linux-xtensa.org \ --cc=mhocko@kernel.org \ --cc=mhocko@suse.com \ --cc=mingo@redhat.com \ --cc=sparclinux@vger.kernel.org \ --cc=tglx@linutronix.de \ --cc=tony.luck@intel.com \ --cc=x86@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.