Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 14 Jul 2023 11:44:06 -0700
From:   "Paul E. McKenney" <paulmck@kernel.org>
To:     Alan Huang <mmpgouride@gmail.com>
Cc:     Joel Fernandes <joel@joelfernandes.org>,
        Gao Xiang <hsiangkao@linux.alibaba.com>,
        Sandeep Dhavale <dhavale@google.com>,
        Frederic Weisbecker <frederic@kernel.org>,
        Neeraj Upadhyay <quic_neeraju@quicinc.com>,
        Josh Triplett <josh@joshtriplett.org>,
        Boqun Feng <boqun.feng@gmail.com>,
        Steven Rostedt <rostedt@goodmis.org>,
        Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
        Lai Jiangshan <jiangshanlai@gmail.com>,
        Zqiang <qiang.zhang1211@gmail.com>,
        Matthias Brugger <matthias.bgg@gmail.com>,
        AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@collabora.com>,
        linux-erofs@lists.ozlabs.org, xiang@kernel.org,
        Will Shiu <Will.Shiu@mediatek.com>, kernel-team@android.com,
        rcu@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org,
        linux-mediatek@lists.infradead.org
Subject: Re: [PATCH v1] rcu: Fix and improve RCU read lock checks when
 !CONFIG_DEBUG_LOCK_ALLOC
Message-ID: <453bcbbd-94f4-46da-98f6-c9fa5f931231@paulmck-laptop>
Reply-To: paulmck@kernel.org
References: <f1c60dcb-e32f-7b7e-bf0d-5dec999e9299@linux.alibaba.com>
 <CAEXW_YSODXRfgkR0D2G-x=0uZdsqvF3kZL+LL3DcRX-5CULJ1Q@mail.gmail.com>
 <894a3b64-a369-7bc6-c8a8-0910843cc587@linux.alibaba.com>
 <CAEXW_YSM1yik4yWTgZoxCS9RM6TbsL26VCVCH=41+uMA8chfAQ@mail.gmail.com>
 <58b661d0-0ebb-4b45-a10d-c5927fb791cd@paulmck-laptop>
 <7d433fac-a62d-4e81-b8e5-57cf5f2d1d55@paulmck-laptop>
 <F160D7F8-57DC-4986-90A9-EB50F7C89891@gmail.com>
 <6E5326AD-9A5D-4570-906A-BDE8257B6F0C@gmail.com>
 <e8ee7006-c1d0-4c04-bd25-0f518fb6534b@paulmck-laptop>
 <D042B1CB-2ED4-4DF9-8CF5-5E455E7EAB73@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <D042B1CB-2ED4-4DF9-8CF5-5E455E7EAB73@gmail.com>
Precedence: bulk

On Sat, Jul 15, 2023 at 02:40:08AM +0800, Alan Huang wrote:
> > 2023年7月15日 01:02，Paul E. McKenney <paulmck@kernel.org> 写道：
> > On Fri, Jul 14, 2023 at 11:54:47PM +0800, Alan Huang wrote:
> >>> 2023年7月14日 23:35，Alan Huang <mmpgouride@gmail.com> 写道：
> >>>> 2023年7月14日 10:16，Paul E. McKenney <paulmck@kernel.org> 写道：

[ . . . ]

> >>>>> How can this be solved?
> >>>>> 
> >>>>> 1. Always use a workqueue.  Simple, but is said to have performance
> >>>>> issues.
> >>>>> 
> >>>>> 2. Pass a flag in that indicates whether or not the caller is in an
> >>>>> RCU read-side critical section.  Conceptually simple, but might
> >>>>> or might not be reasonable to actually implement in the code as
> >>>>> it exists now. (You tell me!)
> >>>>> 
> >>>>> 3. Create a function in z_erofs that gives you a decent
> >>>>> approximation, maybe something like the following.
> >>>>> 
> >>>>> 4. Other ideas here.
> >>>> 
> >>>> 5. #3 plus make the corresponding Kconfig option select
> >>>> PREEMPT_COUNT, assuming that any users needing compression in
> >>>> non-preemptible kernels are OK with PREEMPT_COUNT being set.
> >>>> (Some users of non-preemptible kernels object strenuously
> >>>> to the added overhead from CONFIG_PREEMPT_COUNT=y.)
> >>> 
> >>> 6. Set one bit in bio->bi_private, check the bit and flip it in rcu_read_lock() path,
> >>> then in z_erofs_decompressqueue_endio, check if the bit has changed.
> >> 
> >> Seems bad, read and modify bi_private is a bad idea.
> > 
> > Is there some other field that would work?
> 
> Maybe bio->bi_opf, btrfs uses some bits of it.

Sandeep, thoughts?


							Thanx, Paul

> >>> Not sure if this is feasible or acceptable. :)
> >>> 
> >>>> 
> >>>> Thanx, Paul
> >>>> 
> >>>>> The following is untested, and is probably quite buggy, but it should
> >>>>> provide you with a starting point.
> >>>>> 
> >>>>> static bool z_erofs_wq_needed(void)
> >>>>> {
> >>>>> if (IS_ENABLED(CONFIG_PREEMPTION) && rcu_preempt_depth())
> >>>>> return true;  // RCU reader
> >>>>> if (IS_ENABLED(CONFIG_PREEMPT_COUNT) && !preemptible())
> >>>>> return true;  // non-preemptible
> >>>>> if (!IS_ENABLED(CONFIG_PREEMPT_COUNT))
> >>>>> return true;  // non-preeemptible kernel, so play it safe
> >>>>> return false;
> >>>>> }
> >>>>> 
> >>>>> You break it, you buy it!  ;-)
> >>>>> 
> >>>>> Thanx, Paul
> 
>