From: Maxime Chevallier <maxime.chevallier@bootlin.com>
To: Vladimir Oltean <vladimir.oltean@nxp.com>
Cc: "davem@davemloft.net" <davem@davemloft.net>,
Rob Herring <robh+dt@kernel.org>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
"thomas.petazzoni@bootlin.com" <thomas.petazzoni@bootlin.com>,
Andrew Lunn <andrew@lunn.ch>,
Florian Fainelli <f.fainelli@gmail.com>,
Heiner Kallweit <hkallweit1@gmail.com>,
Russell King <linux@armlinux.org.uk>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
Luka Perkov <luka.perkov@sartura.hr>,
Robert Marko <robert.marko@sartura.hr>
Subject: Re: [PATCH net-next v2 1/5] net: ipqess: introduce the Qualcomm IPQESS driver
Date: Tue, 17 May 2022 09:11:34 +0200 [thread overview]
Message-ID: <20220517091134.4b67b4a0@pc-20.home> (raw)
In-Reply-To: <20220514204438.urxot42jfazwnjlz@skbuf>
Hello Vlad,
On Sat, 14 May 2022 20:44:38 +0000
Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
> On Sat, May 14, 2022 at 05:06:52PM +0200, Maxime Chevallier wrote:
> > +/* locking is handled by the caller */
> > +static int ipqess_rx_buf_alloc_napi(struct ipqess_rx_ring *rx_ring)
> > +{
> > + struct ipqess_buf *buf = &rx_ring->buf[rx_ring->head];
> > +
> > + buf->skb = napi_alloc_skb(&rx_ring->napi_rx,
> > IPQESS_RX_HEAD_BUFF_SIZE);
> > + if (!buf->skb)
> > + return -ENOMEM;
> > +
> > + return ipqess_rx_buf_prepare(buf, rx_ring);
> > +}
> > +
> > +static int ipqess_rx_buf_alloc(struct ipqess_rx_ring *rx_ring)
> > +{
> > + struct ipqess_buf *buf = &rx_ring->buf[rx_ring->head];
> > +
> > + buf->skb = netdev_alloc_skb_ip_align(rx_ring->ess->netdev,
> > +
> > IPQESS_RX_HEAD_BUFF_SIZE); +
> > + if (!buf->skb)
> > + return -ENOMEM;
> > +
> > + return ipqess_rx_buf_prepare(buf, rx_ring);
> > +}
> > +
> > +static void ipqess_refill_work(struct work_struct *work)
> > +{
> > + struct ipqess_rx_ring_refill *rx_refill =
> > container_of(work,
> > + struct ipqess_rx_ring_refill, refill_work);
> > + struct ipqess_rx_ring *rx_ring = rx_refill->rx_ring;
> > + int refill = 0;
> > +
> > + /* don't let this loop by accident. */
> > + while (atomic_dec_and_test(&rx_ring->refill_count)) {
> > + napi_disable(&rx_ring->napi_rx);
> > + if (ipqess_rx_buf_alloc(rx_ring)) {
> > + refill++;
> > + dev_dbg(rx_ring->ppdev,
> > + "Not all buffers were
> > reallocated");
> > + }
> > + napi_enable(&rx_ring->napi_rx);
> > + }
> > +
> > + if (atomic_add_return(refill, &rx_ring->refill_count))
> > + schedule_work(&rx_refill->refill_work);
> > +}
> > +
> > +static int ipqess_rx_poll(struct ipqess_rx_ring *rx_ring, int
> > budget) +{
>
> > + while (done < budget) {
>
> > + num_desc += atomic_xchg(&rx_ring->refill_count, 0);
> > + while (num_desc) {
> > + if (ipqess_rx_buf_alloc_napi(rx_ring)) {
> > + num_desc =
> > atomic_add_return(num_desc,
> > +
> > &rx_ring->refill_count);
> > + if (num_desc >= ((4 *
> > IPQESS_RX_RING_SIZE + 6) / 7))
>
> DIV_ROUND_UP(IPQESS_RX_RING_SIZE * 4, 7)
> Also, why this number?
Ah this was from the original out-of-tree driver... I'll try to figure
out what's going on an replace that by some #define that would make
more sense.
> > +
> > schedule_work(&rx_ring->ess->rx_refill[rx_ring->ring_id].refill_work);
> > + break;
> > + }
> > + num_desc--;
> > + }
> > + }
> > +
> > + ipqess_w32(rx_ring->ess,
> > IPQESS_REG_RX_SW_CONS_IDX_Q(rx_ring->idx),
> > + rx_ring_tail);
> > + rx_ring->tail = rx_ring_tail;
> > +
> > + return done;
> > +}
>
> > +static void ipqess_rx_ring_free(struct ipqess *ess)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < IPQESS_NETDEV_QUEUES; i++) {
> > + int j;
> > +
> > + atomic_set(&ess->rx_ring[i].refill_count, 0);
> > + cancel_work_sync(&ess->rx_refill[i].refill_work);
>
> When refill_work is currently scheduled and executing the while loop,
> will refill_count underflow due to the possibility of calling
> atomic_dec_and_test(0)?
Good question, I'll double-check, you might be correct. Nice catch
> > +
> > + for (j = 0; j < IPQESS_RX_RING_SIZE; j++) {
> > + dma_unmap_single(&ess->pdev->dev,
> > +
> > ess->rx_ring[i].buf[j].dma,
> > +
> > ess->rx_ring[i].buf[j].length,
> > + DMA_FROM_DEVICE);
> > +
> > dev_kfree_skb_any(ess->rx_ring[i].buf[j].skb);
> > + }
> > + }
> > +
Thanks,
Maxime
next prev parent reply other threads:[~2022-05-17 7:12 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-14 15:06 [PATCH net-next v2 0/5] net: ipqess: introduce Qualcomm IPQESS driver Maxime Chevallier
2022-05-14 15:06 ` [PATCH net-next v2 1/5] net: ipqess: introduce the " Maxime Chevallier
2022-05-14 17:18 ` Russell King (Oracle)
2022-05-17 7:09 ` Maxime Chevallier
2022-05-14 20:44 ` Vladimir Oltean
2022-05-17 7:11 ` Maxime Chevallier [this message]
2022-05-16 2:51 ` Andrew Lunn
2022-05-17 7:13 ` Maxime Chevallier
2022-05-17 21:03 ` Christophe JAILLET
2022-05-14 15:06 ` [PATCH net-next v2 2/5] net: dsa: add out-of-band tagging protocol Maxime Chevallier
2022-05-14 16:33 ` Florian Fainelli
2022-05-17 7:06 ` Maxime Chevallier
2022-05-14 22:40 ` Vladimir Oltean
2022-05-17 7:01 ` Maxime Chevallier
2022-05-19 14:52 ` Vladimir Oltean
2022-05-19 17:11 ` Florian Fainelli
2022-05-19 17:34 ` Vladimir Oltean
2022-05-16 19:20 ` Jakub Kicinski
2022-05-17 6:53 ` Maxime Chevallier
2022-05-17 20:58 ` Jakub Kicinski
2022-05-14 15:06 ` [PATCH net-next v2 3/5] net: ipqess: Add out-of-band DSA tagging support Maxime Chevallier
2022-05-14 15:06 ` [PATCH net-next v2 4/5] net: dt-bindings: Introduce the Qualcomm IPQESS Ethernet controller Maxime Chevallier
2022-05-18 0:52 ` Rob Herring
2022-05-14 15:06 ` [PATCH net-next v2 5/5] ARM: dts: qcom: ipq4019: Add description for the " Maxime Chevallier
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=20220517091134.4b67b4a0@pc-20.home \
--to=maxime.chevallier@bootlin.com \
--cc=andrew@lunn.ch \
--cc=davem@davemloft.net \
--cc=devicetree@vger.kernel.org \
--cc=f.fainelli@gmail.com \
--cc=hkallweit1@gmail.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@armlinux.org.uk \
--cc=luka.perkov@sartura.hr \
--cc=netdev@vger.kernel.org \
--cc=robert.marko@sartura.hr \
--cc=robh+dt@kernel.org \
--cc=thomas.petazzoni@bootlin.com \
--cc=vladimir.oltean@nxp.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).