On Fri, Aug 26, 2022 at 7:49 PM Vlastimil Babka (SUSE)
<vbabka@xxxxxxxxxx> wrote:
>
> On 8/26/22 08:40, zhaoyang.huang wrote:
> > From: Zhaoyang Huang <zhaoyang.huang@xxxxxxxxxx>
> >
> > Kthread and drivers could fetch memory via alloc_pages directly which make them
> > hard to debug when leaking. Solve this by introducing __GFP_TRACELEAK and reuse
> > kmemleak mechanism which unified most of kernel cosuming pages into kmemleak.
>
> Can you expand how exactly this is expected to be used? So you first have to
> suspect some concrete allocation and add __GFP_TRACKLEAK to it?
It could be used to investigate suspicious drivers or kthread which
may introduce in-kernel leak during debug phase by enabling kmemleak.
This can be done via page owner things now. I would like to unified it
together with slab leak into kmemleak.
>
> > Signed-off-by: Zhaoyang Huang <zhaoyang.huang@xxxxxxxxxx>
> > ---
> > include/linux/gfp.h | 4 +++-
> > include/linux/page-flags.h | 3 +++
> > mm/kmemleak.c | 2 +-
> > mm/page_alloc.c | 6 ++++++
> > 4 files changed, 13 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/linux/gfp.h b/include/linux/gfp.h
> > index 2d2ccae..081ab54 100644
> > --- a/include/linux/gfp.h
> > +++ b/include/linux/gfp.h
> > @@ -68,6 +68,7 @@
> > #else
> > #define ___GFP_NOLOCKDEP 0
> > #endif
> > +#define ___GFP_TRACKLEAK 0x10000000u
> > /* If the above are modified, __GFP_BITS_SHIFT may need updating */
> >
> > /*
> > @@ -259,12 +260,13 @@
> > #define __GFP_SKIP_ZERO ((__force gfp_t)___GFP_SKIP_ZERO)
> > #define __GFP_SKIP_KASAN_UNPOISON ((__force gfp_t)___GFP_SKIP_KASAN_UNPOISON)
> > #define __GFP_SKIP_KASAN_POISON ((__force gfp_t)___GFP_SKIP_KASAN_POISON)
> > +#define __GFP_TRACKLEAK ((__force gfp_t)___GFP_TRACKLEAK)
> >
> > /* Disable lockdep for GFP context tracking */
> > #define __GFP_NOLOCKDEP ((__force gfp_t)___GFP_NOLOCKDEP)
> >
> > /* Room for N __GFP_FOO bits */
> > -#define __GFP_BITS_SHIFT (27 + IS_ENABLED(CONFIG_LOCKDEP))
> > +#define __GFP_BITS_SHIFT (28 + IS_ENABLED(CONFIG_LOCKDEP))
> > #define __GFP_BITS_MASK ((__force gfp_t)((1 << __GFP_BITS_SHIFT) - 1))
> >
> > /**
> > diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> > index e66f7aa..ef0f814 100644
> > --- a/include/linux/page-flags.h
> > +++ b/include/linux/page-flags.h
> > @@ -942,6 +942,7 @@ static inline bool is_page_hwpoison(struct page *page)
> > #define PG_offline 0x00000100
> > #define PG_table 0x00000200
> > #define PG_guard 0x00000400
> > +#define PG_trackleak 0x00000800
> >
> > #define PageType(page, flag) \
> > ((page->page_type & (PAGE_TYPE_BASE | flag)) == PAGE_TYPE_BASE)
> > @@ -1012,6 +1013,8 @@ static inline int page_has_type(struct page *page)
> > */
> > PAGE_TYPE_OPS(Guard, guard)
> >
> > +PAGE_TYPE_OPS(Trackleak, trackleak)
> > +
> > extern bool is_free_buddy_page(struct page *page);
> >
> > PAGEFLAG(Isolated, isolated, PF_ANY);
> > diff --git a/mm/kmemleak.c b/mm/kmemleak.c
> > index 422f28f..a182f5d 100644
> > --- a/mm/kmemleak.c
> > +++ b/mm/kmemleak.c
> > @@ -1471,7 +1471,7 @@ static void kmemleak_scan(void)
> > if (page_zone(page) != zone)
> > continue;
> > /* only scan if page is in use */
> > - if (page_count(page) == 0 || PageReserved(page))
> > + if (page_count(page) == 0)
> > continue;
> > scan_block(page, page + 1, NULL);
> > if (!(pfn & 63))
> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > index e008a3d..d8995c6 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -1361,6 +1361,8 @@ static __always_inline bool free_pages_prepare(struct page *page,
> > page->mapping = NULL;
> > if (memcg_kmem_enabled() && PageMemcgKmem(page))
> > __memcg_kmem_uncharge_page(page, order);
> > + if (PageTrackleak(page))
> > + kmemleak_free(page);
> > if (check_free)
> > bad += check_free_page(page);
> > if (bad)
> > @@ -5444,6 +5446,10 @@ struct page *__alloc_pages(gfp_t gfp, unsigned int order, int preferred_nid,
> > __free_pages(page, order);
> > page = NULL;
> > }
> > + if (gfp & __GFP_TRACKLEAK) {
> > + kmemleak_alloc(page_address(page), PAGE_SIZE << order, 1, gfp & ~__GFP_TRACKLEAK);
> > + __SetPageTrackleak(page);
> > + }
> >
> > trace_mm_page_alloc(page, order, alloc_gfp, ac.migratetype);
> >
>
