On Sat, Jul 15, 2023 at 02:40:08AM +0800, Alan Huang wrote:
> > 2023年7月15日 01:02，Paul E. McKenney <paulmck@xxxxxxxxxx> 写道：
> > On Fri, Jul 14, 2023 at 11:54:47PM +0800, Alan Huang wrote:
> >>> 2023年7月14日 23:35，Alan Huang <mmpgouride@xxxxxxxxx> 写道：
> >>>> 2023年7月14日 10:16，Paul E. McKenney <paulmck@xxxxxxxxxx> 写道：
[ . . . ]
> >>>>> 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
>
>
