Re: [PATCH] tcm_loop: Increase maximum request size

[Mike Christie <michael.christie@oracle.com>]

On 12/19/22 8:39 AM, Nikos Tsironis wrote:
> On 12/7/22 20:35, Christoph Hellwig wrote:
>> On Wed, Dec 07, 2022 at 12:29:56PM -0600, Mike Christie wrote:
>>> I think you need to make this configurable.
>>>
>>> If you use loop with pscsi, then the sgl that loop now gets might be too
>>> big for the backend device so we now fail in:
>>>
>>> pscsi_map_sg -> blk_rq_append_bio -> ll_back_merge_fn
>>>
>>> So some users might be relying on the smaller limit.
>>
>> Note that this could happen even now, you just need sufficiently
>> horrible hardware to pass through for it.  But yes, for pscsi
>> this needs to look at the underlying device, and increasing the
>> limit might be a good point to do that.  I'm not sure it's worth
>> to add user configuration, though.
> 
> Thanks for the feedback.
> 
> What should I do? Change pscsi to look at the underlying device limits?

Yes. I think there might be 2 things to do here:

1. SCSI VPD values.

I think we would do something similar to iblock_configure_device
where when we setup the device we look at the underlying device's limits
and then set the se_device's limits. Those values then get reported to
the initiator in the VPD info, so that would be useful for normal drivers
like iscsi and FC.

For the max segments there's not a SCSI VPD value, but we have this code:

        /*
         * Set MAXIMUM TRANSFER LENGTH
         *
         * XXX: Currently assumes single PAGE_SIZE per scatterlist for fabrics
         * enforcing maximum HW scatter-gather-list entry limit
         */
        if (cmd->se_tfo->max_data_sg_nents) {
                mtl = (cmd->se_tfo->max_data_sg_nents * PAGE_SIZE) /
                       dev->dev_attrib.block_size;
        }
        put_unaligned_be32(min_not_zero(mtl, dev->dev_attrib.hw_max_sectors), &buf[8]);

which would limit the IO size. max_data_sg_nents is only currently a fabric
driver (front end drivers like iscsi, fc, etc) limit, so we would need to make
that more generic so backends like pscsi can set it.


2. For loop we have the sg_tablesize setting like you saw, but the problem is
that it's host level and we don't know the devices when we add the host.

I think we would have to modify tcm_loop_port_link so if adds the device
successfully, we call blk_queue_max_segments.

Christoph is that what you were thinking?