Re: [PATCH 0/4] int to bool conversion

[Peter Hurley <peter@hurleysoftware.com>]

Hi Louis,

On 01/30/2015 12:32 PM, Louis Langholtz wrote:
> While it may not be productive to perturb seemingly working
> code (as Rafael argues), it may also not be productive to
> have decreased code readability (as Quentin suggests).
> 
> Personally I prefer readability enhancements over worrying
> about possibly breaking working code. I don't want to start
> a flame war so I won't go into arguing this as a better
> position. I'd just like to thank Quentin for his efforts to
> identify boolean uses of variables. It's something I'm
> interested in as well and have been working on in a branch
> of my own git repository.
> 
> Quentin if you want to work on this together at all, that'd
> be great. Please contact me directly as I'm not subscribed to
> the LKML. As for the original semantic patch code, it's
> unlikely that it would be safe to not exclude variables that
> are passed by address (and seemingly the ampersand operator
> applied on x - as in '&x' - should be a part of the exclusion
> set).

Just a quick note about bools vs. ints in kernel code:
one of the required arch guarantees is that an int is a
unique memory location, whereas a bool does not provide that
guarantee. Much kernel code requires unique memory locations.

For instance, in the example below, do_something() may not execute.

static bool x;
static bool y;

CPU 0                     | CPU 1
                          |
                          | y = 1;
x = 1;                    |
                          | if (y)
                          |    do_something();
                          |

The reason is that the 'x = 1' statement may be a RMW operation
if the compiler has merged x and y into the same memory
location. So that what really happens is

u8 xy;

CPU 0                     | CPU 1
                          |
                          | load [xy]=> R
			  | R |= Y_BIT
load [xy] => R            |
R |= X_BIT                |
                          | store R => [xy]
store R => [xy]           |
                          | if ([xy] & Y_BIT)
                          |    do_something();
                          |

I looked over the patches when they were first posted and
none involve concurrent access, so I didn't mention it.
But a general campaign of int=>bool will need to be aware
of this.

Regards,
Peter Hurley

PS - In fact, even chars or shorts can be RMW on the entire
machine word.