Linux PCI subsystem development
 help / color / mirror / Atom feed
From: Bjorn Helgaas <helgaas@kernel.org>
To: Ammar Qadri <ammarq@google.com>
Cc: Manivannan Sadhasivam <mani@kernel.org>,
	Bjorn Helgaas <bhelgaas@google.com>,
	linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] PCI: Reduce verbosity of device enable messages
Date: Fri, 13 Jun 2025 17:09:50 -0500	[thread overview]
Message-ID: <20250613220950.GA986935@bhelgaas> (raw)
In-Reply-To: <CAFbbpayW+y8s3i4qxzHcoY0Yz5qeAhb7ziey=FayDiZbC_mm7w@mail.gmail.com>

On Fri, Jun 13, 2025 at 02:40:40PM -0700, Ammar Qadri wrote:
> Hi Mani,
> 
> The issue we are experiencing is not caused from
> removing/reattaching the device driver, so the other messages have
> not been problematic.
> 
> The vfio-pci driver is attached to each VF once. Clients in our
> system call open and close on the vfio-pci driver, respectively, at
> the start and end of their use, with fairly short-term tenancy,
> which ends up triggering these enable messages.  This message is
> proving challenging not only because they are not particularly
> useful,  but because they are causing log files to rotate once every
> 30 minutes or so, and we lose a lot of other more valuable logging
> as a consequence.  I'm open to other solutions, but in my opinion
> this preserves the message, without over-engineering and introducing
> throttling or other behaviour.

Are there any other messages associated with the open/close?  I assume
probably not, or you would want to demote those as well.

I did happen to find some value in this particular message just the
other day because it showed that a config read was successful after
previous ones had failed.

But I agree in general that it's fairly low value and at least the
uninterpreted "%04x -> %04x" part is not really user-friendly.

If people think there's enough value in retaining it at KERN_INFO, I
suppose there's always the option of carrying an out-of-tree patch to
demote it?

> On Thu, Jun 12, 2025 at 11:12 PM Manivannan Sadhasivam <mani@kernel.org> wrote:
> >
> > On Wed, May 07, 2025 at 11:29:19PM +0000, Ammar Qadri wrote:
> > > Excessive logging of PCIe device enable operations can create significant
> > > noise in system logs, especially in environments with a high number of
> > > such devices, especially VFs.
> > >
> > > High-rate logging can cause log files to rotate too quickly, losing
> > > valuable information from other system components.This commit addresses
> > > this issue by downgrading the logging level of "enabling device" messages
> > > from `info` to `dbg`.
> > >
> >
> > While I generally prefer reduced verbosity of the device drivers, demoting an
> > existing log to debug might surprise users. Especially in this case, the message
> > is widely used to identify the enablement of a PCI device. So I don't think it
> > is a good idea to demote it to a debug log.
> >
> > But I'm surprised that this single message is creating much overhead in the
> > logging. I understand that you might have 100s of VFs in cloud environments, but
> > when a VF is added, a bunch of other messages would also get printed (resource,
> > IRQ, device driver etc...). Or you considered that this message is not that
> > important compared to the rest?
> >
> > - Mani
> >
> > > Signed-off-by: Ammar Qadri <ammarq@google.com>
> > > ---
> > >  drivers/pci/setup-res.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
> > > index c6657cdd06f67..be669ff6ca240 100644
> > > --- a/drivers/pci/setup-res.c
> > > +++ b/drivers/pci/setup-res.c
> > > @@ -516,7 +516,7 @@ int pci_enable_resources(struct pci_dev *dev, int mask)
> > >       }
> > >
> > >       if (cmd != old_cmd) {
> > > -             pci_info(dev, "enabling device (%04x -> %04x)\n", old_cmd, cmd);
> > > +             pci_dbg(dev, "enabling device (%04x -> %04x)\n", old_cmd, cmd);
> > >               pci_write_config_word(dev, PCI_COMMAND, cmd);
> > >       }
> > >       return 0;
> > > --
> > > 2.49.0.987.g0cc8ee98dc-goog
> > >
> >
> > --
> > மணிவண்ணன் சதாசிவம்

  reply	other threads:[~2025-06-13 22:09 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-05-07 23:29 [PATCH] PCI: Reduce verbosity of device enable messages Ammar Qadri
2025-06-13  6:12 ` Manivannan Sadhasivam
2025-06-13 21:40   ` Ammar Qadri
2025-06-13 22:09     ` Bjorn Helgaas [this message]
2025-07-09 18:25       ` Ammar Qadri
2025-07-09 20:26         ` Bjorn Helgaas

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=20250613220950.GA986935@bhelgaas \
    --to=helgaas@kernel.org \
    --cc=ammarq@google.com \
    --cc=bhelgaas@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=mani@kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox