On Thu, Mar 22, 2018 at 8:31 AM, Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote:
> From: Matthew Wilcox <mawilcox@xxxxxxxxxxxxx>
>
> Save 4/8 bytes by moving the pfmemalloc indicator from its own bool
> to the top bit of pagecnt_bias. This has no effect on the fastpath
> of the allocator since the pagecnt_bias cannot go negative. It's
> a couple of extra instructions in the slowpath.
>
> Signed-off-by: Matthew Wilcox <mawilcox@xxxxxxxxxxxxx>
So I was just thinking about this and it would probably make more
sense to look at addressing this after you take care of your
conversion from size/offset to a mask. One thing with the mask is that
it should never reach 64K since that is the largest page size if I
recall. With that being the case we could look at dropping mask to a
u16 value and then add a u16 flags field where you could store things
like this. Then you could avoid having to do the masking and math you
are having to do below.
> ---
> include/linux/mm_types.h | 4 +++-
> mm/page_alloc.c | 8 +++++---
> net/core/skbuff.c | 5 ++---
> 3 files changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index fd1af6b9591d..a63b138ad1a4 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -218,6 +218,7 @@ struct page {
>
> #define PAGE_FRAG_CACHE_MAX_SIZE __ALIGN_MASK(32768, ~PAGE_MASK)
> #define PAGE_FRAG_CACHE_MAX_ORDER get_order(PAGE_FRAG_CACHE_MAX_SIZE)
> +#define PFC_MEMALLOC (1U << 31)
>
> struct page_frag_cache {
> void * va;
> @@ -231,9 +232,10 @@ struct page_frag_cache {
> * containing page->_refcount every time we allocate a fragment.
> */
> unsigned int pagecnt_bias;
> - bool pfmemalloc;
> };
>
> +#define page_frag_cache_pfmemalloc(pfc) ((pfc)->pagecnt_bias & PFC_MEMALLOC)
> +
> typedef unsigned long vm_flags_t;
>
> /*
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 635d7dd29d7f..61366f23e8c8 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -4395,16 +4395,18 @@ void *page_frag_alloc(struct page_frag_cache *nc,
> page_ref_add(page, size - 1);
>
> /* reset page count bias and offset to start of new frag */
> - nc->pfmemalloc = page_is_pfmemalloc(page);
> nc->pagecnt_bias = size;
> + if (page_is_pfmemalloc(page))
> + nc->pagecnt_bias |= PFC_MEMALLOC;
> nc->offset = size;
> }
>
> offset = nc->offset - fragsz;
> if (unlikely(offset < 0)) {
> + unsigned int pagecnt_bias = nc->pagecnt_bias & ~PFC_MEMALLOC;
> page = virt_to_page(nc->va);
>
> - if (!page_ref_sub_and_test(page, nc->pagecnt_bias))
> + if (!page_ref_sub_and_test(page, pagecnt_bias))
> goto refill;
>
> #if (PAGE_SIZE < PAGE_FRAG_CACHE_MAX_SIZE)
> @@ -4415,7 +4417,7 @@ void *page_frag_alloc(struct page_frag_cache *nc,
> set_page_count(page, size);
>
> /* reset page count bias and offset to start of new frag */
> - nc->pagecnt_bias = size;
> + nc->pagecnt_bias = size | (nc->pagecnt_bias - pagecnt_bias);
> offset = size - fragsz;
> }
>
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 0bb0d8877954..54bbde8f7541 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -412,7 +412,7 @@ struct sk_buff *__netdev_alloc_skb(struct net_device *dev, unsigned int len,
>
> nc = this_cpu_ptr(&netdev_alloc_cache);
> data = page_frag_alloc(nc, len, gfp_mask);
> - pfmemalloc = nc->pfmemalloc;
> + pfmemalloc = page_frag_cache_pfmemalloc(nc);
>
> local_irq_restore(flags);
>
> @@ -485,8 +485,7 @@ struct sk_buff *__napi_alloc_skb(struct napi_struct *napi, unsigned int len,
> return NULL;
> }
>
> - /* use OR instead of assignment to avoid clearing of bits in mask */
> - if (nc->page.pfmemalloc)
> + if (page_frag_cache_pfmemalloc(&nc->page))
> skb->pfmemalloc = 1;
> skb->head_frag = 1;
>
> --
> 2.16.2
>
