re: nilfs2: fix issue with flush kernel thread after remount in RO mode because of driver's internal error or metadata corruption

re: nilfs2: fix issue with flush kernel thread after remount in RO mode because of driver's internal error or metadata corruption

[Dan Carpenter]

[ It's weird that kbuild didn't catch this ].

Hello Vyacheslav Dubeyko,

The patch bb594c4767b0: "nilfs2: fix issue with flush kernel thread
after remount in RO mode because of driver's internal error or
metadata corruption" from Apr 18, 2013, leads to the following 
warning:
"fs/nilfs2/inode.c:211 nilfs_writepage()
	 error: we previously assumed 'inode' could be null (see line 195)"

fs/nilfs2/inode.c
   190  static int nilfs_writepage(struct page *page, struct writeback_control *wbc)
   191  {
   192          struct inode *inode = page->mapping->host;
   193          int err;
   194  
   195          if (inode && (inode->i_sb->s_flags & MS_RDONLY)) {
                    ^^^^^
New check.

   196                  /*
   197                   * It means that filesystem was remounted in read-only
   198                   * mode because of error or metadata corruption. But we
   199                   * have dirty pages that try to be flushed in background.
   200                   * So, here we simply discard this dirty page.
   201                   */
   202                  nilfs_clear_dirty_page(page, false);
   203                  unlock_page(page);
   204                  return -EROFS;
   205          }
   206  
   207          redirty_page_for_writepage(wbc, page);
   208          unlock_page(page);
   209  
   210          if (wbc->sync_mode == WB_SYNC_ALL) {
   211                  err = nilfs_construct_segment(inode->i_sb);
                                                      ^^^^^^^^^^^
Old dereference.

   212                  if (unlikely(err))
   213                          return err;
   214          } else if (wbc->for_reclaim)
   215                  nilfs_flush_segment(inode->i_sb, inode->i_ino);
                                            ^^^^^^^^^^^
Old dereference.

   216  
   217          return 0;
   218  }

regards,
dan carpenter

--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] nilfs2: fix warning in nilfs_writepage()

[Vyacheslav Dubeyko]

On Thu, 2013-04-25 at 10:49 +0300, Dan Carpenter wrote:
> [ It's weird that kbuild didn't catch this ].
> 
> Hello Vyacheslav Dubeyko,
> 
> The patch bb594c4767b0: "nilfs2: fix issue with flush kernel thread
> after remount in RO mode because of driver's internal error or
> metadata corruption" from Apr 18, 2013, leads to the following 
> warning:
> "fs/nilfs2/inode.c:211 nilfs_writepage()
> 	 error: we previously assumed 'inode' could be null (see line 195)"
> 

Please, find below the patch with fix.

Thanks,
Vyacheslav Dubeyko.
--
From: Vyacheslav Dubeyko <slava-yeENwD64cLxBDgjK7y7TUQ@public.gmane.org>
Subject: [PATCH] nilfs2: fix warning in nilfs_writepage()

The patch fixes warning: "fs/nilfs2/inode.c:211 nilfs_writepage() error: we previously assumed 'inode' could be null (see line 195)".

Reported-by: Dan Carpenter <dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Vyacheslav Dubeyko <slava-yeENwD64cLxBDgjK7y7TUQ@public.gmane.org>
CC: Ryusuke Konishi <konishi.ryusuke-Zyj7fXuS5i5L9jVzuh4AOg@public.gmane.org>
---
 fs/nilfs2/inode.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/nilfs2/inode.c b/fs/nilfs2/inode.c
index ba7a1da..cf02f55 100644
--- a/fs/nilfs2/inode.c
+++ b/fs/nilfs2/inode.c
@@ -192,7 +192,7 @@ static int nilfs_writepage(struct page *page, struct writeback_control *wbc)
 	struct inode *inode = page->mapping->host;
 	int err;
 
-	if (inode && (inode->i_sb->s_flags & MS_RDONLY)) {
+	if (inode->i_sb->s_flags & MS_RDONLY) {
 		/*
 		 * It means that filesystem was remounted in read-only
 		 * mode because of error or metadata corruption. But we
-- 
1.7.9.5



--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] nilfs2: fix warning in nilfs_writepage()

[Andrew Morton]

On Thu, 25 Apr 2013 13:38:35 +0400 Vyacheslav Dubeyko <slava-yeENwD64cLxBDgjK7y7TUQ@public.gmane.org> wrote:

> On Thu, 2013-04-25 at 10:49 +0300, Dan Carpenter wrote:
> > [ It's weird that kbuild didn't catch this ].
> > 
> > Hello Vyacheslav Dubeyko,
> > 
> > The patch bb594c4767b0: "nilfs2: fix issue with flush kernel thread
> > after remount in RO mode because of driver's internal error or
> > metadata corruption" from Apr 18, 2013, leads to the following 
> > warning:
> > "fs/nilfs2/inode.c:211 nilfs_writepage()
> > 	 error: we previously assumed 'inode' could be null (see line 195)"
> > 
> 
> Please, find below the patch with fix.
> 
> Thanks,
> Vyacheslav Dubeyko.
> --
> From: Vyacheslav Dubeyko <slava-yeENwD64cLxBDgjK7y7TUQ@public.gmane.org>
> Subject: [PATCH] nilfs2: fix warning in nilfs_writepage()
> 
> The patch fixes warning: "fs/nilfs2/inode.c:211 nilfs_writepage() error: we previously assumed 'inode' could be null (see line 195)".

The changelog doesn't tell us what tool emitted the warning.  Was it sparse?

> --- a/fs/nilfs2/inode.c
> +++ b/fs/nilfs2/inode.c
> @@ -192,7 +192,7 @@ static int nilfs_writepage(struct page *page, struct writeback_control *wbc)
>  	struct inode *inode = page->mapping->host;
>  	int err;
>  
> -	if (inode && (inode->i_sb->s_flags & MS_RDONLY)) {
> +	if (inode->i_sb->s_flags & MS_RDONLY) {
>  		/*
>  		 * It means that filesystem was remounted in read-only
>  		 * mode because of error or metadata corruption. But we

I'll going to fix up the patch description and subject.  In particular,
the objective of this patch isn't really to fix a warning - it's to
remove some unneeded code.  The warning merely alerted us to its
presence.

From: Vyacheslav Dubeyko <slava-yeENwD64cLxBDgjK7y7TUQ@public.gmane.org>
Subject: nilfs2: remove unneeded test in nilfs_writepage()

page->mapping->host cannot be NULL in nilfs_writepage(), so remove the
unneeded test.

The fixes the sparse warning: "fs/nilfs2/inode.c:211 nilfs_writepage()
error: we previously assumed 'inode' could be null (see line 195)".

Reported-by: Dan Carpenter <dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Vyacheslav Dubeyko <slava-yeENwD64cLxBDgjK7y7TUQ@public.gmane.org>
Cc: Ryusuke Konishi <konishi.ryusuke-Zyj7fXuS5i5L9jVzuh4AOg@public.gmane.org>
Signed-off-by: Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
---

 fs/nilfs2/inode.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff -puN fs/nilfs2/inode.c~nilfs2-fix-warning-in-nilfs_writepage fs/nilfs2/inode.c
--- a/fs/nilfs2/inode.c~nilfs2-fix-warning-in-nilfs_writepage
+++ a/fs/nilfs2/inode.c
@@ -192,7 +192,7 @@ static int nilfs_writepage(struct page *
 	struct inode *inode = page->mapping->host;
 	int err;
 
-	if (inode && (inode->i_sb->s_flags & MS_RDONLY)) {
+	if (inode->i_sb->s_flags & MS_RDONLY) {
 		/*
 		 * It means that filesystem was remounted in read-only
 		 * mode because of error or metadata corruption. But we
_

--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] nilfs2: fix warning in nilfs_writepage()

[Dan Carpenter]

On Thu, Apr 25, 2013 at 01:46:10PM -0700, Andrew Morton wrote:
> On Thu, 25 Apr 2013 13:38:35 +0400 Vyacheslav Dubeyko <slava-yeENwD64cLxBDgjK7y7TUQ@public.gmane.org> wrote:
> 
> > On Thu, 2013-04-25 at 10:49 +0300, Dan Carpenter wrote:
> > > [ It's weird that kbuild didn't catch this ].
> > > 
> > > Hello Vyacheslav Dubeyko,
> > > 
> > > The patch bb594c4767b0: "nilfs2: fix issue with flush kernel thread
> > > after remount in RO mode because of driver's internal error or
> > > metadata corruption" from Apr 18, 2013, leads to the following 
> > > warning:
> > > "fs/nilfs2/inode.c:211 nilfs_writepage()
> > > 	 error: we previously assumed 'inode' could be null (see line 195)"
> > > 
> > 
> > Please, find below the patch with fix.
> > 
> > Thanks,
> > Vyacheslav Dubeyko.
> > --
> > From: Vyacheslav Dubeyko <slava-yeENwD64cLxBDgjK7y7TUQ@public.gmane.org>
> > Subject: [PATCH] nilfs2: fix warning in nilfs_writepage()
> > 
> > The patch fixes warning: "fs/nilfs2/inode.c:211 nilfs_writepage() error: we previously assumed 'inode' could be null (see line 195)".
> 
> The changelog doesn't tell us what tool emitted the warning.  Was it sparse?
> 

That's my fault.  It's a Smatch warning.

regards,
dan carpenter


--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [kbuild] nilfs2: fix issue with flush kernel thread after remount in RO mode because of driver's internal error or metadata corruption

[Fengguang Wu]

On Thu, Apr 25, 2013 at 10:49:47AM +0300, Dan Carpenter wrote:
> [ It's weird that kbuild didn't catch this ].

Sorry about that! I made a big change in the test scripts and took
several days to fixup the new bugs. Quite some branches missed the
tests during the time..

Thanks,
Fengguang
--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html