linux-next: manual merge of the akpm tree with the ext4 tree

linux-next: manual merge of the akpm tree with the ext4 tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 2519 bytes --]

Hi Andrew,

Today's linux-next merge of the akpm tree got a conflict in
fs/ext4/extents_status.c between commit decae3aab047 ("ext4: make the
extent_status code more robust against ENOMEM failures") from the ext4
tree and commit  "fs: convert fs shrinkers to new scan/count API" from
the akpm tree.

I fixed it up (I think - see below) and can carry the fix as necessary
(no action is required).

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

diff --cc fs/ext4/extents_status.c
index 91cb110,fb3b95e..0000000
--- a/fs/ext4/extents_status.c
+++ b/fs/ext4/extents_status.c
@@@ -903,9 -891,21 +903,20 @@@ static int ext4_inode_touch_time_cmp(vo
  		return -1;
  }
  
+ static long ext4_es_count(struct shrinker *shrink, struct shrink_control *sc)
+ {
+ 	long nr;
+ 	struct ext4_sb_info *sbi = container_of(shrink,
+ 					struct ext4_sb_info, s_es_shrinker);
+ 
+ 	nr = percpu_counter_read_positive(&sbi->s_extent_cache_cnt);
+ 	trace_ext4_es_shrink_enter(sbi->s_sb, sc->nr_to_scan, nr);
+ 	return nr;
+ }
+ 
 -static long ext4_es_scan(struct shrinker *shrink, struct shrink_control *sc)
 +static int __ext4_es_shrink(struct ext4_sb_info *sbi, int nr_to_scan,
 +			    struct ext4_inode_info *locked_ei)
  {
 -	struct ext4_sb_info *sbi = container_of(shrink,
 -					struct ext4_sb_info, s_es_shrinker);
  	struct ext4_inode_info *ei;
  	struct list_head *cur, *tmp;
  	LIST_HEAD(skiped);
@@@ -958,30 -959,8 +969,23 @@@
  	list_splice_tail(&skiped, &sbi->s_es_lru);
  	spin_unlock(&sbi->s_es_lru_lock);
  
 +	if (locked_ei && nr_shrunk == 0)
 +		nr_shrunk = __es_try_to_reclaim_extents(ei, nr_to_scan);
 +
 +	return nr_shrunk;
 +}
 +
- static int ext4_es_shrink(struct shrinker *shrink, struct shrink_control *sc)
++static long ext4_es_shrink(struct shrinker *shrink, struct shrink_control *sc)
 +{
 +	struct ext4_sb_info *sbi = container_of(shrink,
 +					struct ext4_sb_info, s_es_shrinker);
 +	int nr_to_scan = sc->nr_to_scan;
- 	int ret, nr_shrunk;
- 
- 	ret = percpu_counter_read_positive(&sbi->s_extent_cache_cnt);
- 	trace_ext4_es_shrink_enter(sbi->s_sb, nr_to_scan, ret);
- 
- 	if (!nr_to_scan)
- 		return ret;
++	int nr_shrunk;
 +
 +	nr_shrunk = __ext4_es_shrink(sbi, nr_to_scan, NULL);
 +
- 	ret = percpu_counter_read_positive(&sbi->s_extent_cache_cnt);
  	trace_ext4_es_shrink_exit(sbi->s_sb, nr_shrunk, ret);
- 	return ret;
+ 	return nr_shrunk;
  }
  
  void ext4_es_register_shrinker(struct ext4_sb_info *sbi)

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: linux-next: manual merge of the akpm tree with the ext4 tree

[Andrew Morton]

On Fri, 9 Aug 2013 15:21:04 -0700 Kees Cook <kees@outflux.net> wrote:

> Hi,
> 
> On Wed, Aug 07, 2013 at 03:19:03PM +1000, Stephen Rothwell wrote:
> > Hi Andrew,
> > 
> >   	list_for_each_safe(cur, tmp, &sbi->s_es_lru) {
> > + 		int ret;
> > + 
> >   		/*
> >   		 * If we have already reclaimed all extents from extent
> >   		 * status tree, just stop the loop immediately.
> 
> Which is masking the "ret" at the start, leading to warnings at build-time:
> 
> fs/ext4/extents_status.c: In function _____ext4_es_shrink___:
> fs/ext4/extents_status.c:950:6: warning: unused variable ___ret___ [-Wunused-variable]

yup.

From: Andrew Morton <akpm@linux-foundation.org>
Subject: fs-convert-fs-shrinkers-to-new-scan-count-api-fix-fix-2

fix shadowed local, spell "skipped" correctly

Cc: Dave Chinner <dchinner@redhat.com>
Cc: Glauber Costa <glommer@openvz.org>
Cc: Kees Cook <kees@outflux.net>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 fs/ext4/extents_status.c |   16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff -puN fs/ext4/extents_status.c~fs-convert-fs-shrinkers-to-new-scan-count-api-fix-fix-2 fs/ext4/extents_status.c
--- a/fs/ext4/extents_status.c~fs-convert-fs-shrinkers-to-new-scan-count-api-fix-fix-2
+++ a/fs/ext4/extents_status.c
@@ -946,15 +946,15 @@ static int __ext4_es_shrink(struct ext4_
 {
 	struct ext4_inode_info *ei;
 	struct list_head *cur, *tmp;
-	LIST_HEAD(skiped);
-	int ret, nr_shrunk = 0;
+	LIST_HEAD(skipped);
+	int nr_shrunk = 0;
 	int retried = 0, skip_precached = 1, nr_skipped = 0;
 
 	spin_lock(&sbi->s_es_lru_lock);
 
 retry:
 	list_for_each_safe(cur, tmp, &sbi->s_es_lru) {
-		int ret;
+		int shrunk;
 
 		/*
 		 * If we have already reclaimed all extents from extent
@@ -974,7 +974,7 @@ retry:
 		    (skip_precached && ext4_test_inode_state(&ei->vfs_inode,
 						EXT4_STATE_EXT_PRECACHED))) {
 			nr_skipped++;
-			list_move_tail(cur, &skiped);
+			list_move_tail(cur, &skipped);
 			continue;
 		}
 
@@ -982,19 +982,19 @@ retry:
 			continue;
 
 		write_lock(&ei->i_es_lock);
-		ret = __es_try_to_reclaim_extents(ei, nr_to_scan);
+		shrunk = __es_try_to_reclaim_extents(ei, nr_to_scan);
 		if (ei->i_es_lru_nr == 0)
 			list_del_init(&ei->i_es_lru);
 		write_unlock(&ei->i_es_lock);
 
-		nr_shrunk += ret;
-		nr_to_scan -= ret;
+		nr_shrunk += shrunk;
+		nr_to_scan -= shrunk;
 		if (nr_to_scan == 0)
 			break;
 	}
 
 	/* Move the newer inodes into the tail of the LRU list. */
-	list_splice_tail(&skiped, &sbi->s_es_lru);
+	list_splice_tail(&skipped, &sbi->s_es_lru);
 
 	/*
 	 * If we skipped any inodes, and we weren't able to make any
_

Re: linux-next: manual merge of the akpm tree with the ext4 tree

[Kees Cook]

Hi,

On Wed, Aug 07, 2013 at 03:19:03PM +1000, Stephen Rothwell wrote:
> Hi Andrew,
> 
> Today's linux-next merge of the akpm tree got a conflict in
> fs/ext4/extents_status.c between commit 49c6efc7b80e ("ext4: add new
> ioctl EXT4_IOC_PRECACHE_EXTENTS") from the ext4 tree and commit
> "fs-convert-fs-shrinkers-to-new-scan-count-api-fix" from the akpm tree.
> 
> I fixed it up (see below) and can carry the fix as necessary (no action
> is required).
> 
> -- 
> Cheers,
> Stephen Rothwell                    sfr@canb.auug.org.au
> 
> diff --cc fs/ext4/extents_status.c
> index 28e2627,0361206..0000000
> --- a/fs/ext4/extents_status.c
> +++ b/fs/ext4/extents_status.c
> @@@ -947,13 -909,23 +947,15 @@@ static int __ext4_es_shrink(struct ext4
>   	struct ext4_inode_info *ei;
>   	struct list_head *cur, *tmp;
>   	LIST_HEAD(skiped);
> - 	int ret, nr_shrunk = 0;
> + 	unsigned long nr_shrunk = 0;
>  +	int retried = 0, skip_precached = 1, nr_skipped = 0;
>   
>   	spin_lock(&sbi->s_es_lru_lock);
>   
>  -	/*
>  -	 * If the inode that is at the head of LRU list is newer than
>  -	 * last_sorted time, that means that we need to sort this list.
>  -	 */
>  -	ei = list_first_entry(&sbi->s_es_lru, struct ext4_inode_info, i_es_lru);
>  -	if (sbi->s_es_last_sorted < ei->i_touch_when) {
>  -		list_sort(NULL, &sbi->s_es_lru, ext4_inode_touch_time_cmp);
>  -		sbi->s_es_last_sorted = jiffies;
>  -	}
>  -
>  +retry:

It seems like fs-convert-fs-shrinkers-to-new-scan-count-api-fix is carrying
this needless chunk above:

>   	list_for_each_safe(cur, tmp, &sbi->s_es_lru) {
> + 		int ret;
> + 
>   		/*
>   		 * If we have already reclaimed all extents from extent
>   		 * status tree, just stop the loop immediately.

Which is masking the "ret" at the start, leading to warnings at build-time:

fs/ext4/extents_status.c: In function ‘__ext4_es_shrink’:
fs/ext4/extents_status.c:950:6: warning: unused variable ‘ret’ [-Wunused-variable]

-Kees

-- 
Kees Cook                                            @outflux.net

linux-next: manual merge of the akpm tree with the ext4 tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1496 bytes --]

Hi Andrew,

Today's linux-next merge of the akpm tree got a conflict in
fs/ext4/extents_status.c between commit 49c6efc7b80e ("ext4: add new
ioctl EXT4_IOC_PRECACHE_EXTENTS") from the ext4 tree and commit
"fs-convert-fs-shrinkers-to-new-scan-count-api-fix" from the akpm tree.

I fixed it up (see below) and can carry the fix as necessary (no action
is required).

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

diff --cc fs/ext4/extents_status.c
index 28e2627,0361206..0000000
--- a/fs/ext4/extents_status.c
+++ b/fs/ext4/extents_status.c
@@@ -947,13 -909,23 +947,15 @@@ static int __ext4_es_shrink(struct ext4
  	struct ext4_inode_info *ei;
  	struct list_head *cur, *tmp;
  	LIST_HEAD(skiped);
- 	int ret, nr_shrunk = 0;
+ 	unsigned long nr_shrunk = 0;
 +	int retried = 0, skip_precached = 1, nr_skipped = 0;
  
  	spin_lock(&sbi->s_es_lru_lock);
  
 -	/*
 -	 * If the inode that is at the head of LRU list is newer than
 -	 * last_sorted time, that means that we need to sort this list.
 -	 */
 -	ei = list_first_entry(&sbi->s_es_lru, struct ext4_inode_info, i_es_lru);
 -	if (sbi->s_es_last_sorted < ei->i_touch_when) {
 -		list_sort(NULL, &sbi->s_es_lru, ext4_inode_touch_time_cmp);
 -		sbi->s_es_last_sorted = jiffies;
 -	}
 -
 +retry:
  	list_for_each_safe(cur, tmp, &sbi->s_es_lru) {
+ 		int ret;
+ 
  		/*
  		 * If we have already reclaimed all extents from extent
  		 * status tree, just stop the loop immediately.

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

linux-next: manual merge of the akpm tree with the ext4 tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 957 bytes --]

Hi Andrew,

Today's linux-next merge of the akpm tree got a conflict in
fs/ext4/extents_status.c between commit 49c6efc7b80e ("ext4: add new
ioctl EXT4_IOC_PRECACHE_EXTENTS") from the ext4 tree and commit "fs:
convert fs shrinkers to new scan/count API" from the akpm tree.

I fixed it up (see below) and can carry the fix as necessary (no action
is required).

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

diff --cc fs/ext4/extents_status.c
index 00e6589,9f931b2..0000000
--- a/fs/ext4/extents_status.c
+++ b/fs/ext4/extents_status.c
@@@ -1091,9 -1044,7 +1102,9 @@@ static int __es_try_to_reclaim_extents(
  	struct ext4_es_tree *tree = &ei->i_es_tree;
  	struct rb_node *node;
  	struct extent_status *es;
- 	int nr_shrunk = 0;
+ 	long nr_shrunk = 0;
 +	static DEFINE_RATELIMIT_STATE(_rs, DEFAULT_RATELIMIT_INTERVAL,
 +				      DEFAULT_RATELIMIT_BURST);
  
  	if (ei->i_es_lru_nr == 0)
  		return 0;

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

linux-next: manual merge of the akpm tree with the ext4 tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 2634 bytes --]

Hi Andrew,

Today's linux-next merge of the akpm tree got a conflict in
fs/ext4/extents_status.c between commit decae3aab047 ("ext4: make the
extent_status code more robust against ENOMEM failures") from the ext4
tree and commit "fs-convert-fs-shrinkers-to-new-scan-count-api-fix" from
the akpm tree.

I fixed it up (see below) and can carry the fix as necessary (no action
is required).

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

diff --cc fs/ext4/extents_status.c
index 95d5bde,8a45b86..0000000
--- a/fs/ext4/extents_status.c
+++ b/fs/ext4/extents_status.c
@@@ -148,8 -148,6 +148,8 @@@ static int __es_remove_extent(struct in
  			      ext4_lblk_t end);
  static int __es_try_to_reclaim_extents(struct ext4_inode_info *ei,
  				       int nr_to_scan);
- static int __ext4_es_shrink(struct ext4_sb_info *sbi, int nr_to_scan,
- 			    struct ext4_inode_info *locked_ei);
++static unsigned long __ext4_es_shrink(struct ext4_sb_info *sbi, int nr_to_scan,
++				      struct ext4_inode_info *locked_ei);
  
  int __init ext4_init_es(void)
  {
@@@ -914,13 -903,17 +915,14 @@@ static unsigned long ext4_es_count(stru
  	return nr;
  }
  
- static int __ext4_es_shrink(struct ext4_sb_info *sbi, int nr_to_scan,
 -static unsigned long ext4_es_scan(struct shrinker *shrink,
 -				  struct shrink_control *sc)
++static unsigned long __ext4_es_shrink(struct ext4_sb_info *sbi, int nr_to_scan,
 +			    struct ext4_inode_info *locked_ei)
  {
 -	struct ext4_sb_info *sbi = container_of(shrink,
 -					struct ext4_sb_info, s_es_shrinker);
  	struct ext4_inode_info *ei;
  	struct list_head *cur, *tmp;
  	LIST_HEAD(skiped);
- 	int ret, nr_shrunk = 0;
 -	int nr_to_scan = sc->nr_to_scan;
 -	int ret = 0;
++	int ret;
+ 	unsigned long nr_shrunk = 0;
  
  	spin_lock(&sbi->s_es_lru_lock);
  
@@@ -969,21 -962,6 +971,22 @@@
  	list_splice_tail(&skiped, &sbi->s_es_lru);
  	spin_unlock(&sbi->s_es_lru_lock);
  
 +	if (locked_ei && nr_shrunk == 0)
 +		nr_shrunk = __es_try_to_reclaim_extents(ei, nr_to_scan);
 +
 +	return nr_shrunk;
 +}
 +
- static long ext4_es_shrink(struct shrinker *shrink, struct shrink_control *sc)
++static unsigned long ext4_es_shrink(struct shrinker *shrink,
++				    struct shrink_control *sc)
 +{
 +	struct ext4_sb_info *sbi = container_of(shrink,
 +					struct ext4_sb_info, s_es_shrinker);
 +	int nr_to_scan = sc->nr_to_scan;
- 	int nr_shrunk;
++	unsigned long nr_shrunk;
 +
 +	nr_shrunk = __ext4_es_shrink(sbi, nr_to_scan, NULL);
 +
  	trace_ext4_es_shrink_exit(sbi->s_sb, nr_shrunk, ret);
  	return nr_shrunk;
  }

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: linux-next: manual merge of the akpm tree with the ext4 tree

[Glauber Costa]

On Wed, Jun 19, 2013 at 02:59:17PM -0400, Theodore Ts'o wrote:
> On Wed, Jun 19, 2013 at 10:06:35AM -0700, Andrew Morton wrote:
> > 
> > Merge the ext4 change early, please.  The core shrinker changes aren't
> > 100% certain at this time - first they need to stop oopsing ;)
> 
> Ack, sounds like a plan.
> 
> Are they oopsing often enough that they are likely to interfere with
> running regression tests on linux-next?
> 
I have only received one report so far. It will really depend on how likely it
is for other people to hit it - wether or not other people hit it easily is also
good info...

Re: linux-next: manual merge of the akpm tree with the ext4 tree

[Theodore Ts'o]

On Wed, Jun 19, 2013 at 10:06:35AM -0700, Andrew Morton wrote:
> 
> Merge the ext4 change early, please.  The core shrinker changes aren't
> 100% certain at this time - first they need to stop oopsing ;)

Ack, sounds like a plan.

Are they oopsing often enough that they are likely to interfere with
running regression tests on linux-next?

					- Ted

Re: linux-next: manual merge of the akpm tree with the ext4 tree

[Andrew Morton]

On Wed, 19 Jun 2013 10:20:31 -0400 "Theodore Ts'o" <tytso@mit.edu> wrote:

> Is there some way we can avoid this conflict during the next merge
> window?  Given that this is an API change, it may not be possible.
> Failing that, what's the best merge strategy; should we try to make
> sure your change goes first, and then I can defer the ext4 pull
> request until a little bit later in the merge window?

Merge the ext4 change early, please.  The core shrinker changes aren't
100% certain at this time - first they need to stop oopsing ;)

Re: linux-next: manual merge of the akpm tree with the ext4 tree

[Theodore Ts'o]

Is there some way we can avoid this conflict during the next merge
window?  Given that this is an API change, it may not be possible.
Failing that, what's the best merge strategy; should we try to make
sure your change goes first, and then I can defer the ext4 pull
request until a little bit later in the merge window?

Thanks,

				- Ted

Re: linux-next: manual merge of the akpm tree with the ext4 tree

[Zheng Liu]

Hi Stephen,

On Jun 19, 2013, at 3:27 PM, Stephen Rothwell <sfr@canb.auug.org.au> wrote:

> Hi Andrew,
> 
> Today's linux-next merge of the akpm tree got a conflict in
> fs/ext4/extents_status.c between commit 6480bad916be ("ext4: improve
> extent cache shrink mechanism to avoid to burn CPU time") from the ext
> tree and commit 1f42d0934b4e ("fs: convert fs shrinkers to new scan/count
> API") from the akpm tree.
> 
> I fixed it up (I am not sure if the result makes complete sense - see
> below) and can carry the fix as necessary (no action is required).

The patch looks good to me.  Thanks for fixing it.

Regards,
						- Zheng

> 
> -- 
> Cheers,
> Stephen Rothwell                    sfr@canb.auug.org.au
> 
> diff --cc fs/ext4/extents_status.c
> index 80dcc59,4bce4f0..0000000
> --- a/fs/ext4/extents_status.c
> +++ b/fs/ext4/extents_status.c
> @@@ -876,58 -878,32 +876,63 @@@ int ext4_es_zeroout(struct inode *inode
>  				     EXTENT_STATUS_WRITTEN);
>  }
> 
> +static int ext4_inode_touch_time_cmp(void *priv, struct list_head *a,
> +				     struct list_head *b)
> +{
> +	struct ext4_inode_info *eia, *eib;
> +	unsigned long diff;
> +
> +	eia = list_entry(a, struct ext4_inode_info, i_es_lru);
> +	eib = list_entry(b, struct ext4_inode_info, i_es_lru);
> +
> +	diff = eia->i_touch_when - eib->i_touch_when;
> +	if (diff < 0)
> +		return -1;
> +	if (diff > 0)
> +		return 1;
> +	return 0;
> +}
> 
> - static int ext4_es_shrink(struct shrinker *shrink, struct shrink_control *sc)
> + static long ext4_es_count(struct shrinker *shrink, struct shrink_control *sc)
> + {
> + 	long nr;
> + 	struct ext4_sb_info *sbi = container_of(shrink,
> + 					struct ext4_sb_info, s_es_shrinker);
> + 
> + 	nr = percpu_counter_read_positive(&sbi->s_extent_cache_cnt);
> + 	trace_ext4_es_shrink_enter(sbi->s_sb, sc->nr_to_scan, nr);
> + 	return nr;
> + }
> + 
> + static long ext4_es_scan(struct shrinker *shrink, struct shrink_control *sc)
>  {
>  	struct ext4_sb_info *sbi = container_of(shrink,
>  					struct ext4_sb_info, s_es_shrinker);
>  	struct ext4_inode_info *ei;
> -	struct list_head *cur, *tmp, scanned;
> +	struct list_head *cur, *tmp;
> +	LIST_HEAD(skiped);
>  	int nr_to_scan = sc->nr_to_scan;
> - 	int ret, nr_shrunk = 0;
> - 
> - 	ret = percpu_counter_read_positive(&sbi->s_extent_cache_cnt);
> - 	trace_ext4_es_shrink_enter(sbi->s_sb, nr_to_scan, ret);
> - 
> - 	if (!nr_to_scan)
> - 		return ret;
> + 	int ret = 0, nr_shrunk = 0;
> 
> -	INIT_LIST_HEAD(&scanned);
> -
>  	spin_lock(&sbi->s_es_lru_lock);
> +
> +	/*
> +	 * If the inode that is at the head of LRU list is newer than
> +	 * last_sorted time, that means that we need to sort this list.
> +	 */
> +	ei = list_first_entry(&sbi->s_es_lru, struct ext4_inode_info, i_es_lru);
> +	if (sbi->s_es_last_sorted < ei->i_touch_when) {
> +		list_sort(NULL, &sbi->s_es_lru, ext4_inode_touch_time_cmp);
> +		sbi->s_es_last_sorted = jiffies;
> +	}
> +
>  	list_for_each_safe(cur, tmp, &sbi->s_es_lru) {
> -		list_move_tail(cur, &scanned);
> +		/*
> +		 * If we have already reclaimed all extents from extent
> +		 * status tree, just stop the loop immediately.
> +		 */
> +		if (percpu_counter_read_positive(&sbi->s_extent_cache_cnt) == 0)
> +			break;
> 
>  		ei = list_entry(cur, struct ext4_inode_info, i_es_lru);
> 
> @@@ -951,22 -923,22 +956,22 @@@
>  		if (nr_to_scan == 0)
>  			break;
>  	}
> -	list_splice_tail(&scanned, &sbi->s_es_lru);
> +
> +	/* Move the newer inodes into the tail of the LRU list. */
> +	list_splice_tail(&skiped, &sbi->s_es_lru);
>  	spin_unlock(&sbi->s_es_lru_lock);
> 
> - 	ret = percpu_counter_read_positive(&sbi->s_extent_cache_cnt);
>  	trace_ext4_es_shrink_exit(sbi->s_sb, nr_shrunk, ret);
> - 	return ret;
> + 	return nr_shrunk;
>  }
> 
> -void ext4_es_register_shrinker(struct super_block *sb)
> +void ext4_es_register_shrinker(struct ext4_sb_info *sbi)
>  {
> -	struct ext4_sb_info *sbi;
> -
> -	sbi = EXT4_SB(sb);
>  	INIT_LIST_HEAD(&sbi->s_es_lru);
>  	spin_lock_init(&sbi->s_es_lru_lock);
> +	sbi->s_es_last_sorted = 0;
> - 	sbi->s_es_shrinker.shrink = ext4_es_shrink;
> + 	sbi->s_es_shrinker.scan_objects = ext4_es_scan;
> + 	sbi->s_es_shrinker.count_objects = ext4_es_count;
>  	sbi->s_es_shrinker.seeks = DEFAULT_SEEKS;
>  	register_shrinker(&sbi->s_es_shrinker);
>  }

Re: linux-next: manual merge of the akpm tree with the ext4 tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 245 bytes --]

Hi,

On Wed, 19 Jun 2013 11:44:04 +0400 Glauber Costa <glommer@gmail.com> wrote:
>
> I believe the resolution is okay, at least from our PoV.

Thanks for checking.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: linux-next: manual merge of the akpm tree with the ext4 tree

[Glauber Costa]

On Wed, Jun 19, 2013 at 05:27:21PM +1000, Stephen Rothwell wrote:
> Hi Andrew,
> 
> Today's linux-next merge of the akpm tree got a conflict in
> fs/ext4/extents_status.c between commit 6480bad916be ("ext4: improve
> extent cache shrink mechanism to avoid to burn CPU time") from the ext
> tree and commit 1f42d0934b4e ("fs: convert fs shrinkers to new scan/count
> API") from the akpm tree.
> 
> I fixed it up (I am not sure if the result makes complete sense - see
> below) and can carry the fix as necessary (no action is required).
> 
> -- 
> Cheers,
> Stephen Rothwell                    sfr@canb.auug.org.au
> 
> diff --cc fs/ext4/extents_status.c
> index 80dcc59,4bce4f0..0000000
> --- a/fs/ext4/extents_status.c
> +++ b/fs/ext4/extents_status.c
> @@@ -876,58 -878,32 +876,63 @@@ int ext4_es_zeroout(struct inode *inode
>   				     EXTENT_STATUS_WRITTEN);
>   }
>   
>  +static int ext4_inode_touch_time_cmp(void *priv, struct list_head *a,
>  +				     struct list_head *b)
>  +{
>  +	struct ext4_inode_info *eia, *eib;
>  +	unsigned long diff;
>  +
>  +	eia = list_entry(a, struct ext4_inode_info, i_es_lru);
>  +	eib = list_entry(b, struct ext4_inode_info, i_es_lru);
>  +
>  +	diff = eia->i_touch_when - eib->i_touch_when;
>  +	if (diff < 0)
>  +		return -1;
>  +	if (diff > 0)
>  +		return 1;
>  +	return 0;
>  +}
>   
This comes straight from ext4, so no problem.

Now we just need to make sure that the shrinker is clearly separated
between a count part and a scan part.

> - static int ext4_es_shrink(struct shrinker *shrink, struct shrink_control *sc)
> + static long ext4_es_count(struct shrinker *shrink, struct shrink_control *sc)
> + {
> + 	long nr;
> + 	struct ext4_sb_info *sbi = container_of(shrink,
> + 					struct ext4_sb_info, s_es_shrinker);
> + 
> + 	nr = percpu_counter_read_positive(&sbi->s_extent_cache_cnt);
> + 	trace_ext4_es_shrink_enter(sbi->s_sb, sc->nr_to_scan, nr);
> + 	return nr;
> + }
The count seems okay.

> + 
> + static long ext4_es_scan(struct shrinker *shrink, struct shrink_control *sc)
>   {
>   	struct ext4_sb_info *sbi = container_of(shrink,
>   					struct ext4_sb_info, s_es_shrinker);
>   	struct ext4_inode_info *ei;
>  -	struct list_head *cur, *tmp, scanned;
>  +	struct list_head *cur, *tmp;
>  +	LIST_HEAD(skiped);
>   	int nr_to_scan = sc->nr_to_scan;
> - 	int ret, nr_shrunk = 0;
> - 
> - 	ret = percpu_counter_read_positive(&sbi->s_extent_cache_cnt);
> - 	trace_ext4_es_shrink_enter(sbi->s_sb, nr_to_scan, ret);
> - 
> - 	if (!nr_to_scan)
> - 		return ret;
> + 	int ret = 0, nr_shrunk = 0;
>   
>  -	INIT_LIST_HEAD(&scanned);
>  -
>   	spin_lock(&sbi->s_es_lru_lock);
>  +
>  +	/*
>  +	 * If the inode that is at the head of LRU list is newer than
>  +	 * last_sorted time, that means that we need to sort this list.
>  +	 */
>  +	ei = list_first_entry(&sbi->s_es_lru, struct ext4_inode_info, i_es_lru);
>  +	if (sbi->s_es_last_sorted < ei->i_touch_when) {
>  +		list_sort(NULL, &sbi->s_es_lru, ext4_inode_touch_time_cmp);
>  +		sbi->s_es_last_sorted = jiffies;
>  +	}
>  +
They are sorting the list, but that doesn't change the ammount of items they report.
So far it is fine. My only concern here would be locking, but it seems to be all
protected by the s_es_lru_lock.

>   	list_for_each_safe(cur, tmp, &sbi->s_es_lru) {
>  -		list_move_tail(cur, &scanned);
>  +		/*
>  +		 * If we have already reclaimed all extents from extent
>  +		 * status tree, just stop the loop immediately.
>  +		 */
>  +		if (percpu_counter_read_positive(&sbi->s_extent_cache_cnt) == 0)
>  +			break;
>   
>   		ei = list_entry(cur, struct ext4_inode_info, i_es_lru);
>   
> @@@ -951,22 -923,22 +956,22 @@@
>   		if (nr_to_scan == 0)
>   			break;
>   	}
>  -	list_splice_tail(&scanned, &sbi->s_es_lru);
>  +
>  +	/* Move the newer inodes into the tail of the LRU list. */
>  +	list_splice_tail(&skiped, &sbi->s_es_lru);
>   	spin_unlock(&sbi->s_es_lru_lock);
>   
> - 	ret = percpu_counter_read_positive(&sbi->s_extent_cache_cnt);
>   	trace_ext4_es_shrink_exit(sbi->s_sb, nr_shrunk, ret);
> - 	return ret;
> + 	return nr_shrunk;
>   }
>   
This part also seems fine.

>  -void ext4_es_register_shrinker(struct super_block *sb)
>  +void ext4_es_register_shrinker(struct ext4_sb_info *sbi)
>   {
>  -	struct ext4_sb_info *sbi;
>  -
>  -	sbi = EXT4_SB(sb);
>   	INIT_LIST_HEAD(&sbi->s_es_lru);
>   	spin_lock_init(&sbi->s_es_lru_lock);
>  +	sbi->s_es_last_sorted = 0;
> - 	sbi->s_es_shrinker.shrink = ext4_es_shrink;
> + 	sbi->s_es_shrinker.scan_objects = ext4_es_scan;
> + 	sbi->s_es_shrinker.count_objects = ext4_es_count;
>   	sbi->s_es_shrinker.seeks = DEFAULT_SEEKS;
>   	register_shrinker(&sbi->s_es_shrinker);
>   }

And so does this.

I believe the resolution is okay, at least from our PoV.

linux-next: manual merge of the akpm tree with the ext4 tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 4069 bytes --]

Hi Andrew,

Today's linux-next merge of the akpm tree got a conflict in
fs/ext4/extents_status.c between commit 6480bad916be ("ext4: improve
extent cache shrink mechanism to avoid to burn CPU time") from the ext
tree and commit 1f42d0934b4e ("fs: convert fs shrinkers to new scan/count
API") from the akpm tree.

I fixed it up (I am not sure if the result makes complete sense - see
below) and can carry the fix as necessary (no action is required).

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

diff --cc fs/ext4/extents_status.c
index 80dcc59,4bce4f0..0000000
--- a/fs/ext4/extents_status.c
+++ b/fs/ext4/extents_status.c
@@@ -876,58 -878,32 +876,63 @@@ int ext4_es_zeroout(struct inode *inode
  				     EXTENT_STATUS_WRITTEN);
  }
  
 +static int ext4_inode_touch_time_cmp(void *priv, struct list_head *a,
 +				     struct list_head *b)
 +{
 +	struct ext4_inode_info *eia, *eib;
 +	unsigned long diff;
 +
 +	eia = list_entry(a, struct ext4_inode_info, i_es_lru);
 +	eib = list_entry(b, struct ext4_inode_info, i_es_lru);
 +
 +	diff = eia->i_touch_when - eib->i_touch_when;
 +	if (diff < 0)
 +		return -1;
 +	if (diff > 0)
 +		return 1;
 +	return 0;
 +}
  
- static int ext4_es_shrink(struct shrinker *shrink, struct shrink_control *sc)
+ static long ext4_es_count(struct shrinker *shrink, struct shrink_control *sc)
+ {
+ 	long nr;
+ 	struct ext4_sb_info *sbi = container_of(shrink,
+ 					struct ext4_sb_info, s_es_shrinker);
+ 
+ 	nr = percpu_counter_read_positive(&sbi->s_extent_cache_cnt);
+ 	trace_ext4_es_shrink_enter(sbi->s_sb, sc->nr_to_scan, nr);
+ 	return nr;
+ }
+ 
+ static long ext4_es_scan(struct shrinker *shrink, struct shrink_control *sc)
  {
  	struct ext4_sb_info *sbi = container_of(shrink,
  					struct ext4_sb_info, s_es_shrinker);
  	struct ext4_inode_info *ei;
 -	struct list_head *cur, *tmp, scanned;
 +	struct list_head *cur, *tmp;
 +	LIST_HEAD(skiped);
  	int nr_to_scan = sc->nr_to_scan;
- 	int ret, nr_shrunk = 0;
- 
- 	ret = percpu_counter_read_positive(&sbi->s_extent_cache_cnt);
- 	trace_ext4_es_shrink_enter(sbi->s_sb, nr_to_scan, ret);
- 
- 	if (!nr_to_scan)
- 		return ret;
+ 	int ret = 0, nr_shrunk = 0;
  
 -	INIT_LIST_HEAD(&scanned);
 -
  	spin_lock(&sbi->s_es_lru_lock);
 +
 +	/*
 +	 * If the inode that is at the head of LRU list is newer than
 +	 * last_sorted time, that means that we need to sort this list.
 +	 */
 +	ei = list_first_entry(&sbi->s_es_lru, struct ext4_inode_info, i_es_lru);
 +	if (sbi->s_es_last_sorted < ei->i_touch_when) {
 +		list_sort(NULL, &sbi->s_es_lru, ext4_inode_touch_time_cmp);
 +		sbi->s_es_last_sorted = jiffies;
 +	}
 +
  	list_for_each_safe(cur, tmp, &sbi->s_es_lru) {
 -		list_move_tail(cur, &scanned);
 +		/*
 +		 * If we have already reclaimed all extents from extent
 +		 * status tree, just stop the loop immediately.
 +		 */
 +		if (percpu_counter_read_positive(&sbi->s_extent_cache_cnt) == 0)
 +			break;
  
  		ei = list_entry(cur, struct ext4_inode_info, i_es_lru);
  
@@@ -951,22 -923,22 +956,22 @@@
  		if (nr_to_scan == 0)
  			break;
  	}
 -	list_splice_tail(&scanned, &sbi->s_es_lru);
 +
 +	/* Move the newer inodes into the tail of the LRU list. */
 +	list_splice_tail(&skiped, &sbi->s_es_lru);
  	spin_unlock(&sbi->s_es_lru_lock);
  
- 	ret = percpu_counter_read_positive(&sbi->s_extent_cache_cnt);
  	trace_ext4_es_shrink_exit(sbi->s_sb, nr_shrunk, ret);
- 	return ret;
+ 	return nr_shrunk;
  }
  
 -void ext4_es_register_shrinker(struct super_block *sb)
 +void ext4_es_register_shrinker(struct ext4_sb_info *sbi)
  {
 -	struct ext4_sb_info *sbi;
 -
 -	sbi = EXT4_SB(sb);
  	INIT_LIST_HEAD(&sbi->s_es_lru);
  	spin_lock_init(&sbi->s_es_lru_lock);
 +	sbi->s_es_last_sorted = 0;
- 	sbi->s_es_shrinker.shrink = ext4_es_shrink;
+ 	sbi->s_es_shrinker.scan_objects = ext4_es_scan;
+ 	sbi->s_es_shrinker.count_objects = ext4_es_count;
  	sbi->s_es_shrinker.seeks = DEFAULT_SEEKS;
  	register_shrinker(&sbi->s_es_shrinker);
  }

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

linux-next: manual merge of the akpm tree with the ext4 tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1459 bytes --]

Hi Andrew,

Today's linux-next merge of the akpm tree got a conflict in
fs/ext4/bitmap.c between commit f6fb99cadcd4 ("ext4: pass a char * to
ext4_count_free() instead of a buffer_head ptr") from the ext4 tree and
commit "ext4: use memweight()" from the akpm tree.

I fixed it up (I hope - see below) and can carry the fix as necessary.

N.B. this code is now used by more that just the EXT4FS_DEBUG code.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

diff --cc fs/ext4/bitmap.c
index a94b9c6,ff53d5d..0000000
--- a/fs/ext4/bitmap.c
+++ b/fs/ext4/bitmap.c
@@@ -11,18 -11,15 +11,11 @@@
  #include <linux/jbd2.h>
  #include "ext4.h"
  
- static const int nibblemap[] = {4, 3, 3, 2, 3, 2, 2, 1, 3, 2, 2, 1, 2, 1, 1, 0};
- 
 -#ifdef EXT4FS_DEBUG
 -
 -unsigned int ext4_count_free(struct buffer_head *map, unsigned int numchars)
 +unsigned int ext4_count_free(char *bitmap, unsigned int numchars)
  {
- 	unsigned int i, sum = 0;
- 
- 	for (i = 0; i < numchars; i++)
- 		sum += nibblemap[bitmap[i] & 0xf] +
- 			nibblemap[(bitmap[i] >> 4) & 0xf];
- 	return sum;
 -	return numchars * BITS_PER_BYTE - memweight(map->b_data, numchars);
++	return numchars * BITS_PER_BYTE - memweight(bitmap, numchars);
  }
  
 -#endif  /*  EXT4FS_DEBUG  */
 -
  int ext4_inode_bitmap_csum_verify(struct super_block *sb, ext4_group_t group,
  				  struct ext4_group_desc *gdp,
  				  struct buffer_head *bh, int sz)

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]