Re: [PATCH] pci: call _cond_resched() after pci_bus_write_config

[Menglong Dong <menglong8.dong@gmail.com>]

Hello,

On Thu, Oct 14, 2021 at 3:00 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
[...]
>
> s/pci/PCI/ above.
> Add space before "(".
> Add "()" after function names consistently (some have it, some don't).

Thanks, get it!

>
> What exactly is the problem?  I expect __pci_bus_assign_resources() to
> be used mostly during boot-time enumeration.  How much of a problem is
> the latency at that point?  Why is this particularly a problem in the
> KVM environment?  Or is it also a problem on bare metal?
>

In fact, this is a problem on KVM when hotplug virtual devices. The
initialization
of this devices will be done in a workqueue, which can be seen from the call
stack:

62485   62485   kworker/u8:0    pcibios_resource_survey_bus
        b'pcibios_resource_survey_bus+0x1 [kernel]'
        b'acpiphp_check_bridge.part.13+0x11c [kernel]'
        b'acpiphp_hotplug_notify+0x14b [kernel]'
        b'acpi_device_hotplug+0xe0 [kernel]'
        b'acpi_hotplug_work_fn+0x1e [kernel]'
        b'process_one_work+0x19f [kernel]'
        b'worker_thread+0x37 [kernel]'
        b'kthread+0x117 [kernel]'
        b'ret_from_fork+0x24 [kernel]'

And __pci_bus_assign_resources() will be called too. However, as the device
is virtual, it is simulated by qemu, which makes its pci config can't be written
directly. During pci config writing, it will cause KVM exit guest mode and qemu
in HOST can process the pci config writing. Therefore, the kworker in KVM will
block the CPU.

It is't a problem on bare metal.

The latency can be different in different machines. With 4-core CPU and 2G
memory, single pci config writing can cost 1-2ms, and
__pci_bus_assign_resources()
can cost up to 30ms.


> Are there other config write paths that should have a similar change?
>
> _cond_resched() only appears here:
>
>   $ git grep "\<_cond_resched\>"
>   include/linux/sched.h:static __always_inline int _cond_resched(void)
>   include/linux/sched.h:static inline int _cond_resched(void)
>   include/linux/sched.h:static inline int _cond_resched(void) { return 0; }
>   include/linux/sched.h:  _cond_resched();
>
> so I don't believe PCI is so special that this needs to be the only
> other use.  Maybe a different resched interface is more appropriate?

Seems _cond_resched() is not directly used any more. cond_resched()
should be used here.

Thanks!
Menglong Dong