Hello Yann,
On Fri, May 30, 2014 at 10:02:47PM +0200, Yann Droneaud wrote:
> Up to 7 bytes are wasted at the end of struct platform_object
> in the form of padding after name field: unfortunately this
> padding is not used when allocating the memory to hold the
> name.
>
> This patch converts name array from name[1] to C99 flexible
> array name[] (equivalent to name[0]) so that no padding is
> required by the presence of this field. Memory allocation
> is updated to take care of allocating an additional byte for
> the NUL terminating character.
>
> Built on Fedora 20, using GCC 4.8, for ARM, i386, SPARC64 and
> x86_64 architectures, the data structure layout can be reported
> with following command:
>
> $ pahole drivers/base/platform.o \
> --recursive \
> --class_name device,pdev_archdata,platform_device,platform_object
>
> Please find below some comparisons of structure layout for arm,
> i386, sparc64 and x86_64 architecture before and after the patch.
>
> --- obj-arm/drivers/base/platform.o.pahole.v3.15-rc7-79-gfe45736f4134 2014-05-30 10:32:06.290960701 +0200
> +++ obj-arm/drivers/base/platform.o.pahole.v3.15-rc7-80-g2cdb06858d71 2014-05-30 11:26:20.851988347 +0200
> @@ -81,10 +81,9 @@
> /* XXX last struct has 4 bytes of padding */
>
> /* --- cacheline 6 boundary (384 bytes) was 8 bytes ago --- */
> - char name[1]; /* 392 1 */
> + char name[0]; /* 392 0 */
>
> - /* size: 400, cachelines: 7, members: 2 */
> - /* padding: 7 */
> + /* size: 392, cachelines: 7, members: 2 */
> /* paddings: 1, sum paddings: 4 */
> - /* last cacheline: 16 bytes */
> + /* last cacheline: 8 bytes */
> };
>
> --- obj-i386/drivers/base/platform.o.pahole.v3.15-rc7-79-gfe45736f4134 2014-05-30 10:32:06.305960691 +0200
> +++ obj-i386/drivers/base/platform.o.pahole.v3.15-rc7-80-g2cdb06858d71 2014-05-30 11:26:20.875988332 +0200
> @@ -73,9 +73,8 @@
> struct platform_object {
> struct platform_device pdev; /* 0 396 */
> /* --- cacheline 6 boundary (384 bytes) was 12 bytes ago --- */
> - char name[1]; /* 396 1 */
> + char name[0]; /* 396 0 */
>
> - /* size: 400, cachelines: 7, members: 2 */
> - /* padding: 3 */
> - /* last cacheline: 16 bytes */
> + /* size: 396, cachelines: 7, members: 2 */
> + /* last cacheline: 12 bytes */
> };
>
> --- obj-sparc64/drivers/base/platform.o.pahole.v3.15-rc7-79-gfe45736f4134 2014-05-30 10:32:06.406960625 +0200
> +++ obj-sparc64/drivers/base/platform.o.pahole.v3.15-rc7-80-g2cdb06858d71 2014-05-30 11:26:20.971988269 +0200
> @@ -94,9 +94,8 @@
> struct platform_object {
> struct platform_device pdev; /* 0 2208 */
> /* --- cacheline 34 boundary (2176 bytes) was 32 bytes ago --- */
> - char name[1]; /* 2208 1 */
> + char name[0]; /* 2208 0 */
>
> - /* size: 2216, cachelines: 35, members: 2 */
> - /* padding: 7 */
> - /* last cacheline: 40 bytes */
> + /* size: 2208, cachelines: 35, members: 2 */
> + /* last cacheline: 32 bytes */
> };
>
> --- obj-x86_64/drivers/base/platform.o.pahole.v3.15-rc7-79-gfe45736f4134 2014-05-30 10:32:06.432960608 +0200
> +++ obj-x86_64/drivers/base/platform.o.pahole.v3.15-rc7-80-g2cdb06858d71 2014-05-30 11:26:21.000988250 +0200
> @@ -84,9 +84,8 @@
> struct platform_object {
> struct platform_device pdev; /* 0 720 */
> /* --- cacheline 11 boundary (704 bytes) was 16 bytes ago --- */
> - char name[1]; /* 720 1 */
> + char name[0]; /* 720 0 */
>
> - /* size: 728, cachelines: 12, members: 2 */
> - /* padding: 7 */
> - /* last cacheline: 24 bytes */
> + /* size: 720, cachelines: 12, members: 2 */
> + /* last cacheline: 16 bytes */
> };
>
> Changes from v5 [1]:
> - dropped dma_mask allocation changes and only kept padding
> removal changes (name array length set to 0).
>
> Changes from v4 [2]:
> [by Emil Goode <emilgoode@xxxxxxxxx>:]
> - Split v4 of the patch into two separate patches.
> - Generated new object file size and data structure layout info.
> - Updated the changelog message.
>
> Changes from v3 [3]:
> - fixed commit message so that git am doesn't fail.
>
> Changes from v2 [4]:
> - move 'dma_mask' to platform_object so that it's always
> allocated and won't leak on release; remove all previously
> added support functions.
> - use C99 flexible array member for 'name' to remove padding
> at the end of platform_object.
>
> Changes from v1 [5]:
> - remove unneeded kfree() from error path
> - add reference to author/commit adding allocation of dmamask
>
> Changes from v0 [6]:
> - small rewrite to squeeze the patch to a bare minimal
I'd drop the changes from the commit log, but other than that I like it.
Acked-by: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>
Thanks
Uwe
>
> [1] http://lkml.kernel.org/r/1401122483-31603-2-git-send-email-emilgoode@xxxxxxxxx
> http://lkml.kernel.org/r/1401122483-31603-1-git-send-email-emilgoode@xxxxxxxxx
> http://lkml.kernel.org/r/1401122483-31603-3-git-send-email-emilgoode@xxxxxxxxx
>
> [2] http://lkml.kernel.org/r/1390817152-30898-1-git-send-email-ydroneaud@xxxxxxxxxx
> https://patchwork.kernel.org/patch/3541871/
>
> [3] http://lkml.kernel.org/r/1390771138-28348-1-git-send-email-ydroneaud@xxxxxxxxxx
> https://patchwork.kernel.org/patch/3540081/
>
> [4] http://lkml.kernel.org/r/1389683909-17495-1-git-send-email-ydroneaud@xxxxxxxxxx
> https://patchwork.kernel.org/patch/3484411/
>
> [5] http://lkml.kernel.org/r/1389649085-7365-1-git-send-email-ydroneaud@xxxxxxxxxx
> https://patchwork.kernel.org/patch/3480961/
>
> [6] http://lkml.kernel.org/r/1386886207-2735-1-git-send-email-ydroneaud@xxxxxxxxxx
>
> Cc: Emil Goode <emilgoode@xxxxxxxxx>
> Cc: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> Cc: Shawn Guo <shawn.guo@xxxxxxxxxxxxx>
> Cc: Sascha Hauer <kernel@xxxxxxxxxxxxxx>
> Cc: Russell King <linux@xxxxxxxxxxxxxxxx>
> Cc: Olof Johansson <olof@xxxxxxxxx>
> Cc: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>
> Cc: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Yann Droneaud <ydroneaud@xxxxxxxxxx>
> ---
> drivers/base/platform.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index 5b47210889e0..b126c9363f53 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -162,7 +162,7 @@ EXPORT_SYMBOL_GPL(platform_add_devices);
>
> struct platform_object {
> struct platform_device pdev;
> - char name[1];
> + char name[];
> };
>
> /**
> @@ -203,7 +203,7 @@ struct platform_device *platform_device_alloc(const char *name, int id)
> {
> struct platform_object *pa;
>
> - pa = kzalloc(sizeof(struct platform_object) + strlen(name), GFP_KERNEL);
> + pa = kzalloc(sizeof(*pa) + strlen(name) + 1, GFP_KERNEL);
> if (pa) {
> strcpy(pa->name, name);
> pa->pdev.name = pa->name;
> --
> 1.9.3
>
>
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
