On Fri, Jul 19, 2024 at 2:37 AM Yunsheng Lin <linyunsheng@xxxxxxxxxx> wrote:
diff --git a/mm/page_frag_cache.c b/mm/page_frag_cache.c
index 609a485cd02a..2958fe006fe7 100644
--- a/mm/page_frag_cache.c
+++ b/mm/page_frag_cache.c
@@ -22,6 +22,7 @@
static struct page *__page_frag_cache_refill(struct page_frag_cache *nc,
gfp_t gfp_mask)
{
+ unsigned int page_size = PAGE_FRAG_CACHE_MAX_SIZE;
struct page *page = NULL;
gfp_t gfp = gfp_mask;
@@ -30,12 +31,21 @@ static struct page *__page_frag_cache_refill(struct page_frag_cache *nc,
__GFP_NOWARN | __GFP_NORETRY | __GFP_NOMEMALLOC;
page = alloc_pages_node(NUMA_NO_NODE, gfp_mask,
PAGE_FRAG_CACHE_MAX_ORDER);
- nc->size = page ? PAGE_FRAG_CACHE_MAX_SIZE : PAGE_SIZE;
#endif
- if (unlikely(!page))
+ if (unlikely(!page)) {
page = alloc_pages_node(NUMA_NO_NODE, gfp, 0);
+ if (unlikely(!page)) {
+ nc->va = NULL;
+ return NULL;
+ }
- nc->va = page ? page_address(page) : NULL;
+ page_size = PAGE_SIZE;
+ }
+
+#if (PAGE_SIZE < PAGE_FRAG_CACHE_MAX_SIZE)
+ nc->size = page_size;
+#endif
+ nc->va = page_address(page);
return page;
}
Not a huge fan of the changes here. If we are changing the direction
then just do that. I don't see the point of these changes. As far as I
can tell it is just adding noise to the diff and has no effect on the
final code as the outcome is mostly the same except for you don't
update size in the event that you overwrite nc->va to NULL.
@@ -64,8 +74,8 @@ void *__page_frag_alloc_align(struct page_frag_cache *nc,
unsigned int align_mask)
{
unsigned int size = PAGE_SIZE;
+ unsigned int remaining;
struct page *page;
- int offset;
if (unlikely(!nc->va)) {
refill:
@@ -82,35 +92,20 @@ void *__page_frag_alloc_align(struct page_frag_cache *nc,
*/
page_ref_add(page, PAGE_FRAG_CACHE_MAX_SIZE);
- /* reset page count bias and offset to start of new frag */
+ /* reset page count bias and remaining to start of new frag */
nc->pfmemalloc = page_is_pfmemalloc(page);
nc->pagecnt_bias = PAGE_FRAG_CACHE_MAX_SIZE + 1;
- nc->offset = size;
+ nc->remaining = size;
}
- offset = nc->offset - fragsz;
- if (unlikely(offset < 0)) {
- page = virt_to_page(nc->va);
-
- if (!page_ref_sub_and_test(page, nc->pagecnt_bias))
- goto refill;
-
- if (unlikely(nc->pfmemalloc)) {
- free_unref_page(page, compound_order(page));
- goto refill;
- }
-
#if (PAGE_SIZE < PAGE_FRAG_CACHE_MAX_SIZE)
- /* if size can vary use size else just use PAGE_SIZE */
- size = nc->size;
+ /* if size can vary use size else just use PAGE_SIZE */
+ size = nc->size;
#endif
Rather than pulling this out and placing it here it might make more
sense at the start of the function. Basically just overwrite size w/
either PAGE_SIZE or nc->size right at the start. Then if we have to
reallocate we overwrite it. That way we can avoid some redundancy and
this will be easier to read.
- /* OK, page count is 0, we can safely set it */
- set_page_count(page, PAGE_FRAG_CACHE_MAX_SIZE + 1);
- /* reset page count bias and offset to start of new frag */
- nc->pagecnt_bias = PAGE_FRAG_CACHE_MAX_SIZE + 1;
- offset = size - fragsz;
- if (unlikely(offset < 0)) {
+ remaining = nc->remaining & align_mask;
+ if (unlikely(remaining < fragsz)) {
+ if (unlikely(fragsz > PAGE_SIZE)) {
/*
* The caller is trying to allocate a fragment
* with fragsz > PAGE_SIZE but the cache isn't big
@@ -122,13 +117,31 @@ void *__page_frag_alloc_align(struct page_frag_cache *nc,
*/
return NULL;
}
+
+ page = virt_to_page(nc->va);
+
+ if (!page_ref_sub_and_test(page, nc->pagecnt_bias))
+ goto refill;
+
+ if (unlikely(nc->pfmemalloc)) {
+ free_unref_page(page, compound_order(page));
+ goto refill;
+ }
+
+ /* OK, page count is 0, we can safely set it */
+ set_page_count(page, PAGE_FRAG_CACHE_MAX_SIZE + 1);
+
+ /* reset page count bias and remaining to start of new frag */
+ nc->pagecnt_bias = PAGE_FRAG_CACHE_MAX_SIZE + 1;
+ nc->remaining = size;
Why are you setting nc->remaining here? You set it a few lines below.
This is redundant.
+
+ remaining = size;
}
nc->pagecnt_bias--;
- offset &= align_mask;
- nc->offset = offset;
+ nc->remaining = remaining - fragsz;
- return nc->va + offset;
+ return nc->va + (size - remaining);
}
EXPORT_SYMBOL(__page_frag_alloc_align);