public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] xfs: don't leak root inode reference
@ 2013-08-26 20:47 Ben Myers
  2013-08-26 21:03 ` Al Viro
  2013-08-26 21:24 ` Dave Chinner
  0 siblings, 2 replies; 8+ messages in thread
From: Ben Myers @ 2013-08-26 20:47 UTC (permalink / raw)
  To: xfs; +Cc: Al Viro

Looks like in 48fde701 we removed the iput of the root inode in
xfs_fs_fill_super for the error case.  Add it back.

Signed-off-by: Ben Myers <bpm@sgi.com>

---
 fs/xfs/xfs_super.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Index: b/fs/xfs/xfs_super.c
===================================================================
--- a/fs/xfs/xfs_super.c	2013-08-26 15:36:09.170848579 -0500
+++ b/fs/xfs/xfs_super.c	2013-08-26 15:40:19.450817933 -0500
@@ -1493,12 +1493,12 @@ xfs_fs_fill_super(
 	}
 	if (is_bad_inode(root)) {
 		error = EINVAL;
-		goto out_unmount;
+		goto out_iput;
 	}
 	sb->s_root = d_make_root(root);
 	if (!sb->s_root) {
 		error = ENOMEM;
-		goto out_unmount;
+		goto out_iput;
 	}
 
 	return 0;
@@ -1519,6 +1519,8 @@ out_destroy_workqueues:
  out:
 	return -error;
 
+ out_iput:
+	iput(root);
  out_unmount:
 	xfs_filestream_unmount(mp);
 	xfs_unmountfs(mp);

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH] xfs: don't leak root inode reference
  2013-08-26 20:47 [PATCH] xfs: don't leak root inode reference Ben Myers
@ 2013-08-26 21:03 ` Al Viro
  2013-08-26 21:24 ` Dave Chinner
  1 sibling, 0 replies; 8+ messages in thread
From: Al Viro @ 2013-08-26 21:03 UTC (permalink / raw)
  To: Ben Myers; +Cc: xfs

On Mon, Aug 26, 2013 at 03:47:30PM -0500, Ben Myers wrote:
> Looks like in 48fde701 we removed the iput of the root inode in
> xfs_fs_fill_super for the error case.  Add it back.
>  	if (is_bad_inode(root)) {
>  		error = EINVAL;
> -		goto out_unmount;
> +		goto out_iput;

This one makes sense

>  	}
>  	sb->s_root = d_make_root(root);
>  	if (!sb->s_root) {
>  		error = ENOMEM;
> -		goto out_unmount;
> +		goto out_iput;


... this one, OTOH, is simply wrong - take a look at what d_make_root()
does on failure.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH] xfs: don't leak root inode reference
  2013-08-26 20:47 [PATCH] xfs: don't leak root inode reference Ben Myers
  2013-08-26 21:03 ` Al Viro
@ 2013-08-26 21:24 ` Dave Chinner
  2013-08-27 21:25   ` [PATCH v2] " Ben Myers
  1 sibling, 1 reply; 8+ messages in thread
From: Dave Chinner @ 2013-08-26 21:24 UTC (permalink / raw)
  To: Ben Myers; +Cc: Al Viro, xfs

On Mon, Aug 26, 2013 at 03:47:30PM -0500, Ben Myers wrote:
> Looks like in 48fde701 we removed the iput of the root inode in
> xfs_fs_fill_super for the error case.  Add it back.
> 
> Signed-off-by: Ben Myers <bpm@sgi.com>
> 
> ---
>  fs/xfs/xfs_super.c |    6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> Index: b/fs/xfs/xfs_super.c
> ===================================================================
> --- a/fs/xfs/xfs_super.c	2013-08-26 15:36:09.170848579 -0500
> +++ b/fs/xfs/xfs_super.c	2013-08-26 15:40:19.450817933 -0500
> @@ -1493,12 +1493,12 @@ xfs_fs_fill_super(
>  	}
>  	if (is_bad_inode(root)) {
>  		error = EINVAL;
> -		goto out_unmount;
> +		goto out_iput;
>  	}
>  	sb->s_root = d_make_root(root);
>  	if (!sb->s_root) {
>  		error = ENOMEM;
> -		goto out_unmount;
> +		goto out_iput;
>  	}

That's wrong. d_make_root() drops the reference to the inode on
failure itself, and so the change in 48fde701 is correct and valid.

The leak on bad inodes (which, AFAICT, can never happen on XFS) has
been around a lot longer than Al's change - this commit introduced
it:

	2bcf6e9 xfs: start periodic workers later

with this hunk:

        if (is_bad_inode(root)) {
                error = EINVAL;
-               goto fail_vnrele;
+               goto out_syncd_stop;
        }

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 8+ messages in thread
* [PATCH v2] xfs: don't leak root inode reference
  2013-08-26 21:24 ` Dave Chinner
@ 2013-08-27 21:25   ` Ben Myers
  2013-08-27 22:08     ` Dave Chinner
  0 siblings, 1 reply; 8+ messages in thread
From: Ben Myers @ 2013-08-27 21:25 UTC (permalink / raw)
  To: Dave Chinner; +Cc: Al Viro, xfs

On Tue, Aug 27, 2013 at 07:24:23AM +1000, Dave Chinner wrote:
> On Mon, Aug 26, 2013 at 03:47:30PM -0500, Ben Myers wrote:
> > Looks like in 48fde701 we removed the iput of the root inode in
> > xfs_fs_fill_super for the error case.  Add it back.
> > 
> > Signed-off-by: Ben Myers <bpm@sgi.com>
> > 
> > ---
> >  fs/xfs/xfs_super.c |    6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > Index: b/fs/xfs/xfs_super.c
> > ===================================================================
> > --- a/fs/xfs/xfs_super.c	2013-08-26 15:36:09.170848579 -0500
> > +++ b/fs/xfs/xfs_super.c	2013-08-26 15:40:19.450817933 -0500
> > @@ -1493,12 +1493,12 @@ xfs_fs_fill_super(
> >  	}
> >  	if (is_bad_inode(root)) {
> >  		error = EINVAL;
> > -		goto out_unmount;
> > +		goto out_iput;
> >  	}
> >  	sb->s_root = d_make_root(root);
> >  	if (!sb->s_root) {
> >  		error = ENOMEM;
> > -		goto out_unmount;
> > +		goto out_iput;
> >  	}
> 
> That's wrong. d_make_root() drops the reference to the inode on
> failure itself, and so the change in 48fde701 is correct and valid.
> 
> The leak on bad inodes (which, AFAICT, can never happen on XFS) has
> been around a lot longer than Al's change - this commit introduced
> it:
> 
> 	2bcf6e9 xfs: start periodic workers later
> 
> with this hunk:
> 
>         if (is_bad_inode(root)) {
>                 error = EINVAL;
> -               goto fail_vnrele;
> +               goto out_syncd_stop;
>         }

Thanks Gents.  Here's another try:

xfs: don't leak root inode reference

Looks like in 2bcf6e9 we removed the iput of the root inode in
xfs_fs_fill_super for the is_bad_inode error case.  Add it back.

Signed-off-by: Ben Myers <bpm@sgi.com>

---

 fs/xfs/xfs_super.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Index: b/fs/xfs/xfs_super.c
===================================================================
--- a/fs/xfs/xfs_super.c	2013-08-26 15:43:55.530817462 -0500
+++ b/fs/xfs/xfs_super.c	2013-08-27 16:20:50.100857436 -0500
@@ -1493,7 +1493,7 @@ xfs_fs_fill_super(
 	}
 	if (is_bad_inode(root)) {
 		error = EINVAL;
-		goto out_unmount;
+		goto out_iput;
 	}
 	sb->s_root = d_make_root(root);
 	if (!sb->s_root) {
@@ -1519,6 +1519,8 @@ out_destroy_workqueues:
  out:
 	return -error;
 
+ out_iput:
+	iput(root);
  out_unmount:
 	xfs_filestream_unmount(mp);
 	xfs_unmountfs(mp);

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH v2] xfs: don't leak root inode reference
  2013-08-27 21:25   ` [PATCH v2] " Ben Myers
@ 2013-08-27 22:08     ` Dave Chinner
  2013-09-10 23:11       ` Ben Myers
  0 siblings, 1 reply; 8+ messages in thread
From: Dave Chinner @ 2013-08-27 22:08 UTC (permalink / raw)
  To: Ben Myers; +Cc: Al Viro, xfs

On Tue, Aug 27, 2013 at 04:25:58PM -0500, Ben Myers wrote:
> On Tue, Aug 27, 2013 at 07:24:23AM +1000, Dave Chinner wrote:
> > On Mon, Aug 26, 2013 at 03:47:30PM -0500, Ben Myers wrote:
> > > Looks like in 48fde701 we removed the iput of the root inode in
> > > xfs_fs_fill_super for the error case.  Add it back.
> > > 
> > > Signed-off-by: Ben Myers <bpm@sgi.com>
> > > 
> > > ---
> > >  fs/xfs/xfs_super.c |    6 ++++--
> > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > > 
> > > Index: b/fs/xfs/xfs_super.c
> > > ===================================================================
> > > --- a/fs/xfs/xfs_super.c	2013-08-26 15:36:09.170848579 -0500
> > > +++ b/fs/xfs/xfs_super.c	2013-08-26 15:40:19.450817933 -0500
> > > @@ -1493,12 +1493,12 @@ xfs_fs_fill_super(
> > >  	}
> > >  	if (is_bad_inode(root)) {
> > >  		error = EINVAL;
> > > -		goto out_unmount;
> > > +		goto out_iput;
> > >  	}
> > >  	sb->s_root = d_make_root(root);
> > >  	if (!sb->s_root) {
> > >  		error = ENOMEM;
> > > -		goto out_unmount;
> > > +		goto out_iput;
> > >  	}
> > 
> > That's wrong. d_make_root() drops the reference to the inode on
> > failure itself, and so the change in 48fde701 is correct and valid.
> > 
> > The leak on bad inodes (which, AFAICT, can never happen on XFS) has
> > been around a lot longer than Al's change - this commit introduced
> > it:
> > 
> > 	2bcf6e9 xfs: start periodic workers later
> > 
> > with this hunk:
> > 
> >         if (is_bad_inode(root)) {
> >                 error = EINVAL;
> > -               goto fail_vnrele;
> > +               goto out_syncd_stop;
> >         }
> 
> Thanks Gents.  Here's another try:
> 
> xfs: don't leak root inode reference
> 
> Looks like in 2bcf6e9 we removed the iput of the root inode in
> xfs_fs_fill_super for the is_bad_inode error case.  Add it back.
> 
> Signed-off-by: Ben Myers <bpm@sgi.com>

I don't think this is right, either.

As I said in my previous reply, I don't think that XFS can ever see
a bad inode. The fact is that we're grabbing mp->m_rootip, which is
we already have a reference to and is in cache and validated thanks
to an xfs_iget() call in xfs_mountfs().  If we fail validation when
reading the root inode into cache then xfs_mountfs() will fail and
we won't ever get to this check.

Further, XFS never marks inodes bad - even on a failed lookup or a
shut down filesystem - and so AFAICT we cannot ever see the root
inode (or any other XFS inode) as a bad inode.

Hence I think that the is_bad_inode(root) check should just go away.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH v2] xfs: don't leak root inode reference
  2013-08-27 22:08     ` Dave Chinner
@ 2013-09-10 23:11       ` Ben Myers
  2013-09-18  3:13         ` Dave Chinner
  2013-10-01 22:40         ` Ben Myers
  0 siblings, 2 replies; 8+ messages in thread
From: Ben Myers @ 2013-09-10 23:11 UTC (permalink / raw)
  To: Dave Chinner; +Cc: Al Viro, xfs

Hi Dave,

On Wed, Aug 28, 2013 at 08:08:33AM +1000, Dave Chinner wrote:
> On Tue, Aug 27, 2013 at 04:25:58PM -0500, Ben Myers wrote:
> > On Tue, Aug 27, 2013 at 07:24:23AM +1000, Dave Chinner wrote:
> > > On Mon, Aug 26, 2013 at 03:47:30PM -0500, Ben Myers wrote:
> > > > Looks like in 48fde701 we removed the iput of the root inode in
> > > > xfs_fs_fill_super for the error case.  Add it back.
> > > > 
> > > > Signed-off-by: Ben Myers <bpm@sgi.com>
> > > > 
> > > > ---
> > > >  fs/xfs/xfs_super.c |    6 ++++--
> > > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > > > 
> > > > Index: b/fs/xfs/xfs_super.c
> > > > ===================================================================
> > > > --- a/fs/xfs/xfs_super.c	2013-08-26 15:36:09.170848579 -0500
> > > > +++ b/fs/xfs/xfs_super.c	2013-08-26 15:40:19.450817933 -0500
> > > > @@ -1493,12 +1493,12 @@ xfs_fs_fill_super(
> > > >  	}
> > > >  	if (is_bad_inode(root)) {
> > > >  		error = EINVAL;
> > > > -		goto out_unmount;
> > > > +		goto out_iput;
> > > >  	}
> > > >  	sb->s_root = d_make_root(root);
> > > >  	if (!sb->s_root) {
> > > >  		error = ENOMEM;
> > > > -		goto out_unmount;
> > > > +		goto out_iput;
> > > >  	}
> > > 
> > > That's wrong. d_make_root() drops the reference to the inode on
> > > failure itself, and so the change in 48fde701 is correct and valid.
> > > 
> > > The leak on bad inodes (which, AFAICT, can never happen on XFS) has
> > > been around a lot longer than Al's change - this commit introduced
> > > it:
> > > 
> > > 	2bcf6e9 xfs: start periodic workers later
> > > 
> > > with this hunk:
> > > 
> > >         if (is_bad_inode(root)) {
> > >                 error = EINVAL;
> > > -               goto fail_vnrele;
> > > +               goto out_syncd_stop;
> > >         }
> > 
> > Thanks Gents.  Here's another try:
> > 
> > xfs: don't leak root inode reference
> > 
> > Looks like in 2bcf6e9 we removed the iput of the root inode in
> > xfs_fs_fill_super for the is_bad_inode error case.  Add it back.
> > 
> > Signed-off-by: Ben Myers <bpm@sgi.com>
> 
> I don't think this is right, either.
> 
> As I said in my previous reply, I don't think that XFS can ever see
> a bad inode. The fact is that we're grabbing mp->m_rootip, which is
> we already have a reference to and is in cache and validated thanks
> to an xfs_iget() call in xfs_mountfs().  If we fail validation when
> reading the root inode into cache then xfs_mountfs() will fail and
> we won't ever get to this check.
> 
> Further, XFS never marks inodes bad - even on a failed lookup or a
> shut down filesystem - and so AFAICT we cannot ever see the root
> inode (or any other XFS inode) as a bad inode.
> 
> Hence I think that the is_bad_inode(root) check should just go away.

Ok.  Lets try this.  ;)

xfs: remove usage of is_bad_inode

XFS never calls mark_inode_bad or iget_failed, so it will never see a
bad inode.  Remove all checks for is_bad_inode because they are
unnecessary.

Signed-off-by: Ben Myers <bpm@sgi.com>
---
 fs/xfs/xfs_icache.c |    7 -------
 fs/xfs/xfs_inode.c  |    2 +-
 fs/xfs/xfs_super.c  |    9 ---------
 3 files changed, 1 insertion(+), 17 deletions(-)

Index: b/fs/xfs/xfs_icache.c
===================================================================
--- a/fs/xfs/xfs_icache.c	2013-09-10 14:55:38.574338256 -0500
+++ b/fs/xfs/xfs_icache.c	2013-09-10 14:56:09.514377472 -0500
@@ -501,11 +501,6 @@ xfs_inode_ag_walk_grab(
 	if (!igrab(inode))
 		return ENOENT;
 
-	if (is_bad_inode(inode)) {
-		IRELE(ip);
-		return ENOENT;
-	}
-
 	/* inode is valid */
 	return 0;
 
@@ -919,8 +914,6 @@ restart:
 		xfs_iflock(ip);
 	}
 
-	if (is_bad_inode(VFS_I(ip)))
-		goto reclaim;
 	if (XFS_FORCED_SHUTDOWN(ip->i_mount)) {
 		xfs_iunpin_wait(ip);
 		xfs_iflush_abort(ip, false);
Index: b/fs/xfs/xfs_inode.c
===================================================================
--- a/fs/xfs/xfs_inode.c	2013-09-10 14:56:38.124337715 -0500
+++ b/fs/xfs/xfs_inode.c	2013-09-10 14:56:40.777424778 -0500
@@ -1687,7 +1687,7 @@ xfs_inactive(
 	 * If the inode is already free, then there can be nothing
 	 * to clean up here.
 	 */
-	if (ip->i_d.di_mode == 0 || is_bad_inode(VFS_I(ip))) {
+	if (ip->i_d.di_mode == 0) {
 		ASSERT(ip->i_df.if_real_bytes == 0);
 		ASSERT(ip->i_df.if_broot_bytes == 0);
 		return VN_INACTIVE_CACHE;
Index: b/fs/xfs/xfs_super.c
===================================================================
--- a/fs/xfs/xfs_super.c	2013-09-10 14:38:48.144338844 -0500
+++ b/fs/xfs/xfs_super.c	2013-09-10 14:57:06.793813184 -0500
@@ -946,10 +946,6 @@ xfs_fs_destroy_inode(
 
 	XFS_STATS_INC(vn_reclaim);
 
-	/* bad inode, get out here ASAP */
-	if (is_bad_inode(inode))
-		goto out_reclaim;
-
 	ASSERT(XFS_FORCED_SHUTDOWN(ip->i_mount) || ip->i_delayed_blks == 0);
 
 	/*
@@ -965,7 +961,6 @@ xfs_fs_destroy_inode(
 	 * this more efficiently than we can here, so simply let background
 	 * reclaim tear down all inodes.
 	 */
-out_reclaim:
 	xfs_inode_set_reclaim_tag(ip);
 }
 
@@ -1491,10 +1486,6 @@ xfs_fs_fill_super(
 		error = ENOENT;
 		goto out_unmount;
 	}
-	if (is_bad_inode(root)) {
-		error = EINVAL;
-		goto out_unmount;
-	}
 	sb->s_root = d_make_root(root);
 	if (!sb->s_root) {
 		error = ENOMEM;

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH v2] xfs: don't leak root inode reference
  2013-09-10 23:11       ` Ben Myers
@ 2013-09-18  3:13         ` Dave Chinner
  2013-10-01 22:40         ` Ben Myers
  1 sibling, 0 replies; 8+ messages in thread
From: Dave Chinner @ 2013-09-18  3:13 UTC (permalink / raw)
  To: Ben Myers; +Cc: Al Viro, xfs

On Tue, Sep 10, 2013 at 06:11:22PM -0500, Ben Myers wrote:
> Ok.  Lets try this.  ;)
> 
> xfs: remove usage of is_bad_inode
> 
> XFS never calls mark_inode_bad or iget_failed, so it will never see a
> bad inode.  Remove all checks for is_bad_inode because they are
> unnecessary.
> 
> Signed-off-by: Ben Myers <bpm@sgi.com>

Looks good to me.

Reviewed-by: Dave Chinner <dchinner@redhat.com>
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH v2] xfs: don't leak root inode reference
  2013-09-10 23:11       ` Ben Myers
  2013-09-18  3:13         ` Dave Chinner
@ 2013-10-01 22:40         ` Ben Myers
  1 sibling, 0 replies; 8+ messages in thread
From: Ben Myers @ 2013-10-01 22:40 UTC (permalink / raw)
  To: Ben Myers; +Cc: xfs

On Tue, Sep 10, 2013 at 06:11:22PM -0500, Ben Myers wrote:
> Hi Dave,
> 
> On Wed, Aug 28, 2013 at 08:08:33AM +1000, Dave Chinner wrote:
> > On Tue, Aug 27, 2013 at 04:25:58PM -0500, Ben Myers wrote:
> > > On Tue, Aug 27, 2013 at 07:24:23AM +1000, Dave Chinner wrote:
> > > > On Mon, Aug 26, 2013 at 03:47:30PM -0500, Ben Myers wrote:
> > > > > Looks like in 48fde701 we removed the iput of the root inode in
> > > > > xfs_fs_fill_super for the error case.  Add it back.
> > > > > 
> > > > > Signed-off-by: Ben Myers <bpm@sgi.com>
> > > > > 
> > > > > ---
> > > > >  fs/xfs/xfs_super.c |    6 ++++--
> > > > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > > > > 
> > > > > Index: b/fs/xfs/xfs_super.c
> > > > > ===================================================================
> > > > > --- a/fs/xfs/xfs_super.c	2013-08-26 15:36:09.170848579 -0500
> > > > > +++ b/fs/xfs/xfs_super.c	2013-08-26 15:40:19.450817933 -0500
> > > > > @@ -1493,12 +1493,12 @@ xfs_fs_fill_super(
> > > > >  	}
> > > > >  	if (is_bad_inode(root)) {
> > > > >  		error = EINVAL;
> > > > > -		goto out_unmount;
> > > > > +		goto out_iput;
> > > > >  	}
> > > > >  	sb->s_root = d_make_root(root);
> > > > >  	if (!sb->s_root) {
> > > > >  		error = ENOMEM;
> > > > > -		goto out_unmount;
> > > > > +		goto out_iput;
> > > > >  	}
> > > > 
> > > > That's wrong. d_make_root() drops the reference to the inode on
> > > > failure itself, and so the change in 48fde701 is correct and valid.
> > > > 
> > > > The leak on bad inodes (which, AFAICT, can never happen on XFS) has
> > > > been around a lot longer than Al's change - this commit introduced
> > > > it:
> > > > 
> > > > 	2bcf6e9 xfs: start periodic workers later
> > > > 
> > > > with this hunk:
> > > > 
> > > >         if (is_bad_inode(root)) {
> > > >                 error = EINVAL;
> > > > -               goto fail_vnrele;
> > > > +               goto out_syncd_stop;
> > > >         }
> > > 
> > > Thanks Gents.  Here's another try:
> > > 
> > > xfs: don't leak root inode reference
> > > 
> > > Looks like in 2bcf6e9 we removed the iput of the root inode in
> > > xfs_fs_fill_super for the is_bad_inode error case.  Add it back.
> > > 
> > > Signed-off-by: Ben Myers <bpm@sgi.com>
> > 
> > I don't think this is right, either.
> > 
> > As I said in my previous reply, I don't think that XFS can ever see
> > a bad inode. The fact is that we're grabbing mp->m_rootip, which is
> > we already have a reference to and is in cache and validated thanks
> > to an xfs_iget() call in xfs_mountfs().  If we fail validation when
> > reading the root inode into cache then xfs_mountfs() will fail and
> > we won't ever get to this check.
> > 
> > Further, XFS never marks inodes bad - even on a failed lookup or a
> > shut down filesystem - and so AFAICT we cannot ever see the root
> > inode (or any other XFS inode) as a bad inode.
> > 
> > Hence I think that the is_bad_inode(root) check should just go away.
> 
> Ok.  Lets try this.  ;)
> 
> xfs: remove usage of is_bad_inode
> 
> XFS never calls mark_inode_bad or iget_failed, so it will never see a
> bad inode.  Remove all checks for is_bad_inode because they are
> unnecessary.
> 
> Signed-off-by: Ben Myers <bpm@sgi.com>

Applied this one.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 8+ messages in thread
end of thread, other threads:[~2013-10-01 22:40 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-08-26 20:47 [PATCH] xfs: don't leak root inode reference Ben Myers
2013-08-26 21:03 ` Al Viro
2013-08-26 21:24 ` Dave Chinner
2013-08-27 21:25   ` [PATCH v2] " Ben Myers
2013-08-27 22:08     ` Dave Chinner
2013-09-10 23:11       ` Ben Myers
2013-09-18  3:13         ` Dave Chinner
2013-10-01 22:40         ` Ben Myers

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox