LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Gerald Schaefer <gerald.schaefer@linux.ibm.com>
To: Karsten Graul <kgraul@linux.ibm.com>
Cc: Ioana Ciornei <ioana.ciornei@nxp.com>,
	Jeremy Linton <jeremy.linton@arm.com>,
	Hamza Mahfooz <someguy@effective-light.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Christoph Hellwig <hch@lst.de>,
	Marek Szyprowski <m.szyprowski@samsung.com>,
	Robin Murphy <robin.murphy@arm.com>,
	"iommu@lists.linux-foundation.org"
	<iommu@lists.linux-foundation.org>,
	Dan Williams <dan.j.williams@intel.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	linux-s390 <linux-s390@vger.kernel.org>,
	Gerald Schaefer <gerald.schaefer@linux.ibm.com>
Subject: Re: DPAA2 triggers, [PATCH] dma debug: report -EEXIST errors in add_dma_entry
Date: Fri, 1 Oct 2021 14:52:56 +0200	[thread overview]
Message-ID: <20211001145256.0323957a@thinkpad> (raw)
In-Reply-To: <185e7ee4-3749-4ccb-6d2e-da6bc8f30c04@linux.ibm.com>

On Thu, 30 Sep 2021 15:37:33 +0200
Karsten Graul <kgraul@linux.ibm.com> wrote:

> On 14/09/2021 17:45, Ioana Ciornei wrote:
> > On Wed, Sep 08, 2021 at 10:33:26PM -0500, Jeremy Linton wrote:
> >> +DPAA2, netdev maintainers
> >> Hi,
> >>
> >> On 5/18/21 7:54 AM, Hamza Mahfooz wrote:
> >>> Since, overlapping mappings are not supported by the DMA API we should
> >>> report an error if active_cacheline_insert returns -EEXIST.
> >>
> >> It seems this patch found a victim. I was trying to run iperf3 on a
> >> honeycomb (5.14.0, fedora 35) and the console is blasting this error message
> >> at 100% cpu. So, I changed it to a WARN_ONCE() to get the call trace, which
> >> is attached below.
> >>
> > 
> > These frags are allocated by the stack, transformed into a scatterlist
> > by skb_to_sgvec and then DMA mapped with dma_map_sg. It was not the
> > dpaa2-eth's decision to use two fragments from the same page (that will
> > also end un in the same cacheline) in two different in-flight skbs.
> > 
> > Is this behavior normal?
> > 
> 
> We see the same problem here and it started with 5.15-rc2 in our nightly CI runs.
> The CI has panic_on_warn enabled so we see the panic every day now.

Adding a WARN for a case that be detected false-positive seems not
acceptable, exactly for this reason (kernel panic on unaffected
systems).

So I guess it boils down to the question if the behavior that Ioana
described is legit behavior, on a system that is dma coherent. We
are apparently hitting the same scenario, although it could not yet be
reproduced with debug printks for some reason.

If the answer is yes, than please remove at lease the WARN, so that
it will not make systems crash that behave valid, and have
panic_on_warn set. Even a normal printk feels wrong to me in that
case, it really sounds rather like you want to fix / better refine
the overlap check, if you want to report anything here.

BTW, there is already a WARN in the add_dma_entry() path, related
to cachlline overlap and -EEXIST:

add_dma_entry() -> active_cacheline_insert() -> -EEXIST ->
active_cacheline_inc_overlap()

That will only trigger when "overlap > ACTIVE_CACHELINE_MAX_OVERLAP".
Not familiar with that code, but it seems that there are now two
warnings for more or less the same, and the new warning is much more
prone to false-positives.

How do these 2 warnings relate, are they both really necessary?
I think the new warning was only introduced because of some old
TODO comment in add_dma_entry(), see commit 2b4bbc6231d78
("dma-debug: report -EEXIST errors in add_dma_entry").

That comment was initially added by Dan long time ago, and he
added several fix-ups for overlap detection after that, including
the "overlap > ACTIVE_CACHELINE_MAX_OVERLAP" stuff in
active_cacheline_inc_overlap(). So could it be that the TODO
comment was simply not valid any more, and better be removed
instead of adding new / double warnings, that also generate
false-positives and kernel crashes?

  reply	other threads:[~2021-10-01 12:53 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-18 12:54 Hamza Mahfooz
2021-06-22  7:41 ` Christoph Hellwig
2021-09-09  3:33 ` DPAA2 triggers, " Jeremy Linton
2021-09-09 21:16   ` Ioana Ciornei
2021-09-10 10:23   ` Christoph Hellwig
2021-09-14 15:45   ` Ioana Ciornei
2021-09-30 13:37     ` Karsten Graul
2021-10-01 12:52       ` Gerald Schaefer [this message]
2021-10-06 13:10         ` Gerald Schaefer
2021-10-06 13:21           ` Gerald Schaefer
2021-10-06 14:23           ` Robin Murphy
2021-10-06 15:06             ` Gerald Schaefer
2021-10-07 10:59             ` Karsten Graul
2021-10-07 16:40               ` Gerald Schaefer
2021-10-11 11:47               ` Christoph Hellwig
2021-10-01  4:19     ` Christoph Hellwig
2021-10-01  9:21       ` Ioana Ciornei

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=20211001145256.0323957a@thinkpad \
    --to=gerald.schaefer@linux.ibm.com \
    --cc=dan.j.williams@intel.com \
    --cc=hch@lst.de \
    --cc=ioana.ciornei@nxp.com \
    --cc=iommu@lists.linux-foundation.org \
    --cc=jeremy.linton@arm.com \
    --cc=kgraul@linux.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=m.szyprowski@samsung.com \
    --cc=netdev@vger.kernel.org \
    --cc=robin.murphy@arm.com \
    --cc=someguy@effective-light.com \
    --subject='Re: DPAA2 triggers, [PATCH] dma debug: report -EEXIST errors in add_dma_entry' \
    /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

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).