netdev - RE: [PATCH v3 6/6] PCI: hv: Use async probing to reduce boot time

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

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID:
 <SA1PR21MB13355D8F2A7AC6CA91FE1D1BBF779@SA1PR21MB1335.namprd21.prod.outlook.com>
Date: Wed, 10 May 2023 17:12:23 +0000
From: Dexuan Cui <decui@...rosoft.com>
To: Lorenzo Pieralisi <lpieralisi@...nel.org>
CC: "bhelgaas@...gle.com" <bhelgaas@...gle.com>, "davem@...emloft.net"
	<davem@...emloft.net>, "edumazet@...gle.com" <edumazet@...gle.com>, Haiyang
 Zhang <haiyangz@...rosoft.com>, Jake Oshins <jakeo@...rosoft.com>,
	"kuba@...nel.org" <kuba@...nel.org>, "kw@...ux.com" <kw@...ux.com>, KY
 Srinivasan <kys@...rosoft.com>, "leon@...nel.org" <leon@...nel.org>,
	"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>, "Michael Kelley
 (LINUX)" <mikelley@...rosoft.com>, "pabeni@...hat.com" <pabeni@...hat.com>,
	"robh@...nel.org" <robh@...nel.org>, "saeedm@...dia.com" <saeedm@...dia.com>,
	"wei.liu@...nel.org" <wei.liu@...nel.org>, Long Li <longli@...rosoft.com>,
	"boqun.feng@...il.com" <boqun.feng@...il.com>, Saurabh Singh Sengar
	<ssengar@...rosoft.com>, "helgaas@...nel.org" <helgaas@...nel.org>,
	"linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>, Jose Teuttli Carranco
	<josete@...rosoft.com>, "stable@...r.kernel.org" <stable@...r.kernel.org>
Subject: RE: [PATCH v3 6/6] PCI: hv: Use async probing to reduce boot time

> From: Lorenzo Pieralisi <lpieralisi@...nel.org>
> Sent: Wednesday, May 10, 2023 1:23 AM
> To: Dexuan Cui <decui@...rosoft.com>
> ...
> On Wed, Apr 19, 2023 at 07:40:37PM -0700, Dexuan Cui wrote:
> > Commit 414428c5da1c ("PCI: hv: Lock PCI bus on device eject") added
> > pci_lock_rescan_remove() and pci_unlock_rescan_remove() in
> > create_root_hv_pci_bus() and in hv_eject_device_work() to address the
> > race between create_root_hv_pci_bus() and hv_eject_device_work(), but it
> > turns that grabing the pci_rescan_remove_lock mutex is not enough:
> > refer to the earlier fix "PCI: hv: Add a per-bus mutex state_lock".
> 
> This is meaningless for a commit log reader, there is nothing to
> refer to.
Correct. Because patch 5
[PATCH v3 5/6] PCI: hv: Add a per-bus mutex state_lock
has not been in any upstream tree, so I don't have a commit id yet.
 
> > Now with hbus->state_lock and other fixes, the race is resolved, so
> 
> "other fixes" is meaningless too.
Ditto. 
 
> Explain the problem and how you fix it (this patch should be split
> because the Subject does not represent what you are doing precisely,
> see below).
Ok, I will better explain the boot time issue.

> > remove pci_{lock,unlock}_rescan_remove() in create_root_hv_pci_bus():
> > this removes the serialization in hv_pci_probe() and hence allows
> > async-probing (PROBE_PREFER_ASYNCHRONOUS) to work.
> >
> > Add the async-probing flag to hv_pci_drv.
> 
> Adding the asynchronous probing should be a separate patch and
> I don't think you should send it to stable kernels straight away
> because a) it is not a fix b) it can trigger further regressions.
Agreed. I'll remove the line "Cc: stable".

> > pci_{lock,unlock}_rescan_remove() in hv_eject_device_work() and in
> > hv_pci_remove() are still kept: according to the comment before
> > drivers/pci/probe.c: static DEFINE_MUTEX(pci_rescan_remove_lock),
> > "PCI device removal routines should always be executed under this mutex".
> 
> This patch should be split, first thing is to fix and document what
> you are changing for pci_{lock,unlock}_rescan_remove() then add
> asynchronous probing.
> 
> Lorenzo
Ok, I'll split this patch into two.

Thanks for reviewing the patch. 
Can you please give an "Acked-by" or "Reviewed-by" to patch 1~5 
if they look good to you? The first 5 patches have been there for a
while, and they already got Michael's Reviewed-by. 

I hope the first 5 patches can go through the hyperv-fixes branch in
the hyperv tree
https://git.kernel.org/pub/scm/linux/kernel/git/hyperv/linux.git/log/?h=hyperv-fixes
since they are specific to Hyper-V.

After the first 5 patches are in, I can refer to the commit IDs, and I
will split this patch (patch 6).

Thanks,
Dexuan