2013-08-07 05:19:08

by Stephen Rothwell

[permalink] [raw]
Subject: linux-next: manual merge of the akpm tree with the ext4 tree

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 [email protected]

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.


Attachments:
(No filename) (1.42 kB)
(No filename) (836.00 B)
Download all attachments

2013-08-09 22:21:24

by Kees Cook

[permalink] [raw]
Subject: Re: linux-next: manual merge of the akpm tree with the ext4 tree

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 [email protected]
>
> 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

2013-08-09 22:34:10

by Andrew Morton

[permalink] [raw]
Subject: Re: linux-next: manual merge of the akpm tree with the ext4 tree

On Fri, 9 Aug 2013 15:21:04 -0700 Kees Cook <[email protected]> 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 <[email protected]>
Subject: fs-convert-fs-shrinkers-to-new-scan-count-api-fix-fix-2

fix shadowed local, spell "skipped" correctly

Cc: Dave Chinner <[email protected]>
Cc: Glauber Costa <[email protected]>
Cc: Kees Cook <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
---

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
_