linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Roland Dreier <rdreier@cisco.com>
To: James.Bottomley@HansenPartnership.com,
	linux-scsi@vger.kernel.org, arne.redlich@xiranet.com
Subject: [PATCH] srp.h: avoid padding of structs
Date: Tue, 18 Apr 2006 09:05:39 -0700	[thread overview]
Message-ID: <adavet6j2cs.fsf_-_@cisco.com> (raw)
In-Reply-To: <20050823221723.GE31949@krispykreme> (Anton Blanchard's message of "Wed, 24 Aug 2005 08:17:23 +1000")

Thanks for pointing out this problem.  However I don't think we should
pack every structure in the file, because packing structures that
don't need it leads to gcc generating horrible code on architectures
like ia64.  I'd prefer something like the following.

James, can you queue this up?  I'd be happy to merge it through my
tree but it seems more appropriate for it to go through the SCSI tree.
It's probably 2.6.17 material at this point in the cycle.

Thanks,
  Roland



Several structs in <scsi/srp.h> get padded to a multiple of 8 bytes on
64-bit architectures and end up with a size that does not match the
definition in the SRP spec:

                                     SRP spec     64-bit
    sizeof (struct indirect_buf)        20          24
    sizeof (struct srp_login_rsp)       52          56
    sizeof (struct srp_rsp)             36          40

Fix this by adding __attribute__((packed)) to the offending structs.

Problem pointed out by Arne Redlich <arne.redlich@xiranet.com>.

Signed-off-by: Roland Dreier <rolandd@cisco.com>

diff --git a/include/scsi/srp.h b/include/scsi/srp.h
index 6c2681d..637f77e 100644
--- a/include/scsi/srp.h
+++ b/include/scsi/srp.h
@@ -95,14 +95,15 @@ struct srp_direct_buf {
 
 /*
  * We need the packed attribute because the SRP spec puts the list of
- * descriptors at an offset of 20, which is not aligned to the size
- * of struct srp_direct_buf.
+ * descriptors at an offset of 20, which is not aligned to the size of
+ * struct srp_direct_buf.  The whole structure must be packed to avoid
+ * having the 20-byte structure padded to 24 bytes on 64-bit architectures.
  */
 struct srp_indirect_buf {
 	struct srp_direct_buf	table_desc;
 	__be32			len;
-	struct srp_direct_buf	desc_list[0] __attribute__((packed));
-};
+	struct srp_direct_buf	desc_list[0];
+} __attribute__((packed));
 
 enum {
 	SRP_MULTICHAN_SINGLE = 0,
@@ -122,6 +123,11 @@ struct srp_login_req {
 	u8	target_port_id[16];
 };
 
+/*
+ * The SRP spec defines the size of the LOGIN_RSP structure to be 52
+ * bytes, so it needs to be packed to avoid having it padded to 56
+ * bytes on 64-bit architectures.
+ */
 struct srp_login_rsp {
 	u8	opcode;
 	u8	reserved1[3];
@@ -132,7 +138,7 @@ struct srp_login_rsp {
 	__be16	buf_fmt;
 	u8	rsp_flags;
 	u8	reserved2[25];
-};
+} __attribute__((packed));
 
 struct srp_login_rej {
 	u8	opcode;
@@ -207,6 +213,11 @@ enum {
 	SRP_RSP_FLAG_DIUNDER  = 1 << 5
 };
 
+/*
+ * The SRP spec defines the size of the RSP structure to be 36 bytes,
+ * so it needs to be packed to avoid having it padded to 40 bytes on
+ * 64-bit architectures.
+ */
 struct srp_rsp {
 	u8	opcode;
 	u8	sol_not;
@@ -221,6 +232,6 @@ struct srp_rsp {
 	__be32	sense_data_len;
 	__be32	resp_data_len;
 	u8	data[0];
-};
+} __attribute__((packed));
 
 #endif /* SCSI_SRP_H */

  parent reply	other threads:[~2006-04-18 16:05 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-04-16 19:34 [PATCH] srp.h: avoid padding of structs Arne Redlich
2006-04-17  9:17 ` Arne Redlich
     [not found]   ` <20050823221723.GE31949@krispykreme>
2006-04-18 16:05     ` Roland Dreier [this message]
2006-04-18 16:27       ` Arne Redlich
2006-04-18 16:33         ` Roland Dreier
2006-04-28 14:53       ` Arne Redlich

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=adavet6j2cs.fsf_-_@cisco.com \
    --to=rdreier@cisco.com \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=arne.redlich@xiranet.com \
    --cc=linux-scsi@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).