linux-kernel - Re: [PATCH net-next 2/4] mvpp2: use page

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 2 Jul 2020 11:42:01 +0200
From:   Matteo Croce <mcroce@...ux.microsoft.com>
To:     ilias.apalodimas@...aro.org
Cc:     netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        bpf@...r.kernel.org, Sven Auhagen <sven.auhagen@...eatech.de>,
        Lorenzo Bianconi <lorenzo@...nel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Jesper Dangaard Brouer <brouer@...hat.com>,
        Stefan Chulski <stefanc@...vell.com>,
        Marcin Wojtas <mw@...ihalf.com>, maxime.chevallier@...tlin.com,
        antoine.tenart@...tlin.com, thomas.petazzoni@...tlin.com
Subject: Re: [PATCH net-next 2/4] mvpp2: use page_pool allocator

On Thu, Jul 2, 2020 at 9:31 AM <ilias.apalodimas@...aro.org> wrote:
>
> Hi Matteo,
>
> Thanks for working on this!
>

:)

> On Tue, Jun 30, 2020 at 08:09:28PM +0200, Matteo Croce wrote:
> > From: Matteo Croce <mcroce@...rosoft.com>
> > -static void *mvpp2_frag_alloc(const struct mvpp2_bm_pool *pool)
> > +/* Returns a struct page if page_pool is set, otherwise a buffer */
> > +static void *mvpp2_frag_alloc(const struct mvpp2_bm_pool *pool,
> > +                           struct page_pool *page_pool)
> >  {
> > +     if (page_pool)
> > +             return page_pool_alloc_pages(page_pool,
> > +                                          GFP_ATOMIC | __GFP_NOWARN);
>
> page_pool_dev_alloc_pages() can set these flags for you, instead of explicitly
> calling them
>

Ok

> >
> > +     if (priv->percpu_pools) {
> > +             err = xdp_rxq_info_reg(&rxq->xdp_rxq_short, port->dev, rxq->id);
> > +             if (err < 0)
> > +                     goto err_free_dma;
> > +
> > +             err = xdp_rxq_info_reg(&rxq->xdp_rxq_long, port->dev, rxq->id);
> > +             if (err < 0)
> > +                     goto err_unregister_rxq_short;
> > +
> > +             /* Every RXQ has a pool for short and another for long packets */
> > +             err = xdp_rxq_info_reg_mem_model(&rxq->xdp_rxq_short,
> > +                                              MEM_TYPE_PAGE_POOL,
> > +                                              priv->page_pool[rxq->logic_rxq]);
> > +             if (err < 0)
> > +                     goto err_unregister_rxq_short;
> > +
> > +             err = xdp_rxq_info_reg_mem_model(&rxq->xdp_rxq_long,
> > +                                              MEM_TYPE_PAGE_POOL,
> > +                                              priv->page_pool[rxq->logic_rxq +
> > +                                                              port->nrxqs]);
> > +             if (err < 0)
> > +                     goto err_unregister_rxq_long;
>
> Since mvpp2_rxq_init() will return an error shouldn't we unregister the short
> memory pool as well?
>

Ok, I'll add another label like:

err_unregister_mem_rxq_short:
        xdp_rxq_info_unreg_mem_model(&rxq->xdp_rxq_short);

-- 
per aspera ad upstream