Re: [Intel-gfx] [PATCH v2 1/2] mm: Export nr_swap_pages

From: Daniel Vetter <daniel@ffwll.ch>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Johannes Weiner <hannes@cmpxchg.org>,
	Daniel Vetter <daniel@ffwll.ch>,
	Chris Wilson <chris@chris-wilson.co.uk>,
	linux-mm@kvack.org, intel-gfx@lists.freedesktop.org, "Goel,
	Akash" <akash.goel@intel.com>
Subject: Re: [Intel-gfx] [PATCH v2 1/2] mm: Export nr_swap_pages
Date: Tue, 5 Jan 2016 11:05:27 +0100	[thread overview]
Message-ID: <20160105100527.GO8076@phenom.ffwll.local> (raw)
In-Reply-To: <20151223142611.63907890.akpm@linux-foundation.org>

On Wed, Dec 23, 2015 at 02:26:11PM -0800, Andrew Morton wrote:
> On Wed, 23 Dec 2015 17:04:27 -0500 Johannes Weiner <hannes@cmpxchg.org> wrote:
> 
> > On Thu, Dec 10, 2015 at 10:32:42AM +0100, Daniel Vetter wrote:
> > > On Fri, Dec 04, 2015 at 11:09:52AM -0500, Johannes Weiner wrote:
> > > > On Fri, Dec 04, 2015 at 03:58:53PM +0000, Chris Wilson wrote:
> > > > > Some modules, like i915.ko, use swappable objects and may try to swap
> > > > > them out under memory pressure (via the shrinker). Before doing so, they
> > > > > want to check using get_nr_swap_pages() to see if any swap space is
> > > > > available as otherwise they will waste time purging the object from the
> > > > > device without recovering any memory for the system. This requires the
> > > > > nr_swap_pages counter to be exported to the modules.
> > > > > 
> > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > > > Cc: "Goel, Akash" <akash.goel@intel.com>
> > > > > Cc: Johannes Weiner <hannes@cmpxchg.org>
> > > > > Cc: linux-mm@kvack.org
> > > > 
> > > > Acked-by: Johannes Weiner <hannes@cmpxchg.org>
> > > 
> > > Ack for merging this through drm-intel trees for 4.5? I'm a bit unclear
> > > who's ack I need for that for linux-mm topics ...
> > 
> > Andrew would be the -mm maintainer. CC'd.
> 
> yup, please go ahead and merge that via the DRM tree.
> 
> nr_swap_pages is a crappy name.  That means "number of pages in swap",
> which isn't the case.  Something like "swap_pages_available" would be
> better.
> 
> And your swap_available() isn't good either ;) It can mean "is any swap
> online" or "what is the amount of free swap space (in unknown units!)".
> I'd call it "swap_is_full()" and put a ! in the caller.  But it's
> hardly important for a wee little static helper.

Yeah it's not super-pretty, but then the entire core mm/shrinker
abstraction is more than just a bit leaky (at least i915 has plenty of
code to make sure we don't bite our own tail). In case of doubt I prefer
the simplest export and avoid the mistake of fake abstraction in the form
of an inline helper with a pretty name.

Merged to drm-intel.git as-is, but missed the 4.5 train so will only land
in 4.6.

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>