linux-next: build warning after merge of the akpm-current tree

linux-next: build warning after merge of the akpm-current tree

[Stephen Rothwell]

Hi Andrew,

After merging the akpm-current tree, today's linux-next build (powerpc
ppc64_defconfig) produced this warning:

fs/jfs/jfs_metapage.c: In function 'force_metapage':
fs/jfs/jfs_metapage.c:714:2: warning: ignoring return value of 'write_one_page', declared with attribute warn_unused_result [-Wunused-result]
  write_one_page(page);
  ^
fs/jfs/jfs_metapage.c: In function 'release_metapage':
fs/jfs/jfs_metapage.c:759:4: warning: ignoring return value of 'write_one_page', declared with attribute warn_unused_result [-Wunused-result]
    write_one_page(page);
    ^

Introduced by commit

  f8652aebee02 ("mm: drop "wait" parameter from write_one_page()")

These call sites were updated for the droppping of the argument, but
not for the addition of __must_check :-(

-- 
Cheers,
Stephen Rothwell

Re: linux-next: build warning after merge of the akpm-current tree

[Jeff Layton]

On Fri, 2017-05-26 at 12:43 +1000, Stephen Rothwell wrote:
> Hi Andrew,
> 
> After merging the akpm-current tree, today's linux-next build (powerpc
> ppc64_defconfig) produced this warning:
> 
> fs/jfs/jfs_metapage.c: In function 'force_metapage':
> fs/jfs/jfs_metapage.c:714:2: warning: ignoring return value of 'write_one_page', declared with attribute warn_unused_result [-Wunused-result]
>   write_one_page(page);
>   ^
> fs/jfs/jfs_metapage.c: In function 'release_metapage':
> fs/jfs/jfs_metapage.c:759:4: warning: ignoring return value of 'write_one_page', declared with attribute warn_unused_result [-Wunused-result]
>     write_one_page(page);
>     ^
> 
> Introduced by commit
> 
>   f8652aebee02 ("mm: drop "wait" parameter from write_one_page()")
> 
> These call sites were updated for the droppping of the argument, but
> not for the addition of __must_check :-(
> 

(cc'ing Dave...)

Yeah, that's a known issue. When Willy reviewed the patch originally he
asked me to add a __must_check there so that JFS would pick up some
warnings for this.

JFS really ought to check the return code there and do something sane
with it. Dave?

-- 
Jeff Layton <jlayton@redhat.com>

Re: linux-next: build warning after merge of the akpm-current tree

[Dave Kleikamp]

On 05/26/2017 05:16 AM, Jeff Layton wrote:
> On Fri, 2017-05-26 at 12:43 +1000, Stephen Rothwell wrote:
>> Hi Andrew,
>>
>> After merging the akpm-current tree, today's linux-next build (powerpc
>> ppc64_defconfig) produced this warning:
>>
>> fs/jfs/jfs_metapage.c: In function 'force_metapage':
>> fs/jfs/jfs_metapage.c:714:2: warning: ignoring return value of 'write_one_page', declared with attribute warn_unused_result [-Wunused-result]
>>   write_one_page(page);
>>   ^
>> fs/jfs/jfs_metapage.c: In function 'release_metapage':
>> fs/jfs/jfs_metapage.c:759:4: warning: ignoring return value of 'write_one_page', declared with attribute warn_unused_result [-Wunused-result]
>>     write_one_page(page);
>>     ^
>>
>> Introduced by commit
>>
>>   f8652aebee02 ("mm: drop "wait" parameter from write_one_page()")
>>
>> These call sites were updated for the droppping of the argument, but
>> not for the addition of __must_check :-(
>>
> 
> (cc'ing Dave...)
> 
> Yeah, that's a known issue. When Willy reviewed the patch originally he
> asked me to add a __must_check there so that JFS would pick up some
> warnings for this.
> 
> JFS really ought to check the return code there and do something sane
> with it. Dave?

This is true. I promised to do something about it. I'll try to get a
patch out later today.

Dave

[PATCH] JFS: do not ignore return code from write_one_page()

[Dave Kleikamp]

There are a couple places where jfs calls write_one_page() where
clean recovery is not possible. In these cases, the file system
should be marked dirty. To do this, it is now necessary to store the
superblock in the metapage structure.

Signed-off-by: Dave Kleikamp <dave.kleikamp@oracle.com>
---
 fs/jfs/jfs_metapage.c | 7 +++++--
 fs/jfs/jfs_metapage.h | 1 +
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/fs/jfs/jfs_metapage.c b/fs/jfs/jfs_metapage.c
index 744fa3c079e6..7cc1c85f4508 100644
--- a/fs/jfs/jfs_metapage.c
+++ b/fs/jfs/jfs_metapage.c
@@ -664,6 +664,7 @@ struct metapage *__get_metapage(struct inode *inode, unsigned long lblock,
 		INCREMENT(mpStat.pagealloc);
 		mp = alloc_metapage(GFP_NOFS);
 		mp->page = page;
+		mp->sb = inode->i_sb;
 		mp->flag = 0;
 		mp->xflag = COMMIT_PAGE;
 		mp->count = 1;
@@ -711,7 +712,8 @@ void force_metapage(struct metapage *mp)
 	get_page(page);
 	lock_page(page);
 	set_page_dirty(page);
-	write_one_page(page);
+	if (write_one_page(page))
+		jfs_error(mp->sb, "write_one_page() failed\n");
 	clear_bit(META_forcewrite, &mp->flag);
 	put_page(page);
 }
@@ -756,7 +758,8 @@ void release_metapage(struct metapage * mp)
 		set_page_dirty(page);
 		if (test_bit(META_sync, &mp->flag)) {
 			clear_bit(META_sync, &mp->flag);
-			write_one_page(page);
+			if (write_one_page(page))
+				jfs_error(mp->sb, "write_one_page() failed\n");
 			lock_page(page); /* write_one_page unlocks the page */
 		}
 	} else if (mp->lsn)	/* discard_metapage doesn't remove it */
diff --git a/fs/jfs/jfs_metapage.h b/fs/jfs/jfs_metapage.h
index a869fb4a20d6..8b0ee514eb84 100644
--- a/fs/jfs/jfs_metapage.h
+++ b/fs/jfs/jfs_metapage.h
@@ -38,6 +38,7 @@ struct metapage {
 
 	/* implementation */
 	struct page *page;
+	struct super_block *sb;
 	unsigned int logical_size;
 
 	/* Journal management */

Re: [PATCH] JFS: do not ignore return code from write_one_page()

[Dave Kleikamp]

Andrew,

Do you want to pick this up into akpm-current? I could push it through
the jfs tree, but without the change to write_one_page(), my version of
the patch would need a manual merge. It'd be a simple one, so maybe
that's not a big deal.

Thanks,
Shaggy

On 05/26/2017 03:45 PM, Dave Kleikamp wrote:
> There are a couple places where jfs calls write_one_page() where
> clean recovery is not possible. In these cases, the file system
> should be marked dirty. To do this, it is now necessary to store the
> superblock in the metapage structure.
> 
> Signed-off-by: Dave Kleikamp <dave.kleikamp@oracle.com>
> ---
>  fs/jfs/jfs_metapage.c | 7 +++++--
>  fs/jfs/jfs_metapage.h | 1 +
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/jfs/jfs_metapage.c b/fs/jfs/jfs_metapage.c
> index 744fa3c079e6..7cc1c85f4508 100644
> --- a/fs/jfs/jfs_metapage.c
> +++ b/fs/jfs/jfs_metapage.c
> @@ -664,6 +664,7 @@ struct metapage *__get_metapage(struct inode *inode, unsigned long lblock,
>  		INCREMENT(mpStat.pagealloc);
>  		mp = alloc_metapage(GFP_NOFS);
>  		mp->page = page;
> +		mp->sb = inode->i_sb;
>  		mp->flag = 0;
>  		mp->xflag = COMMIT_PAGE;
>  		mp->count = 1;
> @@ -711,7 +712,8 @@ void force_metapage(struct metapage *mp)
>  	get_page(page);
>  	lock_page(page);
>  	set_page_dirty(page);
> -	write_one_page(page);
> +	if (write_one_page(page))
> +		jfs_error(mp->sb, "write_one_page() failed\n");
>  	clear_bit(META_forcewrite, &mp->flag);
>  	put_page(page);
>  }
> @@ -756,7 +758,8 @@ void release_metapage(struct metapage * mp)
>  		set_page_dirty(page);
>  		if (test_bit(META_sync, &mp->flag)) {
>  			clear_bit(META_sync, &mp->flag);
> -			write_one_page(page);
> +			if (write_one_page(page))
> +				jfs_error(mp->sb, "write_one_page() failed\n");
>  			lock_page(page); /* write_one_page unlocks the page */
>  		}
>  	} else if (mp->lsn)	/* discard_metapage doesn't remove it */
> diff --git a/fs/jfs/jfs_metapage.h b/fs/jfs/jfs_metapage.h
> index a869fb4a20d6..8b0ee514eb84 100644
> --- a/fs/jfs/jfs_metapage.h
> +++ b/fs/jfs/jfs_metapage.h
> @@ -38,6 +38,7 @@ struct metapage {
>  
>  	/* implementation */
>  	struct page *page;
> +	struct super_block *sb;
>  	unsigned int logical_size;
>  
>  	/* Journal management */
> 

Re: [PATCH] JFS: do not ignore return code from write_one_page()

[Andrew Morton]

On Fri, 26 May 2017 15:48:51 -0500 Dave Kleikamp <dave.kleikamp@oracle.com> wrote:

> Andrew,
> 
> Do you want to pick this up into akpm-current? I could push it through
> the jfs tree, but without the change to write_one_page(), my version of
> the patch would need a manual merge. It'd be a simple one, so maybe
> that's not a big deal.

Yes, I'll add this to Jeff's series, thanks.

Re: [PATCH] JFS: do not ignore return code from write_one_page()

[Stephen Rothwell]

Hi Andrew,

On Fri, 26 May 2017 23:36:00 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Fri, 26 May 2017 15:48:51 -0500 Dave Kleikamp <dave.kleikamp@oracle.com> wrote:
> 
> > Andrew,
> > 
> > Do you want to pick this up into akpm-current? I could push it through
> > the jfs tree, but without the change to write_one_page(), my version of
> > the patch would need a manual merge. It'd be a simple one, so maybe
> > that's not a big deal.  
> 
> Yes, I'll add this to Jeff's series, thanks.

I have added this to linux-next today.

-- 
Cheers,
Stephen Rothwell