On Wed, Apr 12, 2023 at 2:57 AM Qi Zheng <zhengqi.arch@xxxxxxxxxxxxx> wrote: > > > > On 2023/4/12 14:44, Zhang, Qiang1 wrote: [..] > > Maybe no need to convert ->list_lock to raw_spinlock. > > > > --- a/lib/debugobjects.c > > +++ b/lib/debugobjects.c > > @@ -562,10 +562,10 @@ __debug_object_init(void *addr, const struct debug_obj_descr *descr, int onstack > > unsigned long flags; > > > > /* > > - * On RT enabled kernels the pool refill must happen in preemptible > > + * The pool refill must happen in preemptible > > * context: > > */ > > - if (!IS_ENABLED(CONFIG_PREEMPT_RT) || preemptible()) > > + if (preemptible()) > > fill_pool(); > > > > db = get_bucket((unsigned long) addr); > > Ah, this does fix the warning I was encountered! Actually fill_pool() should be safe to call on !CONFIG_PREEMPT_RT kernels as it is GFP_ATOMIC, however with the above change, that goes away just to satisfy a false-positive report. Because now all !preemptible() sections on !CONFIG_PREEMPT_RT kernels cannot call fill_pool(), right? So you will not end up filling the pool when it is safe to do so? I think it would be better to fix PROVE_LOCKING / CONFIG_PREEMPT_RT instead of degrading !CONFIG_PREEMPT_RT just to satisfy a false-positive report. +Steven Rostedt as well. thanks, - Joel > > > > > > > > > Thanks > > Zqiang > > > >> > >> > >> Regards, > >> Boqun > >> > >>>> > >>>> It's indeed unfortunate for the warning in the commit message. But > >>>> functions like kmem_cache_alloc(GFP_ATOMIC) may indeed be called > >>>> in the critical section of raw_spinlock or in the hardirq context, which > >>> > >>> Hmm, I thought they may not, actually. > >>> > >>>> will cause problem in the PREEMPT_RT kernel. So I still think it is > >>>> reasonable to convert kmem_cache_node->list_lock to raw_spinlock type. > >>> > >>> It wouldn't be the complete solution anyway. Once we allow even a GFP_ATOMIC > >>> slab allocation for such context, it means also page allocation can happen > >>> to refill the slabs, so lockdep will eventually complain about zone->lock, > >>> and who knows what else. > >> > >> Oh, indeed. :( > >> > >>> > >>>> In addition, there are many fix patches for this kind of warning in the > >>>> git log, so I also think there should be a general and better solution. :) > >>> > >>> Maybe, but given above, I doubt it's this one. > >>> > >>>> > >>>>> > >>>> > >>> > >> > >> -- > >> Thanks, > >> Qi > > -- > Thanks, > Qi
