Re: [PATCH 1/3] block: Respect the device's maximum segment size

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1722 bytes --]

On Tue, Sep 10, 2019 at 08:13:48AM +0200, Christoph Hellwig wrote:
> On Tue, Sep 10, 2019 at 02:03:17AM +0000, Yoshihiro Shimoda wrote:
> > I'm sorry for causing this trouble on your environment. I have a proposal to
> > resolve this issue. The mmc_host struct will have a new caps2 flag
> > like MMC_CAP2_MERGE_CAPABLE and add a condition into the queue.c like below.
> > 
> > +	if (host->caps2 & MMC_CAP2_MERGE_CAPABLE &&
> > +	    host->max_segs < MMC_DMA_MAP_MERGE_SEGMENTS &&
> > 	    dma_get_merge_boundary(mmc_dev(host)))
> > 		host->can_dma_map_merge = 1;
> > 	else
> > 		host->can_dma_map_merge = 0;
> > 
> > After that, all mmc controllers disable the feature as default, and if a mmc
> > controller has such capable, the host driver should set the flag.
> 
> That sounds sensible to me.  Alternatively we'd have to limit
> max_sectors to 16-bit values for sdhci if using an iommu that can
> merge.

Isn't that effectively what dma_set_max_seg_size() is supposed to be
doing? That tells the DMA API what the maximum size of a segment can
be for the given device, right? If we make sure never to exceed that
when compacting the SG, the SG that we get back should map just fine
into the descriptors that SDHCI supports.

Now, for devices that support larger segments (or in fact no concept
of segments at all), if we set the maximum segment size to something
really big, isn't that going to automatically allow the huge, merged
segments that the original patch intended?

To me this sounds simply like an issue of the queue code thinking it
knows better than the driver and just overriding the maximum segment
size. Isn't that the real bug here that needs to be fixed?

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]