| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <YWbdvc7EWEZLVTHM@smile.fi.intel.com>
Date: Wed, 13 Oct 2021 16:23:09 +0300
From: Andy Shevchenko <andy.shevchenko@...il.com>
To: Bjorn Helgaas <helgaas@...nel.org>
Cc: Uwe Kleine-König
<u.kleine-koenig@...gutronix.de>,
linux-pci <linux-pci@...r.kernel.org>,
Sascha Hauer <kernel@...gutronix.de>,
Alexander Duyck <alexanderduyck@...com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Andrew Donnellan <ajd@...ux.ibm.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>,
Arnd Bergmann <arnd@...db.de>,
Benjamin Herrenschmidt <benh@...nel.crashing.org>,
Bjorn Helgaas <bhelgaas@...gle.com>,
Borislav Petkov <bp@...en8.de>,
Boris Ostrovsky <boris.ostrovsky@...cle.com>,
"David S. Miller" <davem@...emloft.net>,
Fiona Trahe <fiona.trahe@...el.com>,
Frederic Barrat <fbarrat@...ux.ibm.com>,
Giovanni Cabiddu <giovanni.cabiddu@...el.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Herbert Xu <herbert@...dor.apana.org.au>,
"H. Peter Anvin" <hpa@...or.com>, Ido Schimmel <idosch@...dia.com>,
Ingo Molnar <mingo@...hat.com>, Jack Xu <jack.xu@...el.com>,
Jakub Kicinski <kuba@...nel.org>,
Jesse Brandeburg <jesse.brandeburg@...el.com>,
Jiri Olsa <jolsa@...hat.com>, Jiri Pirko <jiri@...dia.com>,
Juergen Gross <jgross@...e.com>,
Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>,
Marco Chiappero <marco.chiappero@...el.com>,
Mark Rutland <mark.rutland@....com>,
Mathias Nyman <mathias.nyman@...el.com>,
Michael Buesch <m@...s.ch>,
Michael Ellerman <mpe@...erman.id.au>,
Namhyung Kim <namhyung@...nel.org>,
Oliver O'Halloran <oohall@...il.com>,
Paul Mackerras <paulus@...ba.org>,
Peter Zijlstra <peterz@...radead.org>,
Rafał Miłecki <zajec5@...il.com>,
Russell Currey <ruscur@...sell.cc>,
Salil Mehta <salil.mehta@...wei.com>,
Sathya Prakash <sathya.prakash@...adcom.com>,
Simon Horman <simon.horman@...igine.com>,
Sreekanth Reddy <sreekanth.reddy@...adcom.com>,
Stefano Stabellini <sstabellini@...nel.org>,
Suganath Prabu Subramani
<suganath-prabu.subramani@...adcom.com>,
Taras Chornyi <tchornyi@...vell.com>,
Thomas Gleixner <tglx@...utronix.de>,
Tomaszx Kowalik <tomaszx.kowalik@...el.com>,
Vadym Kochan <vkochan@...vell.com>,
Wojciech Ziemba <wojciech.ziemba@...el.com>,
Yisen Zhuang <yisen.zhuang@...wei.com>,
Zhou Wang <wangzhou1@...ilicon.com>,
linux-crypto <linux-crypto@...r.kernel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
linux-perf-users@...r.kernel.org,
"open list:LINUX FOR POWERPC PA SEMI PWRFICIENT"
<linuxppc-dev@...ts.ozlabs.org>,
linux-scsi <linux-scsi@...r.kernel.org>,
USB <linux-usb@...r.kernel.org>,
"open list:TI WILINK WIRELES..." <linux-wireless@...r.kernel.org>,
MPT-FusionLinux.pdl@...adcom.com, netdev <netdev@...r.kernel.org>,
oss-drivers@...igine.com, qat-linux@...el.com,
"maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)" <x86@...nel.org>,
xen-devel@...ts.xenproject.org
Subject: Re: [PATCH v6 00/11] PCI: Drop duplicated tracking of a pci_dev's
bound driver
On Wed, Oct 13, 2021 at 06:33:56AM -0500, Bjorn Helgaas wrote:
> On Wed, Oct 13, 2021 at 12:26:42PM +0300, Andy Shevchenko wrote:
> > On Wed, Oct 13, 2021 at 2:33 AM Bjorn Helgaas <helgaas@...nel.org> wrote:
> > > On Mon, Oct 04, 2021 at 02:59:24PM +0200, Uwe Kleine-König wrote:
...
> > It's a bit unusual. Other to_*_dev() are not NULL-aware IIRC.
>
> It is a little unusual. I only found three of 77 that are NULL-aware:
>
> to_moxtet_driver()
> to_siox_driver()
> to_spi_driver()
>
> It seems worthwhile to me because it makes the patch and the resulting
> code significantly cleaner.
I'm not objecting the change, just a remark.
...
> > > + for (id = drv ? drv->id_table : NULL; id && id->vendor; id++)
> > > + if (id->vendor == vendor && id->device == device)
> >
> > > + break;
> >
> > return true;
> >
> > > return id && id->vendor;
> >
> > return false;
>
> Good cleanup for a follow-up patch, but doesn't seem directly related
> to the objective here.
True. Maybe you can bake one while not forgotten?
...
> > > + return drv && drv->resume ?
> > > + drv->resume(pci_dev) : pci_pm_reenable_device(pci_dev);
> >
> > One line?
>
> I don't think I touched that line.
Then why they are both in + section?
...
> > > + struct pci_driver *drv = to_pci_driver(dev->dev.driver);
> > > const struct pci_error_handlers *err_handler =
> > > - dev->dev.driver ? to_pci_driver(dev->dev.driver)->err_handler : NULL;
> > > + drv ? drv->err_handler : NULL;
> >
> > Isn't dev->driver == to_pci_driver(dev->dev.driver)?
>
> Yes, I think so, but not sure what you're getting at here, can you
> elaborate?
Getting pointer from another pointer seems waste of resources, why we
can't simply
struct pci_driver *drv = dev->driver;
?
...
> > Stray change? Or is it in a separate patch in your tree?
>
> Could be skipped. The string now fits on one line so I combined it to
> make it more greppable.
This is inconsistency in your changes, in one case you are objecting of
doing something close to the changed lines, in the other you are doing
unrelated change.
--
With Best Regards,
Andy Shevchenko
Powered by blists - more mailing lists