All of lore.kernel.org
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: "Christoph Hellwig" <hch@lst.de>,
	"Dominique MARTINET" <dominique.martinet@atmark-techno.com>,
	"jianxiong Gao" <jxgao@google.com>,
	"Horia Geantă" <horia.geanta@nxp.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"Lukas Hartmann" <lukas@mntmn.com>,
	"Aymen Sghaier" <aymen.sghaier@nxp.com>,
	"Herbert Xu" <herbert@gondor.apana.org.au>,
	"David S. Miller" <davem@davemloft.net>,
	"linux-crypto@vger.kernel.org" <linux-crypto@vger.kernel.org>,
	"iommu@lists.linux-foundation.org"
	<iommu@lists.linux-foundation.org>
Subject: Re: swiotlb/caamjr regression (Was: [GIT PULL] (swiotlb) stable/for-linus-5.12)
Date: Fri, 11 Jun 2021 09:21:42 -0700	[thread overview]
Message-ID: <CAHk-=wgxgTB=G7P6KRneAd0s310WYK2NDisXM5P-wsNibgLrQA@mail.gmail.com> (raw)
In-Reply-To: <YMM8Ua0HMmErLIQg@0xbeefdead.lan>

On Fri, Jun 11, 2021 at 3:35 AM Konrad Rzeszutek Wilk
<konrad.wilk@oracle.com> wrote:
>
> Linus,
>
> Would you be terribly offended if I took your code (s/unsigned
> long/unsigned int), and used Chanho's description of the problem (see below)?

No offense to that at all - that looks like the right solution. See my
answer to Christoph: I do think my patch does the right one, but I
can't test it and my knowledge of the swiotlb code is not complete
enough to really do anything else than "this looks right".

And adding my sign-off to the patch is fine, but I don't necessarily
need the authorship credit - mine was a throw-away patch just looking
at what the bisection report said. All the real effort was by the
reporters (and for the commit message, Bumyong Lee & co).

Finally - looking at the two places that do have that
swiotlb_align_offset(), my reaction is "I don't understand what that
code is doing".

The index does that

        index = find_slots(dev, orig_addr, alloc_size + offset);

so that offset does seem to depend on how the find_slots code works.
Which in turn does use the same dma_get_min_align_mask() thing that
swiotlb_align_offset() uses.  So the offsets do seem to match, but
find_slots(dev() does a lot of stuff that I don't know. so...

IOW, it does reinforce my "I don't know this code AT ALL". Which just
makes me more convinced that I shouldn't get authorship of the patch
because if something goes wrong with it, I can't help.

So at most maybe a "Suggested-by:".

My patch really was based on very little context and "this is the
calculation that makes sense given the other calculations in the
function".

              Linus

WARNING: multiple messages have this Message-ID (diff)
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: "Dominique MARTINET" <dominique.martinet@atmark-techno.com>,
	"Aymen Sghaier" <aymen.sghaier@nxp.com>,
	"Herbert Xu" <herbert@gondor.apana.org.au>,
	"Horia Geantă" <horia.geanta@nxp.com>,
	"Lukas Hartmann" <lukas@mntmn.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"David S. Miller" <davem@davemloft.net>,
	"iommu@lists.linux-foundation.org"
	<iommu@lists.linux-foundation.org>,
	"linux-crypto@vger.kernel.org" <linux-crypto@vger.kernel.org>,
	"Christoph Hellwig" <hch@lst.de>,
	"jianxiong Gao" <jxgao@google.com>
Subject: Re: swiotlb/caamjr regression (Was: [GIT PULL] (swiotlb) stable/for-linus-5.12)
Date: Fri, 11 Jun 2021 09:21:42 -0700	[thread overview]
Message-ID: <CAHk-=wgxgTB=G7P6KRneAd0s310WYK2NDisXM5P-wsNibgLrQA@mail.gmail.com> (raw)
In-Reply-To: <YMM8Ua0HMmErLIQg@0xbeefdead.lan>

On Fri, Jun 11, 2021 at 3:35 AM Konrad Rzeszutek Wilk
<konrad.wilk@oracle.com> wrote:
>
> Linus,
>
> Would you be terribly offended if I took your code (s/unsigned
> long/unsigned int), and used Chanho's description of the problem (see below)?

No offense to that at all - that looks like the right solution. See my
answer to Christoph: I do think my patch does the right one, but I
can't test it and my knowledge of the swiotlb code is not complete
enough to really do anything else than "this looks right".

And adding my sign-off to the patch is fine, but I don't necessarily
need the authorship credit - mine was a throw-away patch just looking
at what the bisection report said. All the real effort was by the
reporters (and for the commit message, Bumyong Lee & co).

Finally - looking at the two places that do have that
swiotlb_align_offset(), my reaction is "I don't understand what that
code is doing".

The index does that

        index = find_slots(dev, orig_addr, alloc_size + offset);

so that offset does seem to depend on how the find_slots code works.
Which in turn does use the same dma_get_min_align_mask() thing that
swiotlb_align_offset() uses.  So the offsets do seem to match, but
find_slots(dev() does a lot of stuff that I don't know. so...

IOW, it does reinforce my "I don't know this code AT ALL". Which just
makes me more convinced that I shouldn't get authorship of the patch
because if something goes wrong with it, I can't help.

So at most maybe a "Suggested-by:".

My patch really was based on very little context and "this is the
calculation that makes sense given the other calculations in the
function".

              Linus
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

  parent reply	other threads:[~2021-06-11 16:22 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-26 16:00 [GIT PULL] (swiotlb) stable/for-linus-5.12 Konrad Rzeszutek Wilk
2021-02-26 22:24 ` pr-tracker-bot
2021-06-08  2:35 ` swiotlb/caamjr regression (Was: [GIT PULL] (swiotlb) stable/for-linus-5.12) Dominique MARTINET
2021-06-10 14:52   ` Horia Geantă
2021-06-10 14:52     ` Horia Geantă
2021-06-10 19:41     ` Linus Torvalds
2021-06-10 19:41       ` Linus Torvalds
2021-06-10 23:20       ` Horia Geantă
2021-06-10 23:20         ` Horia Geantă
2021-06-11  6:21     ` Christoph Hellwig
2021-06-11  6:21       ` Christoph Hellwig
2021-06-11 10:34       ` Konrad Rzeszutek Wilk
2021-06-11 10:34         ` Konrad Rzeszutek Wilk
2021-06-11 10:59         ` Horia Geantă
2021-06-11 10:59           ` Horia Geantă
2021-06-11 16:21         ` Linus Torvalds [this message]
2021-06-11 16:21           ` Linus Torvalds
2021-06-16 20:49         ` Jianxiong Gao
2021-06-16 20:49           ` Jianxiong Gao via iommu
2021-06-17  0:27           ` Konrad Rzeszutek Wilk
2021-06-17  0:27             ` Konrad Rzeszutek Wilk
2021-06-17  0:39             ` Dominique MARTINET
2021-06-17  0:39               ` Dominique MARTINET
2021-06-17  5:12               ` Christoph Hellwig
2021-06-17  5:12                 ` Christoph Hellwig
2021-06-17  5:36                 ` Dominique MARTINET
2021-06-17  5:36                   ` Dominique MARTINET
2021-06-18 18:01                   ` Jianxiong Gao
2021-06-18 18:01                     ` Jianxiong Gao via iommu
2021-06-21  2:03                     ` Dominique MARTINET
2021-06-21  2:03                       ` Dominique MARTINET
2021-06-21  2:55                       ` Chanho Park
2021-06-21  2:55                         ` Chanho Park
2021-06-21  4:14                         ` 'Dominique MARTINET'
2021-06-21  4:14                           ` 'Dominique MARTINET'
2021-06-21 13:16                           ` Konrad Rzeszutek Wilk
2021-06-21 13:16                             ` Konrad Rzeszutek Wilk
2021-06-22  7:48                             ` 'Dominique MARTINET'
2021-06-22  7:48                               ` 'Dominique MARTINET'
2021-06-22 21:58                               ` Konrad Rzeszutek Wilk
2021-06-22 21:58                                 ` Konrad Rzeszutek Wilk
2021-06-22 23:04                                 ` 'Dominique MARTINET'
2021-06-22 23:04                                   ` 'Dominique MARTINET'
2021-06-17 11:33             ` Christoph Hellwig
2021-06-17 11:33               ` Christoph Hellwig
2021-06-11 16:01       ` Linus Torvalds
2021-06-11 16:01         ` Linus Torvalds

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='CAHk-=wgxgTB=G7P6KRneAd0s310WYK2NDisXM5P-wsNibgLrQA@mail.gmail.com' \
    --to=torvalds@linux-foundation.org \
    --cc=aymen.sghaier@nxp.com \
    --cc=davem@davemloft.net \
    --cc=dominique.martinet@atmark-techno.com \
    --cc=hch@lst.de \
    --cc=herbert@gondor.apana.org.au \
    --cc=horia.geanta@nxp.com \
    --cc=iommu@lists.linux-foundation.org \
    --cc=jxgao@google.com \
    --cc=konrad.wilk@oracle.com \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lukas@mntmn.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.