Re: [PATCH v2 0/6] scsi: fix scsi_cmd::cmd_len

[Christoph Hellwig <hch@lst.de>]

On Mon, Apr 11, 2022 at 11:06:17AM -0400, Douglas Gilbert wrote:
> On 2022-04-11 01:03, Christoph Hellwig wrote:
>> This still misses any good explanation of why we want all this.
>
> Advantages:
>    - undoes regression in ce70fd9a551af, that is:
>        - cdb_len > 32 no longer allowed (visible to the user space), undone

What exact regression causes this for real users and no just people
playing around with scsi_debug?

>        - but we still have this one:
>            - prior to lk5.18 sizeof(scsi_cmnd::cmnd) is that of a
>              pointer but >= lk5.18 sizeof(scsi_cmnd::cmnd) is 32 (or 16)

Please check the total size of struct scsi_cmnd, which is what really
matters.

>    - makes all scsi_cmnd objects 16 bytes smaller

Do we have data why this matters?

>    - hides the poorly named dtor for scsi_cmnd objects (blk_mq_free_request)
>      within a more intuitively named inline: scsi_free_cmnd

I don't think this is in any way poorly named.

> Disadvantages:
>     - burdens each access to a CDB with (scsi_cmnd::flags & SCMD_LONG_CDB)
>       check
>     - LLDs that want to fetch 32 byte CDBs (or longer) need to use the
>       scsi_cmnd_get_cdb() access function. For CDB lengths <= 16 bytes
>       they can continue to access scsi_cmnd::cmnd directly

It adds back the dynamic allocation for 32-byte CDBs that we got rid of.
It also breaks all LLDS actually using 32-byte CDBS currently as far as
I can tell.