Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Ratheesh Kannoth <rkannoth@marvell.com>
To:     Jakub Kicinski <kuba@kernel.org>
CC:     "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Sunil Kovvuri Goutham <sgoutham@marvell.com>,
        "davem@davemloft.net" <davem@davemloft.net>,
        "edumazet@google.com" <edumazet@google.com>,
        "pabeni@redhat.com" <pabeni@redhat.com>,
        Subbaraya Sundeep Bhatta <sbhatta@marvell.com>,
        Geethasowjanya Akula <gakula@marvell.com>,
        Srujana Challa <schalla@marvell.com>,
        Hariprasad Kelam <hkelam@marvell.com>
Subject: RE: Re: [PATH net-next v1] octeontx2-pf: Add support for page pool
Thread-Topic: Re: [PATH net-next v1] octeontx2-pf: Add support for page pool
Thread-Index: AQHZiUlvwclaroU3Xk+4nrIRMuBdfA==
Date:   Thu, 18 May 2023 05:27:27 +0000
Message-ID: <MWHPR1801MB1918540ACD82ECEF1F7D92ACD37F9@MWHPR1801MB1918.namprd18.prod.outlook.com>
References: <20230517041511.2532997-1-rkannoth@marvell.com>
 <20230517204632.5f80a7bf@kernel.org>
In-Reply-To: <20230517204632.5f80a7bf@kernel.org>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?OIII756Q0qWM1Ubs43RUZYvK1FzXNV5fWdAX84H7b19O2TozimG/PhE9ye5w?=
 =?us-ascii?Q?QnylBYF6MnOr4bHF00qrgEUJMjc51CoseU4zwqZo/UnNmlmlGTICbyzmqlWj?=
 =?us-ascii?Q?L4HZdNYht+qK/dSrtGwyytPVRBZ1csthbDqZnjCnL/M8WEjomnZdVeQ1FX9I?=
 =?us-ascii?Q?hw9IEE3U2X9dNdfmkX7UKsI47d+yHnXV7VqGijbE4OCyPPu6bZsAJ5QtVVHv?=
 =?us-ascii?Q?xp5wHNid5sJWTD39c8Ovv0I2zKiYFjRsKJD7YbjE1jGD/IPqUciHv4K9jSYK?=
 =?us-ascii?Q?dnf9JJ49lXVBjVIXr9aH5NARs040mD1t4czFhzOIKZWrKEiw/3zbegDpVbVJ?=
 =?us-ascii?Q?eSgd+z3ou0z8uctl4z+XEoTvbBixqUN9So7mxcglZg89+CtmwDYrXVb8plGh?=
 =?us-ascii?Q?o25/ViJKPzcDkSla+unFDPGr/IbuEgLP1SUjEAMWKukuSOloebUQr0PU+5fO?=
 =?us-ascii?Q?L/LvzNRaM+T1U4It4owAsiIz2Rrx/R+81gOFxBk1Tu966AAt35AwBRQH00BW?=
 =?us-ascii?Q?xKwusFCDVC/KOAWB0FOn91UxBIohNNB8lkGzrMUxWReSuKOZcD0WFJbSFbx2?=
 =?us-ascii?Q?1KCdDClhmZOQJggiVkZSHUQ+FeCkhigL0FZEG25Nq26UmUyJkv0Cfji1GoMe?=
 =?us-ascii?Q?1lIIkUmUHDYbZcxVYmU5fdqU7mPCvYMJ5cwO7Ra3Spm3HsIZ0GtnkvvvJdra?=
 =?us-ascii?Q?F3gngbqJ/OXyFKeLs4Rt67261B7psmk+U1ENTsnkmFGQN9IX2PLj0BgYqSFM?=
 =?us-ascii?Q?VHu3OaFX5BYX7wASOk3lIbHfXNlduax0tD3r66Msx3feWPAIXjBOvYjZGfva?=
 =?us-ascii?Q?bez3ixWXhPNekQk43crnG7qzipnmVhbAjkr55hrC2GpnPrKjOmuQaP4Hk9TU?=
 =?us-ascii?Q?iKG3a1FOKi48nm339SyNJgIdWv5E/WVJhznOJY5L4oFDwEnPi3ypjigkUWNj?=
 =?us-ascii?Q?Hq9cE4Axgh4YcQ37quOs8ybf6Wdup8Uic6csKX98NQVhVB/LMgt2fJ9948SY?=
 =?us-ascii?Q?wQj5SQ0+s6PvBGkxSQCf7Y60cpUCYvzAkoRu3LSB1FSlPTORj+n0Rjd9GhZ5?=
 =?us-ascii?Q?KsAB7DS2Z0gW3CHeb8U2p7H9YsEPSqD5sMTC/gaqCV27XFtPfWO3cv4WCDja?=
 =?us-ascii?Q?2mMBWDCz9hCqsOnAmFu/dJd/hd3veaX7izD9W2V7x5RaqM6UJ3PSpRBq7DhQ?=
 =?us-ascii?Q?ln80yI4Ij8aY/q1R0F5XcM8Zczkn/TaaTubrrzGI0iSgZ0kFHqVQXO2i5U9l?=
 =?us-ascii?Q?zh4f7qDFUrTl1kw7VkfeuCuTpPZSVWh9QrF/6NKaivkMMSogS7pAguip89km?=
 =?us-ascii?Q?252rqjIvk59r8owv3GiPXdiNkQQVlrK9U+WFTxsCeXPqRp5W4M54+LOxm/Wg?=
 =?us-ascii?Q?axZFXqjIB7zoMRPOYHvjNcmju4YA8g2X3v3LThdS+Z/METW1ZBKh2a6kgaoG?=
 =?us-ascii?Q?L3ycEJy7RPeoidrTxclJ/Sjj83bsonT4uyz5dvHsBTG8S0G3e2KXOe9LJuSb?=
 =?us-ascii?Q?oAu9Lnz8UpZhuzOu5V8WrqlxBy8tqRvGpaHI15DJWcKrn7gYUMxXDgUX3exG?=
 =?us-ascii?Q?k6bMLszhBxBjPTzx6YVkp1tBjIHjC7SNMin9FBfS?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: marvell.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: MWHPR1801MB1918.namprd18.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: ca0323d9-6da4-44bf-c28a-08db576091f0
X-MS-Exchange-CrossTenant-originalarrivaltime: 18 May 2023 05:27:27.9521
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 70e1fb47-1155-421d-87fc-2e58f638b6e0
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: T+oDY6D9n/qD9SlRVT/0I7ARUY1/CaRJJ3Rv8uzCZyCc1FpP5ClTWFf6VFzYHHcUUwj/L3UbyinyBW+W3GIB7w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH2PR18MB3272
X-Proofpoint-GUID: HLgI4cLlmlQDVPc3ZFeKD-aXvy6c73WG
X-Proofpoint-ORIG-GUID: HLgI4cLlmlQDVPc3ZFeKD-aXvy6c73WG
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.254,Aquarius:18.0.957,Hydra:6.0.573,FMLib:17.11.170.22
 definitions=2023-05-18_03,2023-05-17_02,2023-02-09_01
X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,RCVD_IN_DNSWL_LOW,SPF_HELO_NONE,SPF_NONE,
        T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

> -----Original Message-----
> From: Jakub Kicinski <kuba@kernel.org>
> Sent: Thursday, May 18, 2023 9:17 AM
> To: Ratheesh Kannoth <rkannoth@marvell.com>
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Sunil Kovvuri
> Goutham <sgoutham@marvell.com>; davem@davemloft.net;
> edumazet@google.com; pabeni@redhat.com; Subbaraya Sundeep Bhatta
> <sbhatta@marvell.com>; Geethasowjanya Akula <gakula@marvell.com>;
> Srujana Challa <schalla@marvell.com>; Hariprasad Kelam
> <hkelam@marvell.com>
> Subject: [EXT] Re: [PATH net-next v1] octeontx2-pf: Add support for page
> pool
> On Wed, 17 May 2023 09:45:11 +0530 Ratheesh Kannoth wrote:
> > Page pool for each rx queue enhance rx side performance by reclaiming
> > buffers back to each queue specific pool. DMA mapping is done only for
> > first allocation of buffers.
> > As subsequent buffers allocation avoid DMA mapping, it results in
> > performance improvement.
> >
> > Image        |  Performance with Linux kernel Packet Generator
> > ------------ | -----------------------------------------------
> > Vannila      |   3Mpps
> >              |
> > with this    |   42Mpps
> > change	     |
> > ----------------------------------------------------------------
> >
> > Signed-off-by: Ratheesh Kannoth <rkannoth@marvell.com>
> >
>=20
> Put an extra --- here, to place the change log outside the normal commit
> message.
Okay.=20

>=20
> > ChangeLog
> > v0 -> v1: Removed CONFIG_PAGE_POOL #ifdefs in code
> > 	  Used compound page APIs
> > 	  Replaced page_pool_put_page API with page_pool_put_full_page
> API
>=20
> > diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
> > b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
> > index f9286648e45c..49df1876eca3 100644
> > --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
> > +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
> > @@ -518,11 +518,36 @@ void otx2_config_irq_coalescing(struct otx2_nic
> *pfvf, int qidx)
> >  		     (pfvf->hw.cq_ecount_wait - 1));  }
> >
> > +static int otx2_alloc_pool_buf(struct otx2_nic *pfvf, struct otx2_pool
> *pool,
> > +			       dma_addr_t *dma)
> > +{
> > +	unsigned int offset =3D 0;
> > +	struct page *page;
> > +	size_t sz;
> > +
> > +	sz =3D SKB_DATA_ALIGN(pool->rbsize);
> > +	sz =3D ALIGN(sz, OTX2_ALIGN);
> > +
> > +	page =3D page_pool_alloc_frag(pool->page_pool, &offset, sz,
> > +				    (in_interrupt() ? GFP_ATOMIC :
> GFP_KERNEL) |
>=20
> in_interrupt() should not be used in drivers, AFAIR.
Okay.=20

> Pass the correct flags from the caller (or don't -- it seems like the onl=
y caller
> assumes softirq context already).
I agree. Will Remove GFP_KERNEL.

>=20
> > +				    GFP_DMA);
>=20
> GFP_DMA? Why?
No need. Will Remove.=20

>=20
> > +	if (unlikely(!page)) {
> > +		netdev_err(pfvf->netdev, "Allocation of page pool
> failed\n");
>=20
> No prints on allocation errors, please, it only adds stress to the system=
. You
> can add a statistic if you want.
>=20
> > +		return -ENOMEM;
> > +	}
> > +
> > +	*dma =3D page_pool_get_dma_addr(page) + offset;
> > +	return 0;
> > +}
>=20
> > +	pp_params.flags =3D PP_FLAG_PAGE_FRAG | PP_FLAG_DMA_MAP;
> > +	pp_params.pool_size =3D numptrs;
> > +	pp_params.nid =3D NUMA_NO_NODE;
> > +	pp_params.dev =3D pfvf->dev;
> > +	pp_params.dma_dir =3D DMA_FROM_DEVICE;
> > +	pool->page_pool =3D page_pool_create(&pp_params);
> > +	if (!pool->page_pool) {
> > +		netdev_err(pfvf->netdev, "Creation of page pool failed\n");
> > +		return -EFAULT;
>=20
> EFAULT =3D=3D "Bad address", doesn't sound right
Agree. Will fix to return correct err code.=20

>=20
> > +	}
> > +
> >  	return 0;
> >  }
> --
> pw-bot: cr