Re: [PATCH v6 1/1] scsi: sas: skip opt_sectors when DMA reports no real optimization hint

[John Garry]

On 26/03/2026 08:46, Ionut Nechita (Wind River) wrote:
> sas_host_setup() unconditionally sets shost->opt_sectors from
> dma_opt_mapping_size().  When the IOMMU is disabled or in passthrough
> mode and no DMA ops provide an opt_mapping_size callback,
> dma_opt_mapping_size() returns min(dma_max_mapping_size(), SIZE_MAX)
> which equals dma_max_mapping_size() — a hard upper bound, not an
> optimization hint.
>
> On a Dell PowerEdge R750 with mpt3sas (Broadcom SAS3816, FW 33.15.00.00)
> and intel_iommu=off the following values are observed:
>
>    dma_opt_mapping_size()  = dma_max_mapping_size() (no real hint)
>    shost->max_sectors      = 32767
>    opt_sectors             = min(32767, huge >> 9) = 32767
>    optimal_io_size         = 32767 << 9 = 16776704
>                            → round_down(16776704, 4096) = 16773120
>
> The SAS disk (SAMSUNG MZILT800HBHQ0D3) does not report an
> Optimal Transfer Length in VPD page B0, so sdkp->opt_xfer_blocks
> remains 0.  sd_revalidate_disk() then uses min_not_zero(0, opt_sectors)
> = opt_sectors, propagating the bogus value into the block device's
> optimal_io_size (visible as OPT-IO = 16773120 in lsblk --topology).
>
> mkfs.xfs picks up optimal_io_size and minimum_io_size and computes:
>
>    swidth = 16773120 / 4096 = 4095
>    sunit  = 8192 / 4096     = 2
>
> Since 4095 % 2 != 0, XFS rejects the geometry:
>
>    SB stripe unit sanity check failed
>
> This makes it impossible to create XFS filesystems (e.g. for
> /var/lib/docker) during system bootstrap.
>
> Fix this by introducing a sas_dma_setup_opt_sectors() helper that
> sets opt_sectors only when dma_opt_mapping_size() is strictly less
> than dma_max_mapping_size(), indicating a genuine DMA optimization
> constraint.  The helper computes min(opt_sectors, max_sectors) first,
> then rounds down to a power of two so that filesystem geometry
> calculations always produce clean results.  When the two DMA values
> are equal, no backend provided a real hint, so opt_sectors stays at
> 0 ("no preference").
>
> Fixes: 4cbfca5f7750 ("scsi: scsi_transport_sas: cap shost opt_sectors according to DMA optimal limit")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Ionut Nechita <ionut.nechita@xxxxxxxxxxxxx>

I have some nits below, regardless of that, FWIW:

Reviewed-by: John Garry <john.g.garry@xxxxxxxxxx>


> ---
>   drivers/scsi/scsi_transport_sas.c | 38 +++++++++++++++++++++++++++----
>   1 file changed, 33 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/scsi/scsi_transport_sas.c b/drivers/scsi/scsi_transport_sas.c
> index 13412702188e4..fa79a0883bb3d 100644
> --- a/drivers/scsi/scsi_transport_sas.c
> +++ b/drivers/scsi/scsi_transport_sas.c
> @@ -27,6 +27,7 @@
>   #include <linux/module.h>
>   #include <linux/jiffies.h>
>   #include <linux/err.h>
> +#include <linux/log2.h>
>   #include <linux/slab.h>
>   #include <linux/string.h>
>   #include <linux/blkdev.h>
> @@ -222,12 +223,42 @@ static int sas_bsg_initialize(struct Scsi_Host *shost, struct sas_rphy *rphy)
>    * SAS host attributes
>    */
>   
> +/*
> + * Set shost->opt_sectors from the DMA optimal mapping size, but only
> + * when dma_opt_mapping_size() is strictly less than dma_max_mapping_size(),

Aside from this patch, dma_opt_mapping_size() may be better named to 
dma_max_opt_mapping_size() or similar, to indicate that it is an upper 
limit of good performance and not a sweet spot which we should aim for

> + * indicating a genuine optimization hint from an IOMMU or DMA backend.
> + * When the two are equal (e.g. IOMMU disabled / passthrough), no real
> + * hint exists, so leave opt_sectors at 0 to avoid bogus optimal_io_size
> + * values that break filesystem geometry (e.g. mkfs.xfs stripe alignment).
> + */
> +static void sas_dma_setup_opt_sectors(struct Scsi_Host *shost)
> +{
> +	struct device *dma_dev = shost->dma_dev;
> +	size_t opt, max;
> +	unsigned int opt_sectors;
> +
> +	if (!dma_dev->dma_mask)
> +		return;
> +
> +	opt = dma_opt_mapping_size(dma_dev);
> +	max = dma_max_mapping_size(dma_dev);
> +
> +	if (!opt || opt >= max)
> +		return;

opt > max should not be possible, but I suppose no harm to check. And I 
think that the opt == 0 check is really covered by the !opt_sectors 
check, below

> +
> +	opt_sectors = min_t(unsigned int, opt >> SECTOR_SHIFT,
> +			    shost->max_sectors);
> +	if (!opt_sectors)
> +		return;

I don't think that opt_sectors == 0 is possible as max_sectors == 0 is 
not possible unless someone hacks their SCSI LLD to override it to zero 
after scsi_host_alloc(), so I suppose that the check is ok since 
rounddown_pow_of_two(0) gives undefined behaviour

> +
> +	shost->opt_sectors = rounddown_pow_of_two(opt_sectors);
> +}
> +
>   static int sas_host_setup(struct transport_container *tc, struct device *dev,
>   			  struct device *cdev)
>   {
>   	struct Scsi_Host *shost = dev_to_shost(dev);
>   	struct sas_host_attrs *sas_host = to_sas_host_attrs(shost);
> -	struct device *dma_dev = shost->dma_dev;
>   
>   	INIT_LIST_HEAD(&sas_host->rphy_list);
>   	mutex_init(&sas_host->lock);
> @@ -239,10 +270,7 @@ static int sas_host_setup(struct transport_container *tc, struct device *dev,
>   		dev_printk(KERN_ERR, dev, "fail to a bsg device %d\n",
>   			   shost->host_no);
>   
> -	if (dma_dev->dma_mask) {
> -		shost->opt_sectors = min_t(unsigned int, shost->max_sectors,
> -				dma_opt_mapping_size(dma_dev) >> SECTOR_SHIFT);
> -	}
> +	sas_dma_setup_opt_sectors(shost);
>   
>   	return 0;
>   }