Skip to content

Commit

Permalink
hpilo: Replace one-element array with flexible-array member
Browse files Browse the repository at this point in the history
There is a regular need in the kernel to provide a way to declare
having a dynamically sized set of trailing elements in a structure.
Kernel code should always use “flexible array members”[1] for these
cases. The older style of one-element or zero-length arrays should
no longer be used[2].

For this particular case, it is important to notice that the cachelines
change from 7 to 6 after the flexible-array conversion:

$ pahole -C 'fifo' drivers/misc/hpilo.o
struct fifo {
	u64                        nrents;               /*     0     8 */
	u64                        imask;                /*     8     8 */
	u64                        merge;                /*    16     8 */
	u64                        reset;                /*    24     8 */
	u8                         pad_0[96];            /*    32    96 */
	/* --- cacheline 2 boundary (128 bytes) --- */
	u64                        head;                 /*   128     8 */
	u8                         pad_1[120];           /*   136   120 */
	/* --- cacheline 4 boundary (256 bytes) --- */
	u64                        tail;                 /*   256     8 */
	u8                         pad_2[120];           /*   264   120 */
	/* --- cacheline 6 boundary (384 bytes) --- */
	u64                        fifobar[1];           /*   384     8 */

	/* size: 392, cachelines: 7, members: 10 */
	/* last cacheline: 8 bytes */
};

$ pahole -C 'fifo' drivers/misc/hpilo.o
struct fifo {
	u64                        nrents;               /*     0     8 */
	u64                        imask;                /*     8     8 */
	u64                        merge;                /*    16     8 */
	u64                        reset;                /*    24     8 */
	u8                         pad_0[96];            /*    32    96 */
	/* --- cacheline 2 boundary (128 bytes) --- */
	u64                        head;                 /*   128     8 */
	u8                         pad_1[120];           /*   136   120 */
	/* --- cacheline 4 boundary (256 bytes) --- */
	u64                        tail;                 /*   256     8 */
	u8                         pad_2[120];           /*   264   120 */
	/* --- cacheline 6 boundary (384 bytes) --- */
	u64                        fifobar[];            /*   384     0 */

	/* size: 384, cachelines: 6, members: 10 */
};

Lastly, remove unnecessary parentheses in fifo_sz() and fix the following
checkpatch.pl warning for the whole fifo structure:

WARNING: please, no spaces at the start of a line

[1] https://en.wikipedia.org/wiki/Flexible_array_member
[2] KSPP#79

Tested-by: kernel test robot <lkp@intel.com>
Link: https://github.com/GustavoARSilva/linux-hardening/blob/master/cii/kernel-ci/hpilo-20200714.md
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Link: https://lore.kernel.org/r/20200714154449.GA26153@embeddedor
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
GustavoARSilva authored and gregkh committed Jul 14, 2020
1 parent 3a12c2b commit fadbfc3
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 12 deletions.
2 changes: 1 addition & 1 deletion drivers/misc/hpilo.c
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ static void ctrl_setup(struct ccb *ccb, int nr_desc, int l2desc_sz)
static inline int fifo_sz(int nr_entry)
{
/* size of a fifo is determined by the number of entries it contains */
return (nr_entry * sizeof(u64)) + FIFOHANDLESIZE;
return nr_entry * sizeof(u64) + FIFOHANDLESIZE;
}

static void fifo_setup(void *base_addr, int nr_entry)
Expand Down
22 changes: 11 additions & 11 deletions drivers/misc/hpilo.h
Original file line number Diff line number Diff line change
Expand Up @@ -160,23 +160,23 @@ struct ccb_data {
#define ILO_START_ALIGN 4096
#define ILO_CACHE_SZ 128
struct fifo {
u64 nrents; /* user requested number of fifo entries */
u64 imask; /* mask to extract valid fifo index */
u64 merge; /* O/C bits to merge in during enqueue operation */
u64 reset; /* set to non-zero when the target device resets */
u8 pad_0[ILO_CACHE_SZ - (sizeof(u64) * 4)];
u64 nrents; /* user requested number of fifo entries */
u64 imask; /* mask to extract valid fifo index */
u64 merge; /* O/C bits to merge in during enqueue operation */
u64 reset; /* set to non-zero when the target device resets */
u8 pad_0[ILO_CACHE_SZ - (sizeof(u64) * 4)];

u64 head;
u8 pad_1[ILO_CACHE_SZ - (sizeof(u64))];
u64 head;
u8 pad_1[ILO_CACHE_SZ - (sizeof(u64))];

u64 tail;
u8 pad_2[ILO_CACHE_SZ - (sizeof(u64))];
u64 tail;
u8 pad_2[ILO_CACHE_SZ - (sizeof(u64))];

u64 fifobar[1];
u64 fifobar[];
};

/* convert between struct fifo, and the fifobar, which is saved in the ccb */
#define FIFOHANDLESIZE (sizeof(struct fifo) - sizeof(u64))
#define FIFOHANDLESIZE (sizeof(struct fifo))
#define FIFOBARTOHANDLE(_fifo) \
((struct fifo *)(((char *)(_fifo)) - FIFOHANDLESIZE))

Expand Down

0 comments on commit fadbfc3

Please sign in to comment.