All of lore.kernel.org
 help / color / mirror / Atom feed
From: Edward Cree <ecree.xilinx@gmail.com>
To: Ignat Korchagin <ignat@cloudflare.com>
Cc: habetsm.xilinx@gmail.com, "David S. Miller" <davem@davemloft.net>,
	kuba@kernel.org, netdev@vger.kernel.org,
	kernel-team <kernel-team@cloudflare.com>,
	stable@vger.kernel.org
Subject: Re: [PATCH] sfc: adjust efx->xdp_tx_queue_count with the real number of initialized queues
Date: Thu, 29 Apr 2021 16:04:51 +0100	[thread overview]
Message-ID: <f7951d69-0c67-7455-2b0c-530cb959bff5@gmail.com> (raw)
In-Reply-To: <CALrw=nF+rD+GdWAZndKGxTW4cpao+x2W0dvDfUacXjD=A5mCKA@mail.gmail.com>

On 29/04/2021 15:49, Ignat Korchagin wrote:
> On Thu, Apr 29, 2021 at 3:22 PM Edward Cree <ecree.xilinx@gmail.com> wrote:
>>
>> On 27/04/2021 22:09, Ignat Korchagin wrote:
>>> +     if (xdp_queue_number)
>> Wait, why is this guard condition needed?
>> What happens if we had nonzero efx->xdp_tx_queue_count initially, but we end up
>>  with no TXQs available for XDP at all (so xdp_queue_number == 0)?
>>
>> -ed
> 
> My thoughts were: efx->xdp_tx_queue_count is originally used to
> allocate efx->xdp_tx_queues.
> So, if xdp_queue_number ends up being 0, we should keep
> efx->xdp_tx_queue_count positive not
> to forget to release efx->xdp_tx_queues (because most checks are
> efx->xdp_tx_queue_count && efx->xdp_tx_queues).
Well, we allocated it in this function, so could we not just free it
 (and NULL it) if we get here with xdp_queue_number == 0?
Assuming it even makes sense for those checks to be that conjunction,
 and not just efx->xdp_tx_queues.

> I'm not familiar enough with SFC internals to definitely say if it is
> even possible to have
> xdp_queue_number == 0 while having efx->xdp_tx_queue_count > 0
If it's possible for us to get xdp_queue_number != efx->xdp_tx_queue_count
 at all (which I can't remember exactly how it happens, but I think it's a
 case of not getting as many VIs back from firmware as we wanted, which
 happens after the initial determination of numbers of queues & channels),
 then it's possible that our number of available TXQs is reduced far
 enough that we don't have any left for XDP.
At least, I think so; this part of the driver confuses me too :S

-ed

      reply	other threads:[~2021-04-29 15:04 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-27 21:09 [PATCH] sfc: adjust efx->xdp_tx_queue_count with the real number of initialized queues Ignat Korchagin
2021-04-27 22:40 ` patchwork-bot+netdevbpf
2021-04-29 14:22 ` Edward Cree
2021-04-29 14:49   ` Ignat Korchagin
2021-04-29 15:04     ` Edward Cree [this message]

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=f7951d69-0c67-7455-2b0c-530cb959bff5@gmail.com \
    --to=ecree.xilinx@gmail.com \
    --cc=davem@davemloft.net \
    --cc=habetsm.xilinx@gmail.com \
    --cc=ignat@cloudflare.com \
    --cc=kernel-team@cloudflare.com \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=stable@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.