From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.ilvokhin.com (mail.ilvokhin.com [178.62.254.231]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A6F303B813D for ; Tue, 12 May 2026 14:37:30 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=178.62.254.231 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1778596652; cv=none; b=MxuJUmujK/Al4jfLIlBV6jnWSNqfylickgV85carIqu7EWGa42LIZ7f5png0Lqdz996dV9A5FvjYgdUvmdmFfC54MRd1WUFUTlKWvD1CYYvmUrLlW5m6o0+26sAGhnfDnTQOJP1SrySdqDnszzDXkw00PPytrLfn8n4SNieMQ2I= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1778596652; c=relaxed/simple; bh=DR0IOGwlH9wtTWgc8AcvWqHspSqG1E3XZmTOTGiDCGY=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=Mr4g9iy/WJ25F0ufeKhQiid5KXsDuk3BnPsF0stgopWCiLGopScRWofcowY/peKKAHsf+bgDYpn+He0gfvA5FJC7Tqm+DGlVmlfNim0Pr1ptmDGkLxTs78/mxRf6Hg0CYbFh+IE+lzjVCHnfXUCf1SfB1xzyBNQazR8DQyPjoek= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=ilvokhin.com; spf=pass smtp.mailfrom=ilvokhin.com; dkim=pass (1024-bit key) header.d=ilvokhin.com header.i=@ilvokhin.com header.b=oxzVwwmU; arc=none smtp.client-ip=178.62.254.231 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=ilvokhin.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=ilvokhin.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=ilvokhin.com header.i=@ilvokhin.com header.b="oxzVwwmU" Received: from shell.ilvokhin.com (shell.ilvokhin.com [138.68.190.75]) (Authenticated sender: d@ilvokhin.com) by mail.ilvokhin.com (Postfix) with ESMTPSA id 8429AD041B; Tue, 12 May 2026 14:37:28 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ilvokhin.com; s=mail; t=1778596648; bh=MW3E0b/P8o/cx8sVzzjLUdmNXwzTnFCf0K7UbXufGbc=; h=Date:From:To:Cc:Subject:References:In-Reply-To; b=oxzVwwmULOSd0cxBnZNVR3SvxbVbPIUBUE9BWe9ujkDkT/QBd/YkYGERr6HDDSlvs PPiKnNvb82q/+tz7/y1EXV0IDluzQ1vGQzzk9pnLdBrdp2P9bJ85a5u+IJEJ95XzjN XwkuqMz4pYAtUB7Fqiuf0nJUdLtz9tY5AQLh2wFA= Date: Tue, 12 May 2026 14:37:25 +0000 From: Dmitry Ilvokhin To: Peter Zijlstra Cc: Christian Brauner , Dan Williams , Dave Jiang , Marco Elver , "H. Peter Anvin" , Andrew Morton , linux-mm@kvack.org, linux-kernel@vger.kernel.org, kernel-team@meta.com Subject: Re: [PATCH v2] cleanup: Remove NULL check from unconditional guards Message-ID: References: <20260512071510.92451-1-d@ilvokhin.com> <20260512124557.GD1889694@noisy.programming.kicks-ass.net> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260512124557.GD1889694@noisy.programming.kicks-ass.net> On Tue, May 12, 2026 at 02:45:57PM +0200, Peter Zijlstra wrote: > > What about class_irqdesc_lock_constructor() ? AFAICT > __irq_get_desc_lock() can return NULL. Thanks for taking a look, Peter. Yes, that is actually a very good catch. For some reason I naively thought __DEFINE_UNLOCK_GUARD() wouldn't be used outside of include/linux/cleanup.h, but this is obviously wrong assumption. There are cases, where DEFINE_LOCK_GUARD_1() doesn't fit callers needs, so __DEFINE_UNLOCK_GUARD() is used directly. - kernel/irq/internals.h: the case you pointed out. Can be fixed by moving the NULL check into the irqdesc_lock unlock expression directly. - include/linux/tty_port.h: similar use case. NULL check can be moved into tty_port_tty as well, similar to previous case. - kernel/sched/sched.h: lock and lock2 shouldn't be NULL at destructor time, since they are dereferenced unconditionally at constructor. Below is example how this will look like. Does this look reasonable to you, or would you prefer a different approach? >From 835d6c13caf8169db7bd049c9c549d5988fd0a82 Mon Sep 17 00:00:00 2001 From: Dmitry Ilvokhin Date: Tue, 12 May 2026 06:59:04 -0700 Subject: [PATCH] cleanup: Move NULL check into conditional guard unlock expressions irqdesc_lock and tty_port_tty use __DEFINE_UNLOCK_GUARD() directly with custom constructors that can set .lock to NULL. Move the NULL check from the generic __DEFINE_UNLOCK_GUARD() destructor into their _unlock expressions, making the NULL handling explicit at the call site. Signed-off-by: Dmitry Ilvokhin --- include/linux/tty_port.h | 2 +- kernel/irq/internals.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/linux/tty_port.h b/include/linux/tty_port.h index d2a7882c0b58..cbbff6aabdad 100644 --- a/include/linux/tty_port.h +++ b/include/linux/tty_port.h @@ -286,7 +286,7 @@ static inline void tty_port_tty_vhangup(struct tty_port *port) #ifdef CONFIG_TTY void tty_kref_put(struct tty_struct *tty); __DEFINE_CLASS_IS_CONDITIONAL(tty_port_tty, true); -__DEFINE_UNLOCK_GUARD(tty_port_tty, struct tty_struct, tty_kref_put(_T->lock)); +__DEFINE_UNLOCK_GUARD(tty_port_tty, struct tty_struct, if (_T->lock) tty_kref_put(_T->lock)); static inline class_tty_port_tty_t class_tty_port_tty_constructor(struct tty_port *tport) { class_tty_port_tty_t _t = { diff --git a/kernel/irq/internals.h b/kernel/irq/internals.h index 9412e57056f5..347cb333b9fe 100644 --- a/kernel/irq/internals.h +++ b/kernel/irq/internals.h @@ -171,7 +171,7 @@ void __irq_put_desc_unlock(struct irq_desc *desc, unsigned long flags, bool bus) __DEFINE_CLASS_IS_CONDITIONAL(irqdesc_lock, true); __DEFINE_UNLOCK_GUARD(irqdesc_lock, struct irq_desc, - __irq_put_desc_unlock(_T->lock, _T->flags, _T->bus), + if (_T->lock) __irq_put_desc_unlock(_T->lock, _T->flags, _T->bus), unsigned long flags; bool bus); static inline class_irqdesc_lock_t class_irqdesc_lock_constructor(unsigned int irq, bool bus, -- 2.53.0-Meta