Re: [PATCH net-next 2/4] mvpp2: use page_pool allocator

[Matteo Croce]

On Thu, Jul 2, 2020 at 9:31 AM <ilias.apalodimas@xxxxxxxxxx> 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@xxxxxxxxxxxxx>
> > -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