linux-kernel - Re: Memory barrier needed with wake_up

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <8737lh79mm.fsf@linux.intel.com>
Date:   Sat, 03 Sep 2016 09:58:09 +0300
From:   Felipe Balbi <felipe.balbi@...ux.intel.com>
To:     Peter Zijlstra <peterz@...radead.org>,
        Alan Stern <stern@...land.harvard.edu>
Cc:     "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
        Ingo Molnar <mingo@...hat.com>,
        USB list <linux-usb@...r.kernel.org>,
        Kernel development list <linux-kernel@...r.kernel.org>
Subject: Re: Memory barrier needed with wake_up_process()?


hi,

Peter Zijlstra <peterz@...radead.org> writes:
> On Fri, Sep 02, 2016 at 04:16:54PM -0400, Alan Stern wrote:
>> Felipe, your tests will show whether my guess was totally off-base.  
>> For the new people, Felipe is tracking down a problem that involves
>> exactly the code sequence listed at the top of the email, where we know
>> that the wakeup routine runs but nevertheless the task sleeps.  At
>> least, that's what it looks like at the moment.
>
> What arch are you seeing this on?

x86. Skylake to be exact.

The following change survived through the night:

diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c
index 8f3659b65f53..d31581dd5ce5 100644
--- a/drivers/usb/gadget/function/f_mass_storage.c
+++ b/drivers/usb/gadget/function/f_mass_storage.c
@@ -395,7 +395,7 @@ static int fsg_set_halt(struct fsg_dev *fsg, struct usb_ep *ep)
 /* Caller must hold fsg->lock */
 static void wakeup_thread(struct fsg_common *common)
 {
-	smp_wmb();	/* ensure the write of bh->state is complete */
+	smp_mb();	/* ensure the write of bh->state is complete */
 	/* Tell the main thread that something has happened */
 	common->thread_wakeup_needed = 1;
 	if (common->thread_task)
@@ -626,7 +626,7 @@ static int sleep_thread(struct fsg_common *common, bool can_freeze)
 	}
 	__set_current_state(TASK_RUNNING);
 	common->thread_wakeup_needed = 0;
-	smp_rmb();	/* ensure the latest bh->state is visible */
+	smp_mb();	/* ensure the latest bh->state is visible */
 	return rc;
 }
 

I'll keep it running until Monday at least

-- 
balbi