On 1/16/25 00:44, Jakub Kicinski wrote:
On Wed, 8 Jan 2025 14:06:27 -0800 David Wei wrote:
From: Jakub Kicinski <kuba@xxxxxxxxxx>
A spin off from the original page pool memory providers patch by Jakub,
which allows extending page pools with custom allocators. One of such
providers is devmem TCP, and the other is io_uring zerocopy added in
following patches.
Suggested-by: Jakub Kicinski <kuba@xxxxxxxxxx>
Something odd with authorship here. You list me as author (From)
but didn't add my SoB. Maybe add something like "Based on
earlier work by Jakub" to the commit and reset the tags?
The intention was to change the author (failed) and put it as
suggested-by since you said before you don't care and changes pile
up, and even modification notes got deleted for unknown to me
reasons in v9. Based-on, Co-authored also sound good if you
have a preference.
Or the Suggested-by is just for the warning on ops not being built in?
Signed-off-by: Pavel Begunkov <asml.silence@xxxxxxxxx>
Signed-off-by: David Wei <dw@xxxxxxxxxxx>
---
include/net/page_pool/memory_provider.h | 20 ++++++++++++++++++++
include/net/page_pool/types.h | 4 ++++
net/core/devmem.c | 15 ++++++++++++++-
net/core/page_pool.c | 23 +++++++++++++++--------
4 files changed, 53 insertions(+), 9 deletions(-)
create mode 100644 include/net/page_pool/memory_provider.h
diff --git a/include/net/page_pool/memory_provider.h b/include/net/page_pool/memory_provider.h
new file mode 100644
index 000000000000..79412a8714fa
--- /dev/null
+++ b/include/net/page_pool/memory_provider.h
@@ -0,0 +1,20 @@
+/* SPDX-License-Identifier: GPL-2.0
+ *
+ * page_pool/memory_provider.h
+ * Author: Pavel Begunkov <asml.silence@xxxxxxxxx>
+ * Author: David Wei <dw@xxxxxxxxxxx>
Not a customary thing in networking to list authors in comments.
I'm not used to that, but _all_ networking files I quickly looked
through before adding that had authors listed including some new
ones. Maybe I was just too lucky. I can kill it.
+ */
+#ifndef _NET_PAGE_POOL_MEMORY_PROVIDER_H
+#define _NET_PAGE_POOL_MEMORY_PROVIDER_H
+
+#include <net/netmem.h>
+#include <net/page_pool/types.h>
No need? All you need is forward declarations for types at this stage.
That's getting extremely nit picky. I don't see why it's preferable
forward declaring all structures instead of including a "types.h"
file. Not like that matters or there is a dependency problem. Even
from your comments to v18, the list would likely need to grow with
an mp_params declaration.
+struct memory_provider_ops {
+ netmem_ref (*alloc_netmems)(struct page_pool *pool, gfp_t gfp);
+ bool (*release_netmem)(struct page_pool *pool, netmem_ref netmem);
+ int (*init)(struct page_pool *pool);
+ void (*destroy)(struct page_pool *pool);
+};
+
+#endif
Rest LGTM.
--
Pavel Begunkov
