Skip to content

ioctl: Remove attribute packed and alignedof for args structs#321

Merged
igaw merged 1 commit intolinux-nvme:masterfrom
igaw:remove-packed-attribute
Mar 31, 2022
Merged

ioctl: Remove attribute packed and alignedof for args structs#321
igaw merged 1 commit intolinux-nvme:masterfrom
igaw:remove-packed-attribute

Conversation

@igaw
Copy link
Copy Markdown
Collaborator

@igaw igaw commented Mar 30, 2022

The attribute packed is usually used to make sure the data structures
is compatible between different compilers in regards of padding rules.

As we have sorted the members of all argument structs according their
naturual size, there are no holes to pad. This makes the packed
attributed superflous as compilers are agree on the data layout in
this case.

The alignedof attribute is used to tell the outer alignmen of the data
structure because the aligment of a packed data structure is 1.

Anyway, both attributes doen't add any benefits to the layout and
pahole agrees on this.

This is an example the diff between the packed/aligneof version and
the plain version:

struct nvme_dim_args {
__u32 * result; /* 0 8 /
void * data; /
8 8 /
@@ -776,4 +777,4 @@
/
size: 40, cachelines: 1, members: 7 /
/
padding: 7 /
/
last cacheline: 40 bytes */
-} attribute((aligned(8)));
+};

Thus, just remove the attributes.

Signed-off-by: Daniel Wagner dwagner@suse.de

Fixes: #285

The attribute packed is usually used to make sure the data structures
is compatible between different compilers in regards of padding rules.

As we have sorted the members of all argument structs according their
naturual size, there are no holes to pad. This makes the packed
attributed superflous as compilers are agree on the data layout in
this case.

The alignedof attribute is used to tell the outer alignmen of the data
structure because the aligment of a packed data structure is 1.

Anyway, both attributes doen't add any benefits to the layout and
pahole agrees on this.

This is an example the diff between the packed/aligneof version and
the plain version:

 struct nvme_dim_args {
        __u32 *                    result;               /*     0     8 */
        void *                     data;                 /*     8     8 */
@@ -776,4 +777,4 @@
        /* size: 40, cachelines: 1, members: 7 */
        /* padding: 7 */
        /* last cacheline: 40 bytes */
-} __attribute__((__aligned__(8)));
+};

Thus, just remove the attributes.

Signed-off-by: Daniel Wagner <dwagner@suse.de>
@igaw
Copy link
Copy Markdown
Collaborator Author

igaw commented Mar 30, 2022

The complete pahole diff output (note we even fix a problem for struct nvme_lockdown_args it just had packed and no alignmentof)

--- /tmp/pahole-ioctl-padded.txt	2022-03-30 09:33:25.888098975 +0200
+++ /tmp/pahole-ioctl.txt	2022-03-30 09:35:21.008584057 +0200
@@ -284,7 +284,7 @@
 	/* size: 48, cachelines: 1, members: 11 */
 	/* padding: 3 */
 	/* last cacheline: 48 bytes */
-} __attribute__((__aligned__(8)));
+};
 struct nvme_get_log_args {
 	__u64                      lpo;                  /*     0     8 */
 	__u32 *                    result;               /*     8     8 */
@@ -304,7 +304,7 @@
 
 	/* size: 64, cachelines: 1, members: 15 */
 	/* padding: 6 */
-} __attribute__((__aligned__(8)));
+};
 struct nvme_set_features_args {
 	__u32 *                    result;               /*     0     8 */
 	void *                     data;                 /*     8     8 */
@@ -324,7 +324,7 @@
 	/* size: 56, cachelines: 1, members: 14 */
 	/* padding: 1 */
 	/* last cacheline: 56 bytes */
-} __attribute__((__aligned__(8)));
+};
 struct nvme_get_features_args {
 	__u32 *                    result;               /*     0     8 */
 	void *                     data;                 /*     8     8 */
@@ -341,7 +341,7 @@
 	/* size: 48, cachelines: 1, members: 11 */
 	/* padding: 2 */
 	/* last cacheline: 48 bytes */
-} __attribute__((__aligned__(8)));
+};
 struct nvme_format_nvm_args {
 	__u32 *                    result;               /*     0     8 */
 	int                        args_size;            /*     8     4 */
@@ -357,7 +357,7 @@
 	/* size: 48, cachelines: 1, members: 10 */
 	/* padding: 7 */
 	/* last cacheline: 48 bytes */
-} __attribute__((__aligned__(8)));
+};
 struct nvme_ns_mgmt_args {
 	__u32 *                    result;               /*     0     8 */
 	struct nvme_id_ns *        ns;                   /*     8     8 */
@@ -371,7 +371,7 @@
 	/* size: 40, cachelines: 1, members: 8 */
 	/* padding: 3 */
 	/* last cacheline: 40 bytes */
-} __attribute__((__aligned__(8)));
+};
 struct nvme_ns_attach_args {
 	__u32 *                    result;               /*     0     8 */
 	struct nvme_ctrl_list *    ctrlist;              /*     8     8 */
@@ -384,7 +384,7 @@
 	/* size: 40, cachelines: 1, members: 7 */
 	/* padding: 4 */
 	/* last cacheline: 40 bytes */
-} __attribute__((__aligned__(8)));
+};
 struct nvme_fw_download_args {
 	__u32 *                    result;               /*     0     8 */
 	void *                     data;                 /*     8     8 */
@@ -397,7 +397,7 @@
 	/* size: 40, cachelines: 1, members: 7 */
 	/* padding: 4 */
 	/* last cacheline: 40 bytes */
-} __attribute__((__aligned__(8)));
+};
 struct nvme_fw_commit_args {
 	__u32 *                    result;               /*     0     8 */
 	int                        args_size;            /*     8     4 */
@@ -410,7 +410,7 @@
 	/* size: 32, cachelines: 1, members: 7 */
 	/* padding: 6 */
 	/* last cacheline: 32 bytes */
-} __attribute__((__aligned__(8)));
+};
 struct nvme_security_send_args {
 	__u32 *                    result;               /*     0     8 */
 	void *                     data;                 /*     8     8 */
@@ -428,7 +428,7 @@
 	/* size: 48, cachelines: 1, members: 12 */
 	/* padding: 4 */
 	/* last cacheline: 48 bytes */
-} __attribute__((__aligned__(8)));
+};
 struct nvme_security_receive_args {
 	__u32 *                    result;               /*     0     8 */
 	void *                     data;                 /*     8     8 */
@@ -446,7 +446,7 @@
 	/* size: 48, cachelines: 1, members: 12 */
 	/* padding: 4 */
 	/* last cacheline: 48 bytes */
-} __attribute__((__aligned__(8)));
+};
 struct nvme_get_lba_status_args {
 	__u64                      slba;                 /*     0     8 */
 	__u32 *                    result;               /*     8     8 */
@@ -462,7 +462,7 @@
 	/* size: 56, cachelines: 1, members: 10 */
 	/* padding: 6 */
 	/* last cacheline: 56 bytes */
-} __attribute__((__aligned__(8)));
+};
 struct nvme_directive_send_args {
 	__u32 *                    result;               /*     0     8 */
 	void *                     data;                 /*     8     8 */
@@ -479,7 +479,7 @@
 	/* size: 56, cachelines: 1, members: 11 */
 	/* padding: 6 */
 	/* last cacheline: 56 bytes */
-} __attribute__((__aligned__(8)));
+};
 struct nvme_directive_recv_args {
 	__u32 *                    result;               /*     0     8 */
 	void *                     data;                 /*     8     8 */
@@ -496,7 +496,7 @@
 	/* size: 56, cachelines: 1, members: 11 */
 	/* padding: 6 */
 	/* last cacheline: 56 bytes */
-} __attribute__((__aligned__(8)));
+};
 struct nvme_capacity_mgmt_args {
 	__u32 *                    result;               /*     0     8 */
 	int                        args_size;            /*     8     4 */
@@ -510,7 +510,7 @@
 	/* size: 32, cachelines: 1, members: 8 */
 	/* padding: 1 */
 	/* last cacheline: 32 bytes */
-} __attribute__((__aligned__(8)));
+};
 struct nvme_lockdown_args {
 	__u32 *                    result;               /*     0     8 */
 	int                        args_size;            /*     8     4 */
@@ -522,9 +522,10 @@
 	__u8                       ofi;                  /*    23     1 */
 	__u8                       uuidx;                /*    24     1 */
 
-	/* size: 25, cachelines: 1, members: 9 */
-	/* last cacheline: 25 bytes */
-} __attribute__((__packed__));
+	/* size: 32, cachelines: 1, members: 9 */
+	/* padding: 7 */
+	/* last cacheline: 32 bytes */
+};
 struct nvme_set_property_args {
 	__u64                      value;                /*     0     8 */
 	__u32 *                    result;               /*     8     8 */
@@ -535,7 +536,7 @@
 
 	/* size: 32, cachelines: 1, members: 6 */
 	/* last cacheline: 32 bytes */
-} __attribute__((__aligned__(8)));
+};
 struct nvme_get_property_args {
 	__u64 *                    value;                /*     0     8 */
 	int                        args_size;            /*     8     4 */
@@ -545,7 +546,7 @@
 
 	/* size: 24, cachelines: 1, members: 5 */
 	/* last cacheline: 24 bytes */
-} __attribute__((__aligned__(8)));
+};
 struct nvme_sanitize_nvm_args {
 	__u32 *                    result;               /*     0     8 */
 	int                        args_size;            /*     8     4 */
@@ -560,7 +561,7 @@
 
 	/* size: 32, cachelines: 1, members: 10 */
 	/* last cacheline: 32 bytes */
-} __attribute__((__aligned__(8)));
+};
 struct nvme_dev_self_test_args {
 	__u32 *                    result;               /*     0     8 */
 	int                        args_size;            /*     8     4 */
@@ -572,7 +573,7 @@
 	/* size: 32, cachelines: 1, members: 6 */
 	/* padding: 4 */
 	/* last cacheline: 32 bytes */
-} __attribute__((__aligned__(8)));
+};
 struct nvme_virtual_mgmt_args {
 	__u32 *                    result;               /*     0     8 */
 	int                        args_size;            /*     8     4 */
@@ -585,7 +586,7 @@
 
 	/* size: 32, cachelines: 1, members: 8 */
 	/* last cacheline: 32 bytes */
-} __attribute__((__aligned__(8)));
+};
 struct nvme_io_args {
 	__u64                      slba;                 /*     0     8 */
 	__u64                      storage_tag;          /*     8     8 */
@@ -610,7 +611,7 @@
 	/* size: 80, cachelines: 2, members: 18 */
 	/* padding: 1 */
 	/* last cacheline: 16 bytes */
-} __attribute__((__aligned__(8)));
+};
 struct nvme_dsm_args {
 	__u32 *                    result;               /*     0     8 */
 	struct nvme_dsm_range *    dsm;                  /*     8     8 */
@@ -624,7 +625,7 @@
 	/* size: 40, cachelines: 1, members: 8 */
 	/* padding: 2 */
 	/* last cacheline: 40 bytes */
-} __attribute__((__aligned__(8)));
+};
 struct nvme_copy_args {
 	__u64                      sdlba;                /*     0     8 */
 	__u32 *                    result;               /*     8     8 */
@@ -646,7 +647,7 @@
 	__u8                       format;               /*    63     1 */
 
 	/* size: 64, cachelines: 1, members: 18 */
-} __attribute__((__aligned__(8)));
+};
 struct nvme_resv_acquire_args {
 	__u64                      crkey;                /*     0     8 */
 	__u64                      nrkey;                /*     8     8 */
@@ -662,7 +663,7 @@
 	/* size: 56, cachelines: 1, members: 10 */
 	/* padding: 7 */
 	/* last cacheline: 56 bytes */
-} __attribute__((__aligned__(8)));
+};
 struct nvme_resv_register_args {
 	__u64                      crkey;                /*     0     8 */
 	__u64                      nrkey;                /*     8     8 */
@@ -678,7 +679,7 @@
 	/* size: 56, cachelines: 1, members: 10 */
 	/* padding: 7 */
 	/* last cacheline: 56 bytes */
-} __attribute__((__aligned__(8)));
+};
 struct nvme_resv_release_args {
 	__u64                      crkey;                /*     0     8 */
 	__u32 *                    result;               /*     8     8 */
@@ -693,7 +694,7 @@
 	/* size: 48, cachelines: 1, members: 9 */
 	/* padding: 7 */
 	/* last cacheline: 48 bytes */
-} __attribute__((__aligned__(8)));
+};
 struct nvme_resv_report_args {
 	__u32 *                    result;               /*     0     8 */
 	struct nvme_resv_status *  report;               /*     8     8 */
@@ -707,7 +708,7 @@
 	/* size: 40, cachelines: 1, members: 8 */
 	/* padding: 3 */
 	/* last cacheline: 40 bytes */
-} __attribute__((__aligned__(8)));
+};
 struct nvme_zns_mgmt_send_args {
 	__u64                      slba;                 /*     0     8 */
 	__u32 *                    result;               /*     8     8 */
@@ -724,7 +725,7 @@
 	/* size: 56, cachelines: 1, members: 11 */
 	/* padding: 6 */
 	/* last cacheline: 56 bytes */
-} __attribute__((__aligned__(8)));
+};
 struct nvme_zns_mgmt_recv_args {
 	__u64                      slba;                 /*     0     8 */
 	__u32 *                    result;               /*     8     8 */
@@ -741,7 +742,7 @@
 	/* size: 56, cachelines: 1, members: 11 */
 	/* padding: 5 */
 	/* last cacheline: 56 bytes */
-} __attribute__((__aligned__(8)));
+};
 struct nvme_zns_append_args {
 	__u64                      zslba;                /*     0     8 */
 	__u64 *                    result;               /*     8     8 */
@@ -763,7 +764,7 @@
 	/* size: 72, cachelines: 2, members: 15 */
 	/* padding: 4 */
 	/* last cacheline: 8 bytes */
-} __attribute__((__aligned__(8)));
+};
 struct nvme_dim_args {
 	__u32 *                    result;               /*     0     8 */
 	void *                     data;                 /*     8     8 */
@@ -776,4 +777,4 @@
 	/* size: 40, cachelines: 1, members: 7 */
 	/* padding: 7 */
 	/* last cacheline: 40 bytes */
-} __attribute__((__aligned__(8)));
+};

@igaw igaw merged commit 451b365 into linux-nvme:master Mar 31, 2022
@igaw igaw deleted the remove-packed-attribute branch March 31, 2022 11:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Should we drop 'attribute packed' in ioctl.h?

2 participants