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

Christoph Hellwig
2022-04-11T15:53:05Z

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.