[Outreachy kernel] [RESEND PATCH 2/2] staging: vboxvideo: Use unsigned int instead bool

Julia Lawall julia.lawall at lip6.fr
Fri Oct 26 20:54:34 UTC 2018


[Adding Joe Perches]

On Fri, 26 Oct 2018, Sasha Levin wrote:

> On Fri, Oct 26, 2018 at 04:04:45PM -0300, Shayenne da Luz Moura wrote:
> > This change was suggested by checkpath.pl. Use unsigned int with bitfield
> > allocate only one bit to the boolean variable.
> >
> > CHECK: Avoid using bool structure members because of possible alignment
> > issues
> >
> > Signed-off-by: Shayenne da Luz Moura <shayenneluzmoura at gmail.com>
> > ---
> > drivers/staging/vboxvideo/vbox_drv.h        | 14 +++++++-------
> > drivers/staging/vboxvideo/vboxvideo_guest.h |  2 +-
> > 2 files changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/staging/vboxvideo/vbox_drv.h
> > b/drivers/staging/vboxvideo/vbox_drv.h
> > index 594f84272957..7d3e329a6b1c 100644
> > --- a/drivers/staging/vboxvideo/vbox_drv.h
> > +++ b/drivers/staging/vboxvideo/vbox_drv.h
> > @@ -81,7 +81,7 @@ struct vbox_private {
> > 	u8 __iomem *vbva_buffers;
> > 	struct gen_pool *guest_pool;
> > 	struct vbva_buf_ctx *vbva_info;
> > -	bool any_pitch;
> > +	unsigned int any_pitch:1;
> > 	u32 num_crtcs;
> > 	/** Amount of available VRAM, including space used for buffers. */
> > 	u32 full_vram_size;
>
> Using bitfields for booleans in these cases is less efficient than just
> using "regular" booleans for two reasons:
>
> 1. It will use the same amount of space. Due to alignment requirements,
> the compiler can't squeeze in anything into the 7 bits that are now
> "free". Each member, unless it's another bitfield, must start at a whole
> byte.
>
> 2. This is actually less efficient (slower) for the compiler to work
> with. The smallest granularity we have to access memory is 1 byte; we
> can't set individual bits directly in memory. For the original code, the
> assembly for 'vbox_private.any_pitch = true' would look something like
> this:
>
> 	movl   $0x1,-0x10(%rsp)
>
> As you can see, the compiler can directly write into the variable.
> However, when we switch to using bitfields, the compiler must preserve
> the original value of the other 7 bits, so it must first read them from
> memory, manipulate the value and write it back. The assembly would
> look something like this:
>
> 	movzbl -0x10(%rsp),%eax
> 	or     $0x1,%eax
> 	mov    %al,-0x10(%rsp)
>
> Which is less efficient than what was previously happening.

Maybe checkpatch could be more precise about what kind of bools should be
changed?

julia


More information about the devel mailing list