Josef Bacik
2011-Oct-04 15:54 UTC
[PATCH] Btrfs: break out of orphan cleanup if we can''t make progress V2
I noticed while running xfstests 83 that if we didn''t have enough space to delete our inode the orphan cleanup would just loop. This is because it keeps finding the same orphan item and keeps trying to kill it but can''t because we don''t get an error back from iput for deleting the inode. So keep track of the last guy we tried to kill, if it''s the same as the one we''re trying to kill currently we know we are having problems and can just error out. I don''t have a way to test this so look hard and make sure it''s right. Thanks, Signed-off-by: Josef Bacik <josef@redhat.com> --- V1->V2: set last_objectid properly fs/btrfs/inode.c | 11 +++++++++++ 1 files changed, 11 insertions(+), 0 deletions(-) diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index 446531a..595a807 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -2230,6 +2230,7 @@ int btrfs_orphan_cleanup(struct btrfs_root *root) struct btrfs_key key, found_key; struct btrfs_trans_handle *trans; struct inode *inode; + u64 last_objectid = 0; int ret = 0, nr_unlink = 0, nr_truncate = 0; if (cmpxchg(&root->orphan_cleanup_state, 0, ORPHAN_CLEANUP_STARTED)) @@ -2281,6 +2282,16 @@ int btrfs_orphan_cleanup(struct btrfs_root *root) * crossing root thing. we store the inode number in the * offset of the orphan item. */ + + if (found_key.offset == last_objectid) { + printk(KERN_ERR "btrfs: Error removing orphan entry, " + "stopping orphan cleanup\n"); + ret = -EINVAL; + goto out; + } + + last_objectid = found_key.offset; + found_key.objectid = found_key.offset; found_key.type = BTRFS_INODE_ITEM_KEY; found_key.offset = 0; -- 1.7.5.2 -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Mitch Harder
2011-Oct-09 19:14 UTC
Re: [PATCH] Btrfs: break out of orphan cleanup if we can''t make progress V2
On Tue, Oct 4, 2011 at 10:54 AM, Josef Bacik <josef@redhat.com> wrote:> I noticed while running xfstests 83 that if we didn''t have enough space to > delete our inode the orphan cleanup would just loop. This is because it keeps > finding the same orphan item and keeps trying to kill it but can''t because we > don''t get an error back from iput for deleting the inode. So keep track of the > last guy we tried to kill, if it''s the same as the one we''re trying to kill > currently we know we are having problems and can just error out. I don''t have a > way to test this so look hard and make sure it''s right. Thanks, > > Signed-off-by: Josef Bacik <josef@redhat.com> > --- > V1->V2: set last_objectid properly > > fs/btrfs/inode.c | 11 +++++++++++ > 1 files changed, 11 insertions(+), 0 deletions(-) > > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c > index 446531a..595a807 100644 > --- a/fs/btrfs/inode.c > +++ b/fs/btrfs/inode.c > @@ -2230,6 +2230,7 @@ int btrfs_orphan_cleanup(struct btrfs_root *root) > struct btrfs_key key, found_key; > struct btrfs_trans_handle *trans; > struct inode *inode; > + u64 last_objectid = 0; > int ret = 0, nr_unlink = 0, nr_truncate = 0; > > if (cmpxchg(&root->orphan_cleanup_state, 0, ORPHAN_CLEANUP_STARTED)) > @@ -2281,6 +2282,16 @@ int btrfs_orphan_cleanup(struct btrfs_root *root) > * crossing root thing. we store the inode number in the > * offset of the orphan item. > */ > + > + if (found_key.offset == last_objectid) { > + printk(KERN_ERR "btrfs: Error removing orphan entry, " > + "stopping orphan cleanup\n"); > + ret = -EINVAL; > + goto out; > + } > + > + last_objectid = found_key.offset; > + > found_key.objectid = found_key.offset; > found_key.type = BTRFS_INODE_ITEM_KEY; > found_key.offset = 0; > -- > 1.7.5.2 >Just in case it gets lost in the shuffle between github and git.kernel.org, and also for the benefit of those testing Chris Mason''s integration-test branch on github, please note that version 1 of this patch is still in Chris'' integration-test branch, as well as in Josef''s github kernel repos (master and for-chris branch only, the unstable branch has this version of the patch). -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Josef Bacik
2011-Oct-10 00:28 UTC
Re: [PATCH] Btrfs: break out of orphan cleanup if we can''t make progress V2
On Sun, Oct 09, 2011 at 02:14:19PM -0500, Mitch Harder wrote:> On Tue, Oct 4, 2011 at 10:54 AM, Josef Bacik <josef@redhat.com> wrote: > > I noticed while running xfstests 83 that if we didn''t have enough space to > > delete our inode the orphan cleanup would just loop. This is because it keeps > > finding the same orphan item and keeps trying to kill it but can''t because we > > don''t get an error back from iput for deleting the inode. So keep track of the > > last guy we tried to kill, if it''s the same as the one we''re trying to kill > > currently we know we are having problems and can just error out. I don''t have a > > way to test this so look hard and make sure it''s right. Thanks, > > > > Signed-off-by: Josef Bacik <josef@redhat.com> > > --- > > V1->V2: set last_objectid properly > > > > fs/btrfs/inode.c | 11 +++++++++++ > > 1 files changed, 11 insertions(+), 0 deletions(-) > > > > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c > > index 446531a..595a807 100644 > > --- a/fs/btrfs/inode.c > > +++ b/fs/btrfs/inode.c > > @@ -2230,6 +2230,7 @@ int btrfs_orphan_cleanup(struct btrfs_root *root) > > struct btrfs_key key, found_key; > > struct btrfs_trans_handle *trans; > > struct inode *inode; > > + u64 last_objectid = 0; > > int ret = 0, nr_unlink = 0, nr_truncate = 0; > > > > if (cmpxchg(&root->orphan_cleanup_state, 0, ORPHAN_CLEANUP_STARTED)) > > @@ -2281,6 +2282,16 @@ int btrfs_orphan_cleanup(struct btrfs_root *root) > > * crossing root thing. we store the inode number in the > > * offset of the orphan item. > > */ > > + > > + if (found_key.offset == last_objectid) { > > + printk(KERN_ERR "btrfs: Error removing orphan entry, " > > + "stopping orphan cleanup\n"); > > + ret = -EINVAL; > > + goto out; > > + } > > + > > + last_objectid = found_key.offset; > > + > > found_key.objectid = found_key.offset; > > found_key.type = BTRFS_INODE_ITEM_KEY; > > found_key.offset = 0; > > -- > > 1.7.5.2 > > > > Just in case it gets lost in the shuffle between github and > git.kernel.org, and also for the benefit of those testing Chris > Mason''s integration-test branch on github, please note that version 1 > of this patch is still in Chris'' integration-test branch, as well as > in Josef''s github kernel repos (master and for-chris branch only, the > unstable branch has this version of the patch).I''ve migrated back to git.kernel.org btw so the right version should be in git.kernel.org. Would it be usefull if I keep up the github tree as well? Thanks, Josef -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Mitch Harder
2011-Oct-10 15:08 UTC
Re: [PATCH] Btrfs: break out of orphan cleanup if we can''t make progress V2
On Sun, Oct 9, 2011 at 7:28 PM, Josef Bacik <josef@redhat.com> wrote:> On Sun, Oct 09, 2011 at 02:14:19PM -0500, Mitch Harder wrote: >> On Tue, Oct 4, 2011 at 10:54 AM, Josef Bacik <josef@redhat.com> wrote: >> > I noticed while running xfstests 83 that if we didn''t have enough space to >> > delete our inode the orphan cleanup would just loop. This is because it keeps >> > finding the same orphan item and keeps trying to kill it but can''t because we >> > don''t get an error back from iput for deleting the inode. So keep track of the >> > last guy we tried to kill, if it''s the same as the one we''re trying to kill >> > currently we know we are having problems and can just error out. I don''t have a >> > way to test this so look hard and make sure it''s right. Thanks, >> > >> > Signed-off-by: Josef Bacik <josef@redhat.com> >> > --- >> > V1->V2: set last_objectid properly >> > >> > fs/btrfs/inode.c | 11 +++++++++++ >> > 1 files changed, 11 insertions(+), 0 deletions(-) >> > >> > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c >> > index 446531a..595a807 100644 >> > --- a/fs/btrfs/inode.c >> > +++ b/fs/btrfs/inode.c >> > @@ -2230,6 +2230,7 @@ int btrfs_orphan_cleanup(struct btrfs_root *root) >> > struct btrfs_key key, found_key; >> > struct btrfs_trans_handle *trans; >> > struct inode *inode; >> > + u64 last_objectid = 0; >> > int ret = 0, nr_unlink = 0, nr_truncate = 0; >> > >> > if (cmpxchg(&root->orphan_cleanup_state, 0, ORPHAN_CLEANUP_STARTED)) >> > @@ -2281,6 +2282,16 @@ int btrfs_orphan_cleanup(struct btrfs_root *root) >> > * crossing root thing. we store the inode number in the >> > * offset of the orphan item. >> > */ >> > + >> > + if (found_key.offset == last_objectid) { >> > + printk(KERN_ERR "btrfs: Error removing orphan entry, " >> > + "stopping orphan cleanup\n"); >> > + ret = -EINVAL; >> > + goto out; >> > + } >> > + >> > + last_objectid = found_key.offset; >> > + >> > found_key.objectid = found_key.offset; >> > found_key.type = BTRFS_INODE_ITEM_KEY; >> > found_key.offset = 0; >> > -- >> > 1.7.5.2 >> > >> >> Just in case it gets lost in the shuffle between github and >> git.kernel.org, and also for the benefit of those testing Chris >> Mason''s integration-test branch on github, please note that version 1 >> of this patch is still in Chris'' integration-test branch, as well as >> in Josef''s github kernel repos (master and for-chris branch only, the >> unstable branch has this version of the patch). > > I''ve migrated back to git.kernel.org btw so the right version should be in > git.kernel.org. Would it be usefull if I keep up the github tree as well? > Thanks, > > Josef >As far as I''m concerned, maintaining multiple public trees isn''t necessary. I just wanted to make sure this patch wasn''t lost in the shuffle. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html