Received-SPF: pass (google.com: domain of linux-kernel+bounces-45066-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Date: Tue, 30 Jan 2024 09:58:25 -0800
From: Dan Williams <dan.j.williams@intel.com>
To: "Fabio M. De Francesco" <fabio.maria.de.francesco@linux.intel.com>,
	<linux-kernel@vger.kernel.org>, Dan Williams <dan.j.williams@intel.com>
CC: 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: <65b938c1ad435_5cc6f294eb@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>
 <22169722.EfDdHjke4D@fdefranc-mobl3>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <22169722.EfDdHjke4D@fdefranc-mobl3>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?WxHO3BeBslceLsvPOzqlflXuINXv6iFcIuFm2au2k4GSzqZFqGvfXzNGbI3w?=
 =?us-ascii?Q?uGlAfvDSdbNnF6V2j/6/MDTeOJKDeweyOmAL+KK9aYzMT+9Zi5NJAs5BHjeI?=
 =?us-ascii?Q?tUzizL/IbRQoDj8CE4lc4cHfpmd1mSSPAXM2GJjRjMxo4fdafMWXPyy0LoP3?=
 =?us-ascii?Q?Ig4QgLx+6v8Ma2slL4ORFdBtWbFnWE9PArBE4Z/Z8GQ6zj9lJ0QmwKH/47nu?=
 =?us-ascii?Q?ErB+hZVqfdFGny/Ei7dZkwe/WwMWWgtn6Ez+QKuNH67WDt4868ljF9BydSEk?=
 =?us-ascii?Q?4/FVoOL4q+bqD7Qy/SPZbFil/loMGlcrOrOErcS5mZ6nC1dE2M7aOvzGYPPm?=
 =?us-ascii?Q?MkaXsLMSckeIxry6ce9p3UOER3P7gPrAGPeZuNixy0c75X0gewH2tFUzaGH+?=
 =?us-ascii?Q?92k5YaAM8/jg5mLxsx5XghuApuXc9SL/ZQhgsd10uUxLwXalf6H2nHtbq3V2?=
 =?us-ascii?Q?WuWTg4fqloFO8yZ44Osao6wJ8s1MGFGhF4NVeU2Sv+7bW3fxXlQy/B5p3GG5?=
 =?us-ascii?Q?DHBK6gZCnWsTPKSKsBJEOzGkQ2dw9eW9jrnLuuXsDpfKgFq8jPNC5ue7OhDu?=
 =?us-ascii?Q?K0+8gyJ/5Pbdhze++h4dUt3hImZ3azNVLbtmXwNIg2/9rvWBMb2gk/KhluKn?=
 =?us-ascii?Q?z7AJS/z9bgJ6Kv3PcqiLjSS4enWMpJMZgmp4IK23Gg1L2ajHaH89sVVlbE4d?=
 =?us-ascii?Q?b0laT1q5t5cKp13prV9VMBFYmyk7D/fltCSqvflkvqLQmbWYkH+isvPuu4IR?=
 =?us-ascii?Q?DP9CyJIONq/UmOo9WCbD0kqSKXVdnr1mULXDAUsJ1M5EDMroD2AEtPb/EtcF?=
 =?us-ascii?Q?MBE1RiDtFj8EgfDmPUOFwvJZySbHT2gHRtrveNnLEyK5MUI92yWhpZBXlZZV?=
 =?us-ascii?Q?spmF2DlC8evSC7jqNtrk2fnFalApW8ypCCwigED5D8p2AoPP6sAGWpNntyDO?=
 =?us-ascii?Q?axQUam1EfOqvPiEvWsn8nP3IlUYZU+fMTPBd1+FrzbRcaEAkW8M789n4xesw?=
 =?us-ascii?Q?WC+/w9IDR9HrW5m3H/8PQgWA3Umbujb24hrbtGkW8Y5S+aFM5SZzBzp8P/Eg?=
 =?us-ascii?Q?8GYuu3r/AjDpKrulXrl4I8lb7RlqLxqfenIEpeSW44Fy8atF+MWLAvHHffNZ?=
 =?us-ascii?Q?OIb0SPWyzY8iO1kNoEjQsJqycqq1Bk8vcBR35zhbqKPVZiZwc/KSO/yGNadQ?=
 =?us-ascii?Q?DNqz1oWczkjRC02X1AIY8Bm5HC5UfVYeXOxkxlIvobUSrv4iE64EQfPrcDVA?=
 =?us-ascii?Q?ngM8IblSuYWgZGV4kSMRh3dq0b7otnxuQOwcjvem8QVMwZdAtYt7dnJRyiuf?=
 =?us-ascii?Q?hG13V60WdoAOqal9yTrteo9RQr++3YYU+IZB59ywoQ9D8TOnXiIQKzWFLgnO?=
 =?us-ascii?Q?KlYSrnPzlue+fVKIlslZd+DuJ+EO8BQfj5cn0kJu8KhCmjbMdaeD5jTgzYyq?=
 =?us-ascii?Q?1MPYP3TdIJzyHAcN/r5JH3YMI8g38mi+zc7k8OYUfYSrgCoenu46ZBxm9Wkv?=
 =?us-ascii?Q?2xi76kM4poOF2dc5bdkcXAnab/r5i6eqen6eC4Zn38pdcIMgptAbc1L8p2Bg?=
 =?us-ascii?Q?/toE8b1v8+dUNCWOn5epJHHnGUQWwKAjf3jtwImNUuXoK114Gj0sbna2O32V?=
 =?us-ascii?Q?hQ=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 4542f544-87fa-45f4-997a-08dc21bd0fe2
X-MS-Exchange-CrossTenant-AuthSource: PH8PR11MB8107.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Jan 2024 17:58:28.0737
 (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: pHd5ikCsiGiOrQ/R+FoooN2v8ohxDbFWBZozWY9L5k81NmgT67Q1/LSSL1H1gtrClGUPolYbgUWytkhFHS+UgFZLk5B+VJLVLCOlSA8st28=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA1PR11MB7296
X-OriginatorOrg: intel.com

Fabio M. De Francesco wrote:
> On Tuesday, 30 January 2024 18:02:09 CET 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...
> 
> As you propose I can't see how to handle multi-line error path like in:
> 
> 	cond_guard(...) {
> 		dev_dbg(...);
> 		return -EINTR;
> 	} 

The _fail argument is a statement, to make it a compound statement maybe just
add braces, something like:

    cond_guard(..., { dev_dbg(...); return -EINTR; }, ...)

..another possibility is something like 

    int rc = 0;

    cond_guard(..., rc = -EINTR, ...)
    if (rc) {
        ...
        return rc;
    }

..so, I don't think we need separate macros for the multi-statement
case.