• Tetsuo Handa's avatar
    drm/ttm: Avoid memory allocation from shrinker functions. · 881fdaa5
    Tetsuo Handa authored
    Andrew Morton wrote:
    > On Wed, 12 Nov 2014 13:08:55 +0900 Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> wrote:
    >
    > > Andrew Morton wrote:
    > > > Poor ttm guys - this is a bit of a trap we set for them.
    > >
    > > Commit a91576d7 ("drm/ttm: Pass GFP flags in order to avoid deadlock.")
    > > changed to use sc->gfp_mask rather than GFP_KERNEL.
    > >
    > > -       pages_to_free = kmalloc(npages_to_free * sizeof(struct page *),
    > > -                       GFP_KERNEL);
    > > +       pages_to_free = kmalloc(npages_to_free * sizeof(struct page *), gfp);
    > >
    > > But this bug is caused by sc->gfp_mask containing some flags which are not
    > > in GFP_KERNEL, right? Then, I think
    > >
    > > -       pages_to_free = kmalloc(npages_to_free * sizeof(struct page *), gfp);
    > > +       pages_to_free = kmalloc(npages_to_free * sizeof(struct page *), gfp & GFP_KERNEL);
    > >
    > > would hide this bug.
    > >
    > > But I think we should use GFP_ATOMIC (or drop __GFP_WAIT flag)
    >
    > Well no - ttm_page_pool_free() should stop calling kmalloc altogether.
    > Just do
    >
    > 	struct page *pages_to_free[16];
    >
    > and rework the code to free 16 pages at a time.  Easy.
    
    Well, ttm code wants to process 512 pages at a time for performance.
    Memory footprint increased by 512 * sizeof(struct page *) buffer is
    only 4096 bytes. What about using static buffer like below?
    ----------
    >From d3cb5393c9c8099d6b37e769f78c31af1541fe8c Mon Sep 17 00:00:00 2001
    From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
    Date: Thu, 13 Nov 2014 22:21:54 +0900
    Subject: [PATCH] drm/ttm: Avoid memory allocation from shrinker functions.
    
    Commit a91576d7 ("drm/ttm: Pass GFP flags in order to avoid
    deadlock.") caused BUG_ON() due to sc->gfp_mask containing flags
    which are not in GFP_KERNEL.
    
      https://bugzilla.kernel.org/show_bug.cgi?id=87891
    
    Changing from sc->gfp_mask to (sc->gfp_mask & GFP_KERNEL) would
    avoid the BUG_ON(), but avoiding memory allocation from shrinker
    function is better and reliable fix.
    
    Shrinker function is already serialized by global lock, and
    clean up function is called after shrinker function is unregistered.
    Thus, we can use static buffer when called from shrinker function
    and clean up function.
    Signed-off-by: default avatarTetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
    Cc: stable <stable@kernel.org> [2.6.35+]
    Signed-off-by: default avatarDave Airlie <airlied@redhat.com>
    881fdaa5
ttm_page_alloc.c 22.9 KB