On Fri, 2018-10-26 at 22:54 +0200, Julia Lawall wrote:
> [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@xxxxxxxxx>
> > > ---
> > > 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?
Probably so, what verbiage would you suggest?
Also, any conversion from bool to int would
have to take care than any assigment uses !!
where appropriate.
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
