Patchwork [10/12] container quota: bill container inodes alloc/free on ext4.

login
register
mail settings
Submitter jeff.liu
Date May 30, 2012, 2:59 p.m.
Message ID <1338389946-13711-11-git-send-email-jeff.liu@oracle.com>
Download mbox | patch
Permalink /patch/162001/
State Not Applicable
Headers show

Comments

jeff.liu - May 30, 2012, 2:59 p.m.
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(-)
Theodore Ts'o - May 30, 2012, 3:55 p.m.
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
jeff.liu - May 31, 2012, 1:43 a.m.
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
Theodore Ts'o - May 31, 2012, 1:54 a.m.
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
jeff.liu - May 31, 2012, 2:37 a.m.
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
jeff.liu - May 31, 2012, 3:24 a.m.
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
Glauber Costa - May 31, 2012, 9:15 a.m.
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
jeff.liu - May 31, 2012, 12:58 p.m.
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
Glauber Costa - May 31, 2012, 1:14 p.m.
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
jeff.liu - May 31, 2012, 1:43 p.m.
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
Dave Chinner - June 5, 2012, 12:03 a.m.
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.

Patch

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);