Subject: [PATCH] [PATCH] time: tick-sched: use bool for tick_stopped

From: John Zhao <[email protected]>

Variable tick_stopped returned by tick_nohz_tick_stopped can only be true / false values. Since the return type of the tick_nohz_tick_stopped is also bool, variable tick_stopped nice to have data type as 'bool' in place of the 'unsigned int'.
Moreover, the executed instructions cost could be minimal without potiential data type conversion.

Signed-off-by John Zhao <[email protected]>

---
 kernel/time/tick-sched.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

kernel/time/tick-sched.h +1 -1

index 6de959a..4d34309 100644
@@ -48,8 +48,8 @@ struct tick_sched {
 	unsigned long			check_clocks;
 	enum tick_nohz_mode		nohz_mode;
 
+	bool				tick_stopped	: 1;
 	unsigned int			inidle		: 1;
-	unsigned int			tick_stopped	: 1;
 	unsigned int			idle_active	: 1;
 	unsigned int			do_timer_last	: 1;
 	unsigned int			got_idle_tick	: 1;

From: John Zhao <[email protected]>

Variable tick_stopped returned by tick_nohz_tick_stopped can have only true / forse values. Since the return type of the tick_nohz_tick_stopped is also bool, variable tick_stopped nice to have data type as bool in place of int.

Quoted 6 lines · On Tue, 10 Apr 2018, [email protected] wrote:

From: John Zhao <[email protected]>

Variable tick_stopped returned by tick_nohz_tick_stopped can have only true / forse values. Since the return type of the tick_nohz_tick_stopped is also bool, variable tick_stopped nice to have data type as bool in place of int.

The data type is not int.

It's part of an integer bitfield and occupies
exactly one bit of storage, while bool has an architecture dependend size and is at least 1 byte, i.e. 8 bit. So with alignment effects you grew the size of the data structure and therefore the cache foot print.

This is not about 'nice to have' ....

Thanks,

tglx

From: John Zhao <[email protected]>

Variable tick_stopped returned by tick_nohz_tick_stopped can have only true / forse values. Since the return type of the tick_nohz_tick_stopped is also bool, variable tick_stopped nice to have data type as bool in place of int. Moreover, the executed instructions cost could be minimal without potiential data type conversion.

Signed-off-by John Zhao <[email protected]>

---
 kernel/time/tick-sched.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

kernel/time/tick-sched.h +0 -0

index 6de959a..4d34309 100644

Quoted 19 lines · On Tue, Apr 10, 2018 at 9:33 AM, <[email protected]> wrote:

From: John Zhao <[email protected]>

Variable tick_stopped returned by tick_nohz_tick_stopped can have only true / forse values. Since the return type of the tick_nohz_tick_stopped is also bool, variable tick_stopped nice to have data type as bool in place of int. Moreover, the executed instructions cost could be minimal without potiential data type conversion.

Signed-off-by John Zhao <[email protected]>

---
 kernel/time/tick-sched.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

kernel/time/tick-sched.h +0 -0

index 6de959a..4d34309 100644

kernel/time/tick-sched.h struct tick_sched { · line 55

Quoted 9 lines

kernel/time/tick-sched.h +1 -1

@@ -48,8 +48,8 @@ struct tick_sched {
        unsigned long                   check_clocks;
        enum tick_nohz_mode             nohz_mode;
 
+       bool                            tick_stopped    : 1;
        unsigned int                    inidle          : 1;
-       unsigned int                    tick_stopped    : 1;
        unsigned int                    idle_active     : 1;
        unsigned int                    do_timer_last   : 1;
        unsigned int                    got_idle_tick   : 1;

I don't think this is a good idea at all.

Please see https://lkml.org/lkml/2017/11/21/384 for example.

Thanks!

Quoted 32 lines · Rafael J. Wysocki

Quoted 19 lines · On Tue, Apr 10, 2018 at 9:33 AM, <[email protected]> wrote:

From: John Zhao <[email protected]>

Variable tick_stopped returned by tick_nohz_tick_stopped can have only true / forse values. Since the return type of the tick_nohz_tick_stopped is also bool, variable tick_stopped nice to have data type as bool in place of int. Moreover, the executed instructions cost could be minimal without potiential data type conversion.

Signed-off-by John Zhao <[email protected]>

---
 kernel/time/tick-sched.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

kernel/time/tick-sched.h +0 -0

index 6de959a..4d34309 100644

kernel/time/tick-sched.h struct tick_sched { · line 55

Quoted 9 lines

kernel/time/tick-sched.h +1 -1

@@ -48,8 +48,8 @@ struct tick_sched {
        unsigned long                   check_clocks;
        enum tick_nohz_mode             nohz_mode;
 
+       bool                            tick_stopped    : 1;
        unsigned int                    inidle          : 1;
-       unsigned int                    tick_stopped    : 1;
        unsigned int                    idle_active     : 1;
        unsigned int                    do_timer_last   : 1;
        unsigned int                    got_idle_tick   : 1;

I don't think this is a good idea at all.

Quoted 1 line

Please see https://lkml.org/lkml/2017/11/21/384 for example.

[ZJ] Thanks for this sharing. Looks like, this patch fall into the case of "Maybe".

Quoted 33 lines · On Tue, 10 Apr 2018, [email protected] wrote:

Quoted 32 lines · Rafael J. Wysocki

Quoted 19 lines · On Tue, Apr 10, 2018 at 9:33 AM, <[email protected]> wrote:

From: John Zhao <[email protected]>

Variable tick_stopped returned by tick_nohz_tick_stopped can have only true / false values. Since the return type of the tick_nohz_tick_stopped is also bool, variable tick_stopped nice to have data type as bool in place of unsigned int. Moreover, the executed instructions cost could be minimal without potiential data type conversion.

Signed-off-by John Zhao <[email protected]>

---
 kernel/time/tick-sched.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

kernel/time/tick-sched.h +0 -0

index 6de959a..4d34309 100644

kernel/time/tick-sched.h struct tick_sched { · line 55

Quoted 9 lines

kernel/time/tick-sched.h +1 -1

@@ -48,8 +48,8 @@ struct tick_sched {
        unsigned long                   check_clocks;
        enum tick_nohz_mode             nohz_mode;
 
+       bool                            tick_stopped    : 1;
        unsigned int                    inidle          : 1;
-       unsigned int                    tick_stopped    : 1;
        unsigned int                    idle_active     : 1;
        unsigned int                    do_timer_last   : 1;
        unsigned int                    got_idle_tick   : 1;

I don't think this is a good idea at all.

Quoted 3 lines

Quoted 1 line

Please see https://lkml.org/lkml/2017/11/21/384 for example.

[ZJ] Thanks for this sharing. Looks like, this patch fall into the case of "Maybe".

This patch falls into the case 'pointless' because it adds extra storage

for no benefit at all.

Thanks,

tglx

Quoted 34 lines · Thomas Gleixner

Quoted 33 lines · On Tue, 10 Apr 2018, [email protected] wrote:

Quoted 32 lines · Rafael J. Wysocki

Quoted 19 lines · On Tue, Apr 10, 2018 at 9:33 AM, <[email protected]> wrote:

From: John Zhao <[email protected]>

Variable tick_stopped returned by tick_nohz_tick_stopped can have only true / false values. Since the return type of the tick_nohz_tick_stopped is also bool, variable tick_stopped nice to have data type as bool in place of unsigned int. Moreover, the executed instructions cost could be minimal without potiential data type conversion.

Signed-off-by John Zhao <[email protected]>

---
 kernel/time/tick-sched.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

kernel/time/tick-sched.h +0 -0

index 6de959a..4d34309 100644

kernel/time/tick-sched.h struct tick_sched { · line 55

Quoted 9 lines

kernel/time/tick-sched.h +1 -1

@@ -48,8 +48,8 @@ struct tick_sched {
        unsigned long                   check_clocks;
        enum tick_nohz_mode             nohz_mode;
 
+       bool                            tick_stopped    : 1;
        unsigned int                    inidle          : 1;
-       unsigned int                    tick_stopped    : 1;
        unsigned int                    idle_active     : 1;
        unsigned int                    do_timer_last   : 1;
        unsigned int                    got_idle_tick   : 1;

I don't think this is a good idea at all.

Quoted 5 lines

Quoted 3 lines

Quoted 1 line

Please see https://lkml.org/lkml/2017/11/21/384 for example.

[ZJ] Thanks for this sharing. Looks like, this patch fall into the case of "Maybe".

This patch falls into the case 'pointless' because it adds extra storage

[ZJ] 1 bit vs 1 bit. no more.

Quoted 1 line

for no benefit at all.

[ZJ] tick_stopped is returned by the tick_nohz_tick_stopped() which is bool. The benefit is no any potiential type conversion could be minded.

Quoted 35 lines · On Tue, 10 Apr 2018, [email protected] wrote:

Quoted 34 lines · Thomas Gleixner

Quoted 33 lines · On Tue, 10 Apr 2018, [email protected] wrote:

Quoted 32 lines · Rafael J. Wysocki

Quoted 19 lines · On Tue, Apr 10, 2018 at 9:33 AM, <[email protected]> wrote:

From: John Zhao <[email protected]>

Variable tick_stopped returned by tick_nohz_tick_stopped can have only true / false values. Since the return type of the tick_nohz_tick_stopped is also bool, variable tick_stopped nice to have data type as bool in place of unsigned int. Moreover, the executed instructions cost could be minimal without potiential data type conversion.

Signed-off-by John Zhao <[email protected]>

---
 kernel/time/tick-sched.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

kernel/time/tick-sched.h +0 -0

index 6de959a..4d34309 100644

kernel/time/tick-sched.h struct tick_sched { · line 55

Quoted 9 lines

kernel/time/tick-sched.h +1 -1

@@ -48,8 +48,8 @@ struct tick_sched {
        unsigned long                   check_clocks;
        enum tick_nohz_mode             nohz_mode;
 
+       bool                            tick_stopped    : 1;
        unsigned int                    inidle          : 1;
-       unsigned int                    tick_stopped    : 1;
        unsigned int                    idle_active     : 1;
        unsigned int                    do_timer_last   : 1;
        unsigned int                    got_idle_tick   : 1;

I don't think this is a good idea at all.

Quoted 6 lines

Quoted 5 lines

Quoted 3 lines

Quoted 1 line

Please see https://lkml.org/lkml/2017/11/21/384 for example.

[ZJ] Thanks for this sharing. Looks like, this patch fall into the case of "Maybe".

This patch falls into the case 'pointless' because it adds extra storage

[ZJ] 1 bit vs 1 bit. no more.

Groan. No. Care to look at the data structure? You create a new storage,

which is incidentally merged into the other bitfield by the compiler at a different bit position, but there is no guarantee that a compiler does that. It's free to use distinct storage for that bool based bit.

Quoted 3 lines

Quoted 1 line

Quoted 1 line

for no benefit at all.

[ZJ] tick_stopped is returned by the tick_nohz_tick_stopped() which is bool. The benefit is no any potiential type conversion could be minded.

A bit stays a bit. 'bool foo : 1;' or 'unsigned int foo : 1' has to be evaluated as a bit. So there is a type conversion from BIT to bool required because BIT != bool.

By chance the evaluation can be done by evaluating the byte in which the bit is placed just because the compiler knows that the remaining bits are not used. There is no guarantee that this is done, it happens to be true for a particular compiler.

But that does not make it any more interesting. It just makes the code harder to read and eventually leads to bigger storage.

Thanks,

tglx

Quoted 36 lines · Thomas Gleixner

Quoted 35 lines · On Tue, 10 Apr 2018, [email protected] wrote:

Quoted 34 lines · Thomas Gleixner

Quoted 33 lines · On Tue, 10 Apr 2018, [email protected] wrote:

Quoted 32 lines · Rafael J. Wysocki

Quoted 19 lines · On Tue, Apr 10, 2018 at 9:33 AM, <[email protected]> wrote:

From: John Zhao <[email protected]>

Variable tick_stopped returned by tick_nohz_tick_stopped can have only true / false values. Since the return type of the tick_nohz_tick_stopped is also bool, variable tick_stopped nice to have data type as bool in place of unsigned int. Moreover, the executed instructions cost could be minimal without potiential data type conversion.

Signed-off-by John Zhao <[email protected]>

---
 kernel/time/tick-sched.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

kernel/time/tick-sched.h +0 -0

index 6de959a..4d34309 100644

kernel/time/tick-sched.h struct tick_sched { · line 55

Quoted 9 lines

kernel/time/tick-sched.h +1 -1

@@ -48,8 +48,8 @@ struct tick_sched {
        unsigned long                   check_clocks;
        enum tick_nohz_mode             nohz_mode;
 
+       bool                            tick_stopped    : 1;
        unsigned int                    inidle          : 1;
-       unsigned int                    tick_stopped    : 1;
        unsigned int                    idle_active     : 1;
        unsigned int                    do_timer_last   : 1;
        unsigned int                    got_idle_tick   : 1;

I don't think this is a good idea at all.

Quoted 8 lines

Quoted 6 lines

Quoted 5 lines

Quoted 3 lines

Quoted 1 line

Please see https://lkml.org/lkml/2017/11/21/384 for example.

[ZJ] Thanks for this sharing. Looks like, this patch fall into the case of "Maybe".

This patch falls into the case 'pointless' because it adds extra storage

[ZJ] 1 bit vs 1 bit. no more.

Groan. No. Care to look at the data structure? You create a new storage,

[ZJ] Say, {unsigned int, unsigned int, unsigned int, unsigned int, unsigned int} becomes
{bool , unsigned int, unsigned int, unsigned int,
unsigned int}
As specified by the rule No.10 at the section 6.7.2.1 of C99 TC2 as:
"If enough space remains, a bit-field that immediately follows another bit-field in a
structure shall be packed into adjacent bits of the same unit." What is the new storage so far?

Quoted 3 lines

which is incidentally merged into the other bitfield by the compiler at a different bit position, but there is no guarantee that a compiler does that. It's free to use distinct storage for that bool based bit.

[ZJ] Per the rule No.10 at section 6.7.2.1 of C99 TC2 as:
" If insufficient space remains, whether a bit-field that does not fit is put into
the next unit or overlaps adjacent units is implementation-defined." So, implementation is never mind which type will be stored if any.

Quoted 7 lines

Quoted 3 lines

Quoted 1 line

Quoted 1 line

for no benefit at all.

[ZJ] tick_stopped is returned by the tick_nohz_tick_stopped() which is bool. The benefit is no any potiential type conversion could be minded.

A bit stays a bit. 'bool foo : 1;' or 'unsigned int foo : 1' has to be evaluated as a bit. So there is a type conversion from BIT to bool required because BIT != bool.

[ZJ] Per the rule No.9 at section 6.7.2.1 of C99 TC2 as:
"If the value 0 or 1 is stored into a nonzero-width bit-field of types
_Bool, the value of the bit-field shall compare equal to the value stored." Obviously, it is nothing related to type conversion actually.

Quoted 5 lines

By chance the evaluation can be done by evaluating the byte in which the bit is placed just because the compiler knows that the remaining bits are not used. There is no guarantee that this is done, it happens to be true for a particular compiler.

[ZJ] Actually, such as GCC owe that kind of guarantee to be promised by ABI.

Quoted 3 lines

But that does not make it any more interesting. It just makes the code harder to read and eventually leads to bigger storage.

[ZJ] To get the benctifit to be profiled, it is given as:
number of instructions of function tick_nohz_tick_stopped():

original: 17
patched: 14
Which was saved is:
movzbl %al, %eax
testl %eax, %eax
setne %al
Say, 3 / 17 = 17 % could be gained in the instruction executed
for this function can be evaluated.

Note:
The environment I used is:
OS : Ubuntu Desktop 16.04 LTS
gcc: 6.3.0 (without optimization
for in general purpose)

Quoted 1 line

Typo...

Quoted 81 lines · On 2018-04-10 10:08 PM, [email protected] wrote:

Quoted 36 lines · Thomas Gleixner

Quoted 35 lines · On Tue, 10 Apr 2018, [email protected] wrote:

Quoted 34 lines · Thomas Gleixner

Quoted 33 lines · On Tue, 10 Apr 2018, [email protected] wrote:

Quoted 32 lines · Rafael J. Wysocki

Quoted 19 lines · On Tue, Apr 10, 2018 at 9:33 AM, <[email protected]> wrote:

From: John Zhao <[email protected]>

Variable tick_stopped returned by tick_nohz_tick_stopped can have only true / false values. Since the return type of the tick_nohz_tick_stopped is also bool, variable tick_stopped nice to have data type as bool in place of unsigned int. Moreover, the executed instructions cost could be minimal without potiential data type conversion.

Signed-off-by John Zhao <[email protected]>

---
 kernel/time/tick-sched.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

kernel/time/tick-sched.h +0 -0

index 6de959a..4d34309 100644

kernel/time/tick-sched.h struct tick_sched { · line 55

Quoted 9 lines

kernel/time/tick-sched.h +1 -1

@@ -48,8 +48,8 @@ struct tick_sched {
        unsigned long                   check_clocks;
        enum tick_nohz_mode             nohz_mode;
 
+       bool                            tick_stopped    : 1;
        unsigned int                    inidle          : 1;
-       unsigned int                    tick_stopped    : 1;
        unsigned int                    idle_active     : 1;
        unsigned int                    do_timer_last   : 1;
        unsigned int                    got_idle_tick   : 1;

I don't think this is a good idea at all.

Quoted 8 lines

Quoted 6 lines

Quoted 5 lines

Quoted 3 lines

Quoted 1 line

Please see https://lkml.org/lkml/2017/11/21/384 for example.

[ZJ] Thanks for this sharing. Looks like, this patch fall into the case of "Maybe".

This patch falls into the case 'pointless' because it adds extra storage

[ZJ] 1 bit vs 1 bit. no more.

Groan. No. Care to look at the data structure? You create a new storage,

[ZJ] Say, {unsigned int, unsigned int, unsigned int, unsigned int, unsigned int} becomes
{bool , unsigned int, unsigned int, unsigned int,
unsigned int}
As specified by the rule No.10 at the section 6.7.2.1 of C99 TC2 as:
"If enough space remains, a bit-field that immediately follows another bit-field in a
structure shall be packed into adjacent bits of the same unit." What is the new storage so far?

Quoted 3 lines

which is incidentally merged into the other bitfield by the compiler at a different bit position, but there is no guarantee that a compiler does that. It's free to use distinct storage for that bool based bit.

[ZJ] Per the rule No.10 at section 6.7.2.1 of C99 TC2 as:
" If insufficient space remains, whether a bit-field that does not fit is put into
the next unit or overlaps adjacent units is implementation-defined." So, implementation is never mind which type will be stored if any.

Quoted 7 lines

Quoted 3 lines

Quoted 1 line

Quoted 1 line

for no benefit at all.

[ZJ] tick_stopped is returned by the tick_nohz_tick_stopped() which is bool. The benefit is no any potiential type conversion could be minded.

A bit stays a bit. 'bool foo : 1;' or 'unsigned int foo : 1' has to be evaluated as a bit. So there is a type conversion from BIT to bool required because BIT != bool.

[ZJ] Per the rule No.9 at section 6.7.2.1 of C99 TC2 as:
"If the value 0 or 1 is stored into a nonzero-width bit-field of types
_Bool, the value of the bit-field shall compare equal to the value stored." Obviously, it is nothing related to type conversion actually.

Quoted 5 lines

By chance the evaluation can be done by evaluating the byte in which the bit is placed just because the compiler knows that the remaining bits are not used. There is no guarantee that this is done, it happens to be true for a particular compiler.

Quoted 6 lines

[ZJ] Actually, such as GCC owe that kind of guarantee to be promised by ABI.

Quoted 3 lines

But that does not make it any more interesting. It just makes the code harder to read and eventually leads to bigger storage.

[ZJ] To get the benctifit to be profiled, it is given as:
number of instructions of function tick_nohz_tick_stopped():

[ZJ] Here, I used is not the tick_nohz_tick_stopped(), but an evaluation() as:
#include <stdio.h>
#include <stdbool.h>

struct tick_sched {
        unsigned int inidle             : 1;
        unsigned int tick_stopped       : 1;
};

bool get_status()

{
        struct tick_sched *ts;
        ts->tick_stopped = 1;
        return ts->tick_stopped;
}

int main()

{
        if (get_status()) return 0;
        return 0;
}

[ZJ] Toggle the declaration of tick_stopped in side of the tick_sched structure for comparison.

Quoted 9 lines

original: 17
patched: 14
Which was saved is:
movzbl %al, %eax
testl %eax, %eax
setne %al
Say, 3 / 17 = 17 % could be gained in the instruction executed
for this function can be evaluated.

Quoted 7 lines

Note:
The environment I used is:
OS : Ubuntu Desktop 16.04 LTS
gcc: 6.3.0 (without optimization
for in general purpose)

Quoted 1 line

Just FYI.

Thanks,
ZJ

++

Quoted 137 lines · On 2018-04-10 10:49 PM, [email protected] wrote:

Typo...

Quoted 81 lines · On 2018-04-10 10:08 PM, [email protected] wrote:

Quoted 36 lines · Thomas Gleixner

Quoted 35 lines · On Tue, 10 Apr 2018, [email protected] wrote:

Quoted 34 lines · Thomas Gleixner

Quoted 33 lines · On Tue, 10 Apr 2018, [email protected] wrote:

Quoted 32 lines · Rafael J. Wysocki

Quoted 19 lines · On Tue, Apr 10, 2018 at 9:33 AM, <[email protected]> wrote:

From: John Zhao <[email protected]>

Variable tick_stopped returned by tick_nohz_tick_stopped can have only true / false values. Since the return type of the tick_nohz_tick_stopped is also bool, variable tick_stopped nice to have data type as bool in place of unsigned int. Moreover, the executed instructions cost could be minimal without potiential data type conversion.

Signed-off-by John Zhao <[email protected]>

---
 kernel/time/tick-sched.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

kernel/time/tick-sched.h +0 -0

index 6de959a..4d34309 100644

kernel/time/tick-sched.h struct tick_sched { · line 55

Quoted 9 lines

kernel/time/tick-sched.h +1 -1

@@ -48,8 +48,8 @@ struct tick_sched {
        unsigned long                   check_clocks;
        enum tick_nohz_mode             nohz_mode;
 
+       bool                            tick_stopped    : 1;
        unsigned int                    inidle          : 1;
-       unsigned int                    tick_stopped    : 1;
        unsigned int                    idle_active     : 1;
        unsigned int                    do_timer_last   : 1;
        unsigned int                    got_idle_tick   : 1;

I don't think this is a good idea at all.

Quoted 8 lines

Quoted 6 lines

Quoted 5 lines

Quoted 3 lines

Quoted 1 line

Please see https://lkml.org/lkml/2017/11/21/384 for example.

[ZJ] Thanks for this sharing. Looks like, this patch fall into the case of "Maybe".

This patch falls into the case 'pointless' because it adds extra storage

[ZJ] 1 bit vs 1 bit. no more.

Groan. No. Care to look at the data structure? You create a new storage,

[ZJ] Say, {unsigned int, unsigned int, unsigned int, unsigned int, unsigned int} becomes
{bool , unsigned int, unsigned int, unsigned int,
unsigned int}
As specified by the rule No.10 at the section 6.7.2.1 of C99 TC2 as:
"If enough space remains, a bit-field that immediately follows another bit-field in a
structure shall be packed into adjacent bits of the same unit." What is the new storage so far?

Quoted 3 lines

which is incidentally merged into the other bitfield by the compiler at a different bit position, but there is no guarantee that a compiler does that. It's free to use distinct storage for that bool based bit.

[ZJ] Per the rule No.10 at section 6.7.2.1 of C99 TC2 as:
" If insufficient space remains, whether a bit-field that does not fit is put into
the next unit or overlaps adjacent units is implementation-defined." So, implementation is never mind which type will be stored if any.

Quoted 7 lines

Quoted 3 lines

Quoted 1 line

Quoted 1 line

for no benefit at all.

[ZJ] tick_stopped is returned by the tick_nohz_tick_stopped() which is bool. The benefit is no any potiential type conversion could be minded.

A bit stays a bit. 'bool foo : 1;' or 'unsigned int foo : 1' has to be evaluated as a bit. So there is a type conversion from BIT to bool required because BIT != bool.

[ZJ] Per the rule No.9 at section 6.7.2.1 of C99 TC2 as:
"If the value 0 or 1 is stored into a nonzero-width bit-field of types
_Bool, the value of the bit-field shall compare equal to the value stored." Obviously, it is nothing related to type conversion actually.

Quoted 5 lines

By chance the evaluation can be done by evaluating the byte in which the bit is placed just because the compiler knows that the remaining bits are not used. There is no guarantee that this is done, it happens to be true for a particular compiler.

Quoted 6 lines

[ZJ] Actually, such as GCC owe that kind of guarantee to be promised by ABI.

Quoted 3 lines

But that does not make it any more interesting. It just makes the code harder to read and eventually leads to bigger storage.

[ZJ] To get the benctifit to be profiled, it is given as:
number of instructions of function tick_nohz_tick_stopped():

[ZJ] Here, I used is not the tick_nohz_tick_stopped(), but an evaluation() as:
#include <stdio.h>
#include <stdbool.h>

struct tick_sched {
        unsigned int inidle             : 1;
        unsigned int tick_stopped       : 1;
};

bool get_status()

{
        struct tick_sched *ts;
        ts->tick_stopped = 1;
        return ts->tick_stopped;
}

int main()

{
        if (get_status()) return 0;
        return 0;
}

[ZJ] Toggle the declaration of tick_stopped in side of the tick_sched structure for comparison.

Quoted 9 lines

original: 17
patched: 14
Which was saved is:
movzbl %al, %eax
testl %eax, %eax
setne %al
Say, 3 / 17 = 17 % could be gained in the instruction executed
for this function can be evaluated.

Quoted 7 lines

Note:
The environment I used is:
OS : Ubuntu Desktop 16.04 LTS
gcc: 6.3.0 (without optimization
for in general purpose)

Quoted 1 line

Just FYI.

Thanks,
ZJ

++

Quoted 60 lines · On 2018-04-11 07:09 AM, [email protected] wrote:

++

Quoted 57 lines · On 2018-04-10 10:49 PM, [email protected] wrote:

Typo...

Quoted 54 lines · On 2018-04-10 10:08 PM, [email protected] wrote:

Quoted 36 lines · Thomas Gleixner

Quoted 35 lines · On Tue, 10 Apr 2018, [email protected] wrote:

Quoted 34 lines · Thomas Gleixner

Quoted 33 lines · On Tue, 10 Apr 2018, [email protected] wrote:

Quoted 32 lines · Rafael J. Wysocki

Quoted 19 lines · On Tue, Apr 10, 2018 at 9:33 AM, <[email protected]> wrote:

From: John Zhao <[email protected]>

Variable tick_stopped returned by tick_nohz_tick_stopped can have only true / false values. Since the return type of the tick_nohz_tick_stopped is also bool, variable tick_stopped nice to have data type as bool in place of unsigned int. Moreover, the executed instructions cost could be minimal without potiential data type conversion.

Signed-off-by John Zhao <[email protected]>

---
 kernel/time/tick-sched.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

kernel/time/tick-sched.h +0 -0

index 6de959a..4d34309 100644

kernel/time/tick-sched.h struct tick_sched { · line 55

Quoted 9 lines

kernel/time/tick-sched.h +1 -1

@@ -48,8 +48,8 @@ struct tick_sched {
        unsigned long                   check_clocks;
        enum tick_nohz_mode             nohz_mode;
 
+       bool                            tick_stopped    : 1;
        unsigned int                    inidle          : 1;
-       unsigned int                    tick_stopped    : 1;
        unsigned int                    idle_active     : 1;
        unsigned int                    do_timer_last   : 1;
        unsigned int                    got_idle_tick   : 1;

I don't think this is a good idea at all.

Quoted 8 lines

Quoted 6 lines

Quoted 5 lines

Quoted 3 lines

Quoted 1 line

Please see https://lkml.org/lkml/2017/11/21/384 for example.

[ZJ] Thanks for this sharing. Looks like, this patch fall into the case of "Maybe".

This patch falls into the case 'pointless' because it adds extra storage

[ZJ] 1 bit vs 1 bit. no more.

Groan. No. Care to look at the data structure? You create a new storage,

[ZJ] Say, {unsigned int, unsigned int, unsigned int, unsigned int, unsigned int} becomes
{bool , unsigned int, unsigned int, unsigned int,
unsigned int}
As specified by the rule No.10 at the section 6.7.2.1 of C99 TC2 as:
"If enough space remains, a bit-field that immediately follows another bit-field in a
structure shall be packed into adjacent bits of the same unit." What is the new storage so far?

Quoted 81 lines

Quoted 81 lines

Quoted 27 lines

Quoted 4 lines

which is incidentally merged into the other bitfield by the compiler at a different bit position, but there is no guarantee that a compiler does that. It's free to use distinct storage for that bool based bit.

[ZJ] Per the rule No.10 at section 6.7.2.1 of C99 TC2 as:
" If insufficient space remains, whether a bit-field that does not fit is put into
the next unit or overlaps adjacent units is implementation-defined." So, implementation is never mind which type will be stored if any.

Quoted 7 lines

Quoted 3 lines

Quoted 1 line

Quoted 1 line

for no benefit at all.

[ZJ] tick_stopped is returned by the tick_nohz_tick_stopped() which is bool. The benefit is no any potiential type conversion could be minded.

A bit stays a bit. 'bool foo : 1;' or 'unsigned int foo : 1' has to be evaluated as a bit. So there is a type conversion from BIT to bool required because BIT != bool.

[ZJ] Per the rule No.9 at section 6.7.2.1 of C99 TC2 as:
"If the value 0 or 1 is stored into a nonzero-width bit-field of types
_Bool, the value of the bit-field shall compare equal to the value stored." Obviously, it is nothing related to type conversion actually.

Quoted 5 lines

By chance the evaluation can be done by evaluating the byte in which the bit is placed just because the compiler knows that the remaining bits are not used. There is no guarantee that this is done, it happens to be true for a particular compiler.

Quoted 6 lines

[ZJ] Actually, such as GCC owe that kind of guarantee to be promised by ABI.

Quoted 3 lines

But that does not make it any more interesting. It just makes the code harder to read and eventually leads to bigger storage.

[ZJ] To get the benctifit to be profiled, it is given as:
number of instructions of function tick_nohz_tick_stopped():

[ZJ] Here, I used is not the tick_nohz_tick_stopped(), but an evaluation() as:
#include <stdio.h>
#include <stdbool.h>

struct tick_sched {
        unsigned int inidle             : 1;
        unsigned int tick_stopped       : 1;
};

bool get_status()

{
        struct tick_sched *ts;
        ts->tick_stopped = 1;
        return ts->tick_stopped;
}

int main()

{
        if (get_status()) return 0;
        return 0;
}

[ZJ] Toggle the declaration of tick_stopped in side of the tick_sched structure for comparison.

Quoted 9 lines

original: 17
patched: 14
Which was saved is:
movzbl %al, %eax
testl %eax, %eax
setne %al
Say, 3 / 17 = 17 % could be gained in the instruction executed
for this function can be evaluated.

Quoted 8 lines

Note:
The environment I used is:
OS : Ubuntu Desktop 16.04 LTS
gcc: 6.3.0 (without
optimization
for in general purpose)

Quoted 1 line

Just FYI.

Thanks,
ZJ

Quoted 62 lines · On 2018-04-11 07:20 AM, [email protected] wrote:

++

Quoted 60 lines · On 2018-04-11 07:09 AM, [email protected] wrote:

++

Quoted 57 lines · On 2018-04-10 10:49 PM, [email protected] wrote:

Typo...

Quoted 54 lines · On 2018-04-10 10:08 PM, [email protected] wrote:

Quoted 36 lines · Thomas Gleixner

Quoted 35 lines · On Tue, 10 Apr 2018, [email protected] wrote:

Quoted 34 lines · Thomas Gleixner

Quoted 33 lines · On Tue, 10 Apr 2018, [email protected] wrote:

Quoted 32 lines · Rafael J. Wysocki

Quoted 19 lines · On Tue, Apr 10, 2018 at 9:33 AM, <[email protected]> wrote:

From: John Zhao <[email protected]>

Variable tick_stopped returned by tick_nohz_tick_stopped can have only true / false values. Since the return type of the tick_nohz_tick_stopped is also bool, variable tick_stopped nice to have data type as bool in place of unsigned int. Moreover, the executed instructions cost could be minimal without potiential data type conversion.

Signed-off-by John Zhao <[email protected]>

---
 kernel/time/tick-sched.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

kernel/time/tick-sched.h +0 -0

index 6de959a..4d34309 100644

kernel/time/tick-sched.h struct tick_sched { · line 55

Quoted 9 lines

kernel/time/tick-sched.h +1 -1

@@ -48,8 +48,8 @@ struct tick_sched {
        unsigned long                   check_clocks;
        enum tick_nohz_mode             nohz_mode;
 
+       bool                            tick_stopped    : 1;
        unsigned int                    inidle          : 1;
-       unsigned int                    tick_stopped    : 1;
        unsigned int                    idle_active     : 1;
        unsigned int                    do_timer_last   : 1;
        unsigned int                    got_idle_tick   : 1;

I don't think this is a good idea at all.

Quoted 8 lines

Quoted 6 lines

Quoted 5 lines

Quoted 3 lines

Quoted 1 line

Please see https://lkml.org/lkml/2017/11/21/384 for example.

[ZJ] Thanks for this sharing. Looks like, this patch fall into the case of "Maybe".

This patch falls into the case 'pointless' because it adds extra storage

[ZJ] 1 bit vs 1 bit. no more.

Groan. No. Care to look at the data structure? You create a new storage,

[ZJ] Say, {unsigned int, unsigned int, unsigned int, unsigned int, unsigned int} becomes
{bool , unsigned int, unsigned int, unsigned int,
unsigned int}
As specified by the rule No.10 at the section 6.7.2.1 of C99 TC2 as:
"If enough space remains, a bit-field that immediately follows another bit-field in a
structure shall be packed into adjacent bits of the same unit." What is the new storage so far?

[ZJ] Further prototyping has been given based on gcc for both of x86_64 and armv8-a,
unsigned int and bool share the same 1 bytes without the
addtional storage for sure.
Open this and welcome if any other difference behaviour could be
captured.

Quoted 23 lines

Quoted 23 lines

Quoted 23 lines

Quoted 23 lines

Quoted 4 lines

which is incidentally merged into the other bitfield by the compiler at a different bit position, but there is no guarantee that a compiler does that. It's free to use distinct storage for that bool based bit.

[ZJ] Per the rule No.10 at section 6.7.2.1 of C99 TC2 as:
" If insufficient space remains, whether a bit-field that does not fit is put into
the next unit or overlaps adjacent units is implementation-defined." So, implementation is never mind which type will be stored if any.

Quoted 7 lines

Quoted 3 lines

Quoted 1 line

Quoted 1 line

for no benefit at all.

[ZJ] tick_stopped is returned by the tick_nohz_tick_stopped() which is bool. The benefit is no any potiential type conversion could be minded.

A bit stays a bit. 'bool foo : 1;' or 'unsigned int foo : 1' has to be evaluated as a bit. So there is a type conversion from BIT to bool required because BIT != bool.

[ZJ] Per the rule No.9 at section 6.7.2.1 of C99 TC2 as:
"If the value 0 or 1 is stored into a nonzero-width bit-field of types
_Bool, the value of the bit-field shall compare equal to the value stored." Obviously, it is nothing related to type conversion actually.

Quoted 1 line

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

Quoted 4 lines

By chance the evaluation can be done by evaluating the byte in which the bit is placed just because the compiler knows that the remaining bits are not used. There is no guarantee that this is done, it happens to be true for a particular compiler.

[ZJ] Actually, such as GCC owe that kind of guarantee to be promised by ABI.

Quoted 53 lines

Quoted 53 lines

Quoted 53 lines

Quoted 5 lines

Quoted 3 lines

But that does not make it any more interesting. It just makes the code harder to read and eventually leads to bigger storage.

[ZJ] To get the benctifit to be profiled, it is given as:
number of instructions of function tick_nohz_tick_stopped():

[ZJ] Here, I used is not the tick_nohz_tick_stopped(), but an evaluation() as:
#include <stdio.h>
#include <stdbool.h>

struct tick_sched {
        unsigned int inidle             : 1;
        unsigned int tick_stopped       : 1;
};

bool get_status()

{
        struct tick_sched *ts;
        ts->tick_stopped = 1;
        return ts->tick_stopped;
}

int main()

{
        if (get_status()) return 0;
        return 0;
}

[ZJ] Toggle the declaration of tick_stopped in side of the tick_sched structure for comparison.

Quoted 9 lines

original: 17
patched: 14
Which was saved is:
movzbl %al, %eax
testl %eax, %eax
setne %al
Say, 3 / 17 = 17 % could be gained in the instruction executed
for this function can be evaluated.

Quoted 8 lines

Note:
The environment I used is:
OS : Ubuntu Desktop 16.04 LTS
gcc: 6.3.0 (without
optimization
for in general purpose)

Quoted 1 line

Just FYI.

Thanks,
ZJ

On 2018-04-20 09:47 AM, [email protected] wrote:

Quoted 6 lines

[ZJ] Further prototyping has been given based on gcc for both of x86_64 and armv8-a,
unsigned int and bool share the same 1 bytes without the
addtional storage for sure.
Open this and welcome if any other difference behaviour could be
captured.

[ZJ] Typo.. change 4 bytes above to 1 byte actually.

Quoted 1 line · On Fri, 2018-04-20 at 14:44 +0800, [email protected] wrote:

On 2018-04-20 09:47 AM, [email protected] wrote:

[]

Quoted 7 lines

Quoted 6 lines

[ZJ] Further prototyping has been given based on gcc for both of x86_64 and armv8-a,
unsigned int and bool share the same 1 bytes without the
addtional storage for sure.
Open this and welcome if any other difference behaviour could be
captured.

Quoted 1 line

[ZJ] Typo.. change 4 bytes above to 1 byte actually.

Not really.

unsigned int is 4 and bool is generally 1.
Alignment padding after a bool may exist.

Quoted 13 lines

Quoted 13 lines

Quoted 13 lines

kernel/time/tick-sched.h struct tick_sched { · line 55

Quoted 10 lines

kernel/time/tick-sched.h +1 -1

@@ -48,8 +48,8 @@ struct tick_sched {
        unsigned long                   check_clocks;
        enum tick_nohz_mode             nohz_mode;
 
+       bool                            tick_stopped    : 1;
        unsigned int                    inidle          : 1;
-       unsigned int                    tick_stopped    : 1;
        unsigned int                    idle_active     : 1;
        unsigned int                    do_timer_last   : 1;
        unsigned int                    got_idle_tick   : 1;

I don't think this is a good idea at all.

Quoted 6 lines

Quoted 4 lines

Quoted 1 line

Please see https://lkml.org/lkml/2017/11/21/384 for example.

[ZJ] Thanks for this sharing. Looks like, this patch fall into the case of
"Maybe".

This patch falls into the case 'pointless' because it adds extra storage

[ZJ] 1 bit vs 1 bit. no more.

Quoted 1 line

for no benefit at all.

[ZJ] tick_stopped is returned by the tick_nohz_tick_stopped() which is bool. The benefit is no any potiential type conversion could be minded.

Quoted 13 lines · On Tue, Apr 10, 2018 at 06:07:17PM +0800, [email protected] wrote:

Quoted 13 lines

Quoted 13 lines

Quoted 13 lines

kernel/time/tick-sched.h struct tick_sched { · line 55

Quoted 10 lines

kernel/time/tick-sched.h +1 -1

@@ -48,8 +48,8 @@ struct tick_sched {
        unsigned long                   check_clocks;
        enum tick_nohz_mode             nohz_mode;
 
+       bool                            tick_stopped    : 1;
        unsigned int                    inidle          : 1;
-       unsigned int                    tick_stopped    : 1;
        unsigned int                    idle_active     : 1;
        unsigned int                    do_timer_last   : 1;
        unsigned int                    got_idle_tick   : 1;

I don't think this is a good idea at all.

Quoted 7 lines

Quoted 6 lines

Quoted 4 lines

Quoted 1 line

Please see https://lkml.org/lkml/2017/11/21/384 for example.

[ZJ] Thanks for this sharing. Looks like, this patch fall into the case of
"Maybe".

This patch falls into the case 'pointless' because it adds extra storage

[ZJ] 1 bit vs 1 bit. no more.

Since its a different type, the bitfields will not be merged. Also I'm surprised a bitfield with base-type _Bool is even allowed.

Quoted 3 lines

Quoted 1 line

for no benefit at all.

[ZJ] tick_stopped is returned by the tick_nohz_tick_stopped() which is bool. The benefit is no any potiential type conversion could be minded.

Do you have any actual evidence for that? Is there a compiler stupid enough to generate code to convert a bool to a 1bit value?

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

+ bool tick_stopped : 1;

Quoted 2 lines · Peter Zijlstra

Do you have any actual evidence for that? Is there a compiler stupid enough to generate code to convert a bool to a 1bit value?

Sure, if you do:

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

+ bool tick_stopped : 1;

which is stupidly allowed by the standard....

kernel/time/tick-sched.h struct tick_sched { · line 55

Quoted 11 lines · On Tue, Apr 10, 2018 at 9:33 AM, <[email protected]> wrote:

+++ b/kernel/time/tick-sched.h

kernel/time/tick-sched.h +1 -1

@@ -48,8 +48,8 @@ struct tick_sched {
        unsigned long                   check_clocks;
        enum tick_nohz_mode             nohz_mode;
 
+       bool                            tick_stopped    : 1;
        unsigned int                    inidle          : 1;
-       unsigned int                    tick_stopped    : 1;
        unsigned int                    idle_active     : 1;
        unsigned int                    do_timer_last   : 1;
        unsigned int                    got_idle_tick   : 1;

I don't think this is a good idea at all.

Please see https://lkml.org/lkml/2017/11/21/384 for example.

Quoted 15 lines · Rafael J. Wysocki

kernel/time/tick-sched.h struct tick_sched { · line 55

Quoted 11 lines · On Tue, Apr 10, 2018 at 9:33 AM, <[email protected]> wrote:

+++ b/kernel/time/tick-sched.h

kernel/time/tick-sched.h +1 -1

@@ -48,8 +48,8 @@ struct tick_sched {
        unsigned long                   check_clocks;
        enum tick_nohz_mode             nohz_mode;
 
+       bool                            tick_stopped    : 1;
        unsigned int                    inidle          : 1;
-       unsigned int                    tick_stopped    : 1;
        unsigned int                    idle_active     : 1;
        unsigned int                    do_timer_last   : 1;
        unsigned int                    got_idle_tick   : 1;

I don't think this is a good idea at all.

Quoted 1 line

Please see https://lkml.org/lkml/2017/11/21/384 for example.

Joe, apw, could we get Checkpatch to whinge about _Bool in composite types? That should immediately also disqualify using it as the base type of bitfields.

Whinging about bool <foo> : <x> seems entirely sensible and straightforward to do.

I'm not so sure about bool in structs as a patch context could be adding a bool to local stack definitions and there's no real ability to determine if the bool is in a struct or on the stack.

Also, I think there's nothing really wrong with using bool in structs. Steven Rostedt's rationale in
https://lkml.org/lkml/2017/11/21/207 isn't really right as sizeof(int) is 4 not 1 and sizeof(bool) is 1 on arches without alignment issues. I believe when using gcc, sizeof(bool) is always 1 and there may be alignment padding added on some arches. Dunno.

But I think the battle is already lost anyway.

git grep -P '(?<!static|extern)\s+bool\s+\w+\s*;' include | wc -l 1543

A struct with a bool member can have different sizes on various architectures because neither bool size nor alignment is standardized.

So emit a message on the use of bool in structs only in .h files and not .c files.

There is the real possibility that this test could have a false positive when a bool is declared as an automatic, so limit the test to .h files where the only false positive is for declarations in static inline functions.

scripts/checkpatch.pl +2 -0

+		}
+
 # check for semaphores initialized locked
 		if ($line =~ /^.\s*sema_init.+,\W?0\W?\)/) {
 			WARN("CONSIDER_COMPLETION",

Quoted 2 lines · On Tue, 10 Apr 2018 11:19:54 -0700 Joe Perches <[email protected]> wrote:

A struct with a bool member can have different sizes on various architectures because neither bool size nor alignment is standardized.

What's wrong with bools in structs?

Quoted 6 lines · Andrew Morton

Quoted 2 lines · On Tue, 10 Apr 2018 11:19:54 -0700 Joe Perches <[email protected]> wrote:

A struct with a bool member can have different sizes on various architectures because neither bool size nor alignment is standardized.

What's wrong with bools in structs?

See above.

Quoted 6 lines · On Tue, 10 Apr 2018 14:53:51 -0700 Joe Perches <[email protected]> wrote:

Quoted 5 lines · Andrew Morton

Quoted 2 lines · On Tue, 10 Apr 2018 11:19:54 -0700 Joe Perches <[email protected]> wrote:

A struct with a bool member can have different sizes on various architectures because neither bool size nor alignment is standardized.

Quoted 3 lines

Quoted 1 line

What's wrong with bools in structs?

See above.

Yeah, but so what? `long' has different sizes on different architectures too.

Quoted 8 lines · Andrew Morton

Quoted 6 lines · On Tue, 10 Apr 2018 14:53:51 -0700 Joe Perches <[email protected]> wrote:

Quoted 5 lines · Andrew Morton

Quoted 2 lines · On Tue, 10 Apr 2018 11:19:54 -0700 Joe Perches <[email protected]> wrote:

A struct with a bool member can have different sizes on various architectures because neither bool size nor alignment is standardized.

Quoted 6 lines

Quoted 3 lines

Quoted 1 line

What's wrong with bools in structs?

See above.

Yeah, but so what? `long' has different sizes on different architectures too.

Right, so we have ILP32/LP64 for all our 32/64 bit archs respectively. So only 2 possible variations to consider, and if you know your bitness you know your layout.

(+- some really unfortunate alignment exceptions, the worst of which Arnd recently removed, hooray!)

But neither says anything about sizeof(_Bool), and the standard leaves it undefined and only mandates it is large enough to store either 0 or 1 (and I suspect this vagueness is because there are architectures that either have no byte addressibility or it's more expensive than word addressibility).

Typically GCC chooses a single byte to represent _Bool, but there are no guarantees. This means that when you care about structure layout (as we all really should) things go wobbly when you use _Bool.

If GCC were to guarantee a 1 byte _Bool for all Linux ABIs we could reconsider.

We already have some 500 bools-in-structs

and the owners of that code will
be wondering whether they should change them, and whether they should apply those remove-bool-in-struct patches which someone sent them.

So... can we please get some clarity here?

...

(ooh, https://lkml.org/lkml/2017/11/21/384 is working this morning)

hm, Linus suggests that instead of using

	bool mybool;

we should use

	unsigned mybool:1;

However that introduces the risk that alterations of mybool will use nonatomic rmw operations.

	unsigned myboolA:1;
	unsigned myboolB:1;

so

	foo->myboolA = 1;

could scribble on concurrent alterations of foo->myboolB. I think.

(Adding Julia Lawall)

Quoted 1 line · Andrew Morton

We already have some 500 bools-in-structs

I got at least triple that only in include/ so I expect there are at probably an order
of magnitude more than 500 in the kernel.

I suppose some cocci script could count the actual number of instances. A regex can not.

Quoted 4 lines

and the owners of that code will
be wondering whether they should change them, and whether they should apply those remove-bool-in-struct patches which someone sent them.

Which is why the warning is --strict only

Quoted 18 lines

So... can we please get some clarity here?

...

(ooh, https://lkml.org/lkml/2017/11/21/384 is working this morning)

hm, Linus suggests that instead of using

	bool mybool;

we should use

	unsigned mybool:1;

However that introduces the risk that alterations of mybool will use nonatomic rmw operations.

Quoted 8 lines

	unsigned myboolA:1;
	unsigned myboolB:1;

so

	foo->myboolA = 1;

could scribble on concurrent alterations of foo->myboolB. I think.

Without barriers, that could happen anyway.

To me, the biggest problem with conversions from bool to bitfield is logical. ie:

	unsigned int.singlebitfield = 4;

is not the same result as

	bool = 4;

because of implicit truncation vs boolean conversion so a direct change of bool use in structs to unsigned would also require logic analysis.

Quoted 3 lines · Joe Perches

Quoted 1 line · Andrew Morton

We already have some 500 bools-in-structs

Quoted 6 lines

I got at least triple that only in include/ so I expect there are at probably an order of magnitude more than 500 in the kernel.

I suppose some cocci script could count the actual number of instances. A regex can not.

I got 12667.

I'm not sure to understand the issue. Will using a bitfield help if there are no other bitfields in the structure?

Quoted 8 lines · Julia Lawall

Quoted 7 lines · Joe Perches

Quoted 1 line · Andrew Morton

We already have some 500 bools-in-structs

I got at least triple that only in include/ so I expect there are at probably an order of magnitude more than 500 in the kernel.

Quoted 4 lines

Quoted 2 lines

I suppose some cocci script could count the actual number of instances. A regex can not.

I got 12667.

Could you please post the cocci script?

I personally do not find a significant issue with uncontrolled sizes of bool in kernel structs as all of the kernel structs are transitory and not written out to storage.

Quoted 4 lines · Julia Lawall

Quoted 3 lines · Joe Perches

Quoted 1 line · Andrew Morton

We already have some 500 bools-in-structs

Quoted 8 lines

Quoted 6 lines

I got at least triple that only in include/ so I expect there are at probably an order of magnitude more than 500 in the kernel.

I suppose some cocci script could count the actual number of instances. A regex can not.

I got 12667.

Could you please post the cocci script?

Quoted 2 lines

I'm not sure to understand the issue. Will using a bitfield help if there are no other bitfields in the structure?

IMO, not really.

The primary issue is described by Linus here:
https://lkml.org/lkml/2017/11/21/384

I personally do not find a significant issue with uncontrolled sizes of bool in kernel structs as all of the kernel structs are transitory and not written out to storage.

I suppose bool bitfields are also OK, but for the RMW required.

Using unsigned int :1 bitfield instead of bool :1 has the negative of truncation so that the uint has to be set with !! instead of a simple assign.

Quoted 32 lines · Joe Perches

Quoted 4 lines · Julia Lawall

Quoted 3 lines · Joe Perches

Quoted 1 line · Andrew Morton

We already have some 500 bools-in-structs

Quoted 8 lines

Quoted 6 lines

I got at least triple that only in include/ so I expect there are at probably an order of magnitude more than 500 in the kernel.

I suppose some cocci script could count the actual number of instances. A regex can not.

I got 12667.

Could you please post the cocci script?

Quoted 2 lines

I'm not sure to understand the issue. Will using a bitfield help if there are no other bitfields in the structure?

IMO, not really.

The primary issue is described by Linus here:
https://lkml.org/lkml/2017/11/21/384

I personally do not find a significant issue with uncontrolled sizes of bool in kernel structs as all of the kernel structs are transitory and not written out to storage.

I suppose bool bitfields are also OK, but for the RMW required.

Quoted 3 lines

Using unsigned int :1 bitfield instead of bool :1 has the negative of truncation so that the uint has to be set with !! instead of a simple assign.

At least with gcc 5.4.0, a number of structures become larger with unsigned int :1. bool:1 seems to mostly solve this problem. The structure ichx_desc, defined in drivers/gpio/gpio-ich.c seems to become larger with both approaches.

Hi julia,

Quoted 38 lines · Julia Lawall

Quoted 29 lines · Joe Perches

Quoted 4 lines · Julia Lawall

Quoted 3 lines · Joe Perches

Quoted 1 line · Andrew Morton

We already have some 500 bools-in-structs

Quoted 8 lines

Quoted 6 lines

I got at least triple that only in include/ so I expect there are at probably an order of magnitude more than 500 in the kernel.

I suppose some cocci script could count the actual number of instances. A regex can not.

I got 12667.

Could you please post the cocci script?

Quoted 2 lines

I'm not sure to understand the issue. Will using a bitfield help if there are no other bitfields in the structure?

IMO, not really.

The primary issue is described by Linus here:
https://lkml.org/lkml/2017/11/21/384

I personally do not find a significant issue with uncontrolled sizes of bool in kernel structs as all of the kernel structs are transitory and not written out to storage.

Quoted 6 lines

I suppose bool bitfields are also OK, but for the RMW required.

Using unsigned int :1 bitfield instead of bool :1 has the negative of truncation so that the uint has to be set with !! instead of a simple assign.

Quoted 6 lines

At least with gcc 5.4.0, a number of structures become larger with unsigned int :1. bool:1 seems to mostly solve this problem. The structure
ichx_desc, defined in drivers/gpio/gpio-ich.c seems to become larger with
both approaches.

[ZJ] Hopefully, this could make it better in your environment.
IMHO, this is just for double check.

drivers/gpio/gpio-ich.c +8 -0

index 4f6d643..b46e170 100644
@@ -70,6 +70,18 @@ static const u8 avoton_reglen[3] = {
  #define ICHX_READ(reg, base_res)       inl((reg) + (base_res)->start)
 
  struct ichx_desc {
+       /* GPO_BLINK is available on this chipset */
+       bool uses_gpe0:1;
+
+       /* Whether the chipset has GPIO in GPE0_STS in the PM IO region 
 */
+        bool uses_gpe0:1;
+
+        /*
+         * Some chipsets don't let reading output values on GPIO_LVL 

register

drivers/gpio/gpio-ich.c +4 -6

+         * this option allows driver caching written output values
+         */
+        bool use_outlvl_cache:1;
+
         /* Max GPIO pins the chipset can have */
         uint ngpio;

@@ -77,24 +89,12 @@ struct ichx_desc {
         const u8 (*regs)[3];
         const u8 *reglen;
 
-       /* GPO_BLINK is available on this chipset */
-       bool have_blink;
-
-       /* Whether the chipset has GPIO in GPE0_STS in the PM IO region 
 */
-       bool uses_gpe0;
-
         /* USE_SEL is bogus on some chipsets, eg 3100 */
         u32 use_sel_ignore[3];
 
         /* Some chipsets have quirks, let these use their own 

request/get */

         int (*request)(struct gpio_chip *chip, unsigned offset);
         int (*get)(struct gpio_chip *chip, unsigned offset);

drivers/gpio/gpio-ich.c +0 -3

-
-       /*
-        * Some chipsets don't let reading output values on GPIO_LVL 

register

drivers/gpio/gpio-ich.c +0 -3

-        * this option allows driver caching written output values
-        */
-       bool use_outlvl_cache;
  };

ZJ

Quoted 48 lines · On Tue, 2018-04-17 at 17:07 +0800, [email protected] wrote:

Hi julia,

Quoted 44 lines · Julia Lawall

Quoted 29 lines · Joe Perches

Quoted 4 lines · Julia Lawall

Quoted 3 lines · Joe Perches

Quoted 1 line · Andrew Morton

We already have some 500 bools-in-structs

Quoted 8 lines

Quoted 6 lines

I got at least triple that only in include/ so I expect there are at probably an order of magnitude more than 500 in the kernel.

I suppose some cocci script could count the actual number of instances. A regex can not.

I got 12667.

Could you please post the cocci script?

Quoted 2 lines

I'm not sure to understand the issue. Will using a bitfield help if there are no other bitfields in the structure?

IMO, not really.

The primary issue is described by Linus here:
https://lkml.org/lkml/2017/11/21/384

I personally do not find a significant issue with uncontrolled sizes of bool in kernel structs as all of the kernel structs are transitory and not written out to storage.

Quoted 6 lines

I suppose bool bitfields are also OK, but for the RMW required.

Using unsigned int :1 bitfield instead of bool :1 has the negative of truncation so that the uint has to be set with !! instead of a simple assign.

At least with gcc 5.4.0, a number of structures become larger with unsigned int :1. bool:1 seems to mostly solve this problem. The structure
ichx_desc, defined in drivers/gpio/gpio-ich.c seems to become larger with
both approaches.

Quoted 2 lines

[ZJ] Hopefully, this could make it better in your environment.
IMHO, this is just for double check.

I doubt this is actually better or smaller code.

Check the actual object code using objdump and the struct alignment using pahole.

Quoted 55 lines

drivers/gpio/gpio-ich.c +8 -0

index 4f6d643..b46e170 100644
@@ -70,6 +70,18 @@ static const u8 avoton_reglen[3] = {
  #define ICHX_READ(reg, base_res)       inl((reg) + (base_res)->start)
 
  struct ichx_desc {
+       /* GPO_BLINK is available on this chipset */
+       bool uses_gpe0:1;
+
+       /* Whether the chipset has GPIO in GPE0_STS in the PM IO region
 */
+        bool uses_gpe0:1;
+
+        /*
+         * Some chipsets don't let reading output values on GPIO_LVL

register

drivers/gpio/gpio-ich.c +4 -6

+         * this option allows driver caching written output values
+         */
+        bool use_outlvl_cache:1;
+
         /* Max GPIO pins the chipset can have */
         uint ngpio;

@@ -77,24 +89,12 @@ struct ichx_desc {
         const u8 (*regs)[3];
         const u8 *reglen;
 
-       /* GPO_BLINK is available on this chipset */
-       bool have_blink;
-
-       /* Whether the chipset has GPIO in GPE0_STS in the PM IO region
 */
-       bool uses_gpe0;
-
         /* USE_SEL is bogus on some chipsets, eg 3100 */
         u32 use_sel_ignore[3];
 
         /* Some chipsets have quirks, let these use their own

request/get */

         int (*request)(struct gpio_chip *chip, unsigned offset);
         int (*get)(struct gpio_chip *chip, unsigned offset);

drivers/gpio/gpio-ich.c +0 -3

-
-       /*
-        * Some chipsets don't let reading output values on GPIO_LVL

register

drivers/gpio/gpio-ich.c +0 -3

-        * this option allows driver caching written output values
-        */
-       bool use_outlvl_cache;
  };

ZJ

Quoted 52 lines · Joe Perches

Quoted 50 lines · On Tue, 2018-04-17 at 17:07 +0800, [email protected] wrote:

Hi julia,

Quoted 38 lines · Julia Lawall

Quoted 29 lines · Joe Perches

Quoted 4 lines · Julia Lawall

Quoted 3 lines · Joe Perches

Quoted 1 line · Andrew Morton

We already have some 500 bools-in-structs

Quoted 8 lines

Quoted 6 lines

I got at least triple that only in include/ so I expect there are at probably an order of magnitude more than 500 in the kernel.

I suppose some cocci script could count the actual number of instances. A regex can not.

I got 12667.

Could you please post the cocci script?

Quoted 2 lines

I'm not sure to understand the issue. Will using a bitfield help if there are no other bitfields in the structure?

IMO, not really.

The primary issue is described by Linus here:
https://lkml.org/lkml/2017/11/21/384

I personally do not find a significant issue with uncontrolled sizes of bool in kernel structs as all of the kernel structs are transitory and not written out to storage.

Quoted 6 lines

I suppose bool bitfields are also OK, but for the RMW required.

Using unsigned int :1 bitfield instead of bool :1 has the negative of truncation so that the uint has to be set with !! instead of a simple assign.

Quoted 6 lines

At least with gcc 5.4.0, a number of structures become larger with unsigned int :1. bool:1 seems to mostly solve this problem. The structure
ichx_desc, defined in drivers/gpio/gpio-ich.c seems to become larger with
both approaches.

[ZJ] Hopefully, this could make it better in your environment.
IMHO, this is just for double check.

Quoted 4 lines

I doubt this is actually better or smaller code.

Check the actual object code using objdump and the struct alignment using pahole.

I didn't have a chance to try it, but it looks quite likely to result in a smaller data structure based on the other examples that I looked at.

Quoted 55 lines · Julia Lawall

Quoted 52 lines · Joe Perches

Quoted 50 lines · On Tue, 2018-04-17 at 17:07 +0800, [email protected] wrote:

Hi julia,

Quoted 38 lines · Julia Lawall

Quoted 29 lines · Joe Perches

Quoted 4 lines · Julia Lawall

Quoted 3 lines · Joe Perches

Quoted 1 line · Andrew Morton

We already have some 500 bools-in-structs

Quoted 8 lines

Quoted 6 lines

I got at least triple that only in include/ so I expect there are at probably an order of magnitude more than 500 in the kernel.

I suppose some cocci script could count the actual number of instances. A regex can not.

I got 12667.

Could you please post the cocci script?

Quoted 2 lines

I'm not sure to understand the issue. Will using a bitfield help if there are no other bitfields in the structure?

IMO, not really.

The primary issue is described by Linus here:
https://lkml.org/lkml/2017/11/21/384

I personally do not find a significant issue with uncontrolled sizes of bool in kernel structs as all of the kernel structs are transitory and not written out to storage.

Quoted 6 lines

I suppose bool bitfields are also OK, but for the RMW required.

Using unsigned int :1 bitfield instead of bool :1 has the negative of truncation so that the uint has to be set with !! instead of a simple assign.

Quoted 6 lines

At least with gcc 5.4.0, a number of structures become larger with unsigned int :1. bool:1 seems to mostly solve this problem. The structure
ichx_desc, defined in drivers/gpio/gpio-ich.c seems to become larger with
both approaches.

[ZJ] Hopefully, this could make it better in your environment.
IMHO, this is just for double check.

Quoted 7 lines

Quoted 4 lines

I doubt this is actually better or smaller code.

Check the actual object code using objdump and the struct alignment using pahole.

I didn't have a chance to try it, but it looks quite likely to result in a smaller data structure based on the other examples that I looked at.

I _really_ doubt there is any difference in size between the below in any architecture

struct foo {
	int bar;
	bool baz:1;
	int qux;
};

and

struct foo {
	int bar;
	bool baz;
	int qux;
};

Where there would be a difference in size is

struct foo {
	int bar;
	bool baz1:1;
	bool baz2:1;
	int qux;
};

and

struct foo {
	int bar;
	bool baz1;
	bool baz2;

int qux;
};

Quoted 92 lines · Joe Perches

Quoted 55 lines · Julia Lawall

Quoted 52 lines · Joe Perches

Quoted 50 lines · On Tue, 2018-04-17 at 17:07 +0800, [email protected] wrote:

Hi julia,

Quoted 38 lines · Julia Lawall

Quoted 29 lines · Joe Perches

Quoted 4 lines · Julia Lawall

Quoted 3 lines · Joe Perches

Quoted 1 line · Andrew Morton

We already have some 500 bools-in-structs

Quoted 8 lines

Quoted 6 lines

I got at least triple that only in include/ so I expect there are at probably an order of magnitude more than 500 in the kernel.

I suppose some cocci script could count the actual number of instances. A regex can not.

I got 12667.

Could you please post the cocci script?

Quoted 2 lines

I'm not sure to understand the issue. Will using a bitfield help if there are no other bitfields in the structure?

IMO, not really.

The primary issue is described by Linus here:
https://lkml.org/lkml/2017/11/21/384

I personally do not find a significant issue with uncontrolled sizes of bool in kernel structs as all of the kernel structs are transitory and not written out to storage.

Quoted 6 lines

I suppose bool bitfields are also OK, but for the RMW required.

Using unsigned int :1 bitfield instead of bool :1 has the negative of truncation so that the uint has to be set with !! instead of a simple assign.

Quoted 6 lines

At least with gcc 5.4.0, a number of structures become larger with unsigned int :1. bool:1 seems to mostly solve this problem. The structure
ichx_desc, defined in drivers/gpio/gpio-ich.c seems to become larger with
both approaches.

[ZJ] Hopefully, this could make it better in your environment.
IMHO, this is just for double check.

Quoted 7 lines

Quoted 4 lines

I doubt this is actually better or smaller code.

Check the actual object code using objdump and the struct alignment using pahole.

I didn't have a chance to try it, but it looks quite likely to result in a smaller data structure based on the other examples that I looked at.

I _really_ doubt there is any difference in size between the below in any architecture

struct foo {
	int bar;
	bool baz:1;
	int qux;
};

and

struct foo {
	int bar;
	bool baz;
	int qux;
};

Where there would be a difference in size is

struct foo {
	int bar;
	bool baz1:1;
	bool baz2:1;
	int qux;
};

and

Quoted 7 lines

struct foo {
	int bar;
	bool baz1;
	bool baz2;

int qux;
};

In the situation of the example there are two bools together in the middle of the structure and one at the end. Somehow, even converting to bool:1 increases the size. But it seems plausible that putting all three bools together and converting them all to :1 would reduce the size. I don't know. The size increase (more than 8 bytes) seems out of proportion for 3 bools.

I was able to check around 3000 structures that were not declared with any attributes, that don't declare named types internally, and that are compiled for x86. Around 10% become smaller whn using bool:1, typically by at most 8 bytes.

julia

Quoted 2 lines

Quoted 94 lines · Julia Lawall

Quoted 92 lines · Joe Perches

Quoted 55 lines · Julia Lawall

Quoted 52 lines · Joe Perches

Quoted 50 lines · On Tue, 2018-04-17 at 17:07 +0800, [email protected] wrote:

Hi julia,

Quoted 38 lines · Julia Lawall

Quoted 29 lines · Joe Perches

Quoted 4 lines · Julia Lawall

Quoted 3 lines · Joe Perches

Quoted 1 line · Andrew Morton

We already have some 500 bools-in-structs

Quoted 8 lines

Quoted 6 lines

I got at least triple that only in include/ so I expect there are at probably an order of magnitude more than 500 in the kernel.

I suppose some cocci script could count the actual number of instances. A regex can not.

I got 12667.

Could you please post the cocci script?

Quoted 2 lines

I'm not sure to understand the issue. Will using a bitfield help if there are no other bitfields in the structure?

IMO, not really.

The primary issue is described by Linus here:
https://lkml.org/lkml/2017/11/21/384

I personally do not find a significant issue with uncontrolled sizes of bool in kernel structs as all of the kernel structs are transitory and not written out to storage.

Quoted 6 lines

I suppose bool bitfields are also OK, but for the RMW required.

Using unsigned int :1 bitfield instead of bool :1 has the negative of truncation so that the uint has to be set with !! instead of a simple assign.

Quoted 6 lines

At least with gcc 5.4.0, a number of structures become larger with unsigned int :1. bool:1 seems to mostly solve this problem. The structure
ichx_desc, defined in drivers/gpio/gpio-ich.c seems to become larger with
both approaches.

[ZJ] Hopefully, this could make it better in your environment.
IMHO, this is just for double check.

Quoted 7 lines

Quoted 4 lines

I doubt this is actually better or smaller code.

Check the actual object code using objdump and the struct alignment using pahole.

I didn't have a chance to try it, but it looks quite likely to result in a smaller data structure based on the other examples that I looked at.

I _really_ doubt there is any difference in size between the below in any architecture

struct foo {
	int bar;
	bool baz:1;
	int qux;
};

and

struct foo {
	int bar;
	bool baz;
	int qux;
};

Where there would be a difference in size is

struct foo {
	int bar;
	bool baz1:1;
	bool baz2:1;
	int qux;
};

and

Quoted 7 lines

Quoted 7 lines

struct foo {
	int bar;
	bool baz1;
	bool baz2;

int qux;
};

Quoted 7 lines

In the situation of the example there are two bools together in the middle of the structure and one at the end. Somehow, even converting to bool:1 increases the size. But it seems plausible that putting all three bools together and converting them all to :1 would reduce the size. I don't know. The size increase (more than 8 bytes) seems out of proportion for 3 bools.

[ZJ] Typically, addition saving is due for difference padding.

Quoted 5 lines

I was able to check around 3000 structures that were not declared with any attributes, that don't declare named types internally, and that are compiled for x86. Around 10% become smaller whn using bool:1, typically by at most 8 bytes.

Quoted 5 lines

julia

Quoted 2 lines

Quoted 100 lines · On 2018-04-19 02:48 PM, [email protected] wrote:

Quoted 99 lines · Julia Lawall

Quoted 97 lines · Joe Perches

Quoted 55 lines · Julia Lawall

Quoted 52 lines · Joe Perches

Quoted 50 lines · On Tue, 2018-04-17 at 17:07 +0800, [email protected] wrote:

Hi julia,

Quoted 38 lines · Julia Lawall

Quoted 29 lines · Joe Perches

Quoted 4 lines · Julia Lawall

Quoted 3 lines · Joe Perches

Quoted 1 line · Andrew Morton

We already have some 500 bools-in-structs

Quoted 8 lines

Quoted 6 lines

I got at least triple that only in include/ so I expect there are at probably an order of magnitude more than 500 in the kernel.

I suppose some cocci script could count the actual number of instances. A regex can not.

I got 12667.

Could you please post the cocci script?

Quoted 2 lines

I'm not sure to understand the issue. Will using a bitfield help if there are no other bitfields in the structure?

IMO, not really.

The primary issue is described by Linus here:
https://lkml.org/lkml/2017/11/21/384

I personally do not find a significant issue with uncontrolled sizes of bool in kernel structs as all of the kernel structs are transitory and not written out to storage.

Quoted 6 lines

I suppose bool bitfields are also OK, but for the RMW required.

Using unsigned int :1 bitfield instead of bool :1 has the negative of truncation so that the uint has to be set with !! instead of a simple assign.

Quoted 6 lines

At least with gcc 5.4.0, a number of structures become larger with unsigned int :1. bool:1 seems to mostly solve this problem. The structure
ichx_desc, defined in drivers/gpio/gpio-ich.c seems to become larger with
both approaches.

[ZJ] Hopefully, this could make it better in your environment.
IMHO, this is just for double check.

Quoted 7 lines

Quoted 4 lines

I doubt this is actually better or smaller code.

Check the actual object code using objdump and the struct alignment using pahole.

I didn't have a chance to try it, but it looks quite likely to result in a smaller data structure based on the other examples that I looked at.

I _really_ doubt there is any difference in size between the below in any architecture

struct foo {
	int bar;
	bool baz:1;
	int qux;
};

and

struct foo {
	int bar;
	bool baz;
	int qux;
};

Where there would be a difference in size is

struct foo {
	int bar;
	bool baz1:1;
	bool baz2:1;
	int qux;
};

and

struct foo {
	int bar;
	bool baz1;
	bool baz2;

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

int qux;
};

[ZJ] Even though, two bool:1 are grouped in the #3, finally 4 bytes are padded
due for int is the most significant in the type size.
At least, they are all the same per x86 and arm based on gcc.(12
bytes).

Quoted 13 lines

Quoted 7 lines

In the situation of the example there are two bools together in the middle of the structure and one at the end. Somehow, even converting to bool:1 increases the size. But it seems plausible that putting all three bools together and converting them all to :1 would reduce the size. I don't know. The size increase (more than 8 bytes) seems out of proportion for 3 bools.

[ZJ] Typically, addition saving is due for difference padding.

Quoted 5 lines

I was able to check around 3000 structures that were not declared with any attributes, that don't declare named types internally, and that are compiled for x86. Around 10% become smaller whn using bool:1, typically by at most 8 bytes.

[ZJ] As my example, int (*)() requested 8 bytes in x86 arch, then 8 bytes is similiar to that.
While it request 4 bytes in arm arch. Typically, my previous
example struct can
reach to 32 bytes in x86 arch(compared to 40 bytes for original
version).
Similarly, 20 bytes in arm arch(compared to 24 bytes per original
version).

Quoted 5 lines

Quoted 5 lines

julia

Quoted 2 lines

OK. I guess. But I'm not really seeing some snappy description which helps people understand why checkpatch is warning about this.

We already have some 500 bools-in-structs and the owners of that code will be wondering whether they should change them, and whether they should apply those remove-bool-in-struct patches which someone sent them.

(ooh, https://lkml.org/lkml/2017/11/21/384 is working this morning)

hm, Linus suggests that instead of using

	bool mybool;

we should use

	unsigned mybool:1;

However that introduces the risk that alterations of mybool will use nonatomic rmw operations.

	unsigned myboolA:1;
	unsigned myboolB:1;

so

	foo->myboolA = 1;

could scribble on concurrent alterations of foo->myboolB. I think.

I guess that risk is also present if myboolA and myboolB were `bool', too. The compiler could do any old thing with them including, perhaps, using a single-bit bitfield(?).

I still have room in my /dev/null mailbox for pure checkpatch patches.

Quoted 1 line

(ooh, https://lkml.org/lkml/2017/11/21/384 is working this morning)

Yes, we really should not use lkml.org for references. Sadly google displays it very prominently when you search for something.

lkml.org is nice in emails that have a short expected life time and relevance -

Quoted 2 lines · Peter Zijlstra

I still have room in my /dev/null mailbox for pure checkpatch patches.

Quoted 4 lines

Quoted 1 line

(ooh, https://lkml.org/lkml/2017/11/21/384 is working this morning)

Yes, we really should not use lkml.org for references. Sadly google displays it very prominently when you search for something.

lkml.org is nice in emails that have a short expected life time and relevance - but it probably shouldn't be used for permanent references such as kernel messages, code comments and Git log entries.

Thanks,

Ingo

Is there a better or recommended way to reference posts on LKML in commit messages? (I do like the idea of linking to previous discussions, results, ...)

Quoted 3 lines · Andrea Parri

Is there a better or recommended way to reference posts on LKML in commit messages? (I do like the idea of linking to previous discussions, results, ...)

Yes:

https://lkml.kernel.org/r/$MSGID

that has the added benefit that it immediately includes the msg-id, so even if you don't have tubes, you can search for it in your local mailboxes.

Also, since we (kernel.org) control the redirection (currently marc.info) we can always point it to a life archive.

Quoted 13 lines · Peter Zijlstra

Quoted 3 lines · Andrea Parri

Is there a better or recommended way to reference posts on LKML in commit messages? (I do like the idea of linking to previous discussions, results, ...)

Yes:

https://lkml.kernel.org/r/$MSGID

that has the added benefit that it immediately includes the msg-id, so even if you don't have tubes, you can search for it in your local mailboxes.

Quoted 2 lines

Also, since we (kernel.org) control the redirection (currently marc.info) we can always point it to a life archive.

Message IDs are not useful unless you subscribe and keep your emails.

It'd be _much_ nicer if vger.kernel.org stored every email it sent and had a search mechanism available rather than relying on external systems.

Quoted 14 lines · Joe Perches

Quoted 13 lines · Peter Zijlstra

Quoted 3 lines · Andrea Parri

Is there a better or recommended way to reference posts on LKML in commit messages? (I do like the idea of linking to previous discussions, results, ...)

Yes:

https://lkml.kernel.org/r/$MSGID

that has the added benefit that it immediately includes the msg-id, so even if you don't have tubes, you can search for it in your local mailboxes.

Quoted 5 lines

Quoted 2 lines

Also, since we (kernel.org) control the redirection (currently marc.info) we can always point it to a life archive.

Message IDs are not useful unless you subscribe and keep your emails.

I happen to do so..

Quoted 3 lines

It'd be _much_ nicer if vger.kernel.org stored every email it sent and had a search mechanism available rather than relying on external systems.

People are looking at that afaik.

Quoted 3 lines

It'd be _much_ nicer if vger.kernel.org stored every email it sent and had a search mechanism available rather than relying on external systems.

People are looking at that afaik.

Quoted 10 lines · Ingo Molnar

Quoted 6 lines · Peter Zijlstra

I still have room in my /dev/null mailbox for pure checkpatch patches.

Quoted 1 line

(ooh, https://lkml.org/lkml/2017/11/21/384 is working this morning)

Yes, we really should not use lkml.org for references. Sadly google displays it very prominently when you search for something.

Quoted 3 lines

lkml.org is nice in emails that have a short expected life time and relevance - but it probably shouldn't be used for permanent references such as kernel messages, code comments and Git log entries.

Is there a better or recommended way to reference posts on LKML in commit messages? (I do like the idea of linking to previous discussions, results, ...)