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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

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

-- 
Peter Xu





[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux