| Message ID | 1338389946-13711-11-git-send-email-jeff.liu@oracle.com |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Wed, May 30, 2012 at 10:59:04PM +0800, jeff.liu@oracle.com wrote: > dquot_free_inode(inode); > + ns_dquot_free_inode(inode); It looks like you are adding ns_dquot_<foo>() with exactly the same arguments after dquot_<foo>() calls, for various values of <foo>. Instead of needing to change all of the file systems, would it be possible simply to change the quota layer to call the ns_dquot_* functions? It might make your life simpler, and it would certainly reduce the number of filesystem maintainers that would need to look through and review your changes. You'd just have to work with Jan Kara as the quota system maintainer. Regards, - Ted -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Ted, Thanks for your response! On 05/30/2012 11:55 PM, Ted Ts'o wrote: > On Wed, May 30, 2012 at 10:59:04PM +0800, jeff.liu@oracle.com wrote: >> dquot_free_inode(inode); >> + ns_dquot_free_inode(inode); > > It looks like you are adding ns_dquot_<foo>() with exactly the same > arguments after dquot_<foo>() calls, for various values of <foo>. > > Instead of needing to change all of the file systems, would it be > possible simply to change the quota layer to call the ns_dquot_* > functions? I also think that is stupid to change all of the file systems with those hook routines. At first I have tried to change the quota layer by adding ns_dquot_<foo>() to the corresponding dquot_<foo>(), it could works on ext4, and it should works on other file systems which are tightly bound to VFS quota IMHO. However, XFS has its own quota management subsystem, with looser binding to VFS quota. That's why am trying to export those routines over all the file systems and CC to the maintainers, sorry for the noise. Nevertheless, I will try to find out a straightforward way according to your comments. Thanks, -Jeff > It might make your life simpler, and it would certainly > reduce the number of filesystem maintainers that would need to look > through and review your changes. You'd just have to work with Jan > Kara as the quota system maintainer. > > Regards, > > - Ted > -- > To unsubscribe from this list: send the line "unsubscribe linux-ext4" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, May 31, 2012 at 09:43:18AM +0800, Jeff Liu wrote: > I also think that is stupid to change all of the file systems with those > hook routines. > At first I have tried to change the quota layer by adding > ns_dquot_<foo>() to the corresponding dquot_<foo>(), it could works on > ext4, and it should works on other file systems which are tightly bound > to VFS quota IMHO. > > However, XFS has its own quota management subsystem, with looser binding > to VFS quota. That's why am trying to export those routines over all > the file systems and CC to the maintainers, sorry for the noise. No worries about the noise. I was just trying to suggest something that might be easier for you. As near as I can tell XFS doesn't use the fs/quota functions at all, yes? But if you add it into the fs/quota functions, it should get you integration with ext2, ext3, ext4, jfs, ocfs2, and reiserfs. Yes? - Ted -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 05/31/2012 09:54 AM, Ted Ts'o wrote: > On Thu, May 31, 2012 at 09:43:18AM +0800, Jeff Liu wrote: >> I also think that is stupid to change all of the file systems with those >> hook routines. >> At first I have tried to change the quota layer by adding >> ns_dquot_<foo>() to the corresponding dquot_<foo>(), it could works on >> ext4, and it should works on other file systems which are tightly bound >> to VFS quota IMHO. >> >> However, XFS has its own quota management subsystem, with looser binding >> to VFS quota. That's why am trying to export those routines over all >> the file systems and CC to the maintainers, sorry for the noise. > > No worries about the noise. I was just trying to suggest something > that might be easier for you. > > As near as I can tell XFS doesn't use the fs/quota functions at all, > yes? But if you add it into the fs/quota functions, it should get you > integration with ext2, ext3, ext4, jfs, ocfs2, and reiserfs. Yes? Yes, I just took a quick look through over ocfs2, reiserfs and jfs, they are all ok. Thanks, -Jeff > > - Ted > -- > To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 05/31/2012 10:37 AM, Jeff Liu wrote: > On 05/31/2012 09:54 AM, Ted Ts'o wrote: > >> On Thu, May 31, 2012 at 09:43:18AM +0800, Jeff Liu wrote: >>> I also think that is stupid to change all of the file systems with those >>> hook routines. >>> At first I have tried to change the quota layer by adding >>> ns_dquot_<foo>() to the corresponding dquot_<foo>(), it could works on >>> ext4, and it should works on other file systems which are tightly bound >>> to VFS quota IMHO. >>> >>> However, XFS has its own quota management subsystem, with looser binding >>> to VFS quota. That's why am trying to export those routines over all >>> the file systems and CC to the maintainers, sorry for the noise. >> >> No worries about the noise. I was just trying to suggest something >> that might be easier for you. >> >> As near as I can tell XFS doesn't use the fs/quota functions at all, >> yes? But if you add it into the fs/quota functions, it should get you >> integration with ext2, ext3, ext4, jfs, ocfs2, and reiserfs. Yes? > > Yes, I just took a quick look through over ocfs2, reiserfs and jfs, they > are all ok. Such being the case, maybe I should tight ns_dquot_<foo>() to VFS quota to reduce duplication firstly, at lease, it could works with most file systems. Then consider another way to let it works with other file systems doesn't use fs/quota. Thanks, -Jeff > > Thanks, > -Jeff > >> >> - Ted >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 05/30/2012 06:59 PM, jeff.liu@oracle.com wrote: > +#include "../ns_quotaops.h" > + > /* > * ialloc.c contains the inodes allocation and deallocation routines > */ > @@ -233,6 +235,7 @@ void ext4_free_inode(handle_t *handle, struct inode *inode) > dquot_initialize(inode); > ext4_xattr_delete_inode(handle, inode); > dquot_free_inode(inode); > + ns_dquot_free_inode(inode); > dquot_drop(inode); This should be inside dquot_free_inode(). No need to go patch inode.c -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 05/31/2012 05:15 PM, Glauber Costa wrote: > On 05/30/2012 06:59 PM, jeff.liu@oracle.com wrote: >> +#include "../ns_quotaops.h" >> + >> /* >> * ialloc.c contains the inodes allocation and deallocation routines >> */ >> @@ -233,6 +235,7 @@ void ext4_free_inode(handle_t *handle, struct >> inode *inode) >> dquot_initialize(inode); >> ext4_xattr_delete_inode(handle, inode); >> dquot_free_inode(inode); >> + ns_dquot_free_inode(inode); >> dquot_drop(inode); > > This should be inside dquot_free_inode(). Yeah, Ted also mentioned this before, all name space dquot bill routine could be placed to dquot_xxx() for VFS quota user. However, AFAICS, they still have to be exported to other file systems which have their own quota management module, like XFS. Thanks, -Jeff > > No need to go patch inode.c > > -- > To unsubscribe from this list: send the line "unsubscribe linux-ext4" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 05/31/2012 04:58 PM, Jeff Liu wrote: >> > This should be inside dquot_free_inode(). > Yeah, Ted also mentioned this before, all name space dquot bill routine > could be placed to dquot_xxx() for VFS quota user. > > However, AFAICS, they still have to be exported to other file systems > which have their own quota management module, like XFS. > > Thanks, > -Jeff > Do you have a personal need to have it working with xfs, or is the use you are pursuing achievable with the other filesystems? Unless you have a reason yourself to use xfs, I'd say don't bother with it too much at the moment. get it working for the standard interface first. -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 05/31/2012 09:14 PM, Glauber Costa wrote: > On 05/31/2012 04:58 PM, Jeff Liu wrote: >>> > This should be inside dquot_free_inode(). >> Yeah, Ted also mentioned this before, all name space dquot bill routine >> could be placed to dquot_xxx() for VFS quota user. >> >> However, AFAICS, they still have to be exported to other file systems >> which have their own quota management module, like XFS. >> >> Thanks, >> -Jeff >> > Do you have a personal need to have it working with xfs, or is the use > you are pursuing achievable with the other filesystems? I am just trying to figure out a unique interface for all file systems. > > Unless you have a reason yourself to use xfs, I'd say don't bother with > it too much at the moment. get it working for the standard interface first. Ok, It can also make my life easier. Thanks, -Jeff > -- > To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, May 31, 2012 at 05:14:50PM +0400, Glauber Costa wrote: > On 05/31/2012 04:58 PM, Jeff Liu wrote: > >>> This should be inside dquot_free_inode(). > >Yeah, Ted also mentioned this before, all name space dquot bill routine > >could be placed to dquot_xxx() for VFS quota user. > > > >However, AFAICS, they still have to be exported to other file systems > >which have their own quota management module, like XFS. > > > >Thanks, > >-Jeff > > > Do you have a personal need to have it working with xfs, or is the > use you are pursuing achievable with the other filesystems? > > Unless you have a reason yourself to use xfs, I'd say don't bother > with it too much at the moment. get it working for the standard > interface first. Which means you'll end up with something that has to be rewritten to support XFS. Do it right the first time. Cheers, Dave.
diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c index 9f9acac..510fa41 100644 --- a/fs/ext4/ialloc.c +++ b/fs/ext4/ialloc.c @@ -31,6 +31,8 @@ #include <trace/events/ext4.h> +#include "../ns_quotaops.h" + /* * ialloc.c contains the inodes allocation and deallocation routines */ @@ -233,6 +235,7 @@ void ext4_free_inode(handle_t *handle, struct inode *inode) dquot_initialize(inode); ext4_xattr_delete_inode(handle, inode); dquot_free_inode(inode); + ns_dquot_free_inode(inode); dquot_drop(inode); is_directory = S_ISDIR(inode->i_mode); @@ -861,6 +864,10 @@ got: if (err) goto fail_drop; + err = ns_dquot_alloc_inode(inode); + if (err) + goto fail_drop; + err = ext4_init_acl(handle, inode, dir); if (err) goto fail_free_drop; @@ -902,6 +909,7 @@ really_out: fail_free_drop: dquot_free_inode(inode); + ns_dquot_free_inode(inode); fail_drop: dquot_drop(inode);
Bill up container inodes allocation and free on ext4. Signed-off-by: Jie Liu <jeff.liu@oracle.com> --- fs/ext4/ialloc.c | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-)