| Message ID | 1421905582-4539-3-git-send-email-yangds.fnst@cn.fujitsu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hello, > > When removing a subvol, if the related qgroup has no child qgroup, we should destroy > it at the same time. Also remove the TODO entry in qgroup.c. My two cents here: Take a quick look at this, i am not sure whether this is right way to do it. Actually, i think we can only remove a subvolume qgroup safely when both refer and excl are 0, because for subvolume removal case, to make qgroup accounting right, we need walk tree. At this time, qgroup walking might have not finished, if we remove this qgroup directly, accounting for this qgroup will be skipped. Maybe we could remove such qgroup when at qgroup accounting, we find an qgroup’s refer and excl are 0, we could remove it, or we do it at the background.. Also, there is another risk, that is to say, if qgroup accounting not work right, So we might need gurantee that subvolume it going to be deleted or have been deleted at the same time, then we could remove qgroup safely etc. > Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com> > --- > fs/btrfs/inode.c | 4 ++++ > fs/btrfs/qgroup.c | 1 - > 2 files changed, 4 insertions(+), 1 deletion(-) > > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c > index e687bb0..31de211 100644 > --- a/fs/btrfs/inode.c > +++ b/fs/btrfs/inode.c > @@ -59,6 +59,7 @@ > #include "backref.h" > #include "hash.h" > #include "props.h" > +#include "qgroup.h" > > struct btrfs_iget_args { > struct btrfs_key *location; > @@ -4029,6 +4030,9 @@ int btrfs_unlink_subvol(struct btrfs_trans_handle *trans, > ret = btrfs_update_inode_fallback(trans, root, dir); > if (ret) > btrfs_abort_transaction(trans, root, ret); > + ret = btrfs_remove_qgroup(trans, root->fs_info, objectid); > + if (ret == -EBUSY) > + ret = 0; > out: > btrfs_free_path(path); > return ret; > diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c > index c3b1e4f..303c078 100644 > --- a/fs/btrfs/qgroup.c > +++ b/fs/btrfs/qgroup.c > @@ -35,7 +35,6 @@ > #include "qgroup.h" > > /* TODO XXX FIXME > - * - subvol delete -> delete when ref goes to 0? delete limits also? > * - reorganize keys > * - compressed > * - sync > -- > 1.8.4.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 Best Regards, Wang Shilong -- 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
On 01/22/2015 02:16 PM, Wang Shilong wrote: > Hello, > >> When removing a subvol, if the related qgroup has no child qgroup, we should destroy >> it at the same time. Also remove the TODO entry in qgroup.c. > My two cents here: > > Take a quick look at this, i am not sure whether this is right way to do it. > > Actually, i think we can only remove a subvolume qgroup safely when both refer and excl are 0, > because for subvolume removal case, to make qgroup accounting right, we need walk tree. > > At this time, qgroup walking might have not finished, if we remove this qgroup directly, > accounting for this qgroup will be skipped. Good point! > > Maybe we could remove such qgroup when at qgroup accounting, we find an qgroup’s refer and excl > are 0, we could remove it, or we do it at the background.. > > Also, there is another risk, that is to say, if qgroup accounting not work right, So > we might need gurantee that subvolume it going to be deleted or have been deleted at the same time, > then we could remove qgroup safely etc. Yes, you are right. This is not a good timing to remove the qgroup directly. I need some more investigation for a better solution. As I have some emergency to deal with in this period, maybe the V2 will come a little late. Sorry about it. Thanx Yang > > >> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com> >> --- >> fs/btrfs/inode.c | 4 ++++ >> fs/btrfs/qgroup.c | 1 - >> 2 files changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c >> index e687bb0..31de211 100644 >> --- a/fs/btrfs/inode.c >> +++ b/fs/btrfs/inode.c >> @@ -59,6 +59,7 @@ >> #include "backref.h" >> #include "hash.h" >> #include "props.h" >> +#include "qgroup.h" >> >> struct btrfs_iget_args { >> struct btrfs_key *location; >> @@ -4029,6 +4030,9 @@ int btrfs_unlink_subvol(struct btrfs_trans_handle *trans, >> ret = btrfs_update_inode_fallback(trans, root, dir); >> if (ret) >> btrfs_abort_transaction(trans, root, ret); >> + ret = btrfs_remove_qgroup(trans, root->fs_info, objectid); >> + if (ret == -EBUSY) >> + ret = 0; >> out: >> btrfs_free_path(path); >> return ret; >> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c >> index c3b1e4f..303c078 100644 >> --- a/fs/btrfs/qgroup.c >> +++ b/fs/btrfs/qgroup.c >> @@ -35,7 +35,6 @@ >> #include "qgroup.h" >> >> /* TODO XXX FIXME >> - * - subvol delete -> delete when ref goes to 0? delete limits also? >> * - reorganize keys >> * - compressed >> * - sync >> -- >> 1.8.4.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 > Best Regards, > Wang Shilong > > . > -- 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
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index e687bb0..31de211 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -59,6 +59,7 @@ #include "backref.h" #include "hash.h" #include "props.h" +#include "qgroup.h" struct btrfs_iget_args { struct btrfs_key *location; @@ -4029,6 +4030,9 @@ int btrfs_unlink_subvol(struct btrfs_trans_handle *trans, ret = btrfs_update_inode_fallback(trans, root, dir); if (ret) btrfs_abort_transaction(trans, root, ret); + ret = btrfs_remove_qgroup(trans, root->fs_info, objectid); + if (ret == -EBUSY) + ret = 0; out: btrfs_free_path(path); return ret; diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c index c3b1e4f..303c078 100644 --- a/fs/btrfs/qgroup.c +++ b/fs/btrfs/qgroup.c @@ -35,7 +35,6 @@ #include "qgroup.h" /* TODO XXX FIXME - * - subvol delete -> delete when ref goes to 0? delete limits also? * - reorganize keys * - compressed * - sync
When removing a subvol, if the related qgroup has no child qgroup, we should destroy it at the same time. Also remove the TODO entry in qgroup.c. Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com> --- fs/btrfs/inode.c | 4 ++++ fs/btrfs/qgroup.c | 1 - 2 files changed, 4 insertions(+), 1 deletion(-)