linux-kernel - Re: [Intel-gfx] [PATCH] drm/i915: Selectively enable self-reclaim

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <AANLkTilKezSkoNKYxaKlsC7lI-RTHlkE4qVcE5eER-WE@mail.gmail.com>
Date:	Thu, 1 Jul 2010 20:15:16 +1000
From:	Dave Airlie <airlied@...il.com>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Chris Wilson <chris@...is-wilson.co.uk>, earny@...4u.de,
	Roman Jarosz <kedgedev@...il.com>,
	intel-gfx@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
	jcnengel@...glemail.com,
	"A. Boulan" <arnaud.boulan@...ertysurf.fr>,
	Hugh Dickins <hugh.dickins@...cali.co.uk>,
	Pekka Enberg <penberg@...helsinki.fi>,
	A Rojas <nqn1976list@...il.com>,
	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
	rientjes@...gle.com, michael@...nelt.co.at, stable@...nel.org,
	Vefa Bicakci <bicave@...eronline.com>
Subject: Re: [Intel-gfx] [PATCH] drm/i915: Selectively enable self-reclaim

On Thu, Jul 1, 2010 at 11:24 AM, Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
> On Wed, Jun 30, 2010 at 4:07 PM, Linus Torvalds
> <torvalds@...ux-foundation.org> wrote:
>>
>> That commit changes the page cache allocation to use
>>
>> +                                          mapping_gfp_mask (mapping) |
>> +                                          __GFP_COLD |
>> +                                          gfpmask);
>>
>> if I read it right. And the default mapping_gfp_mask() is
>> GFP_HIGHUSER_MOVABLE, so I think you get all of
>> (__GFP_WAIT | __GFP_IO | __GFP_FS | __GFP_HARDWALL | __GFP_HIGHMEM)
>> set by default.
>
> .. and then I left out the one flag I _meant_ to have there, namely
> __GFP_MOVABLE.
>
>> The old code didn't just play games with ~__GFP_NORETRY and change
>> that at runtime (which was buggy - no locking, no protection, no
>> nothing), it also initialized the gfp mask. And that code also got
>> removed:
>
> In fact, I don't really see why we should use that mapping_gfp_mask()
> at all, since all allocations should be going through that
> i915_gem_object_get_pages() function anyway. So why not just change
> that function to ignore the default gfp mask for the mapping, and just
> use the mask that the o915 driver wants?
>
> Btw, why did it want to mark the pages reclaimable?
>
> Anyway, what I'm suggesting somebody who sees this test is just
> something like the patch below (whitespace-damage - I'm cutting and
> pasting, it's a trivial one-liner).  Does this change any behavior?
> Vefa?
>

I think Linus is on to something, I'll finish my testing tomorrow,

I'm stuck testing this on a laptop with a 4200rpm driver, hibernating
takes quite a long time ;-(

But I have reproduced the initial failure,reverted the patch
reproduced success, and then did a couple of cycles with Linus patch
before I left.

Tomorrow I'll do another 3-4 cycles to confirm.

the patch also needs a couple of __ before GFP_HIGHMEM, in  case
anyone else was hacking it.

Dave.


>                    Linus
>
> ---
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 9ded3da..ec8ed6b 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2239,7 +2239,7 @@ i915_gem_object_get_pages(struct drm_gem_object *obj,
>        mapping = inode->i_mapping;
>        for (i = 0; i < page_count; i++) {
>                page = read_cache_page_gfp(mapping, i,
> -                                          mapping_gfp_mask (mapping) |
> +                                          GFP_HIGHMEM |
>                                           __GFP_COLD |
>                                           gfpmask);
>                if (IS_ERR(page))
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/