* Re: [PATCH V2 2/2] change shrinker API by passing shrink_control struct
[not found] ` <BANLkTik3cC9f5M6xB4zpVPpRg8Y_+MtTaw@mail.gmail.com>
@ 2011-05-20 12:30 ` KOSAKI Motohiro
0 siblings, 0 replies; 2+ messages in thread
From: KOSAKI Motohiro @ 2011-05-20 12:30 UTC (permalink / raw)
To: yinghan; +Cc: linux-mm, linux-kernel, akpm
(2011/05/20 12:23), Ying Han wrote:
> On Thu, May 19, 2011 at 7:59 PM, KOSAKI Motohiro<
> kosaki.motohiro@jp.fujitsu.com> wrote:
>
>>> Hmm, got Nick's email wrong.
>>>
>>> --Ying
>>
>> Ping.
>> Can you please explain current status? When I can see your answer?
>>
>
> The patch has been merged into mmotm-04-29-16-25. Sorry if there is a
> question that I missed ?
I know. I know you haven't fix my pointed issue and you haven't answer
my question over two week. :-/
As far as I can remember now, at least I pointed out
- nr_slab_to_reclaim is wrong name.
you misunderstand shrinker->shrink() interface.
- least-recently-us is typo
- don't exporse both nr_scanned and nr_slab_to_reclaim.
Instead, calculate proper argument in shrink_slab.
Andrew, I've fixed it. please apply.
From 104ad1af66b57e4030b2b3bce5e35d2d3ec29e41 Mon Sep 17 00:00:00 2001
From: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Date: Fri, 20 May 2011 20:54:01 +0900
Subject: [PATCH] vmscan: fix up new shrinker API
Current new shrinker API submission has some easy mistake. Fix it up.
- remove nr_scanned field from shrink_control.
we don't have to expose vmscan internal to shrinkers.
- rename nr_slab_to_reclaim to nr_to_scan.
to_reclaim is very wrong name. shrinker API allow shrinker
don't reclaim an slab object if they were recently accessed.
- typo: least-recently-us
This patch also make do_shrinker_shrink() helper function. It
increase code readability a bit.
Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
---
arch/x86/kvm/mmu.c | 2 +-
drivers/gpu/drm/i915/i915_gem.c | 2 +-
drivers/gpu/drm/ttm/ttm_page_alloc.c | 2 +-
drivers/staging/zcache/zcache.c | 2 +-
fs/dcache.c | 2 +-
fs/drop_caches.c | 7 ++---
fs/gfs2/glock.c | 2 +-
fs/inode.c | 2 +-
fs/mbcache.c | 2 +-
fs/nfs/dir.c | 2 +-
fs/quota/dquot.c | 2 +-
fs/xfs/linux-2.6/xfs_buf.c | 2 +-
fs/xfs/linux-2.6/xfs_sync.c | 2 +-
fs/xfs/quota/xfs_qm.c | 2 +-
include/linux/mm.h | 12 +++++-----
mm/memory-failure.c | 3 +-
mm/vmscan.c | 36 +++++++++++++++++----------------
17 files changed, 42 insertions(+), 42 deletions(-)
diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 4cf6c15..bd14bb4 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -3549,7 +3549,7 @@ static int mmu_shrink(struct shrinker *shrink, struct shrink_control *sc)
{
struct kvm *kvm;
struct kvm *kvm_freed = NULL;
- int nr_to_scan = sc->nr_slab_to_reclaim;
+ int nr_to_scan = sc->nr_to_scan;
if (nr_to_scan == 0)
goto out;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index e4f3f6c..ec3d98c 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4100,7 +4100,7 @@ i915_gem_inactive_shrink(struct shrinker *shrinker, struct shrink_control *sc)
mm.inactive_shrinker);
struct drm_device *dev = dev_priv->dev;
struct drm_i915_gem_object *obj, *next;
- int nr_to_scan = sc->nr_slab_to_reclaim;
+ int nr_to_scan = sc->nr_to_scan;
int cnt;
if (!mutex_trylock(&dev->struct_mutex))
diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc.c b/drivers/gpu/drm/ttm/ttm_page_alloc.c
index 02ccf6f..d948575 100644
--- a/drivers/gpu/drm/ttm/ttm_page_alloc.c
+++ b/drivers/gpu/drm/ttm/ttm_page_alloc.c
@@ -402,7 +402,7 @@ static int ttm_pool_mm_shrink(struct shrinker *shrink,
unsigned i;
unsigned pool_offset = atomic_add_return(1, &start_pool);
struct ttm_page_pool *pool;
- int shrink_pages = sc->nr_slab_to_reclaim;
+ int shrink_pages = sc->nr_to_scan;
pool_offset = pool_offset % NUM_POOLS;
/* select start pool in round robin fashion */
diff --git a/drivers/staging/zcache/zcache.c b/drivers/staging/zcache/zcache.c
index 135851a..77ac2d4 100644
--- a/drivers/staging/zcache/zcache.c
+++ b/drivers/staging/zcache/zcache.c
@@ -1185,7 +1185,7 @@ static int shrink_zcache_memory(struct shrinker *shrink,
struct shrink_control *sc)
{
int ret = -1;
- int nr = sc->nr_slab_to_reclaim;
+ int nr = sc->nr_to_scan;
gfp_t gfp_mask = sc->gfp_mask;
if (nr >= 0) {
diff --git a/fs/dcache.c b/fs/dcache.c
index f70abf2..8926cd8 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -1234,7 +1234,7 @@ EXPORT_SYMBOL(shrink_dcache_parent);
static int shrink_dcache_memory(struct shrinker *shrink,
struct shrink_control *sc)
{
- int nr = sc->nr_slab_to_reclaim;
+ int nr = sc->nr_to_scan;
gfp_t gfp_mask = sc->gfp_mask;
if (nr) {
diff --git a/fs/drop_caches.c b/fs/drop_caches.c
index 440999c..e0a2906 100644
--- a/fs/drop_caches.c
+++ b/fs/drop_caches.c
@@ -42,12 +42,11 @@ static void drop_slab(void)
int nr_objects;
struct shrink_control shrink = {
.gfp_mask = GFP_KERNEL,
- .nr_scanned = 1000,
};
- do {
- nr_objects = shrink_slab(&shrink, 1000);
- } while (nr_objects > 10);
+ do
+ nr_objects = shrink_slab(&shrink, 1000, 1000);
+ while (nr_objects > 10);
}
int drop_caches_sysctl_handler(ctl_table *table, int write,
diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
index f3c9b17..2792a79 100644
--- a/fs/gfs2/glock.c
+++ b/fs/gfs2/glock.c
@@ -1352,7 +1352,7 @@ static int gfs2_shrink_glock_memory(struct shrinker *shrink,
struct gfs2_glock *gl;
int may_demote;
int nr_skipped = 0;
- int nr = sc->nr_slab_to_reclaim;
+ int nr = sc->nr_to_scan;
gfp_t gfp_mask = sc->gfp_mask;
LIST_HEAD(skipped);
diff --git a/fs/inode.c b/fs/inode.c
index ce61a1b..fadba5a 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -752,7 +752,7 @@ static void prune_icache(int nr_to_scan)
static int shrink_icache_memory(struct shrinker *shrink,
struct shrink_control *sc)
{
- int nr = sc->nr_slab_to_reclaim;
+ int nr = sc->nr_to_scan;
gfp_t gfp_mask = sc->gfp_mask;
if (nr) {
diff --git a/fs/mbcache.c b/fs/mbcache.c
index 19a2666..8c32ef3 100644
--- a/fs/mbcache.c
+++ b/fs/mbcache.c
@@ -168,7 +168,7 @@ mb_cache_shrink_fn(struct shrinker *shrink, struct shrink_control *sc)
struct mb_cache *cache;
struct mb_cache_entry *entry, *tmp;
int count = 0;
- int nr_to_scan = sc->nr_slab_to_reclaim;
+ int nr_to_scan = sc->nr_to_scan;
gfp_t gfp_mask = sc->gfp_mask;
mb_debug("trying to free %d entries", nr_to_scan);
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 9dee703..424e477 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -2048,7 +2048,7 @@ int nfs_access_cache_shrinker(struct shrinker *shrink,
LIST_HEAD(head);
struct nfs_inode *nfsi, *next;
struct nfs_access_entry *cache;
- int nr_to_scan = sc->nr_slab_to_reclaim;
+ int nr_to_scan = sc->nr_to_scan;
gfp_t gfp_mask = sc->gfp_mask;
if ((gfp_mask & GFP_KERNEL) != GFP_KERNEL)
diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
index b780ee0..5b572c8 100644
--- a/fs/quota/dquot.c
+++ b/fs/quota/dquot.c
@@ -694,7 +694,7 @@ static void prune_dqcache(int count)
static int shrink_dqcache_memory(struct shrinker *shrink,
struct shrink_control *sc)
{
- int nr = sc->nr_slab_to_reclaim;
+ int nr = sc->nr_to_scan;
if (nr) {
spin_lock(&dq_list_lock);
diff --git a/fs/xfs/linux-2.6/xfs_buf.c b/fs/xfs/linux-2.6/xfs_buf.c
index 04b9558..ddac2ec 100644
--- a/fs/xfs/linux-2.6/xfs_buf.c
+++ b/fs/xfs/linux-2.6/xfs_buf.c
@@ -1406,7 +1406,7 @@ xfs_buftarg_shrink(
struct xfs_buftarg *btp = container_of(shrink,
struct xfs_buftarg, bt_shrinker);
struct xfs_buf *bp;
- int nr_to_scan = sc->nr_slab_to_reclaim;
+ int nr_to_scan = sc->nr_to_scan;
LIST_HEAD(dispose);
if (!nr_to_scan)
diff --git a/fs/xfs/linux-2.6/xfs_sync.c b/fs/xfs/linux-2.6/xfs_sync.c
index 3fa9aae..2460114 100644
--- a/fs/xfs/linux-2.6/xfs_sync.c
+++ b/fs/xfs/linux-2.6/xfs_sync.c
@@ -1028,7 +1028,7 @@ xfs_reclaim_inode_shrink(
struct xfs_perag *pag;
xfs_agnumber_t ag;
int reclaimable;
- int nr_to_scan = sc->nr_slab_to_reclaim;
+ int nr_to_scan = sc->nr_to_scan;
gfp_t gfp_mask = sc->gfp_mask;
mp = container_of(shrink, struct xfs_mount, m_inode_shrink);
diff --git a/fs/xfs/quota/xfs_qm.c b/fs/xfs/quota/xfs_qm.c
index 2954330..c31a7ae 100644
--- a/fs/xfs/quota/xfs_qm.c
+++ b/fs/xfs/quota/xfs_qm.c
@@ -2012,7 +2012,7 @@ xfs_qm_shake(
struct shrink_control *sc)
{
int ndqused, nfree, n;
- int nr_to_scan = sc->nr_slab_to_reclaim;
+ int nr_to_scan = sc->nr_to_scan;
gfp_t gfp_mask = sc->gfp_mask;
if (!kmem_shake_allow(gfp_mask))
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 72ba1f5..02be595 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1134,19 +1134,18 @@ static inline void sync_mm_rss(struct task_struct *task, struct mm_struct *mm)
* We consolidate the values for easier extention later.
*/
struct shrink_control {
- unsigned long nr_scanned;
gfp_t gfp_mask;
- /* How many slab objects shrinker() should reclaim */
- unsigned long nr_slab_to_reclaim;
+ /* How many slab objects shrinker() should scan and try to reclaim */
+ unsigned long nr_to_scan;
};
/*
* A callback you can register to apply pressure to ageable caches.
*
- * 'sc' is passed shrink_control which includes a count 'nr_slab_to_reclaim'
- * and a 'gfpmask'. It should look through the least-recently-us
- * 'nr_slab_to_reclaim' entries and attempt to free them up. It should return
+ * 'sc' is passed shrink_control which includes a count 'nr_to_scan'
+ * and a 'gfpmask'. It should look through the least-recently-used
+ * 'nr_to_scan' entries and attempt to free them up. It should return
* the number of objects which remain in the cache. If it returns -1, it means
* it cannot do any scanning at this time (eg. there is a risk of deadlock).
*
@@ -1613,6 +1612,7 @@ int in_gate_area_no_mm(unsigned long addr);
int drop_caches_sysctl_handler(struct ctl_table *, int,
void __user *, size_t *, loff_t *);
unsigned long shrink_slab(struct shrink_control *shrink,
+ unsigned long nr_pages_scanned,
unsigned long lru_pages);
#ifndef CONFIG_MMU
diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index 341341b..5c8f7e0 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -241,10 +241,9 @@ void shake_page(struct page *p, int access)
do {
struct shrink_control shrink = {
.gfp_mask = GFP_KERNEL,
- .nr_scanned = 1000,
};
- nr = shrink_slab(&shrink, 1000);
+ nr = shrink_slab(&shrink, 1000, 1000);
if (page_count(p) == 1)
break;
} while (nr > 10);
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 292582c..89e24f7 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -201,6 +201,14 @@ void unregister_shrinker(struct shrinker *shrinker)
}
EXPORT_SYMBOL(unregister_shrinker);
+static inline int do_shrinker_shrink(struct shrinker *shrinker,
+ struct shrink_control *sc,
+ unsigned long nr_to_scan)
+{
+ sc->nr_to_scan = nr_to_scan;
+ return (*shrinker->shrink)(shrinker, sc);
+}
+
#define SHRINK_BATCH 128
/*
* Call the shrink functions to age shrinkable caches
@@ -222,14 +230,14 @@ EXPORT_SYMBOL(unregister_shrinker);
* Returns the number of slab objects which we shrunk.
*/
unsigned long shrink_slab(struct shrink_control *shrink,
+ unsigned long nr_pages_scanned,
unsigned long lru_pages)
{
struct shrinker *shrinker;
unsigned long ret = 0;
- unsigned long scanned = shrink->nr_scanned;
- if (scanned == 0)
- scanned = SWAP_CLUSTER_MAX;
+ if (nr_pages_scanned == 0)
+ nr_pages_scanned = SWAP_CLUSTER_MAX;
if (!down_read_trylock(&shrinker_rwsem))
return 1; /* Assume we'll be able to shrink next time */
@@ -239,9 +247,8 @@ unsigned long shrink_slab(struct shrink_control *shrink,
unsigned long total_scan;
unsigned long max_pass;
- shrink->nr_slab_to_reclaim = 0;
- max_pass = (*shrinker->shrink)(shrinker, shrink);
- delta = (4 * scanned) / shrinker->seeks;
+ max_pass = do_shrinker_shrink(shrinker, shrink, 0);
+ delta = (4 * nr_pages_scanned) / shrinker->seeks;
delta *= max_pass;
do_div(delta, lru_pages + 1);
shrinker->nr += delta;
@@ -268,11 +275,9 @@ unsigned long shrink_slab(struct shrink_control *shrink,
int shrink_ret;
int nr_before;
- shrink->nr_slab_to_reclaim = 0;
- nr_before = (*shrinker->shrink)(shrinker, shrink);
- shrink->nr_slab_to_reclaim = this_scan;
- shrink_ret = (*shrinker->shrink)(shrinker, shrink);
-
+ nr_before = do_shrinker_shrink(shrinker, shrink, 0);
+ shrink_ret = do_shrinker_shrink(shrinker, shrink,
+ this_scan);
if (shrink_ret == -1)
break;
if (shrink_ret < nr_before)
@@ -2086,8 +2091,7 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
lru_pages += zone_reclaimable_pages(zone);
}
- shrink->nr_scanned = sc->nr_scanned;
- shrink_slab(shrink, lru_pages);
+ shrink_slab(shrink, sc->nr_scanned, lru_pages);
if (reclaim_state) {
sc->nr_reclaimed += reclaim_state->reclaimed_slab;
reclaim_state->reclaimed_slab = 0;
@@ -2488,8 +2492,7 @@ loop_again:
end_zone, 0))
shrink_zone(priority, zone, &sc);
reclaim_state->reclaimed_slab = 0;
- shrink.nr_scanned = sc.nr_scanned;
- nr_slab = shrink_slab(&shrink, lru_pages);
+ nr_slab = shrink_slab(&shrink, sc.nr_scanned, lru_pages);
sc.nr_reclaimed += reclaim_state->reclaimed_slab;
total_scanned += sc.nr_scanned;
@@ -3057,7 +3060,6 @@ static int __zone_reclaim(struct zone *zone, gfp_t gfp_mask, unsigned int order)
}
nr_slab_pages0 = zone_page_state(zone, NR_SLAB_RECLAIMABLE);
- shrink.nr_scanned = sc.nr_scanned;
if (nr_slab_pages0 > zone->min_slab_pages) {
/*
* shrink_slab() does not currently allow us to determine how
@@ -3073,7 +3075,7 @@ static int __zone_reclaim(struct zone *zone, gfp_t gfp_mask, unsigned int order)
unsigned long lru_pages = zone_reclaimable_pages(zone);
/* No reclaimable slab or very low memory pressure */
- if (!shrink_slab(&shrink, lru_pages))
+ if (!shrink_slab(&shrink, sc.nr_scanned, lru_pages))
break;
/* Freed enough memory */
--
1.7.3.1
^ permalink raw reply related [flat|nested] 2+ messages in thread