Re: [PATCH 075/117] nfsd: Convert to the scsi_status union

Chuck Lever III
2021-04-21T14:23:07Z

> On Apr 20, 2021, at 12:44 PM, Bart Van Assche <bvanassche@acm.org> wrote:
>
> On 4/20/21 7:36 AM, Chuck Lever III wrote:
>>> On Apr 19, 2021, at 8:08 PM, Bart Van Assche <bvanassche@acm.org> 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@fieldses.org>
>>> Cc: Chuck Lever <chuck.lever@oracle.com>
>>> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
>>
>> 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@acm.org/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.

> The names in the Cc: list come from the following entry in the
> MAINTAINERS file: "KERNEL NFSD, SUNRPC, AND LOCKD SERVERS".
>
> Bart.

--
Chuck Lever