All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexander Duyck <alexander.duyck@gmail.com>
To: David Miller <davem@davemloft.net>, aduyck@mirantis.com
Cc: netdev@vger.kernel.org
Subject: Re: [PATCH] netpoll: Drop budget parameter from NAPI polling call hierarchy
Date: Sun, 27 Sep 2015 15:58:56 -0700	[thread overview]
Message-ID: <560874B0.1080903@gmail.com> (raw)
In-Reply-To: <20150926.223631.507670918270362716.davem@davemloft.net>

On 09/26/2015 10:36 PM, David Miller wrote:
> From: Alexander Duyck <aduyck@mirantis.com>
> Date: Tue, 22 Sep 2015 14:56:08 -0700
>
>> Rather than carry around a value of budget that is 0 or less we can instead
>> just loop through and pass 0 to each napi->poll call.  If any driver
>> returns a value for work done that is non-zero then we can report that
>> driver and continue rather than allowing a bad actor to make the budget
>> value negative and pass that negative value to napi->poll.
> Unfortunately we have drivers that won't do any TX work if the budget
> is zero.

Well that is what we are doing right now.  The fact is the call starts 
out with a budget of 0, and it is somewhat hidden from the call since 
the budget is assigned a value of 0 in netpoll_poll_dev. That is one of 
the things I was wanting do address because that is clear as mud from 
looking at poll_one_napi.  Based on the code you would assume budget 
starts out as a non-zero value and it doesn't.

> Using the budget for TX work is unfortunate and not the recommended
> way for drivers to do things, but it's not explicitly disallowed
> either.
>
> So I'm not applying this because it definitely has the potential
> to break something.
>
> Sorry.

I don't see how this introduces a regression when all I am doing is 
avoiding tracking a value that should be 0 assuming everything is 
working correctly.  If work returns a non-zero value with the code as it 
currently is then the WARN_ONCE is triggered, and the value of budget is 
becoming negative.  I would consider a negative budget value worse than 
a 0 budget value.

I'll go back through the patch and rebase it since it looks like Neil 
had to submit a v3 of his patch and it may have impacted mine. However 
perhaps we need to revisit this code if you think it is risky as the 
only thing my changes did is remove the ability for the budget value to 
go from 0 to negative and then passing that negative value into the 
function.

- Alex

  reply	other threads:[~2015-09-27 22:58 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-22 21:56 [PATCH] netpoll: Drop budget parameter from NAPI polling call hierarchy Alexander Duyck
2015-09-27  5:36 ` David Miller
2015-09-27 22:58   ` Alexander Duyck [this message]
2015-09-29 20:48     ` David Miller

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=560874B0.1080903@gmail.com \
    --to=alexander.duyck@gmail.com \
    --cc=aduyck@mirantis.com \
    --cc=davem@davemloft.net \
    --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: 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.