Re: [PATCH] scsi: storvsc: set max_segment_size as UINT_MAX explicitly

Christoph Hellwig
2025-06-18T05:44:49Z

On Tue, Jun 17, 2025 at 03:25:21PM +0800, Ming Lei wrote:
> > @@ -473,7 +473,9 @@ struct Scsi_Host *scsi_host_alloc(const struct scsi_host_template *sht, int priv
> > else
> > shost->max_sectors = SCSI_DEFAULT_MAX_SECTORS;
> >
> > - if (sht->max_segment_size)
> > + if (sht->virt_boundary_mask)
> > + shost->virt_boundary_mask = sht->virt_boundary_mask;
> > + else if (sht->max_segment_size)
> > shost->max_segment_size = sht->max_segment_size;
> > else
> > shost->max_segment_size = BLK_MAX_SEGMENT_SIZE;
>
> This way works, but I prefer to set it explicitly in driver, instead of
> making block layer more fragile to deal with def ->max_segment_size
> if ->virt_boundary_mask is defined

The block layer already enforces this as it is a requirement. It is
just the SCSI wrapper that broke it. Without this proper fix iser
is still broken, and srp might or might not.

> - for low level driver, if ->virt_boundary_mask is defined, ->max_segment_size
> should be UINT_MAX obviously since it implies single `virt segment`.
> Setting UINT_MAX in driver has document benefit too.

No, it doesn't. It means you need to cargo cult copy and paste code
instead of solving the problem in the proper place.

> - for logical block device(md, dm, ...), both ->virt_boundary_mask and
> ->max_segment_size may be set, and it is fine since logical block device
> driver needn't to deal with sg

Stacked devices should not inherit the hardware limits at all because
we split below them, but that's a separate story. Either way this is
not relevant here as we don't have stacking drivers that use the
SCSI layer.