From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932481AbdCGOq2 (ORCPT ); Tue, 7 Mar 2017 09:46:28 -0500 Received: from mail-ua0-f169.google.com ([209.85.217.169]:33990 "EHLO mail-ua0-f169.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755721AbdCGOoF (ORCPT ); Tue, 7 Mar 2017 09:44:05 -0500 MIME-Version: 1.0 In-Reply-To: <20170307142740.uh2nnaw44albn3t2@tardis> References: <20170304204052.GC30506@linux.vnet.ibm.com> <20170305184736.GD30506@linux.vnet.ibm.com> <20170306100741.GJ30506@linux.vnet.ibm.com> <20170306230800.GK30506@linux.vnet.ibm.com> <20170307142740.uh2nnaw44albn3t2@tardis> From: Dmitry Vyukov Date: Tue, 7 Mar 2017 15:43:42 +0100 Message-ID: Subject: Re: rcu: WARNING in rcu_seq_end To: Boqun Feng Cc: Paul McKenney , josh@joshtriplett.org, Steven Rostedt , Mathieu Desnoyers , jiangshanlai@gmail.com, LKML , syzkaller Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Mar 7, 2017 at 3:27 PM, Boqun Feng wrote: > On Tue, Mar 07, 2017 at 08:05:19AM +0100, Dmitry Vyukov wrote: > [...] >> >> >> >> What is that mutex? And what locks/unlocks provide synchronization? I >> >> see that one uses exp_mutex and another -- exp_wake_mutex. >> > >> > Both of them. >> > >> > ->exp_mutex is acquired by the task requesting the grace period, and >> > the counter's first increment is done by that task under that mutex. >> > This task then schedules a workqueue, which drives forward the grace >> > period. Upon grace-period completion, the workqueue handler does the >> > second increment (the one that your patch addressed). The workqueue >> > handler then acquires ->exp_wake_mutex and wakes the task that holds >> > ->exp_mutex (along with all other tasks waiting for this grace period), >> > and that task releases ->exp_mutex, which allows the next grace period to >> > start (and the first increment for that next grace period to be carried >> > out under that lock). The workqueue handler releases ->exp_wake_mutex >> > after finishing its wakeups. >> >> >> Then we need the following for the case when task requesting the grace >> period does not block, right? >> > > Won't be necessary I think, as the smp_mb() in rcu_seq_end() and the > smp_mb__before_atomic() in sync_exp_work_done() already provide the > required ordering, no? smp_mb() is probably fine, but smp_mb__before_atomic() is release not acquire. If we want to play that game, then I guess we also need smp_mb__after_atomic() there. But it would be way easier to understand what's happens there and prove that it's correct, if we use store_release/load_acquire.