From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752791AbbJTNRw (ORCPT ); Tue, 20 Oct 2015 09:17:52 -0400 Received: from casper.infradead.org ([85.118.1.10]:56344 "EHLO casper.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752759AbbJTNRs (ORCPT ); Tue, 20 Oct 2015 09:17:48 -0400 Date: Tue, 20 Oct 2015 15:17:45 +0200 From: Peter Zijlstra To: Kosuke Tatsukawa Cc: Ingo Molnar , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH] wait: add comment before waitqueue_active noting memory barrier is required Message-ID: <20151020131745.GF17308@twins.programming.kicks-ass.net> References: <17EC94B0A072C34B8DCF0D30AD16044A028747D0@BPXM09GP.gisp.nec.co.jp> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <17EC94B0A072C34B8DCF0D30AD16044A028747D0@BPXM09GP.gisp.nec.co.jp> User-Agent: Mutt/1.5.21 (2012-12-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Oct 09, 2015 at 12:35:59AM +0000, Kosuke Tatsukawa wrote: > This patch adds a comment before waitqueue_active noting that a memory > barrier is required. > > Besides the original problem in drivers/tty/n_tty.c which caused a > program stall (described in https://lkml.org/lkml/2015/9/28/849), there Do not use lkml.org for links in Changelogs -- preferably do _NOT_ refer to external sources but include all relevant information in the Changelog. If you have to use links, use: http://lkml.kernel.org/r/$msgid which is a stable link format. > were several other places in the linux kernel source, which calls > waitqueue_active without a memory barrier. > > blk-mq: fix waitqueue_active without memory barrier in block/blk-mq-tag.c > media: fix waitqueue_active without memory barrier in cpia2 driver > mei: fix waitqueue_active without memory barrier in mei drivers > brcmfmac: fix waitqueue_active without memory barrier in brcmfmac driver > btrfs: fix waitqueue_active without memory barrier in btrfs > sunrpc: fix waitqueue_active without memory barrier in sunrpc > ALSA: seq_oss: fix waitqueue_active without memory barrier in snd-seq-oss > kvm: fix waitqueue_active without memory barrier in virt/kvm/async_pf.c This seems ill specified and superfluous at this point. > Hopefully, the comment will make people using waitqueue_active a little > more cautious. > > Signed-off-by: Kosuke Tatsukawa > --- > include/linux/wait.h | 8 ++++++++ > 1 files changed, 8 insertions(+), 0 deletions(-) > > diff --git a/include/linux/wait.h b/include/linux/wait.h > index 1e1bf9f..e385564 100644 > --- a/include/linux/wait.h > +++ b/include/linux/wait.h > @@ -102,6 +102,14 @@ init_waitqueue_func_entry(wait_queue_t *q, wait_queue_func_t func) > q->func = func; > } > > +/* > + * Note: Some sort of memory barrier must be called before calling > + * waitqueue_active on SMP, so that any writes done prior to this > + * can be seen by other CPUs. It should very much explain _WHY_ this would be a problem. The below is logically separate from the previous, so a new paragraph is useful. > Also, since waitqueue_active will > + * return 0 even when the queue is locked, the waiter must ensure > + * that a memory barrier is called after add_wait_queue, so that > + * following reads don't get moved up before the queue has changed. And this just doesn't parse at all. It also doesn't fully explain why that is a problem.