Received-SPF: pass (google.com: domain of linux-kernel+bounces-137739-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <1353da1d-4f48-434f-9582-cb89524f869f@intel.com>
Date: Tue, 9 Apr 2024 16:58:17 -0700
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH net-next v5 2/3] net: mirror skb frag ref/unref helpers
To: Mina Almasry <almasrymina@google.com>, <netdev@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>
CC: Ayush Sawal <ayush.sawal@chelsio.com>, "David S. Miller"
	<davem@davemloft.net>, Eric Dumazet <edumazet@google.com>, Jakub Kicinski
	<kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>, Jesper Dangaard Brouer
	<hawk@kernel.org>, Ilias Apalodimas <ilias.apalodimas@linaro.org>, "Steffen
 Klassert" <steffen.klassert@secunet.com>, Herbert Xu
	<herbert@gondor.apana.org.au>, David Ahern <dsahern@kernel.org>, "Boris
 Pismenny" <borisp@nvidia.com>, John Fastabend <john.fastabend@gmail.com>,
	Dragos Tatulea <dtatulea@nvidia.com>
References: <20240408153000.2152844-1-almasrymina@google.com>
 <20240408153000.2152844-3-almasrymina@google.com>
Content-Language: en-US
From: Jacob Keller <jacob.e.keller@intel.com>
In-Reply-To: <20240408153000.2152844-3-almasrymina@google.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?SEdyb3NPaWl1clY2K3RvQTZLZFphbDVRTHRNSkhJc1lORXdLTmEza0t5bnhN?=
 =?utf-8?B?NFlRWUt6dTUyWTh1YmFLcnRabkloUjJjclMvcU9wdlBpanJadXduazMwNE92?=
 =?utf-8?B?K3dIcjhaOXI5cGpoS0F4N2hoV3dYTXY0Nld5NkxzOVU1cGxrMGpQT0VmcTRU?=
 =?utf-8?B?NVhtVjBmczBxSThWQys2LzI5RDlkM3dqSkZUSzJ4ZXQ1RUdTN2p4QnpaQUNa?=
 =?utf-8?B?Tjd3cWdmSGhmUm1mUWxNdGdDQSs5WmNhM0pYcTlFUDVQa1RqYnFDb2JIUklS?=
 =?utf-8?B?bkhQVG1ocHlhYnZTbXJtNTN5UjhPR2N3Nk8xNkp0TlBFL1phOVlKaDlJT2VB?=
 =?utf-8?B?bGtpYWRyUDdETnpZR2MvcHJsN3BlQkRrL2xadysyQWpLdys0L20vWEljQm9Y?=
 =?utf-8?B?ZWxtWVkzalVheDNlb3hsR3BCK0JTQVZrVGRyMm9lZmVJMjVsZUtZem9JaExW?=
 =?utf-8?B?U3BpYjBtY1Vra255bHdJcHp0dlo4c2x0VWx3UFE3bnA5SlJpNFpkWWYzUVVV?=
 =?utf-8?B?TlR2THVVd1IvU0Y5Rk11NGtMWk9VWk96ZHNhYzk4RTEvZ0RhZkNZc2pOVm5r?=
 =?utf-8?B?dW1qeUhkWUFKYzFYczJOTkJkc3kvam1Na2FyNXdnSGZmcVBvL0hpTlNaZDBh?=
 =?utf-8?B?WVVrNkhlaks4cmRFWWc3clJHaHNNSTJ1Tm8zZStvMXhmdTZFOCt4dk5hRThN?=
 =?utf-8?B?Z2llVURBMnpoUlNhN3ZuK0ZQQnZ4QldZZjRUQXExcXM0bmJySWtOTncxSXVz?=
 =?utf-8?B?eE5QL3VvY1BtWE5xUXdCeWdKaXpKRDc3SFRVRG5zQXk2MjVqQ3gzKzZnMTJs?=
 =?utf-8?B?akhzZFJqQUdraWRKWXlIWUhBMGtUNTJ5aFpLTzdSQ0JWKzBBdXNBR21MVldV?=
 =?utf-8?B?UzJYZm9tL2dxMEJ2RWFybEpzRTZnRlR3V3RxK1hHU0c3Rk5BNGFrZlNNbjBK?=
 =?utf-8?B?NTNPMm5HRzdUY3o3RFlVTlFZMmxrNjcvQnZBM2RidG4zN2cxZjR3VExYWWFm?=
 =?utf-8?B?ZGE5UkRjUFMvYU5JSk9aTEE5cXZ5SmpmQXhjdFRtQjQ2R05aMStjdGk4dS9R?=
 =?utf-8?B?UXlENW1hemI1Z29OSU1ZMytZdnBrM2VtTnhseW41YVZacnNENmhXaGJLRXp6?=
 =?utf-8?B?RExDS0FPT2NZVjBuTHR0V05kQmVqdVhIcFpNb2Z4NEVSZlB1M3R5TTE1SDNC?=
 =?utf-8?B?SmpiRGJka0NpTlZjd3dpZS9jenBoNnljcUJ5V3IyVGxBSXAxZnhQSGlHOUho?=
 =?utf-8?B?OC9OdFZ4NzVwV0RIdE8ydHh4SmZMRGdtbG9Jc2xyYllRRUpBU201c1lwOVNR?=
 =?utf-8?B?dmZzQlVZbnpWY2xJM24vYTRVYVBMeFVsNUx4VzcyWTFZbkZNSVhUNDV5ZllK?=
 =?utf-8?B?SVJqZWpMcDBRaDBvZ1VrcXd6amNQTmlHS3AxY014Q29ISFpvSXA1cGhsNk9S?=
 =?utf-8?B?UmhZTThJdFFPSDU0cDExMXNJc1ZqR2RPeUxWSXB1OFl5NjdlUFVMbkUrWkhD?=
 =?utf-8?B?bmFDcDI0em9PVWRrVmVXeTZkQjhFTDg5UTI4MlFaOThtQ0FxYnFJTFZ3bks4?=
 =?utf-8?B?OWpTdVY5UFVNTjJXTjRmdW42TVNBNlV2SzZCTWZxOEdkQUl6S1Vlem53aS9k?=
 =?utf-8?B?eWpGdHo1cEg0c1NHTkgzNml5cndhbUtiWkJKWktrU0tZSU1LL3RJRmhnYktL?=
 =?utf-8?B?L0RBMGVvV2IvbnY4T21YNG8wVEwzUGxyY3VMVUJNa0laZVFsNVlaQ2FrNzM5?=
 =?utf-8?B?UjM3YU9JNjVpVmpBT2gvb3RKQlRSZkxTOVRyQ1NvNXpmTFpkOEpEb2ROWC9t?=
 =?utf-8?B?SUZrcERKSGFNTmwrRXdoYmVteUYvRVJNbGNLb05NTzZuZ0EzRDJwV25yaU81?=
 =?utf-8?B?a1R5YnBtK0p0SVQ1SStndTdZZzhIcEd1YlhLR2wrOXpWbDlpU0NFc2s2YXdI?=
 =?utf-8?B?Rlg0Syt6VUFIK055bkZpS0tLVUhjTG5Ia1NUVHViU0NSUzBCalgwWXdWUGt5?=
 =?utf-8?B?cjNWY3lLRVZobXlOR0FTam1ZKy9rTk5xUThVSFpHd1d1WnhTQ3p4ZHp0VUNY?=
 =?utf-8?B?NzkzMjVPT29IaUUxZDVnT0VZZFZQKy9pdlZCam43aG5JTmkreGhUQ3ZxQ3pi?=
 =?utf-8?B?aURLYkU2S2RvbmZNZUlJN21UWkdmeTV0OFdhNEZKbnlIYWdGdXBDUTB1TkxR?=
 =?utf-8?B?WUE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: cc277460-bd49-47c7-6b41-08dc58f0ee88
X-MS-Exchange-CrossTenant-AuthSource: CO1PR11MB5089.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Apr 2024 23:58:19.7852
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: JGoxStNdw/at8HEmW02GUPZsZIhLz6o56LyQ3uMe3epoXbJdZno1erkkr1uy9rk9Ipn7Qt6UPe8b6lJTvPO9i4b51KczJuHzU4k6NUU5VOM=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN7PR11MB6945
X-OriginatorOrg: intel.com


On 4/8/2024 8:29 AM, Mina Almasry wrote:
> Refactor some of the skb frag ref/unref helpers for improved clarity.
> 
> Implement napi_pp_get_page() to be the mirror counterpart of
> napi_pp_put_page().
> 
> Implement skb_page_ref() to be the mirror of skb_page_unref().
> 
> Improve __skb_frag_ref() to become a mirror counterpart of
> __skb_frag_unref(). Previously unref could handle pp & non-pp pages,
> while the ref could only handle non-pp pages. Now both the ref & unref
> helpers can correctly handle both pp & non-pp pages.
> 
> Now that __skb_frag_ref() can handle both pp & non-pp pages, remove
> skb_pp_frag_ref(), and use __skb_frag_ref() instead.  This lets us
> remove pp specific handling from skb_try_coalesce.
> 
> Additionally, since __skb_frag_ref() can now handle both pp & non-pp
> pages, a latent issue in skb_shift() should now be fixed. Previously
> this function would do a non-pp ref & pp unref on potential pp frags
> (fragfrom). After this patch, skb_shift() should correctly do a pp
> ref/unref on pp frags.
> 

The description sounds like a lot is going on here, and at first I was
thinking this should be split further. However I think that would just
end up with a lot of needless thrash.

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>