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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

--
Thanks,
Sasha
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel



[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux