Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Wed, 24 Mar 2021 10:28:35 +0100
From:   Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
To:     Jesper Dangaard Brouer <brouer@redhat.com>
Cc:     Matteo Croce <mcroce@linux.microsoft.com>, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        Jonathan Lemon <jonathan.lemon@gmail.com>,
        "David S. Miller" <davem@davemloft.net>,
        Ilias Apalodimas <ilias.apalodimas@linaro.org>,
        Jesper Dangaard Brouer <hawk@kernel.org>,
        Lorenzo Bianconi <lorenzo@kernel.org>,
        Saeed Mahameed <saeedm@nvidia.com>,
        David Ahern <dsahern@gmail.com>,
        Saeed Mahameed <saeed@kernel.org>, Andrew Lunn <andrew@lunn.ch>
Subject: Re: [PATCH net-next 6/6] mvneta: recycle buffers
Message-ID: <YFsGQx6XMbYRtBOR@lore-desk>
References: <20210322170301.26017-1-mcroce@linux.microsoft.com>
 <20210322170301.26017-7-mcroce@linux.microsoft.com>
 <20210323160611.28ddc712@carbon>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="PFe7S71U15D4cqrp"
Content-Disposition: inline
In-Reply-To: <20210323160611.28ddc712@carbon>
Precedence: bulk


--PFe7S71U15D4cqrp
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

[...]
> > diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethern=
et/marvell/mvneta.c
> > index a635cf84608a..8b3250394703 100644
> > --- a/drivers/net/ethernet/marvell/mvneta.c
> > +++ b/drivers/net/ethernet/marvell/mvneta.c
> > @@ -2332,7 +2332,7 @@ mvneta_swbm_build_skb(struct mvneta_port *pp, str=
uct mvneta_rx_queue *rxq,
> >  	if (!skb)
> >  		return ERR_PTR(-ENOMEM);
> > =20
> > -	page_pool_release_page(rxq->page_pool, virt_to_page(xdp->data));
> > +	skb_mark_for_recycle(skb, virt_to_page(xdp->data), &xdp->rxq->mem);
> > =20
> >  	skb_reserve(skb, xdp->data - xdp->data_hard_start);
> >  	skb_put(skb, xdp->data_end - xdp->data);
> > @@ -2344,7 +2344,7 @@ mvneta_swbm_build_skb(struct mvneta_port *pp, str=
uct mvneta_rx_queue *rxq,
> >  		skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags,
> >  				skb_frag_page(frag), skb_frag_off(frag),
> >  				skb_frag_size(frag), PAGE_SIZE);
> > -		page_pool_release_page(rxq->page_pool, skb_frag_page(frag));
> > +		skb_mark_for_recycle(skb, skb_frag_page(frag), &xdp->rxq->mem);
> >  	}
> > =20
> >  	return skb;
>=20
> This cause skb_mark_for_recycle() to set 'skb->pp_recycle=3D1' multiple
> times, for the same SKB.  (copy-pasted function below signature to help
> reviewers).
>=20
> This makes me question if we need an API for setting this per page
> fragment?
> Or if the API skb_mark_for_recycle() need to walk the page fragments in
> the SKB and set the info stored in the page for each?

Considering just performances, I guess it is better open-code here since the
driver already performs a loop over fragments to build the skb, but I guess
this approach is quite risky and I would prefer to have a single utility
routine to take care of linear area + fragments. What do you think?

Regards,
Lorenzo

>=20
>=20
> --=20
> Best regards,
>   Jesper Dangaard Brouer
>   MSc.CS, Principal Kernel Engineer at Red Hat
>   LinkedIn: http://www.linkedin.com/in/brouer
>=20

--PFe7S71U15D4cqrp
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iHUEABYIAB0WIQTquNwa3Txd3rGGn7Y6cBh0uS2trAUCYFsGQAAKCRA6cBh0uS2t
rFgiAP0c8Q5ppxzpPYMYk5waAY9QN9APUtx2G3i42vTAQpux1wEAsok6G5Kdb/ww
bYEkrUU4/iizc56IifIQQ8iY05slpgk=
=h2Ta
-----END PGP SIGNATURE-----

--PFe7S71U15D4cqrp--