All of lore.kernel.org
 help / color / mirror / Atom feed
From: Danielle Ratson <danieller@nvidia.com>
To: Alexander Duyck <alexander.duyck@gmail.com>
Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>,
	Oleksandr Natalenko <oleksandr@natalenko.name>,
	Jakub Kicinski <kuba@kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	"Nguyen, Anthony L" <anthony.l.nguyen@intel.com>,
	"David S. Miller" <davem@davemloft.net>,
	intel-wired-lan <intel-wired-lan@lists.osuosl.org>,
	Netdev <netdev@vger.kernel.org>
Subject: RE: [igb] netconsole triggers warning in netpoll_poll_dev
Date: Sun, 21 Nov 2021 11:44:51 +0000	[thread overview]
Message-ID: <DM6PR12MB45162662DF7FAF82E7BD2207D89E9@DM6PR12MB4516.namprd12.prod.outlook.com> (raw)
In-Reply-To: <CAKgT0UfGvcGXAC5VBjXRpR5Y5uAPEPPCsYWjQR8RmW_1kw8TMQ@mail.gmail.com>

> > > >  drivers/net/ethernet/intel/igb/igb_main.c | 12 ++++++++----
> > > >  1 file changed, 8 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/drivers/net/ethernet/intel/igb/igb_main.c
> > > > b/drivers/net/ethernet/intel/igb/igb_main.c
> > > > index 0cd37ad81b4e..b0a9bed14071 100644
> > > > --- a/drivers/net/ethernet/intel/igb/igb_main.c
> > > > +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> > > > @@ -7991,12 +7991,16 @@ static void igb_ring_irq_enable(struct
> > > igb_q_vector *q_vector)
> > > >   **/
> > > >  static int igb_poll(struct napi_struct *napi, int budget)  {
> > > > -       struct igb_q_vector *q_vector = container_of(napi,
> > > > -                                                    struct igb_q_vector,
> > > > -                                                    napi);
> > > > -       bool clean_complete = true;
> > > > +       struct igb_q_vector *q_vector;
> > > > +       bool clean_complete;
> > > >         int work_done = 0;
> > > >
> > > > +       /* if budget is zero, we have a special case for netconsole, so
> > > > +        * make sure to set clean_complete to false in that case.
> > > > +        */
> > > > +       clean_complete = !!budget;
> > > > +
> > > > +       q_vector = container_of(napi, struct igb_q_vector, napi);
> > > >  #ifdef CONFIG_IGB_DCA
> > > >         if (q_vector->adapter->flags & IGB_FLAG_DCA_ENABLED)
> > > >                 igb_update_dca(q_vector);
> > >
> > > I'm not a big fan of moving the q_vector init as a part of this
> > > patch since it just means more backport work.
> > >
> > > That said the change itself should be harmless so I am good with it
> > > either way.
> > >
> > > Reviewed-by: Alexander Duyck <alexanderduyck@fb.com>
> >
> > Hi,
> >
> > I have lately added the netconsole module, and since then we see the
> same warning constantly in the logs.
> > I have tried to apply Jesse's patch but it didn't seem to solve the issue.
> >
> > Did anyone managed to solve the issue and can share with us?
> >
> > Thanks,
> > Danielle
> 
> The one issue I can see is that it basically leaves the igb_poll call stuck in
> polling mode.
> 
> The easiest fix for all of this in the in-kernel driver is to just get rid of the
> "min" at the end and instead just "return work_done;". The extra
> complication is only needed if you were to be polling multiple queues and
> that isn't the case here so we should simplify it and get rid of the buggy
> "budget - 1" return value.

Thank you very much for your reply Alexander!
It seems to work well!

Are you planning to send it upstream?

Thanks,
Danielle


WARNING: multiple messages have this Message-ID (diff)
From: Danielle Ratson <danieller@nvidia.com>
To: intel-wired-lan@osuosl.org
Subject: [Intel-wired-lan] [igb] netconsole triggers warning in netpoll_poll_dev
Date: Sun, 21 Nov 2021 11:44:51 +0000	[thread overview]
Message-ID: <DM6PR12MB45162662DF7FAF82E7BD2207D89E9@DM6PR12MB4516.namprd12.prod.outlook.com> (raw)
In-Reply-To: <CAKgT0UfGvcGXAC5VBjXRpR5Y5uAPEPPCsYWjQR8RmW_1kw8TMQ@mail.gmail.com>

> > > >  drivers/net/ethernet/intel/igb/igb_main.c | 12 ++++++++----
> > > >  1 file changed, 8 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/drivers/net/ethernet/intel/igb/igb_main.c
> > > > b/drivers/net/ethernet/intel/igb/igb_main.c
> > > > index 0cd37ad81b4e..b0a9bed14071 100644
> > > > --- a/drivers/net/ethernet/intel/igb/igb_main.c
> > > > +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> > > > @@ -7991,12 +7991,16 @@ static void igb_ring_irq_enable(struct
> > > igb_q_vector *q_vector)
> > > >   **/
> > > >  static int igb_poll(struct napi_struct *napi, int budget)  {
> > > > -       struct igb_q_vector *q_vector = container_of(napi,
> > > > -                                                    struct igb_q_vector,
> > > > -                                                    napi);
> > > > -       bool clean_complete = true;
> > > > +       struct igb_q_vector *q_vector;
> > > > +       bool clean_complete;
> > > >         int work_done = 0;
> > > >
> > > > +       /* if budget is zero, we have a special case for netconsole, so
> > > > +        * make sure to set clean_complete to false in that case.
> > > > +        */
> > > > +       clean_complete = !!budget;
> > > > +
> > > > +       q_vector = container_of(napi, struct igb_q_vector, napi);
> > > >  #ifdef CONFIG_IGB_DCA
> > > >         if (q_vector->adapter->flags & IGB_FLAG_DCA_ENABLED)
> > > >                 igb_update_dca(q_vector);
> > >
> > > I'm not a big fan of moving the q_vector init as a part of this
> > > patch since it just means more backport work.
> > >
> > > That said the change itself should be harmless so I am good with it
> > > either way.
> > >
> > > Reviewed-by: Alexander Duyck <alexanderduyck@fb.com>
> >
> > Hi,
> >
> > I have lately added the netconsole module, and since then we see the
> same warning constantly in the logs.
> > I have tried to apply Jesse's patch but it didn't seem to solve the issue.
> >
> > Did anyone managed to solve the issue and can share with us?
> >
> > Thanks,
> > Danielle
> 
> The one issue I can see is that it basically leaves the igb_poll call stuck in
> polling mode.
> 
> The easiest fix for all of this in the in-kernel driver is to just get rid of the
> "min" at the end and instead just "return work_done;". The extra
> complication is only needed if you were to be polling multiple queues and
> that isn't the case here so we should simplify it and get rid of the buggy
> "budget - 1" return value.

Thank you very much for your reply Alexander!
It seems to work well!

Are you planning to send it upstream?

Thanks,
Danielle


  reply	other threads:[~2021-11-21 11:44 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-18 15:30 [igb] netconsole triggers warning in netpoll_poll_dev Danielle Ratson
2021-11-18 15:30 ` [Intel-wired-lan] " Danielle Ratson
2021-11-18 16:14 ` Alexander Duyck
2021-11-18 16:14   ` [Intel-wired-lan] " Alexander Duyck
2021-11-21 11:44   ` Danielle Ratson [this message]
2021-11-21 11:44     ` Danielle Ratson
2021-11-21 21:16     ` Alexander Duyck
2021-11-21 21:16       ` [Intel-wired-lan] " Alexander Duyck
2021-11-23 20:36       ` Jesse Brandeburg
2021-11-23 20:36         ` [Intel-wired-lan] " Jesse Brandeburg
  -- strict thread matches above, loose matches on Subject: below --
2021-04-06 12:36 Oleksandr Natalenko
2021-04-06 18:48 ` Jakub Kicinski
2021-04-07  6:00   ` Oleksandr Natalenko
2021-04-07 15:37     ` Jakub Kicinski
2021-04-07 16:25       ` Alexander Duyck
2021-04-07 18:07         ` Jakub Kicinski
2021-04-07 23:06           ` Alexander Duyck
2021-04-23  8:19             ` Oleksandr Natalenko
2021-04-23 22:58               ` Jakub Kicinski
2021-04-26  6:47                 ` Oleksandr Natalenko
2021-04-26 15:28                   ` Alexander Duyck
2021-05-06 23:32                     ` Jesse Brandeburg
2021-05-07  0:38                       ` Alexander Duyck

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=DM6PR12MB45162662DF7FAF82E7BD2207D89E9@DM6PR12MB4516.namprd12.prod.outlook.com \
    --to=danieller@nvidia.com \
    --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 \
    --cc=oleksandr@natalenko.name \
    /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.