Received-SPF: pass (google.com: domain of linux-kernel+bounces-45164-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Tue, 30 Jan 2024 11:06:00 -0800
From: Dan Williams <dan.j.williams@intel.com>
To: Ira Weiny <ira.weiny@intel.com>, Dan Williams <dan.j.williams@intel.com>,
	"Fabio M. De Francesco" <fabio.maria.de.francesco@linux.intel.com>,
	<linux-kernel@vger.kernel.org>
CC: "Fabio M. De Francesco" <fabio.maria.de.francesco@linux.intel.com>, "Peter
 Zijlstra" <peterz@infradead.org>, Dan Williams <dan.j.williams@intel.com>,
	"Ira Weiny" <ira.weiny@intel.com>
Subject: Re: [RFC PATCH v2] cleanup: Add cond_guard() to conditional guards
Message-ID: <65b948987e865_5cc6f294df@dwillia2-mobl3.amr.corp.intel.com.notmuch>
References: <20240130164059.25130-1-fabio.maria.de.francesco@linux.intel.com>
 <65b92b91a41a8_5cc6f29484@dwillia2-mobl3.amr.corp.intel.com.notmuch>
 <65b9434712b57_2d455e294a8@iweiny-mobl.notmuch>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <65b9434712b57_2d455e294a8@iweiny-mobl.notmuch>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?Vvr7mqvDeBXibp50vIjraiOgLniXP77bkxR0pFPVfyEsPZ7C59q802ygDPST?=
 =?us-ascii?Q?0w3l3PPTxwwGJKRtP7fidHi3NiXTj2wkYujGN9AuD0UWgzHABVFKjjxpCQdG?=
 =?us-ascii?Q?Oq05kOYgvkJCnq7RBnOFmOAxJo1xvMnE7tvnwIcAckVNGW7uko/fTU3l1wZI?=
 =?us-ascii?Q?SJZyfOlEvTVZW1BUg3Iw4Cn/OfeKZ5RlSOXjCYsCfdsNdh8a5I8+r3Qc612T?=
 =?us-ascii?Q?dAwAtrQ36ElXlSAPL/5fQP1BFFacRKu+9K06kYXd0CmgB/0nXZ2c2CB12S+W?=
 =?us-ascii?Q?hjKhZqIwyDWhIvGvUHjirqX/5HDvA/buH0jJZPSfbDvzdvOtsVMwR5Eb6Om0?=
 =?us-ascii?Q?vtWjuwagsDMiTdkjf/4xiizz+J9ZZSAmG1AbIAfS4caIy+zdwSG/Jx2J9kUA?=
 =?us-ascii?Q?I4SQ+FARRuuS2so+qb7RmnWqB9UErKkK3T02tsbF1bSPS7wTX3ZN8o4bKs9D?=
 =?us-ascii?Q?uTFXdFfM8JLJFtzJiJCaGXACCZl7A1ix6cDSOLo7SLUzXQQgd4IJ61TUpOTU?=
 =?us-ascii?Q?kdMhdzKnfsKZwd8PmEURQswVHuV2OklD9m2wkTxm6KZ6xFyQmGTPcS60LV5R?=
 =?us-ascii?Q?kgVa5q80+QTFzYfNuvLtq/7kl0vp3xLUONlLzYC35vbz63XDA1rrc9V0BYBy?=
 =?us-ascii?Q?zWmwFNT4MCp+3RToYmGnJQOvdM9Rh4S0zd9MvcbUeqJevyNUM9zkr5+GG7+L?=
 =?us-ascii?Q?BI2N93sCHyXmfq5kjyfi/Z0zb1WuixPvbAgi6PAsyDS9XcoantID3m6eexss?=
 =?us-ascii?Q?vmlzImBbRDYBE/Sww1YsqbhqN8oYXZ5zom+DES3YAMddSFlL0/u889oxR6rL?=
 =?us-ascii?Q?jNW03pAfcDAK0Nk3px6ho04GsCrL7Ok72qk7Nhvt8qTmxkL3ss6CtOmH0P88?=
 =?us-ascii?Q?HN4gqZERhhmvGYBX/7oBtLvGLlIDkyoyFpgkM63iKjDgTbV6UVzDwU+woQOZ?=
 =?us-ascii?Q?2JeMN3d7dT8o42nRZPVXwaPnmjohfFO6T5VX+ZzjfgI2fsd/a4cHQh12huaF?=
 =?us-ascii?Q?iMHhfx0bRI/YJIsGkFzPswo32Pa275fTnIA8ZvLPwXICBuAyZGU5xlS2OjkX?=
 =?us-ascii?Q?cJ/Mq2ertRBGK1o8hfwNDds9WsE3phg7MVYz4apULA5NcJ8D7hYok367uiyw?=
 =?us-ascii?Q?PUB7rhe8L4JbEtlr63BnkIDp4+MoqcHrZCrA+YBY7TxsDToC5HicGafF8Fkw?=
 =?us-ascii?Q?1qL6v5tjRZd4YMLL02E5y2F2go/Qetr46f7c8EZElSaSai/LTUyUyQ//MSwE?=
 =?us-ascii?Q?x7lkK0NmjOGq5ws5uTclhU8DNGw4HW3V61uexIwcBqYTRmyPKFYVha+VmZvF?=
 =?us-ascii?Q?yK0P2qVMi5foj8lPojQFyYHjSYA5d8L68wzTWyKXMXE8Tyi6oQUDY4+dr6ux?=
 =?us-ascii?Q?2RXC7GRjTD37b/64VW5Sr/WdVUBIzM5Ib82e1f+s+f6wQ7d8fHGSCuZb7d4w?=
 =?us-ascii?Q?j/9lmB5XItwjoL+vZaQs0vFIqLKCRcVYDfBWSKU+inDjMBPJwVfV7tCImjuh?=
 =?us-ascii?Q?t5EkFmAQXP9l80DArTwGI7h+JylFku/i8dQTJo+p0vu1uJBq3EbAQ/qIlDjK?=
 =?us-ascii?Q?0hUy1R2G/yLclHoggqmLEL+SO5mvqCb2a2ny06msRQtKIeQFoLQpYA3ppWDA?=
 =?us-ascii?Q?lQ=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 44e90ebf-fac2-4923-9c49-08dc21c68121
X-MS-Exchange-CrossTenant-AuthSource: PH8PR11MB8107.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Jan 2024 19:06:03.4641
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: C3Od4WxKLSxpE5VnVeBoS++CTm0/JuhBamtg00dtAqkPdsE17GLhOlEkS+SI9BS5jBwIR3/6pjujHefea0gblLljpcZyf93bq12M4WyIBmY=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR11MB7521
X-OriginatorOrg: intel.com

Ira Weiny wrote:
> Dan Williams wrote:
> > Fabio M. De Francesco wrote:
> > > Add cond_guard() to conditional guards.
> > > 
> > > cond_guard() is used for the _interruptible(), _killable(), and _try
> > > versions of locks. It expects a block where the failure can be handled
> > > (e.g., calling printk() and returning -EINTR in case of failure).
> > > 
> > > As the other guards, it avoids to open code the release of the lock
> > > after a goto to an 'out' label.
> > > 
> > > This remains an RFC because Dan suggested a slightly different syntax:
> > > 
> > > 	if (cond_guard(...))
> > > 		return -EINTR;
> > > 
> > > But the scoped_cond_guard() macro omits the if statement:
> > > 
> > >     	scoped_cond_guard (...) {
> > >     	}
> > > 
> > > Thus define cond_guard() similarly to scoped_cond_guard() but with a block
> > > to handle the failure case:
> > > 
> > > 	cond_guard(...)
> > > 		return -EINTR;
> > 
> > That's too subtle for me, because of the mistakes that can be made with
> > brackets how about a syntax like:
> > 
> >  	cond_guard(..., return -EINTR, ...)
> > 
> > ...to make it clear what happens if the lock acquisition fails without
> > having to remember there is a hidden incomplete "if ()" statement in
> > that macro? More below...
> 
> I sympathize with the hidden "if" being confusing but there is already
> precedent in the current *_guard macros.  So I'd like to know if Peter has
> an opinion.

What are you asking specifically? The current scoped_cond_guard()
already properly encapsulates the "if ()" and takes an "_fail" so why
wouldn't cond_guard() also safely encpsulate an "if ()" and take an
"_fail" statement argument?