From: Bjorn Helgaas <helgaas@kernel.org>
To: Jon Pan-Doh <pandoh@google.com>
Cc: "Bjorn Helgaas" <bhelgaas@google.com>,
"Karolina Stolarek" <karolina.stolarek@oracle.com>,
linux-pci@vger.kernel.org,
"Martin Petersen" <martin.petersen@oracle.com>,
"Ben Fuller" <ben.fuller@oracle.com>,
"Drew Walton" <drewwalton@microsoft.com>,
"Anil Agrawal" <anilagrawal@meta.com>,
"Tony Luck" <tony.luck@intel.com>,
"Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>,
"Sathyanarayanan Kuppuswamy"
<sathyanarayanan.kuppuswamy@linux.intel.com>,
"Lukas Wunner" <lukas@wunner.de>,
"Jonathan Cameron" <Jonathan.Cameron@huawei.com>,
"Sargun Dhillon" <sargun@meta.com>,
"Paul E . McKenney" <paulmck@kernel.org>
Subject: Re: [PATCH v5 6/8] PCI/AER: Introduce ratelimit for error logs
Date: Tue, 1 Apr 2025 13:02:43 -0500 [thread overview]
Message-ID: <20250401180243.GA1675646@bhelgaas> (raw)
In-Reply-To: <CAMC_AXUDqeJbT3gh48JRL7OiutVqQyf0go_cGcR_xsTfqw+Qsw@mail.gmail.com>
On Mon, Mar 31, 2025 at 05:30:50PM -0700, Jon Pan-Doh wrote:
> On Mon, Mar 31, 2025 at 11:48 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > I found the __ratelimit() return values a little confusing (1 == print
> > the message, 0 == don't print), so this is appealing because it's less
> > confusing by itself.
> >
> > But I think we should name this "aer_ratelimit()" and return the
> > result of __ratelimit() without inverting it so it works the same way
> > as __ratelimit() and similar wrappers like ata_ratelimit(),
> > net_ratelimit(), drbd_ratelimit().
>
> Ack. Caught between readability and consistency :).
>
> > On Thu, Mar 20, 2025 at 06:58:04PM -0700, Jon Pan-Doh wrote:
> > > --- a/drivers/pci/pci.h
> > > +++ b/drivers/pci/pci.h
> > > @@ -533,6 +533,7 @@ static inline bool pci_dev_test_and_set_removed(struct pci_dev *dev)
> > >
> > > struct aer_err_info {
> > > struct pci_dev *dev[AER_MAX_MULTI_ERR_DEVICES];
> > > + bool ratelimited[AER_MAX_MULTI_ERR_DEVICES];
>
> s/ratelimited/ratelimit here as well? Should it store aer_ratelimit()
> or !aer_ratelimit()?
I'm in favor of avoiding negation when possible, so I would name it
"ratelimit" with the semantic of "1 == print", even though that seems
a little backwards to me. But I think it will make sense to people
who read ratelimiting in other areas.
next prev parent reply other threads:[~2025-04-01 18:02 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-03-21 1:57 [PATCH v5 0/8] Rate limit AER logs Jon Pan-Doh
2025-03-21 1:57 ` [PATCH v5 1/8] PCI/AER: Check log level once and propagate down Jon Pan-Doh
2025-05-01 21:43 ` Bjorn Helgaas
2025-05-05 9:30 ` Karolina Stolarek
2025-05-05 17:43 ` Bjorn Helgaas
2025-05-08 15:07 ` Karolina Stolarek
2025-03-21 1:58 ` [PATCH v5 2/8] PCI/AER: Make all pci_print_aer() log levels depend on error type Jon Pan-Doh
2025-03-21 1:58 ` [PATCH v5 3/8] PCI/AER: Move AER stat collection out of __aer_print_error() Jon Pan-Doh
2025-03-21 1:58 ` [PATCH v5 4/8] PCI/AER: Rename aer_print_port_info() to aer_printrp_info() Jon Pan-Doh
2025-03-21 13:39 ` Karolina Stolarek
2025-03-21 19:26 ` Jon Pan-Doh
2025-03-21 1:58 ` [PATCH v5 5/8] PCI/AER: Rename struct aer_stats to aer_report Jon Pan-Doh
2025-03-21 22:01 ` Bjorn Helgaas
2025-03-21 22:15 ` Jon Pan-Doh
2025-03-21 22:30 ` Paul E. McKenney
2025-03-21 22:16 ` Paul E. McKenney
2025-03-21 22:39 ` Bjorn Helgaas
2025-03-21 22:47 ` Paul E. McKenney
2025-05-01 22:02 ` Bjorn Helgaas
2025-05-02 2:16 ` Paul E. McKenney
2025-03-21 1:58 ` [PATCH v5 6/8] PCI/AER: Introduce ratelimit for error logs Jon Pan-Doh
2025-03-21 13:46 ` Karolina Stolarek
2025-03-21 18:41 ` Jon Pan-Doh
2025-04-04 9:32 ` Karolina Stolarek
2025-03-25 17:17 ` Paul E. McKenney
2025-03-27 22:49 ` Jon Pan-Doh
2025-04-03 19:02 ` Paul E. McKenney
2025-03-31 18:48 ` Bjorn Helgaas
2025-04-01 0:30 ` Jon Pan-Doh
2025-04-01 18:02 ` Bjorn Helgaas [this message]
2025-04-24 20:31 ` Bjorn Helgaas
2025-03-21 1:58 ` [PATCH v5 7/8] PCI/AER: Add ratelimits to PCI AER Documentation Jon Pan-Doh
2025-03-21 1:58 ` [PATCH v5 8/8] PCI/AER: Add sysfs attributes for log ratelimits Jon Pan-Doh
2025-03-23 12:20 ` Krzysztof Wilczyński
2025-03-27 22:50 ` Jon Pan-Doh
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=20250401180243.GA1675646@bhelgaas \
--to=helgaas@kernel.org \
--cc=Jonathan.Cameron@huawei.com \
--cc=anilagrawal@meta.com \
--cc=ben.fuller@oracle.com \
--cc=bhelgaas@google.com \
--cc=drewwalton@microsoft.com \
--cc=ilpo.jarvinen@linux.intel.com \
--cc=karolina.stolarek@oracle.com \
--cc=linux-pci@vger.kernel.org \
--cc=lukas@wunner.de \
--cc=martin.petersen@oracle.com \
--cc=pandoh@google.com \
--cc=paulmck@kernel.org \
--cc=sargun@meta.com \
--cc=sathyanarayanan.kuppuswamy@linux.intel.com \
--cc=tony.luck@intel.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.