On 4/21/21 7:22 AM, Chuck Lever III wrote:
On Apr 20, 2021, at 12:44 PM, Bart Van Assche <bvanassche@xxxxxxx> wrote:
On 4/20/21 7:36 AM, Chuck Lever III wrote:
On Apr 19, 2021, at 8:08 PM, Bart Van Assche <bvanassche@xxxxxxx> wrote:
An explanation of the purpose of this patch is available in the patch
"scsi: Introduce the scsi_status union".
Cc: "J. Bruce Fields" <bfields@xxxxxxxxxxxx>
Cc: Chuck Lever <chuck.lever@xxxxxxxxxx>
Signed-off-by: Bart Van Assche <bvanassche@xxxxxxx>
Hi Bart, I assume this is going into v5.13 via the SCSI tree?
Do you need an Acked-by: from the NFSD maintainers?
Hi Chuck,
Thanks for having taken a look. In case you would not yet have found the
"scsi: Introduce the scsi_status union" patch, it is available here:
https://lore.kernel.org/linux-scsi/20210420000845.25873-12-bvanassche@xxxxxxx/T/#u
An Acked-by or Reviewed-by from an NFS expert would be great.
The NFSD patch looks OK to me, but I'm hesitating on sending
an Acked-by.
I went back and looked at the scsi_status union patch, and
that looks dodgy to me.
AFAIK, "enum" doesn't cause the compiler to reserve any
particular size of storage, it just makes a guess. What
keeps those enum fields from being 16- or 32-bits wide?
Shouldn't those be u8 to enforce the correct field size?
I'm not sure where to look for further discussion on that
part of the series.
Hi Chuck,
Although the C standard requires that enums have the same size as an
int, gcc and clang support the attribute "packed" for enums. From the
gcc documentation about the packed attribute: "When attached to an enum
definition, it indicates that the smallest integral type should be used."
Additionally, the following BUILD_BUG_ON() statements verify the size
and endianness of the members of the scsi_status union (see also
https://www.spinics.net/lists/linux-scsi/msg157796.html):
+#define TEST_STATUS ((union scsi_status){.combined = 0x01020308})
+
static int __init init_scsi(void)
{
int error;
+ BUILD_BUG_ON(sizeof(union scsi_status) != 4);
+ BUILD_BUG_ON(TEST_STATUS.combined != 0x01020308);
+ BUILD_BUG_ON(driver_byte(TEST_STATUS) != 1);
+ BUILD_BUG_ON(host_byte(TEST_STATUS) != 2);
+ BUILD_BUG_ON(msg_byte(TEST_STATUS) != 3);
+ BUILD_BUG_ON(status_byte(TEST_STATUS) != 4);
Does this address your concern?
Bart.
