All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jason Gunthorpe <jgg@ziepe.ca>
To: Daniel Vacek <neelx@redhat.com>
Cc: Leon Romanovsky <leon@kernel.org>,
	linux-rdma@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] IB/ipoib: No need to hold the lock while printing the warning
Date: Mon, 11 Dec 2023 11:25:47 -0400	[thread overview]
Message-ID: <20231211152547.GC1489931@ziepe.ca> (raw)
In-Reply-To: <CACjP9X8+CgoQRjs2Y9A+OwWCVxMhKyqzLhEjaguxMavHsy8VRg@mail.gmail.com>

On Mon, Dec 11, 2023 at 03:09:13PM +0100, Daniel Vacek wrote:
> On Mon, Dec 11, 2023 at 2:25 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
> >
> > On Mon, Dec 11, 2023 at 03:22:17PM +0200, Leon Romanovsky wrote:
> >
> > > Please fill some text in commit message.
> >
> > Yes, explain *why* you are doing this
> 
> Oh, sorry. I did not mention it but there's no particular reason
> really. The @Subject says it all. There should be no logical or
> functional change other than reducing the span of that critical
> section. In other words, just nitpicking, not a big deal.
> 
> While checking the code (and past changes) related to the other issue
> I also sent today I just noticed the way 08bc327629cbd added the
> spin_lock before returning from this function and it appeared to me
> it's clearer the way I'm proposing here.
> 
> Honestly, I was not looking into why the lock is released for that
> completion. And I'm not changing that logic.
> 
> If this complete() can be called with priv->lock held, the cleanup
> would look different, of course.

complete() can be called under spinlocks just fine, AFAIK..

Jason

  reply	other threads:[~2023-12-11 15:25 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-11 13:10 [PATCH] IB/ipoib: No need to hold the lock while printing the warning Daniel Vacek
2023-12-11 13:22 ` Leon Romanovsky
2023-12-11 13:25   ` Jason Gunthorpe
2023-12-11 14:09     ` Daniel Vacek
2023-12-11 15:25       ` Jason Gunthorpe [this message]
2023-12-11 16:08         ` Daniel Vacek

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=20231211152547.GC1489931@ziepe.ca \
    --to=jgg@ziepe.ca \
    --cc=leon@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rdma@vger.kernel.org \
    --cc=neelx@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.