Re: [PATCH v2 2/4] userfaultfd: remove (VM_)BUG_ON()s

[David Hildenbrand]

On 10.06.25 15:11, Peter Xu wrote:
> On Sat, Jun 07, 2025 at 02:40:01AM -0400, Tal Zussman wrote:
> > BUG_ON() is deprecated [1]. Convert all the BUG_ON()s and VM_BUG_ON()s
> > to use VM_WARN_ON_ONCE().
> >
> > While at it, also convert the WARN_ON_ONCE()s in move_pages() to use
> > VM_WARN_ON_ONCE(), as the relevant conditions are already checked in
> > validate_range() in move_pages()'s caller.
> >
> > [1] https://www.kernel.org/doc/html/v6.15/process/coding-style.html#use-warn-rather-than-bug
> >
> > Signed-off-by: Tal Zussman <tz2294@xxxxxxxxxxxx>
> > ---
> >   fs/userfaultfd.c | 59 +++++++++++++++++++++++++-------------------------
> >   mm/userfaultfd.c | 66 +++++++++++++++++++++++++++-----------------------------
> >   2 files changed, 61 insertions(+), 64 deletions(-)
> >
> > diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
> > index 22f4bf956ba1..80c95c712266 100644
> > --- a/fs/userfaultfd.c
> > +++ b/fs/userfaultfd.c
> > @@ -165,14 +165,14 @@ static void userfaultfd_ctx_get(struct userfaultfd_ctx *ctx)
> >   static void userfaultfd_ctx_put(struct userfaultfd_ctx *ctx)
> >   {
> >   	if (refcount_dec_and_test(&ctx->refcount)) {
> > -		VM_BUG_ON(spin_is_locked(&ctx->fault_pending_wqh.lock));
> > -		VM_BUG_ON(waitqueue_active(&ctx->fault_pending_wqh));
> > -		VM_BUG_ON(spin_is_locked(&ctx->fault_wqh.lock));
> > -		VM_BUG_ON(waitqueue_active(&ctx->fault_wqh));
> > -		VM_BUG_ON(spin_is_locked(&ctx->event_wqh.lock));
> > -		VM_BUG_ON(waitqueue_active(&ctx->event_wqh));
> > -		VM_BUG_ON(spin_is_locked(&ctx->fd_wqh.lock));
> > -		VM_BUG_ON(waitqueue_active(&ctx->fd_wqh));
> > +		VM_WARN_ON_ONCE(spin_is_locked(&ctx->fault_pending_wqh.lock));
> > +		VM_WARN_ON_ONCE(waitqueue_active(&ctx->fault_pending_wqh));
> > +		VM_WARN_ON_ONCE(spin_is_locked(&ctx->fault_wqh.lock));
> > +		VM_WARN_ON_ONCE(waitqueue_active(&ctx->fault_wqh));
> > +		VM_WARN_ON_ONCE(spin_is_locked(&ctx->event_wqh.lock));
> > +		VM_WARN_ON_ONCE(waitqueue_active(&ctx->event_wqh));
> > +		VM_WARN_ON_ONCE(spin_is_locked(&ctx->fd_wqh.lock));
> > +		VM_WARN_ON_ONCE(waitqueue_active(&ctx->fd_wqh));
> >   		mmdrop(ctx->mm);
> >   		kmem_cache_free(userfaultfd_ctx_cachep, ctx);
>
> I didn't follow closely on the latest discussions on BUG_ON, but here I
> just stumbled on top of this chunk, it does look like a slight overkill
> using tons of bools for each of them.. even if the doc suggested
> WARN_ON_ONCE().
>
> David might have a better picture of what's our plan for mm to properly
> assert while reducing the overhead as much as possible.

There is currently still a discussion whether VM_WARN_ON an 
VM_WARN_ON_ONCE could be unified.

In a CONFIG_DEBUG_VM kernel, the overhead of a couple of booleans is 
usually the least concern (everything is big and slow already) :)

> For this specific one, if we really want to convert we could also merge
> them into one, so one bool to cover all.

One loses precision, but yeah, they are supposed to be found during 
early testing, in which case one can usually reproduce + debug fairly 
easily.

--
Cheers,

David / dhildenb