From: "ameynarkhede03@gmail.com" <ameynarkhede03@gmail.com>
To: Leon Romanovsky <leon@kernel.org>
Cc: Raphael Norwitz <raphael.norwitz@nutanix.com>,
Alex Williamson <alex.williamson@redhat.com>,
"bhelgaas@google.com
<bhelgaas@google.com>,linux-pci@vger.kernel.org"
<linux-pci@vger.kernel.org>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] PCI: merge slot and bus reset implementations
Date: Wed, 7 Apr 2021 13:53:56 +0530 [thread overview]
Message-ID: <20210407082356.53subv4np2fx777x@archlinux> (raw)
In-Reply-To: <YG1eBUY0vCTV+Za/@unreal>
On 21/04/07 10:23AM, Leon Romanovsky wrote:
> On Tue, Apr 06, 2021 at 08:16:26AM -0600, Alex Williamson wrote:
> > On Sun, 4 Apr 2021 11:04:32 +0300
> > Leon Romanovsky <leon@kernel.org> wrote:
> >
> > > On Thu, Apr 01, 2021 at 10:56:16AM -0600, Alex Williamson wrote:
> > > > On Thu, 1 Apr 2021 15:27:37 +0300
> > > > Leon Romanovsky <leon@kernel.org> wrote:
> > > >
> > > > > On Thu, Apr 01, 2021 at 05:37:16AM +0000, Raphael Norwitz wrote:
> > > > > > Slot resets are bus resets with additional logic to prevent a device
> > > > > > from being removed during the reset. Currently slot and bus resets have
> > > > > > separate implementations in pci.c, complicating higher level logic. As
> > > > > > discussed on the mailing list, they should be combined into a generic
> > > > > > function which performs an SBR. This change adds a function,
> > > > > > pci_reset_bus_function(), which first attempts a slot reset and then
> > > > > > attempts a bus reset if -ENOTTY is returned, such that there is now a
> > > > > > single device agnostic function to perform an SBR.
> > > > > >
> > > > > > This new function is also needed to add SBR reset quirks and therefore
> > > > > > is exposed in pci.h.
> > > > > >
> > > > > > Link: https://lkml.org/lkml/2021/3/23/911
> > > > > >
> > > > > > Suggested-by: Alex Williamson <alex.williamson@redhat.com>
> > > > > > Signed-off-by: Amey Narkhede <ameynarkhede03@gmail.com>
> > > > > > Signed-off-by: Raphael Norwitz <raphael.norwitz@nutanix.com>
> > > > > > ---
> > > > > > drivers/pci/pci.c | 17 +++++++++--------
> > > > > > include/linux/pci.h | 1 +
> > > > > > 2 files changed, 10 insertions(+), 8 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > > > > > index 16a17215f633..12a91af2ade4 100644
> > > > > > --- a/drivers/pci/pci.c
> > > > > > +++ b/drivers/pci/pci.c
> > > > > > @@ -4982,6 +4982,13 @@ static int pci_dev_reset_slot_function(struct pci_dev *dev, int probe)
> > > > > > return pci_reset_hotplug_slot(dev->slot->hotplug, probe);
> > > > > > }
> > > > > >
> > > > > > +int pci_reset_bus_function(struct pci_dev *dev, int probe)
> > > > > > +{
> > > > > > + int rc = pci_dev_reset_slot_function(dev, probe);
> > > > > > +
> > > > > > + return (rc == -ENOTTY) ? pci_parent_bus_reset(dev, probe) : rc;
> > > > >
> > > > > The previous coding style is preferable one in the Linux kernel.
> > > > > int rc = pci_dev_reset_slot_function(dev, probe);
> > > > > if (rc != -ENOTTY)
> > > > > return rc;
> > > > > return pci_parent_bus_reset(dev, probe);
> > > >
> > > >
> > > > That'd be news to me, do you have a reference? I've never seen
> > > > complaints for ternaries previously. Thanks,
> > >
> > > The complaint is not to ternaries, but to the function call as one of
> > > the parameters, that makes it harder to read.
> >
> > Sorry, I don't find a function call as a parameter to a ternary to be
> > extraordinary, nor do I find it to be a discouraged usage model within
> > the kernel. This seems like a pretty low bar for hard to read code.
>
> It is up to us where this bar is set.
>
> Thanks
On the side note there are plenty of places where this pattern is used
though
for example -
kernel/time/clockevents.c:328:
return force ? clockevents_program_min_delta(dev) : -ETIME;
kernel/trace/trace_kprobe.c:233:
return tk ? within_error_injection_list(trace_kprobe_address(tk)) :
false;
kernel/signal.c:3104:
return oset ? put_compat_sigset(oset, &old_set, sizeof(*oset)) : 0;
etc
Thanks,
Amey
next prev parent reply other threads:[~2021-04-07 8:24 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-01 5:37 [PATCH] PCI: merge slot and bus reset implementations Raphael Norwitz
2021-04-01 12:27 ` Leon Romanovsky
2021-04-01 16:56 ` Alex Williamson
2021-04-04 8:04 ` Leon Romanovsky
2021-04-06 14:16 ` Alex Williamson
2021-04-07 7:23 ` Leon Romanovsky
2021-04-07 7:49 ` Krzysztof Wilczyński
2021-04-07 7:59 ` Leon Romanovsky
2021-04-07 8:23 ` ameynarkhede03 [this message]
2021-04-07 12:30 ` Leon Romanovsky
2021-04-07 13:06 ` ameynarkhede03
2021-04-07 13:37 ` Leon Romanovsky
2021-04-07 13:43 ` ameynarkhede03
2021-04-08 18:38 ` Raphael Norwitz
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=20210407082356.53subv4np2fx777x@archlinux \
--to=ameynarkhede03@gmail.com \
--cc=alex.williamson@redhat.com \
--cc=leon@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=raphael.norwitz@nutanix.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).