From: Simon Horman <horms@kernel.org>
To: Florian Fainelli <florian.fainelli@broadcom.com>
Cc: Markus Elfring <Markus.Elfring@web.de>,
Justin Chen <justin.chen@broadcom.com>,
bcm-kernel-feedback-list@broadcom.com, netdev@vger.kernel.org,
kernel-janitors@vger.kernel.org,
LKML <linux-kernel@vger.kernel.org>,
"David S. Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>
Subject: Re: [PATCH net] net: bcmasp: fix memory leak when bringing down if
Date: Wed, 17 Apr 2024 21:34:30 +0100 [thread overview]
Message-ID: <20240417203430.GB3935777@kernel.org> (raw)
In-Reply-To: <3a5cb80e-7169-4e82-b10c-843ff1eb0fd3@broadcom.com>
On Wed, Apr 17, 2024 at 09:52:47AM -0700, Florian Fainelli wrote:
> On 4/17/24 09:19, Simon Horman wrote:
> > On Mon, Apr 15, 2024 at 09:46:44PM +0200, Markus Elfring wrote:
> > > > > > When bringing down the TX rings we flush the rings but forget to
> > > > > > reclaimed the flushed packets. This lead to a memory leak since we
> > > > > > do not free the dma mapped buffers. …
> > > > >
> > > > > I find this change description improvable.
> > > > >
> > > > > * How do you think about to avoid typos?
> > > > >
> > > > > * Would another imperative wording be more desirable?
> > > >
> > > > The change description makes sense to me. Can you be a bit more specific as to what isn't clear here?
> > >
> > > Spelling suggestions:
> > > + … forget to reclaim …
> > > + … This leads to …
> >
> > Markus, let's cut to the chase.
> >
> > What portion of your responses of this thread were produced
> > by an LLM or similar technology?
> >
> > The suggestions in your second email are correct.
> > But, ironically, your first response appears to be grammatically incorrect.
> >
> > Specifically:
> >
> > * What does "improvable" mean in this context?
>
> I read it as "improbable", but this patch came out of an actual bug report
> we had internally and code inspection revealed the leaks being plugged by
> this patch.
>
> > * "How do you think about to avoid typos?"
> > is, in my opinion, grammatically incorrect.
> > And, FWIW, I see no typos.
>
> There was one, "This lead to a memory leak" -> "This leads to a memory leak"
>
> > * "Would another imperative wording be more desirable?"
> > is, in my opinion, also grammatically incorrect.
> >
> > And yet your comment is ostensibly about grammar.
> > I'm sorry, but this strikes me as absurd.
>
> Yeah, I share that too, if you are to nitpick on every single word someone
> wrote in a commit message, your responses better be squeaky clean such that
> Shakespeare himself would be proud of you.
>
> There is a track record of what people might consider bike shedding, others
> might consider useless, and others might find uber pedantic comments from
> Markus done under his other email address: elfring@users.sourceforge.net.
>
> Me personally, I read his comments and apply my own judgement as to whether
> they justify spinning a new patch just to address the feedback given. He has
> not landed on my ignore filter, but of course that can change at a moments
> notice.
Thanks Florian,
On reflection, my previous email was inappropriate.
I do have reservations about the review provided by Markus,
but should not reacted as I did. I apologise to every for that.
next prev parent reply other threads:[~2024-04-17 20:34 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-12 18:16 [PATCH net] net: bcmasp: fix memory leak when bringing down if Justin Chen
2024-04-12 21:09 ` Florian Fainelli
2024-04-14 11:23 ` Markus Elfring
2024-04-15 17:36 ` Justin Chen
2024-04-15 19:46 ` Markus Elfring
2024-04-17 16:19 ` Simon Horman
2024-04-17 16:52 ` Florian Fainelli
2024-04-17 18:48 ` Justin Chen
2024-04-18 0:56 ` Jakub Kicinski
2024-04-17 20:34 ` Simon Horman [this message]
2024-04-18 8:02 ` net: bcmasp: Patch review challenges Markus Elfring
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=20240417203430.GB3935777@kernel.org \
--to=horms@kernel.org \
--cc=Markus.Elfring@web.de \
--cc=bcm-kernel-feedback-list@broadcom.com \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=florian.fainelli@broadcom.com \
--cc=justin.chen@broadcom.com \
--cc=kernel-janitors@vger.kernel.org \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.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.