All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lukas Wunner <lukas@wunner.de>
To: Sheng Bi <windy.bi.enflame@gmail.com>
Cc: helgaas@kernel.org, alex.williamson@redhat.com,
	linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] PCI: Fix no-op wait after secondary bus reset
Date: Fri, 20 May 2022 08:41:48 +0200	[thread overview]
Message-ID: <20220520064148.GA20418@wunner.de> (raw)
In-Reply-To: <20220518115432.76183-1-windy.bi.enflame@gmail.com>

On Wed, May 18, 2022 at 07:54:32PM +0800, Sheng Bi wrote:
> +static int pci_bridge_secondary_bus_wait(struct pci_dev *bridge, int timeout)
> +{
> +	struct pci_dev *dev;
> +	int delay = 0;
> +
> +	if (!bridge->subordinate || list_empty(&bridge->subordinate->devices))
> +		return 0;
> +
> +	list_for_each_entry(dev, &bridge->subordinate->devices, bus_list) {
> +		while (!pci_device_is_present(dev)) {
> +			if (delay > timeout) {
> +				pci_warn(dev, "not ready %dms after secondary bus reset; giving up\n",
> +					delay);
> +				return -ENOTTY;
> +			}
> +
> +			msleep(20);
> +			delay += 20;
> +		}
> +
> +		if (delay > 1000)
> +			pci_info(dev, "ready %dms after secondary bus reset\n",
> +				delay);
> +	}
> +
> +	return 0;
> +}

An alternative approach you may want to consider is to call
pci_dev_wait() in the list_for_each_entry loop, but instead of
passing it a constant timeout you'd pass the remaining time.

Get the current time before and after each pci_dev_wait() call
from "jiffies", calculate the difference, convert to msecs with
jiffies_to_msecs() and subtract from the "timeout" parameter
passed in by the caller, then simply pass "timeout" to each
pci_dev_wait() call.

As a side note, traversing the bus list normally requires
holding the pci_bus_sem for reading.  But it's probably unlikely
that devices are added/removed concurrently to a bus reset
and we're doing it wrong pretty much everywhere in the
PCI reset code, so...

(I fixed up one of the reset functions with 10791141a6cf,
but plenty of others remain...)

Thanks,

Lukas

  parent reply	other threads:[~2022-05-20  6:42 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-16 17:30 [PATCH] drivers/pci: wait downstream hierarchy ready instead of slot itself ready, after secondary bus reset windy.bi.enflame
2022-05-16 20:28 ` Bjorn Helgaas
2022-05-16 22:57   ` Alex Williamson
2022-05-17 14:56     ` windy Bi
2022-05-18 11:54     ` [PATCH v2] PCI: Fix no-op wait " Sheng Bi
2022-05-19 17:06       ` Alex Williamson
2022-05-20  3:00         ` windy Bi
2022-05-20  6:41       ` Lukas Wunner [this message]
2022-05-21  8:36         ` Sheng Bi
2022-05-21 12:49           ` Lukas Wunner
2022-05-21 17:37             ` Sheng Bi
2022-05-23 14:20               ` Lukas Wunner
2022-05-23 15:59                 ` Sheng Bi
2022-05-23 17:15                   ` [PATCH v3] " Sheng Bi
2022-06-08 13:16                     ` Sheng Bi
2022-06-08 15:23                     ` Lukas Wunner
2022-05-17  5:34 ` [PATCH] drivers/pci: wait downstream hierarchy ready instead of slot itself ready, " kernel test robot

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=20220520064148.GA20418@wunner.de \
    --to=lukas@wunner.de \
    --cc=alex.williamson@redhat.com \
    --cc=helgaas@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=windy.bi.enflame@gmail.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.