On Tue, Aug 27, 2024 at 5:06 AM Yunsheng Lin <linyunsheng@xxxxxxxxxx> wrote: > > On 2024/8/27 0:46, Alexander Duyck wrote: > > On Mon, Aug 26, 2024 at 5:46 AM Yunsheng Lin <linyunsheng@xxxxxxxxxx> wrote: > >> > >> Currently there is one 'struct page_frag' for every 'struct > >> sock' and 'struct task_struct', we are about to replace the > >> 'struct page_frag' with 'struct page_frag_cache' for them. > >> Before begin the replacing, we need to ensure the size of > >> 'struct page_frag_cache' is not bigger than the size of > >> 'struct page_frag', as there may be tens of thousands of > >> 'struct sock' and 'struct task_struct' instances in the > >> system. > >> > >> By or'ing the page order & pfmemalloc with lower bits of > >> 'va' instead of using 'u16' or 'u32' for page size and 'u8' > >> for pfmemalloc, we are able to avoid 3 or 5 bytes space waste. > >> And page address & pfmemalloc & order is unchanged for the > >> same page in the same 'page_frag_cache' instance, it makes > >> sense to fit them together. > >> > >> After this patch, the size of 'struct page_frag_cache' should be > >> the same as the size of 'struct page_frag'. > >> > >> CC: Alexander Duyck <alexander.duyck@xxxxxxxxx> > >> Signed-off-by: Yunsheng Lin <linyunsheng@xxxxxxxxxx> > >> --- > >> include/linux/mm_types_task.h | 19 ++++++----- > >> include/linux/page_frag_cache.h | 60 +++++++++++++++++++++++++++++++-- > >> mm/page_frag_cache.c | 51 +++++++++++++++------------- > >> 3 files changed, 97 insertions(+), 33 deletions(-) > >> ... > >> void page_frag_cache_drain(struct page_frag_cache *nc); > > > > So how many of these additions are actually needed outside of the > > page_frag_cache.c file? I'm just wondering if we could look at moving > > At least page_frag_cache_is_pfmemalloc(), page_frag_encoded_page_order(), > page_frag_encoded_page_ptr(), page_frag_encoded_page_address() are needed > out of the page_frag_cache.c file for now, which are used mostly in > __page_frag_cache_commit() and __page_frag_alloc_refill_probe_align() for > debugging and performance reason, see patch 7 & 10. As far as the __page_frag_cache_commit I might say that could be moved to page_frag_cache.c, but admittedly I don't know how much that would impact the performance. > The only left one is page_frag_encode_page(), I am not sure if it makes > much sense to move it to page_frag_cache.c while the rest of them are in > .h file. I would move it. There is no point in exposing internals more than necessary. Also since you are carrying a BUILD_BUG_ON it would make sense to keep that internal to your implementation.
