linux-kernel - Re: [PATCH 1/2 v4] cleanup: Add cond

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240215102642.000067c5@Huawei.com>
Date: Thu, 15 Feb 2024 10:26:42 +0000
From: Jonathan Cameron <Jonathan.Cameron@...wei.com>
To: "Fabio M. De Francesco" <fabio.maria.de.francesco@...ux.intel.com>
CC: Peter Zijlstra <peterz@...radead.org>, Dan Williams
	<dan.j.williams@...el.com>, <linux-kernel@...r.kernel.org>,
	<linux-cxl@...r.kernel.org>, Ingo Molnar <mingo@...nel.org>, Dave Jiang
	<dave.jiang@...el.com>, Ira Weiny <ira.weiny@...el.com>
Subject: Re: [PATCH 1/2 v4] cleanup: Add cond_guard() to conditional guards

On Wed, 14 Feb 2024 18:04:52 +0000
Jonathan Cameron <Jonathan.Cameron@...wei.com> wrote:

> On Tue, 13 Feb 2024 17:51:26 +0100
> "Fabio M. De Francesco" <fabio.maria.de.francesco@...ux.intel.com> wrote:
> 
> > On Thursday, 8 February 2024 14:04:23 CET Fabio M. De Francesco wrote:  
> > > Add cond_guard() macro to conditional guards.
> > > 
> > > cond_guard() is a guard to be used with the conditional variants of locks,
> > > like down_read_trylock() or mutex_lock_interruptible().
> > > 
> > > It takes a statement (or statement-expression) that is passed as its
> > > second argument. That statement (or statement-expression) is executed if
> > > waiting for a lock is interrupted or if a _trylock() fails in case of
> > > contention.
> > > 
> > > Usage example:
> > > 
> > > 	cond_guard(mutex_intr, return -EINTR, &mutex);
> > > 
> > > Consistent with other usage of _guard(), locks are unlocked at the exit of
> > > the scope where cond_guard() is called.
> > >     
> > [snip]  
> > > 
> > > +#define cond_guard(_name, _fail, args...) \
> > > +	CLASS(_name, scope)(args); \
> > > +	if (!__guard_ptr(_name)(&scope)) _fail; \
> > > +	else { }
> > > +    
> > 
> > I have converted and tested several functions in drivers/cxl and found that 
> > there are cases where this macro needs to be called twice in the same scope.
> > 
> > The current implementation fails to compile because any subsequent call to 
> > cond_guard() redefines "scope".
> > 
> > I have a solution for this, which is to instantiate a differently named 
> > variable each time cond_guard() is used:
> > 
> > #define __UNIQUE_LINE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __LINE__)
> > #define cond_guard(_name, _fail, args...) \
> >         CLASS(_name, __UNIQUE_LINE_ID(scope))(args); \
> >         if (!__guard_ptr(_name)(&__UNIQUE_LINE_ID(scope))) _fail; \
> >         else { }
> > 
> > But, before sending v5, I think it's best to wait for comments from those with 
> > more experience than me.  
> 
> Ah. So you can't use __UNIQUE_ID as guard does because we need it to be stable
> across the two uses.  What you have looks fine to me.
> We might end up with someone putting multiple calls in a macro but in my
> view anyone doing that level of complexity in a macro is shooting themselves
> in the foot.

Thought more on this whilst cycling home.  Can you use another level
of macros in combination with __UNIQUE_ID that guard uses?
My skills with macros are very limited so I'm sure I got something wrong,
but along the lines of.

#define __cond_class(__unique, _name, _fail, args...) \
   CLASS(_name, __unique)(args); \
         if (!__guard_ptr(_name)(&__unique)) _fail; \
         else { }
#define cond_class(_name, _fail, args... ) \
    __cond_class(__UNIQUE_ID(guard), _name, _fail, args...

?

> 
> Jonathan
> 
> 
> > 
> > Fabio
> > 
> > 
> > 
> >   
> 
>