Received-SPF: pass (google.com: domain of linux-kernel+bounces-45129-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 10:43:19 -0800
From: Ira Weiny <ira.weiny@intel.com>
To: 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: <65b9434712b57_2d455e294a8@iweiny-mobl.notmuch>
References: <20240130164059.25130-1-fabio.maria.de.francesco@linux.intel.com>
 <65b92b91a41a8_5cc6f29484@dwillia2-mobl3.amr.corp.intel.com.notmuch>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <65b92b91a41a8_5cc6f29484@dwillia2-mobl3.amr.corp.intel.com.notmuch>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?UhIk6nczcGmjoVHZw+52KplTez0l4srCZ/KcfK3kfCV6nnyKt7qFg+XEdR+o?=
 =?us-ascii?Q?G3fqraHFrdBcjLRT9H1uHMpP1VRwdJpE1MbIFgMkOYB+0PJsGgslNVXzq8Sr?=
 =?us-ascii?Q?cKUm9C7nabomp/dH+8/i2eu5rp7usJervKWp5ya4Lv2gd02WcpgIkTVjEC5B?=
 =?us-ascii?Q?uRd1ELnSMRW7GM20Qe0LwggRzY+kGKla/ZQcPKfXVY5mPJ8bwxh7z0yW8stT?=
 =?us-ascii?Q?NDKG2EK2vVdCMwRwkiPCLJLiR5GZfzz/UJ/VvnOfDOO+qYCUnzQoeehtdrVu?=
 =?us-ascii?Q?7D6XcGxNGM1IzfObE1MoNg+tLPjtvgG/HnRuwZn7uiuUVLmSGXiPEUgsnStG?=
 =?us-ascii?Q?5RPaubRVMpYzaW3RiFrsmMjlpTYLVNsynGF2Bc9yz9SXKNT6HbgG95ftTAp7?=
 =?us-ascii?Q?vd1+0FGrBHaUzeoieBazNohgO0C7iPJcLgWMWlNgQgU1UX83yeUDqLK2bZQK?=
 =?us-ascii?Q?Pz5xiF5tOG1A/C/a9Yjy6eLpMoh9lWkHeXsyXucXGXWVT2gNXdWX4G7iQEF9?=
 =?us-ascii?Q?wxxAyfErtLWDTkEwc6AqHBHRVO4b6i5WJEV9H9+nHl0LPGkyOfdxvDNCRtDz?=
 =?us-ascii?Q?spYFoZ2SyLkVsPEcy/2uKbpyzOzXAPCPAgb2aWvJQNZNiGvesXF00ILMeeA7?=
 =?us-ascii?Q?liu+5fT0u/UE99dgZVwox3oodFkrCOM8s4xFU5v5edhUvAmsX2u/ftO0+D/c?=
 =?us-ascii?Q?mzCUfa7uwBmuJvYgNaTYFa0IcyhvuBkCfhbBB03w14WY+qCKKanBHQ4Zqem4?=
 =?us-ascii?Q?6PIJbM8pFoH2L9hJOCuQSePKPDOehijYdmQ662Hvg8bEpNFyl7NXGcd4GLAq?=
 =?us-ascii?Q?Pupop1V5Gb/1NhhQkQZNWZtlOZUD7MVifsjYNttdWhMMyyta4bt7K6Uv4J9S?=
 =?us-ascii?Q?ORrIx+JhG85dRfJX3jOvDrQj7CrnlKqsqg/HYVTqGo93n6a0p9ZN13FVjffv?=
 =?us-ascii?Q?XIVjU7GJcPvZ3kBsny27f0RW8+OLx3sYGf+dZj9y/uygU8Z67Vl0gzeuiDek?=
 =?us-ascii?Q?PYyR2KiOClcA9XhTVYhYyj7wK3699S20eKJtDzMGIjelqy4snmOaiu+vxZOL?=
 =?us-ascii?Q?0A6+uAgRc5L1ptmsY8428Uatdp/VEMka5oJyu5gqb+NPngqeQrVaisTXjM01?=
 =?us-ascii?Q?j9AWoxqlXhta4esT4s9/xfHnJ4jIdOHI8HObo5qf4W0e47DBOTfUxzJ3W6hP?=
 =?us-ascii?Q?HjMUJ2sLNSLHqQpRD019VrryhoxhmDike+XinSb4eJPvo4HFlOzjiCnS0SpA?=
 =?us-ascii?Q?X/70NJGlm7ci2WyGlqPcfR7NSaAWBezfnTKrjHYevfftjt/OPYB3p97N55jU?=
 =?us-ascii?Q?/EAdPHSN8ChGl6iWdb3zLMlgByYsCTGBhTiBsQeC73xb7QOiahEDCQAZ/MDs?=
 =?us-ascii?Q?sWOUTx46yXEN+V5jbnUUqSHQ/aExgyP1mx8qhFbTXM/XHBEyXOjeo7UtN/JM?=
 =?us-ascii?Q?/rsDqbS1R/s8uuPQ20C+bZqkZQoO5e1/fCj+hCXKl5gh6x6Y73tHVYKUc3tT?=
 =?us-ascii?Q?GzJ50hKJUKAaleIvKhHKwusJik/o0OA2WAyb2ukPYjnXRHsvF7zKwqrZMVcj?=
 =?us-ascii?Q?QibL8cVJarFS9l/15iDlfhu6Zy7728BkwLaialF+?=
X-MS-Exchange-CrossTenant-Network-Message-Id: a05fe1dd-c767-4f67-3801-08dc21c35698
X-MS-Exchange-CrossTenant-AuthSource: SA1PR11MB6733.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Jan 2024 18:43:23.6633
 (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: v7GAzN5sKj872Xmc+1oQFgXovW726FJDHvNPKggT5k7ntUt6CVgGSVm5BECj0dWuQyssD9mOyx31RPS4sIBOwA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR11MB5983
X-OriginatorOrg: intel.com

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.

Ira