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: Bjorn Helgaas <helgaas@kernel.org>,
	Alex Williamson <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: Mon, 23 May 2022 16:20:42 +0200	[thread overview]
Message-ID: <20220523142042.GA19286@wunner.de> (raw)
In-Reply-To: <CAGdb+H19bfbXM1cPJvhh6gixJbF7Sk=v53d9VpvWY8HEs0mSKg@mail.gmail.com>

On Sun, May 22, 2022 at 01:37:50AM +0800, Sheng Bi wrote:
> On Sat, May 21, 2022 at 8:49 PM Lukas Wunner <lukas@wunner.de> wrote:
> > On Sat, May 21, 2022 at 04:36:10PM +0800, Sheng Bi wrote:
> > > If so, I also want to align the polling things mentioned in the
> > > question from Alex, since pci_dev_wait() is also used for reset
> > > functions other than SBR. To Bjorn, Alex, Lucas, how do you think if
> > > we need to change the polling in pci_dev_wait() to 20ms intervals, or
> > > keep binary exponential back-off with probable unexpected extra
> > > timeout delay.
> >
> > The exponential backoff should probably be capped at some point
> > to avoid excessive wait delays.  I guess the rationale for
> > exponential backoff is to not poll too frequently.
> > Capping at 20 msec or 100 msec may be reasonable, i.e.:
> >
> > -               delay *= 2;
> > +               delay = min(delay * 2, 100);
> 
> Capping at 20 or 100 msec seems reasonable to me. Btw, since 20 msec
> is not a long time in these scenarios, how about changing to a fixed
> 20 msec interval?

The callers of pci_dev_wait() seem to wait for the spec-defined
delay and only call pci_dev_wait() to allow for an additional period
that non-compliant devices may need.  That extra delay can be expected
to be low, which is why it makes sense to start with a short poll interval
and gradually extend it.  So the algorithm seems to be reasonable and
I wouldn't recommend changing it to a constant interval unless that
fixes something which is currently broken.

Thanks,

Lukas

  reply	other threads:[~2022-05-23 14:20 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
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 [this message]
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=20220523142042.GA19286@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.