From: Oleksandr Natalenko <oleksandr@natalenko.name> To: Jakub Kicinski <kuba@kernel.org> Cc: Alexander Duyck <alexander.duyck@gmail.com>, linux-kernel@vger.kernel.org, Jesse Brandeburg <jesse.brandeburg@intel.com>, Tony Nguyen <anthony.l.nguyen@intel.com>, "David S. Miller" <davem@davemloft.net>, intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org Subject: Re: [igb] netconsole triggers warning in netpoll_poll_dev Date: Mon, 26 Apr 2021 08:47:36 +0200 [thread overview] Message-ID: <20210426064736.7efynita4brzos4u@spock.localdomain> (raw) In-Reply-To: <20210423155836.25ef1e77@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> Hello. On Fri, Apr 23, 2021 at 03:58:36PM -0700, Jakub Kicinski wrote: > On Fri, 23 Apr 2021 10:19:44 +0200 Oleksandr Natalenko wrote: > > On Wed, Apr 07, 2021 at 04:06:29PM -0700, Alexander Duyck wrote: > > > On Wed, Apr 7, 2021 at 11:07 AM Jakub Kicinski <kuba@kernel.org> wrote: > > > > Sure, that's simplest. I wasn't sure something is supposed to prevent > > > > this condition or if it's okay to cover it up. > > > > > > I'm pretty sure it is okay to cover it up. In this case the "budget - > > > 1" is supposed to be the upper limit on what can be reported. I think > > > it was assuming an unsigned value anyway. > > > > > > Another alternative would be to default clean_complete to !!budget. > > > Then if budget is 0 clean_complete would always return false. > > > > So, among all the variants, which one to try? Or there was a separate > > patch sent to address this? > > Alex's suggestion is probably best. > > I'm not aware of the fix being posted. Perhaps you could take over and > post the patch if Intel doesn't chime in? So, IIUC, Alex suggests this: ``` diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c index a45cd2b416c8..7503d5bf168a 100644 --- a/drivers/net/ethernet/intel/igb/igb_main.c +++ b/drivers/net/ethernet/intel/igb/igb_main.c @@ -7981,7 +7981,7 @@ static int igb_poll(struct napi_struct *napi, int budget) struct igb_q_vector, napi); bool clean_complete = true; - int work_done = 0; + unsigned int work_done = 0; #ifdef CONFIG_IGB_DCA if (q_vector->adapter->flags & IGB_FLAG_DCA_ENABLED) @@ -8008,7 +8008,7 @@ static int igb_poll(struct napi_struct *napi, int budget) if (likely(napi_complete_done(napi, work_done))) igb_ring_irq_enable(q_vector); - return min(work_done, budget - 1); + return min_t(unsigned int, work_done, budget - 1); } /** ``` Am I right? Thanks. -- Oleksandr Natalenko (post-factum)
WARNING: multiple messages have this Message-ID (diff)
From: Oleksandr Natalenko <oleksandr@natalenko.name> To: intel-wired-lan@osuosl.org Subject: [Intel-wired-lan] [igb] netconsole triggers warning in netpoll_poll_dev Date: Mon, 26 Apr 2021 08:47:36 +0200 [thread overview] Message-ID: <20210426064736.7efynita4brzos4u@spock.localdomain> (raw) In-Reply-To: <20210423155836.25ef1e77@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> Hello. On Fri, Apr 23, 2021 at 03:58:36PM -0700, Jakub Kicinski wrote: > On Fri, 23 Apr 2021 10:19:44 +0200 Oleksandr Natalenko wrote: > > On Wed, Apr 07, 2021 at 04:06:29PM -0700, Alexander Duyck wrote: > > > On Wed, Apr 7, 2021 at 11:07 AM Jakub Kicinski <kuba@kernel.org> wrote: > > > > Sure, that's simplest. I wasn't sure something is supposed to prevent > > > > this condition or if it's okay to cover it up. > > > > > > I'm pretty sure it is okay to cover it up. In this case the "budget - > > > 1" is supposed to be the upper limit on what can be reported. I think > > > it was assuming an unsigned value anyway. > > > > > > Another alternative would be to default clean_complete to !!budget. > > > Then if budget is 0 clean_complete would always return false. > > > > So, among all the variants, which one to try? Or there was a separate > > patch sent to address this? > > Alex's suggestion is probably best. > > I'm not aware of the fix being posted. Perhaps you could take over and > post the patch if Intel doesn't chime in? So, IIUC, Alex suggests this: ``` diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c index a45cd2b416c8..7503d5bf168a 100644 --- a/drivers/net/ethernet/intel/igb/igb_main.c +++ b/drivers/net/ethernet/intel/igb/igb_main.c @@ -7981,7 +7981,7 @@ static int igb_poll(struct napi_struct *napi, int budget) struct igb_q_vector, napi); bool clean_complete = true; - int work_done = 0; + unsigned int work_done = 0; #ifdef CONFIG_IGB_DCA if (q_vector->adapter->flags & IGB_FLAG_DCA_ENABLED) @@ -8008,7 +8008,7 @@ static int igb_poll(struct napi_struct *napi, int budget) if (likely(napi_complete_done(napi, work_done))) igb_ring_irq_enable(q_vector); - return min(work_done, budget - 1); + return min_t(unsigned int, work_done, budget - 1); } /** ``` Am I right? Thanks. -- Oleksandr Natalenko (post-factum)
next prev parent reply other threads:[~2021-04-26 6:47 UTC|newest] Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-04-06 12:36 [igb] netconsole triggers warning in netpoll_poll_dev Oleksandr Natalenko 2021-04-06 12:36 ` [Intel-wired-lan] " Oleksandr Natalenko 2021-04-06 18:48 ` Jakub Kicinski 2021-04-06 18:48 ` [Intel-wired-lan] " Jakub Kicinski 2021-04-07 6:00 ` Oleksandr Natalenko 2021-04-07 6:00 ` [Intel-wired-lan] " Oleksandr Natalenko 2021-04-07 15:37 ` Jakub Kicinski 2021-04-07 15:37 ` [Intel-wired-lan] " Jakub Kicinski 2021-04-07 16:25 ` Alexander Duyck 2021-04-07 16:25 ` [Intel-wired-lan] " Alexander Duyck 2021-04-07 18:07 ` Jakub Kicinski 2021-04-07 18:07 ` [Intel-wired-lan] " Jakub Kicinski 2021-04-07 23:06 ` Alexander Duyck 2021-04-07 23:06 ` [Intel-wired-lan] " Alexander Duyck 2021-04-23 8:19 ` Oleksandr Natalenko 2021-04-23 8:19 ` [Intel-wired-lan] " Oleksandr Natalenko 2021-04-23 22:58 ` Jakub Kicinski 2021-04-23 22:58 ` [Intel-wired-lan] " Jakub Kicinski 2021-04-26 6:47 ` Oleksandr Natalenko [this message] 2021-04-26 6:47 ` Oleksandr Natalenko 2021-04-26 15:28 ` Alexander Duyck 2021-04-26 15:28 ` [Intel-wired-lan] " Alexander Duyck 2021-05-06 23:32 ` Jesse Brandeburg 2021-05-06 23:32 ` [Intel-wired-lan] " Jesse Brandeburg 2021-05-07 0:38 ` Alexander Duyck 2021-05-07 0:38 ` [Intel-wired-lan] " Alexander Duyck 2021-11-18 15:30 Danielle Ratson 2021-11-18 16:14 ` Alexander Duyck 2021-11-21 11:44 ` Danielle Ratson 2021-11-21 21:16 ` Alexander Duyck 2021-11-23 20:36 ` Jesse Brandeburg
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=20210426064736.7efynita4brzos4u@spock.localdomain \ --to=oleksandr@natalenko.name \ --cc=alexander.duyck@gmail.com \ --cc=anthony.l.nguyen@intel.com \ --cc=davem@davemloft.net \ --cc=intel-wired-lan@lists.osuosl.org \ --cc=jesse.brandeburg@intel.com \ --cc=kuba@kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=netdev@vger.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: linkBe 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.