On Wed, 21 May 2025 16:27:04 +0100 Lee Jones lee@kernel.org wrote:
From: Kuniyuki Iwashima kuniyu@amazon.com
[ Upstream commit d0f6dc26346863e1f4a23117f5468614e54df064 ]
This is a prep patch for the last patch in this series so that checkpatch will not warn about BUG_ON().
Does any of this actually make any sense? Either the BUG_ON() should be just deleted because it can't happen (or doesn't matter) or there should be an error path. Blindly replacing with WARN_ON_ONCE() can't be right.
The last change (repeated here)
if (u) {
BUG_ON(!u->inflight);
BUG_ON(list_empty(&u->link));
WARN_ON_ONCE(!u->inflight);
WARN_ON_ONCE(list_empty(&u->link));
u->inflight--; if (!u->inflight)
is clearly just plain wrong. If 'inflight' is zero then 'decrementing' it to ~0 is just going to 'crash and burn' very badly not much later on.
David
Signed-off-by: Kuniyuki Iwashima kuniyu@amazon.com Acked-by: Jens Axboe axboe@kernel.dk Link: https://lore.kernel.org/r/20240129190435.57228-2-kuniyu@amazon.com Signed-off-by: Jakub Kicinski kuba@kernel.org (cherry picked from commit d0f6dc26346863e1f4a23117f5468614e54df064) Signed-off-by: Lee Jones lee@kernel.org
net/unix/garbage.c | 8 ++++---- net/unix/scm.c | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/net/unix/garbage.c b/net/unix/garbage.c index 2934d7b68036..7eeaac165e85 100644 --- a/net/unix/garbage.c +++ b/net/unix/garbage.c @@ -145,7 +145,7 @@ static void scan_children(struct sock *x, void (*func)(struct unix_sock *), /* An embryo cannot be in-flight, so it's safe * to use the list link. */
BUG_ON(!list_empty(&u->link));
} spin_unlock(&x->sk_receive_queue.lock);WARN_ON_ONCE(!list_empty(&u->link)); list_add_tail(&u->link, &embryos);
@@ -224,8 +224,8 @@ static void __unix_gc(struct work_struct *work) total_refs = file_count(sk->sk_socket->file);
BUG_ON(!u->inflight);
BUG_ON(total_refs < u->inflight);
WARN_ON_ONCE(!u->inflight);
if (total_refs == u->inflight) { list_move_tail(&u->link, &gc_candidates); __set_bit(UNIX_GC_CANDIDATE, &u->gc_flags);WARN_ON_ONCE(total_refs < u->inflight);
@@ -318,7 +318,7 @@ static void __unix_gc(struct work_struct *work) list_move_tail(&u->link, &gc_inflight_list); /* All candidates should have been detached by now. */
- BUG_ON(!list_empty(&gc_candidates));
- WARN_ON_ONCE(!list_empty(&gc_candidates));
/* Paired with READ_ONCE() in wait_for_unix_gc(). */ WRITE_ONCE(gc_in_progress, false); diff --git a/net/unix/scm.c b/net/unix/scm.c index 693817a31ad8..6f446dd2deed 100644 --- a/net/unix/scm.c +++ b/net/unix/scm.c @@ -50,10 +50,10 @@ void unix_inflight(struct user_struct *user, struct file *fp) if (u) { if (!u->inflight) {
BUG_ON(!list_empty(&u->link));
} else {WARN_ON_ONCE(!list_empty(&u->link)); list_add_tail(&u->link, &gc_inflight_list);
BUG_ON(list_empty(&u->link));
} u->inflight++; /* Paired with READ_ONCE() in wait_for_unix_gc() */WARN_ON_ONCE(list_empty(&u->link));
@@ -70,8 +70,8 @@ void unix_notinflight(struct user_struct *user, struct file *fp) spin_lock(&unix_gc_lock);