mm: make page pfmemalloc check more robust

commit 2f064f3485cd29633ad1b3cfb00cc519509a3d72 upstream.

Commit c48a11c7ad26 ("netvm: propagate page->pfmemalloc to skb") added
checks for page->pfmemalloc to __skb_fill_page_desc():

        if (page->pfmemalloc && !page->mapping)
                skb->pfmemalloc = true;

It assumes page->mapping == NULL implies that page->pfmemalloc can be
trusted.  However, __delete_from_page_cache() can set set page->mapping
to NULL and leave page->index value alone.  Due to being in union, a
non-zero page->index will be interpreted as true page->pfmemalloc.

So the assumption is invalid if the networking code can see such a page.
And it seems it can.  We have encountered this with a NFS over loopback
setup when such a page is attached to a new skbuf.  There is no copying
going on in this case so the page confuses __skb_fill_page_desc which
interprets the index as pfmemalloc flag and the network stack drops
packets that have been allocated using the reserves unless they are to
be queued on sockets handling the swapping which is the case here and
that leads to hangs when the nfs client waits for a response from the
server which has been dropped and thus never arrive.

The struct page is already heavily packed so rather than finding another
hole to put it in, let's do a trick instead.  We can reuse the index
again but define it to an impossible value (-1UL).  This is the page
index so it should never see the value that large.  Replace all direct
users of page->pfmemalloc by page_is_pfmemalloc which will hide this
nastiness from unspoiled eyes.

The information will get lost if somebody wants to use page->index
obviously but that was the case before and the original code expected
that the information should be persisted somewhere else if that is
really needed (e.g.  what SLAB and SLUB do).

[akpm@linux-foundation.org: fix blooper in slub]
Fixes: c48a11c7ad26 ("netvm: propagate page->pfmemalloc to skb")
Signed-off-by: Michal Hocko <mhocko@suse.com>
Debugged-by: Vlastimil Babka <vbabka@suse.com>
Debugged-by: Jiri Bohac <jbohac@suse.com>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: David Miller <davem@davemloft.net>
Acked-by: Mel Gorman <mgorman@suse.de>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_main.c b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
index c754b2027281f8a2c0b18c079b31c2b7420eedbf..c9da1b5d4804d8d5b65e9f000311681a9de83a38 100644 (file)
--- a/drivers/net/ethernet/intel/fm10k/fm10k_main.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
@@ -216,7 +216,7 @@ static void fm10k_reuse_rx_page(struct fm10k_ring *rx_ring,
 
 static inline bool fm10k_page_is_reserved(struct page *page)
 {
-       return (page_to_nid(page) != numa_mem_id()) || page->pfmemalloc;
+       return (page_to_nid(page) != numa_mem_id()) || page_is_pfmemalloc(page);
 }
 
 static bool fm10k_can_reuse_rx_page(struct fm10k_rx_buffer *rx_buffer,

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index ec79e6461f568fda89552fffe5f16bbc41a052d4..4f6bf996851ec6677811b0c443a7ef744513b4d3 100644 (file)
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -6596,7 +6596,7 @@ static void igb_reuse_rx_page(struct igb_ring *rx_ring,
 
 static inline bool igb_page_is_reserved(struct page *page)
 {
-       return (page_to_nid(page) != numa_mem_id()) || page->pfmemalloc;
+       return (page_to_nid(page) != numa_mem_id()) || page_is_pfmemalloc(page);
 }
 
 static bool igb_can_reuse_rx_page(struct igb_rx_buffer *rx_buffer,

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 5be12a00e1f447744f2497131cea1a70e313fd1f..463ff47200f1ab63ccffc5fc40b9c1db7811d5da 100644 (file)
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -1829,7 +1829,7 @@ static void ixgbe_reuse_rx_page(struct ixgbe_ring *rx_ring,
 
 static inline bool ixgbe_page_is_reserved(struct page *page)
 {
-       return (page_to_nid(page) != numa_mem_id()) || page->pfmemalloc;
+       return (page_to_nid(page) != numa_mem_id()) || page_is_pfmemalloc(page);
 }
 
 /**

diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index e71cdde9cb017aecab834d2f2d9c5d4821c3d42e..1d7b00b038a2ea8c3bdd9394ad3d4988ccee04c8 100644 (file)
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -765,7 +765,7 @@ static void ixgbevf_reuse_rx_page(struct ixgbevf_ring *rx_ring,
 
 static inline bool ixgbevf_page_is_reserved(struct page *page)
 {
-       return (page_to_nid(page) != numa_mem_id()) || page->pfmemalloc;
+       return (page_to_nid(page) != numa_mem_id()) || page_is_pfmemalloc(page);
 }
 
 /**

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 0755b9fd03a7d936e805efb71cfc1d371ddea0d6..b2085582d44e338f8c255723b570d6bf0d6542bd 100644 (file)
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1001,6 +1001,34 @@ static inline int page_mapped(struct page *page)
        return atomic_read(&(page)->_mapcount) >= 0;
 }
 
+/*
+ * Return true only if the page has been allocated with
+ * ALLOC_NO_WATERMARKS and the low watermark was not
+ * met implying that the system is under some pressure.
+ */
+static inline bool page_is_pfmemalloc(struct page *page)
+{
+       /*
+        * Page index cannot be this large so this must be
+        * a pfmemalloc page.
+        */
+       return page->index == -1UL;
+}
+
+/*
+ * Only to be called by the page allocator on a freshly allocated
+ * page.
+ */
+static inline void set_page_pfmemalloc(struct page *page)
+{
+       page->index = -1UL;
+}
+
+static inline void clear_page_pfmemalloc(struct page *page)
+{
+       page->index = 0;
+}
+
 /*
  * Different kinds of faults, as returned by handle_mm_fault().
  * Used to decide whether a process gets delivered SIGBUS or

diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 8d37e26a1007c6493990093e7ce8a03f2b201419..c0c6b33535fb1b165e067ac6b251dcc7dbd481da 100644 (file)
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -63,15 +63,6 @@ struct page {
                union {
                        pgoff_t index;          /* Our offset within mapping. */
                        void *freelist;         /* sl[aou]b first free object */
-                       bool pfmemalloc;        /* If set by the page allocator,
-                                                * ALLOC_NO_WATERMARKS was set
-                                                * and the low watermark was not
-                                                * met implying that the system
-                                                * is under some pressure. The
-                                                * caller should try ensure
-                                                * this page is only used to
-                                                * free other pages.
-                                                */
                };
 
                union {

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index f15154a879c711870ba649f867fc6eeb47212f14..eb1c55b8255a1367377b2fa254443a06e131f8a4 100644 (file)
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -1590,20 +1590,16 @@ static inline void __skb_fill_page_desc(struct sk_buff *skb, int i,
        skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
 
        /*
-        * Propagate page->pfmemalloc to the skb if we can. The problem is
-        * that not all callers have unique ownership of the page. If
-        * pfmemalloc is set, we check the mapping as a mapping implies
-        * page->index is set (index and pfmemalloc share space).
-        * If it's a valid mapping, we cannot use page->pfmemalloc but we
-        * do not lose pfmemalloc information as the pages would not be
-        * allocated using __GFP_MEMALLOC.
+        * Propagate page pfmemalloc to the skb if we can. The problem is
+        * that not all callers have unique ownership of the page but rely
+        * on page_is_pfmemalloc doing the right thing(tm).
         */
        frag->page.p              = page;
        frag->page_offset         = off;
        skb_frag_size_set(frag, size);
 
        page = compound_head(page);
-       if (page->pfmemalloc && !page->mapping)
+       if (page_is_pfmemalloc(page))
                skb->pfmemalloc = true;
 }
 
@@ -2250,7 +2246,7 @@ static inline struct page *dev_alloc_page(void)
 static inline void skb_propagate_pfmemalloc(struct page *page,
                                             struct sk_buff *skb)
 {
-       if (page && page->pfmemalloc)
+       if (page_is_pfmemalloc(page))
                skb->pfmemalloc = true;
 }
 

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index ebffa0e4a9c0451cfbd78bcf4835825ffd72c6de..18490f3bd7f1f0fcbfd0bf8fb5d90fd0597f11ca 100644 (file)
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -983,12 +983,15 @@ static int prep_new_page(struct page *page, unsigned int order, gfp_t gfp_flags,
        set_page_owner(page, order, gfp_flags);
 
        /*
-        * page->pfmemalloc is set when ALLOC_NO_WATERMARKS was necessary to
+        * page is set pfmemalloc when ALLOC_NO_WATERMARKS was necessary to
         * allocate the page. The expectation is that the caller is taking
         * steps that will free more memory. The caller should avoid the page
         * being used for !PFMEMALLOC purposes.
         */
-       page->pfmemalloc = !!(alloc_flags & ALLOC_NO_WATERMARKS);
+       if (alloc_flags & ALLOC_NO_WATERMARKS)
+               set_page_pfmemalloc(page);
+       else
+               clear_page_pfmemalloc(page);
 
        return 0;
 }

diff --git a/mm/slab.c b/mm/slab.c
index 7eb38dd1cefa2f988be6297c1b75b12369e05827..3dd2d1ff9d5d915469ce2417050af5cac0f23310 100644 (file)
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -1602,7 +1602,7 @@ static struct page *kmem_getpages(struct kmem_cache *cachep, gfp_t flags,
        }
 
        /* Record if ALLOC_NO_WATERMARKS was set when allocating the slab */
-       if (unlikely(page->pfmemalloc))
+       if (page_is_pfmemalloc(page))
                pfmemalloc_active = true;
 
        nr_pages = (1 << cachep->gfporder);
@@ -1613,7 +1613,7 @@ static struct page *kmem_getpages(struct kmem_cache *cachep, gfp_t flags,
                add_zone_page_state(page_zone(page),
                        NR_SLAB_UNRECLAIMABLE, nr_pages);
        __SetPageSlab(page);
-       if (page->pfmemalloc)
+       if (page_is_pfmemalloc(page))
                SetPageSlabPfmemalloc(page);
 
        if (kmemcheck_enabled && !(cachep->flags & SLAB_NOTRACK)) {

diff --git a/mm/slub.c b/mm/slub.c
index 54c0876b43d554e4e1d8981e08007a8a3cd0c59a..08342c523a85b0e61887f104260a6e254787f222 100644 (file)
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1427,7 +1427,7 @@ static struct page *new_slab(struct kmem_cache *s, gfp_t flags, int node)
        inc_slabs_node(s, page_to_nid(page), page->objects);
        page->slab_cache = s;
        __SetPageSlab(page);
-       if (page->pfmemalloc)
+       if (page_is_pfmemalloc(page))
                SetPageSlabPfmemalloc(page);
 
        start = page_address(page);

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 41ec02242ea7c2ff57a6b506b685df22c62f3dcc..a2e4e47b28391356e013897d959117039a994ad5 100644 (file)
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -340,7 +340,7 @@ struct sk_buff *build_skb(void *data, unsigned int frag_size)
 
        if (skb && frag_size) {
                skb->head_frag = 1;
-               if (virt_to_head_page(data)->pfmemalloc)
+               if (page_is_pfmemalloc(virt_to_head_page(data)))
                        skb->pfmemalloc = 1;
        }
        return skb;