From: Thierry Reding <thierry.reding@gmail.com>
To: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Cc: Christoph Hellwig <hch@lst.de>,
Ulf Hansson <ulf.hansson@linaro.org>,
Jens Axboe <axboe@kernel.dk>,
Adrian Hunter <adrian.hunter@intel.com>,
Simon Horman <horms+renesas@verge.net.au>,
Jon Hunter <jonathanh@nvidia.com>,
"linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
"linux-mmc@vger.kernel.org" <linux-mmc@vger.kernel.org>,
"linux-tegra@vger.kernel.org" <linux-tegra@vger.kernel.org>
Subject: Re: [PATCH 1/3] block: Respect the device's maximum segment size
Date: Tue, 10 Sep 2019 09:30:32 +0200 [thread overview]
Message-ID: <20190910073032.GA12537@ulmo> (raw)
In-Reply-To: <TYAPR01MB454470364B682B9BF708E557D8B60@TYAPR01MB4544.jpnprd01.prod.outlook.com>
[-- Attachment #1: Type: text/plain, Size: 3226 bytes --]
On Tue, Sep 10, 2019 at 02:03:17AM +0000, Yoshihiro Shimoda wrote:
> Hi Thierry,
>
> > From: Thierry Reding, Sent: Tuesday, September 10, 2019 4:19 AM
> >
> > On Mon, Sep 09, 2019 at 06:13:31PM +0200, Christoph Hellwig wrote:
> > > On Mon, Sep 09, 2019 at 02:56:56PM +0200, Thierry Reding wrote:
> > > > From: Thierry Reding <treding@nvidia.com>
> > > >
> > > > When enabling the DMA map merging capability for a queue, ensure that
> > > > the maximum segment size does not exceed the device's limit.
> > >
> > > We can't do that unfortunately. If we use the virt_boundary setting
> > > we do aggressive merges that there is no accounting for. So we can't
> > > limit the segment size.
> >
> > But that's kind of the point here. My understanding is that the maximum
> > segment size in the device's DMA parameters defines the maximum size of
> > the segment that the device can handle.
> >
> > In the particular case that I'm trying to fix, according to the SDHCI
> > specification, these devices can handle segments that are a maximum of
> > 64 KiB in size. If we allow that segment size to be exceeded, the device
> > will no longer be able to handle them.
> >
> > > And at least for the case how devices usually do the addressing
> > > (page based on not sgl based) that needs the virt_boundary there isn't
> > > really any concept like a segment anyway.
> >
> > I do understand that aspect of it. However, devices that do the
> > addressing this way, wouldn't they want to set the maximum segment size
> > to something large (like UINT_MAX, which many users that don't have the
> > concept of a segment already do)?
> >
> > If you disagree, do you have any alternative proposals other than
> > reverting the offending patch? linux-next is currently broken on all
> > systems where the SDHCI controller is behind an IOMMU.
>
> 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.
> Ulf, is such a patch acceptable for v5.4-rcN? Anyway, I'll submit such a patch later.
I'm sure that would work, but I think that's missing the point. It's not
that the setup isn't capable of merging at all. It just can't deal with
segments that are too large.
While I was debugging this, I was frequently seeing cases where the SG
was on the order of 40 entries initially and after dma_map_sg() it was
reduced to just 4 or 5.
So I think merging is still really useful if a setup supports it via an
IOMMU. I'm just not sure I see why we can't make the code respect what-
ever the maximum segment size that the driver may have configured. That
seems like it should allow us to get the best of both worlds.
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
next prev parent reply other threads:[~2019-09-10 7:30 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-09 12:56 [PATCH 0/3] mmc: Fix scatter/gather on SDHCI Thierry Reding
2019-09-09 12:56 ` [PATCH 1/3] block: Respect the device's maximum segment size Thierry Reding
2019-09-09 16:13 ` Christoph Hellwig
2019-09-09 19:19 ` Thierry Reding
2019-09-10 2:03 ` Yoshihiro Shimoda
2019-09-10 6:13 ` Christoph Hellwig
2019-09-10 7:37 ` Thierry Reding
2019-09-11 10:36 ` Christoph Hellwig
2019-09-10 7:30 ` Thierry Reding [this message]
2019-09-11 7:23 ` Yoshihiro Shimoda
2019-09-11 10:37 ` Christoph Hellwig
2019-09-12 0:57 ` Ming Lei
2019-09-12 8:19 ` Christoph Hellwig
2019-09-09 12:56 ` [PATCH 2/3] mmc: core: Respect MMC host's " Thierry Reding
2019-09-09 12:56 ` [PATCH 3/3] mmc: sdhci: Set DMA maximum segment size to 64 KiB Thierry Reding
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190910073032.GA12537@ulmo \
--to=thierry.reding@gmail.com \
--cc=adrian.hunter@intel.com \
--cc=axboe@kernel.dk \
--cc=hch@lst.de \
--cc=horms+renesas@verge.net.au \
--cc=jonathanh@nvidia.com \
--cc=linux-block@vger.kernel.org \
--cc=linux-mmc@vger.kernel.org \
--cc=linux-tegra@vger.kernel.org \
--cc=ulf.hansson@linaro.org \
--cc=yoshihiro.shimoda.uh@renesas.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).