Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1651044 > unrolled thread

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

Back to article view | Back to linux.kernel

Contents

  linux-next: build warning after merge of the akpm-current tree Stephen Rothwell <sfr@canb.auug.org.au> - 2017-05-26 04:50 +0200
    Re: linux-next: build warning after merge of the akpm-current tree Jeff Layton <jlayton@redhat.com> - 2017-05-26 12:20 +0200
      Re: linux-next: build warning after merge of the akpm-current tree Dave Kleikamp <dave.kleikamp@oracle.com> - 2017-05-26 13:30 +0200
      Re: [PATCH] JFS: do not ignore return code from write_one_page() Dave Kleikamp <dave.kleikamp@oracle.com> - 2017-05-27 03:30 +0200
        Re: [PATCH] JFS: do not ignore return code from write_one_page() Andrew Morton <akpm@linux-foundation.org> - 2017-05-27 08:40 +0200
          Re: [PATCH] JFS: do not ignore return code from write_one_page() Stephen Rothwell <sfr@canb.auug.org.au> - 2017-05-29 01:30 +0200
      [PATCH] JFS: do not ignore return code from write_one_page() Dave Kleikamp <dave.kleikamp@oracle.com> - 2017-05-27 03:40 +0200

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

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

[toc] | [next] | [standalone]

#1651286

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>

[toc] | [prev] | [next] | [standalone]

#1651319

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

[toc] | [prev] | [next] | [standalone]

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

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 */
> 

[toc] | [prev] | [next] | [standalone]

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

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.

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

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 */

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web