[PATCH v6 1/7] bits: split the definition of the asm and non-asm GENMASK()

Vincent Mailhol via B4 Relay posted 7 patches 11 months, 1 week ago
There is a newer version of this series
[PATCH v6 1/7] bits: split the definition of the asm and non-asm GENMASK()
Posted by Vincent Mailhol via B4 Relay 11 months, 1 week ago
From: Vincent Mailhol <mailhol.vincent@wanadoo.fr>

In an upcoming change, GENMASK() and its friends will indirectly
depend on sizeof() which is not available in asm.

Instead of adding further complexity to __GENMASK() to make it work
for both asm and non asm, just split the definition of the two
variants.

Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
---
Changelog:

  v5 -> v6:

    - Restore the comment saying that BUILD_BUG_ON() is not available in asm
      code.

  v4 -> v5:

    - Use tab indentations instead of single space to separate the
      macro name from its body.

  v3 -> v4:

    - New patch in the series
---
 include/linux/bits.h | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/include/linux/bits.h b/include/linux/bits.h
index 14fd0ca9a6cd17339dd2f69e449558312a8a001b..9c1c7ce0bba6bb09490d891904c143a5394fd512 100644
--- a/include/linux/bits.h
+++ b/include/linux/bits.h
@@ -19,23 +19,17 @@
  * GENMASK_ULL(39, 21) gives us the 64bit vector 0x000000ffffe00000.
  */
 #if !defined(__ASSEMBLY__)
+
 #include <linux/build_bug.h>
 #include <linux/compiler.h>
+
 #define GENMASK_INPUT_CHECK(h, l) BUILD_BUG_ON_ZERO(const_true((l) > (h)))
-#else
-/*
- * BUILD_BUG_ON_ZERO is not available in h files included from asm files,
- * disable the input check if that is the case.
- */
-#define GENMASK_INPUT_CHECK(h, l) 0
-#endif
 
 #define GENMASK(h, l) \
 	(GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
 #define GENMASK_ULL(h, l) \
 	(GENMASK_INPUT_CHECK(h, l) + __GENMASK_ULL(h, l))
 
-#if !defined(__ASSEMBLY__)
 /*
  * Missing asm support
  *
@@ -48,6 +42,16 @@
  */
 #define GENMASK_U128(h, l) \
 	(GENMASK_INPUT_CHECK(h, l) + __GENMASK_U128(h, l))
-#endif
+
+#else /* defined(__ASSEMBLY__) */
+
+/*
+ * BUILD_BUG_ON_ZERO() is not available in h files included from asm files, so
+ * no input checks in assembly.
+ */
+#define GENMASK(h, l)		__GENMASK(h, l)
+#define GENMASK_ULL(h, l)	__GENMASK_ULL(h, l)
+
+#endif /* !defined(__ASSEMBLY__) */
 
 #endif	/* __LINUX_BITS_H */

-- 
2.45.3
Re: [PATCH v6 1/7] bits: split the definition of the asm and non-asm GENMASK()
Posted by Andy Shevchenko 11 months, 1 week ago
On Sat, Mar 08, 2025 at 01:48:48AM +0900, Vincent Mailhol via B4 Relay wrote:
> From: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
> 
> In an upcoming change, GENMASK() and its friends will indirectly
> depend on sizeof() which is not available in asm.
> 
> Instead of adding further complexity to __GENMASK() to make it work
> for both asm and non asm, just split the definition of the two
> variants.

...

> -/*
> - * BUILD_BUG_ON_ZERO is not available in h files included from asm files,
> - * disable the input check if that is the case.
> - */

> +/*
> + * BUILD_BUG_ON_ZERO() is not available in h files included from asm files, so
> + * no input checks in assembly.
> + */

In case of a new version I would reformat this as

/*
 * BUILD_BUG_ON_ZERO() is not available in h files included from asm files,
 * so no input checks in assembly.
 */

It makes easier to review the changes and see that the first line is kept
the same.

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v6 1/7] bits: split the definition of the asm and non-asm GENMASK()
Posted by Vincent Mailhol 11 months, 1 week ago
On 08/03/2025 at 02:42, Andy Shevchenko wrote:
> On Sat, Mar 08, 2025 at 01:48:48AM +0900, Vincent Mailhol via B4 Relay wrote:
>> From: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
>>
>> In an upcoming change, GENMASK() and its friends will indirectly
>> depend on sizeof() which is not available in asm.
>>
>> Instead of adding further complexity to __GENMASK() to make it work
>> for both asm and non asm, just split the definition of the two
>> variants.
> 
> ...
> 
>> -/*
>> - * BUILD_BUG_ON_ZERO is not available in h files included from asm files,
>> - * disable the input check if that is the case.
>> - */
> 
>> +/*
>> + * BUILD_BUG_ON_ZERO() is not available in h files included from asm files, so
>> + * no input checks in assembly.
>> + */
> 
> In case of a new version I would reformat this as
> 
> /*
>  * BUILD_BUG_ON_ZERO() is not available in h files included from asm files,
>  * so no input checks in assembly.
>  */
> 
> It makes easier to review the changes and see that the first line is kept
> the same.

Not fully convinced, but why not. I staged this change locally, it will
be in v7.


Yours sincerely,
Vincent Mailhol
Re: [PATCH v6 1/7] bits: split the definition of the asm and non-asm GENMASK()
Posted by Yury Norov 10 months, 4 weeks ago
On Sat, Mar 08, 2025 at 06:10:25PM +0900, Vincent Mailhol wrote:
> On 08/03/2025 at 02:42, Andy Shevchenko wrote:
> > On Sat, Mar 08, 2025 at 01:48:48AM +0900, Vincent Mailhol via B4 Relay wrote:
> >> From: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
> >>
> >> In an upcoming change, GENMASK() and its friends will indirectly
> >> depend on sizeof() which is not available in asm.
> >>
> >> Instead of adding further complexity to __GENMASK() to make it work
> >> for both asm and non asm, just split the definition of the two
> >> variants.
> > 
> > ...
> > 
> >> -/*
> >> - * BUILD_BUG_ON_ZERO is not available in h files included from asm files,
> >> - * disable the input check if that is the case.
> >> - */
> > 
> >> +/*
> >> + * BUILD_BUG_ON_ZERO() is not available in h files included from asm files, so
> >> + * no input checks in assembly.
> >> + */
> > 
> > In case of a new version I would reformat this as
> > 
> > /*
> >  * BUILD_BUG_ON_ZERO() is not available in h files included from asm files,
> >  * so no input checks in assembly.
> >  */
> > 
> > It makes easier to review the changes and see that the first line is kept
> > the same.
> 
> Not fully convinced, but why not. I staged this change locally, it will
> be in v7.

I don't understand why this change is needed at all. The comment
states the same thing before and after, but the history gets wiped.
Maybe just don't touch it?
Re: [PATCH v6 1/7] bits: split the definition of the asm and non-asm GENMASK()
Posted by Vincent Mailhol 10 months, 4 weeks ago
On 19/03/2025 at 01:06, Yury Norov wrote:
> On Sat, Mar 08, 2025 at 06:10:25PM +0900, Vincent Mailhol wrote:
>> On 08/03/2025 at 02:42, Andy Shevchenko wrote:
>>> On Sat, Mar 08, 2025 at 01:48:48AM +0900, Vincent Mailhol via B4 Relay wrote:
>>>> From: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
>>>>
>>>> In an upcoming change, GENMASK() and its friends will indirectly
>>>> depend on sizeof() which is not available in asm.
>>>>
>>>> Instead of adding further complexity to __GENMASK() to make it work
>>>> for both asm and non asm, just split the definition of the two
>>>> variants.
>>>
>>> ...
>>>
>>>> -/*
>>>> - * BUILD_BUG_ON_ZERO is not available in h files included from asm files,
>>>> - * disable the input check if that is the case.
>>>> - */
>>>
>>>> +/*
>>>> + * BUILD_BUG_ON_ZERO() is not available in h files included from asm files, so
>>>> + * no input checks in assembly.
>>>> + */
>>>
>>> In case of a new version I would reformat this as
>>>
>>> /*
>>>  * BUILD_BUG_ON_ZERO() is not available in h files included from asm files,
>>>  * so no input checks in assembly.
>>>  */
>>>
>>> It makes easier to review the changes and see that the first line is kept
>>> the same.
>>
>> Not fully convinced, but why not. I staged this change locally, it will
>> be in v7.
> 
> I don't understand why this change is needed at all. The comment
> states the same thing before and after, but the history gets wiped.
> Maybe just don't touch it?

Ack. I will just move the text down and not rephrase.


Yours sincerely,
Vincent Mailhol