All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Gustavo A. R. Silva" <gustavoars@kernel.org>
To: Arnd Bergmann <arnd@arndb.de>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-kernel@vger.kernel.org,
	"Gustavo A. R. Silva" <gustavoars@kernel.org>
Subject: [PATCH][next] hpilo: Replace one-element array with flexible-array member
Date: Tue, 14 Jul 2020 10:44:49 -0500	[thread overview]
Message-ID: <20200714154449.GA26153@embeddedor> (raw)

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] https://github.com/KSPP/linux/issues/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>
---
 drivers/misc/hpilo.c |  2 +-
 drivers/misc/hpilo.h | 22 +++++++++++-----------
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/misc/hpilo.c b/drivers/misc/hpilo.c
index 927309b86bab..10c975662f8b 100644
--- a/drivers/misc/hpilo.c
+++ b/drivers/misc/hpilo.c
@@ -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)
diff --git a/drivers/misc/hpilo.h b/drivers/misc/hpilo.h
index 1aa433a7f66c..f69ff645cac9 100644
--- a/drivers/misc/hpilo.h
+++ b/drivers/misc/hpilo.h
@@ -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))
 
-- 
2.27.0


             reply	other threads:[~2020-07-14 15:39 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-14 15:44 Gustavo A. R. Silva [this message]
2020-07-14 16:19 ` [PATCH][next] hpilo: Replace one-element array with flexible-array member Greg Kroah-Hartman
2020-07-14 18:06   ` Gustavo A. R. Silva

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200714154449.GA26153@embeddedor \
    --to=gustavoars@kernel.org \
    --cc=arnd@arndb.de \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.