[PATCH -next] quota: fix QUOTACTL kconfig for gfs2

[PATCH -next] quota: fix QUOTACTL kconfig for gfs2

[Randy Dunlap]

From: Randy Dunlap <randy.dunlap@oracle.com>

warning: (GFS2_FS && BLOCK && EXPERIMENTAL && (64BIT || LBDAF)) selects QUOTACTL which has unmet direct dependencies (XFS_QUOTA || QUOTA)

Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
Cc:	Steven Whitehouse <swhiteho@redhat.com>
Cc:	cluster-devel@redhat.com
---
 fs/quota/Kconfig |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

However, the "depends on" line here seems like overkill to me.


--- linux-next-20100810.orig/fs/quota/Kconfig
+++ linux-next-20100810/fs/quota/Kconfig
@@ -65,7 +65,7 @@ config QFMT_V2
 
 config QUOTACTL
 	bool
-	depends on XFS_QUOTA || QUOTA
+	depends on XFS_QUOTA || QUOTA || GFS2_FS
 	default y
 
 config QUOTACTL_COMPAT

Re: [PATCH -next] quota: fix QUOTACTL kconfig for gfs2

[Jan Kara]

On Wed 11-08-10 16:24:35, Randy Dunlap wrote:
> From: Randy Dunlap <randy.dunlap@oracle.com>
> 
> warning: (GFS2_FS && BLOCK && EXPERIMENTAL && (64BIT || LBDAF)) selects QUOTACTL which has unmet direct dependencies (XFS_QUOTA || QUOTA)
> 
> Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
> Cc:	Steven Whitehouse <swhiteho@redhat.com>
> Cc:	cluster-devel@redhat.com
> ---
>  fs/quota/Kconfig |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> However, the "depends on" line here seems like overkill to me.
  OK, so do you mean that making QUOTACTL default to 'n', removing the
depends on line and letting QUOTA, XFS_QUOTA, and GFS2_QUOTA select
QUOTACTL would be better?

								Honza
> 
> 
> --- linux-next-20100810.orig/fs/quota/Kconfig
> +++ linux-next-20100810/fs/quota/Kconfig
> @@ -65,7 +65,7 @@ config QFMT_V2
>  
>  config QUOTACTL
>  	bool
> -	depends on XFS_QUOTA || QUOTA
> +	depends on XFS_QUOTA || QUOTA || GFS2_FS
>  	default y
>  
>  config QUOTACTL_COMPAT
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

Re: [PATCH -next] quota: fix QUOTACTL kconfig for gfs2

[Randy Dunlap]

On 08/11/10 19:46, Jan Kara wrote:
> On Wed 11-08-10 16:24:35, Randy Dunlap wrote:
>> From: Randy Dunlap <randy.dunlap@oracle.com>
>>
>> warning: (GFS2_FS && BLOCK && EXPERIMENTAL && (64BIT || LBDAF)) selects QUOTACTL which has unmet direct dependencies (XFS_QUOTA || QUOTA)
>>
>> Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
>> Cc:	Steven Whitehouse <swhiteho@redhat.com>
>> Cc:	cluster-devel@redhat.com
>> ---
>>  fs/quota/Kconfig |    2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> However, the "depends on" line here seems like overkill to me.
>   OK, so do you mean that making QUOTACTL default to 'n', removing the
> depends on line and letting QUOTA, XFS_QUOTA, and GFS2_QUOTA select
> QUOTACTL would be better?

Yes, I think that would be better.

However, if you like the patch below, we should also kill the 'select QUOTACTL'
in fs/gfs2/Kconfig.

>> --- linux-next-20100810.orig/fs/quota/Kconfig
>> +++ linux-next-20100810/fs/quota/Kconfig
>> @@ -65,7 +65,7 @@ config QFMT_V2
>>  
>>  config QUOTACTL
>>  	bool
>> -	depends on XFS_QUOTA || QUOTA
>> +	depends on XFS_QUOTA || QUOTA || GFS2_FS
>>  	default y
>>  
>>  config QUOTACTL_COMPAT


-- 
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: [PATCH -next] quota: fix QUOTACTL kconfig for gfs2

[Steven Whitehouse]

Hi,

On Thu, 2010-08-12 at 08:39 -0700, Randy Dunlap wrote:
> On 08/11/10 19:46, Jan Kara wrote:
> > On Wed 11-08-10 16:24:35, Randy Dunlap wrote:
> >> From: Randy Dunlap <randy.dunlap@oracle.com>
> >>
> >> warning: (GFS2_FS && BLOCK && EXPERIMENTAL && (64BIT || LBDAF)) selects QUOTACTL which has unmet direct dependencies (XFS_QUOTA || QUOTA)
> >>
> >> Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
> >> Cc:	Steven Whitehouse <swhiteho@redhat.com>
> >> Cc:	cluster-devel@redhat.com
> >> ---
> >>  fs/quota/Kconfig |    2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> However, the "depends on" line here seems like overkill to me.
> >   OK, so do you mean that making QUOTACTL default to 'n', removing the
> > depends on line and letting QUOTA, XFS_QUOTA, and GFS2_QUOTA select
> > QUOTACTL would be better?
> 
I'm not sure I follow. What is GFS2_QUOTA in this case? GFS2 used to
select QUOTA as well, but Christoph removed that. Shouldn't the user
interface part of quota be separate from the core implementation?

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=e9edb1d8a345119c9baafa1b240eb1ec06a44662

Steve.

Re: [PATCH -next] quota: fix QUOTACTL kconfig for gfs2

[Randy Dunlap]

On 08/12/10 08:57, Steven Whitehouse wrote:
> Hi,
> 
> On Thu, 2010-08-12 at 08:39 -0700, Randy Dunlap wrote:
>> On 08/11/10 19:46, Jan Kara wrote:
>>> On Wed 11-08-10 16:24:35, Randy Dunlap wrote:
>>>> From: Randy Dunlap <randy.dunlap@oracle.com>
>>>>
>>>> warning: (GFS2_FS && BLOCK && EXPERIMENTAL && (64BIT || LBDAF)) selects QUOTACTL which has unmet direct dependencies (XFS_QUOTA || QUOTA)
>>>>
>>>> Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
>>>> Cc:	Steven Whitehouse <swhiteho@redhat.com>
>>>> Cc:	cluster-devel@redhat.com
>>>> ---
>>>>  fs/quota/Kconfig |    2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> However, the "depends on" line here seems like overkill to me.
>>>   OK, so do you mean that making QUOTACTL default to 'n', removing the
>>> depends on line and letting QUOTA, XFS_QUOTA, and GFS2_QUOTA select
>>> QUOTACTL would be better?
>>
> I'm not sure I follow. What is GFS2_QUOTA in this case? GFS2 used to

That's OK.  I'm not following you either.  ;)  Where is GFS2_QUOTA?


> select QUOTA as well, but Christoph removed that. Shouldn't the user
> interface part of quota be separate from the core implementation?

Do people use one without the other?

> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=e9edb1d8a345119c9baafa1b240eb1ec06a44662

Do you prefer my original patch that started this thread?

If fs/quota/Kconfig says:

config QUOTACTL
	bool
	depends on XFS_QUOTA || QUOTA || GFS2_FS
	default y

then there is no need for fs/gfs2/Kconfig to select QUOTACTL.


Anyway, this is a centralized kconfig control of QUOTACTL.  I was just suggesting
that having each fs control is locally would be more manageable.  It's not a big deal.

-- 
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: [PATCH -next] quota: fix QUOTACTL kconfig for gfs2

[Christoph Hellwig]

On Thu, Aug 12, 2010 at 09:03:38AM -0700, Randy Dunlap wrote:
> > select QUOTA as well, but Christoph removed that. Shouldn't the user
> > interface part of quota be separate from the core implementation?
> 
> Do people use one without the other?

Yes, there's a the quoatctl system call and a few handlers, of which
the current quota implementation is different.  We currently have the
semi-generionc 'dquot' implementation used by extN, reiserfs and ocfs2,
the xfs implementation and the gfs2 implementation.  We also used to
have some stubs in cifs, but they never became an actually useful
implementation.

So selecting QUOTACTL by XFS, GFS2 and QUOTA (which is a bit misnamed)
would be correct.

Re: [PATCH -next] quota: fix QUOTACTL kconfig for gfs2

[Steven Whitehouse]

Hi,

On Thu, 2010-08-12 at 09:03 -0700, Randy Dunlap wrote:
> On 08/12/10 08:57, Steven Whitehouse wrote:
> > Hi,
> > 
> > On Thu, 2010-08-12 at 08:39 -0700, Randy Dunlap wrote:
> >> On 08/11/10 19:46, Jan Kara wrote:
> >>> On Wed 11-08-10 16:24:35, Randy Dunlap wrote:
> >>>> From: Randy Dunlap <randy.dunlap@oracle.com>
> >>>>
> >>>> warning: (GFS2_FS && BLOCK && EXPERIMENTAL && (64BIT || LBDAF)) selects QUOTACTL which has unmet direct dependencies (XFS_QUOTA || QUOTA)
> >>>>
> >>>> Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
> >>>> Cc:	Steven Whitehouse <swhiteho@redhat.com>
> >>>> Cc:	cluster-devel@redhat.com
> >>>> ---
> >>>>  fs/quota/Kconfig |    2 +-
> >>>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>
> >>>> However, the "depends on" line here seems like overkill to me.
> >>>   OK, so do you mean that making QUOTACTL default to 'n', removing the
> >>> depends on line and letting QUOTA, XFS_QUOTA, and GFS2_QUOTA select
> >>> QUOTACTL would be better?
> >>
> > I'm not sure I follow. What is GFS2_QUOTA in this case? GFS2 used to
> 
> That's OK.  I'm not following you either.  ;)  Where is GFS2_QUOTA?
> 
Well GFS2_QUOTA was referred to in the above comment, but it doesn't
exist as quotas are not optional in GFS2. That is partly a hangover from
GFS1 and partly because it makes things a lot more complicated in a
cluster filesystem if you can't be sure that all nodes understand the
same options.

> 
> > select QUOTA as well, but Christoph removed that. Shouldn't the user
> > interface part of quota be separate from the core implementation?
> 
> Do people use one without the other?
> 
The original GFS2 implementation was totally separate, again it was
largely inherited from GFS1, but some changes (read/write of the quota
file via the gfs2meta fs type, rather than the GFS1 read/write ioctls).
More recently I added support for updating the quotas via the XFS-style
interface with the longer term aim of being able to use quota-tools for
all GFS2 quota operations and (eventually) dropping support for the
gfs2_quota tool.

As a result of that GFS2 uses the quotactl interface code, but retains
all its original, internal, quota code as well.

> > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=e9edb1d8a345119c9baafa1b240eb1ec06a44662
> 
> Do you prefer my original patch that started this thread?
> 
> If fs/quota/Kconfig says:
> 
> config QUOTACTL
> 	bool
> 	depends on XFS_QUOTA || QUOTA || GFS2_FS
> 	default y
> 
> then there is no need for fs/gfs2/Kconfig to select QUOTACTL.
> 
> 
> Anyway, this is a centralized kconfig control of QUOTACTL.  I was just suggesting
> that having each fs control is locally would be more manageable.  It's not a big deal.
> 
Hmm. I'd been looking on the quotactl code as basically being a library
function, so I'd have expected to have the filesystems select it if they
require it.

I thought the original issue was that there was some interdependency
between QUOTA and QUOTACTL or is that not what that message was saying?

Steve.

Re: [PATCH -next] quota: fix QUOTACTL kconfig for gfs2

[Randy Dunlap]

On 08/12/10 09:34, Steven Whitehouse wrote:
> Hi,
> 
> On Thu, 2010-08-12 at 09:03 -0700, Randy Dunlap wrote:
>> On 08/12/10 08:57, Steven Whitehouse wrote:
>>> Hi,
>>>
>>> On Thu, 2010-08-12 at 08:39 -0700, Randy Dunlap wrote:
>>>> On 08/11/10 19:46, Jan Kara wrote:
>>>>> On Wed 11-08-10 16:24:35, Randy Dunlap wrote:
>>>>>> From: Randy Dunlap <randy.dunlap@oracle.com>
>>>>>>
>>>>>> warning: (GFS2_FS && BLOCK && EXPERIMENTAL && (64BIT || LBDAF)) selects QUOTACTL which has unmet direct dependencies (XFS_QUOTA || QUOTA)
>>>>>>
>>>>>> Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
>>>>>> Cc:	Steven Whitehouse <swhiteho@redhat.com>
>>>>>> Cc:	cluster-devel@redhat.com
>>>>>> ---
>>>>>>  fs/quota/Kconfig |    2 +-
>>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>
>>>>>> However, the "depends on" line here seems like overkill to me.
>>>>>   OK, so do you mean that making QUOTACTL default to 'n', removing the
>>>>> depends on line and letting QUOTA, XFS_QUOTA, and GFS2_QUOTA select
>>>>> QUOTACTL would be better?
>>>>
>>> I'm not sure I follow. What is GFS2_QUOTA in this case? GFS2 used to
>>
>> That's OK.  I'm not following you either.  ;)  Where is GFS2_QUOTA?
>>
> Well GFS2_QUOTA was referred to in the above comment, but it doesn't

Oh, in Jan's comment.  I see.

> exist as quotas are not optional in GFS2. That is partly a hangover from
> GFS1 and partly because it makes things a lot more complicated in a
> cluster filesystem if you can't be sure that all nodes understand the
> same options.
> 
>>
>>> select QUOTA as well, but Christoph removed that. Shouldn't the user
>>> interface part of quota be separate from the core implementation?
>>
>> Do people use one without the other?
>>
> The original GFS2 implementation was totally separate, again it was
> largely inherited from GFS1, but some changes (read/write of the quota
> file via the gfs2meta fs type, rather than the GFS1 read/write ioctls).
> More recently I added support for updating the quotas via the XFS-style
> interface with the longer term aim of being able to use quota-tools for
> all GFS2 quota operations and (eventually) dropping support for the
> gfs2_quota tool.
> 
> As a result of that GFS2 uses the quotactl interface code, but retains
> all its original, internal, quota code as well.
> 
>>> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=e9edb1d8a345119c9baafa1b240eb1ec06a44662
>>
>> Do you prefer my original patch that started this thread?
>>
>> If fs/quota/Kconfig says:
>>
>> config QUOTACTL
>> 	bool
>> 	depends on XFS_QUOTA || QUOTA || GFS2_FS
>> 	default y
>>
>> then there is no need for fs/gfs2/Kconfig to select QUOTACTL.
>>
>>
>> Anyway, this is a centralized kconfig control of QUOTACTL.  I was just suggesting
>> that having each fs control is locally would be more manageable.  It's not a big deal.
>>
> Hmm. I'd been looking on the quotactl code as basically being a library
> function, so I'd have expected to have the filesystems select it if they
> require it.

Yes, that's how I see it also, but Jan can correct me on that...

> I thought the original issue was that there was some interdependency
> between QUOTA and QUOTACTL or is that not what that message was saying?

I don't think so.  It's just a Kconfig language muckup & cleanup AFAICT.


-- 
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: [PATCH -next] quota: fix QUOTACTL kconfig for gfs2

[Jan Kara]

On Thu 12-08-10 08:39:45, Randy Dunlap wrote:
> On 08/11/10 19:46, Jan Kara wrote:
> > On Wed 11-08-10 16:24:35, Randy Dunlap wrote:
> >> From: Randy Dunlap <randy.dunlap@oracle.com>
> >>
> >> warning: (GFS2_FS && BLOCK && EXPERIMENTAL && (64BIT || LBDAF)) selects QUOTACTL which has unmet direct dependencies (XFS_QUOTA || QUOTA)
> >>
> >> Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
> >> Cc:	Steven Whitehouse <swhiteho@redhat.com>
> >> Cc:	cluster-devel@redhat.com
> >> ---
> >>  fs/quota/Kconfig |    2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> However, the "depends on" line here seems like overkill to me.
> >   OK, so do you mean that making QUOTACTL default to 'n', removing the
> > depends on line and letting QUOTA, XFS_QUOTA, and GFS2_QUOTA select
> > QUOTACTL would be better?
> 
> Yes, I think that would be better.
  OK, I've pushed the patch below to my tree.

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR
---

>From 7d67aa08b438d5c1681a2e977f8e3f2e44570305 Mon Sep 17 00:00:00 2001
From: Jan Kara <jack@suse.cz>
Date: Tue, 17 Aug 2010 12:14:44 +0200
Subject: [PATCH] quota: Make QUOTACTL config be selected by its users

Remove "depends on" line from QUOTACTL config option and rather select
the option explicitely from config options which need it. It makes more
sense this way and also fixes Kconfig warning due to GFS2 selecting
QUOTACTL but QUOTACTL not depending on it.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/quota/Kconfig |    4 ++--
 fs/xfs/Kconfig   |    1 +
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/quota/Kconfig b/fs/quota/Kconfig
index 3e21b1e..880fd98 100644
--- a/fs/quota/Kconfig
+++ b/fs/quota/Kconfig
@@ -4,6 +4,7 @@
 
 config QUOTA
 	bool "Quota support"
+	select QUOTACTL
 	help
 	  If you say Y here, you will be able to set per user limits for disk
 	  usage (also called disk quotas). Currently, it works for the
@@ -65,8 +66,7 @@ config QFMT_V2
 
 config QUOTACTL
 	bool
-	depends on XFS_QUOTA || QUOTA
-	default y
+	default n
 
 config QUOTACTL_COMPAT
 	bool
diff --git a/fs/xfs/Kconfig b/fs/xfs/Kconfig
index 480f281..6100ec0 100644
--- a/fs/xfs/Kconfig
+++ b/fs/xfs/Kconfig
@@ -22,6 +22,7 @@ config XFS_FS
 config XFS_QUOTA
 	bool "XFS Quota support"
 	depends on XFS_FS
+	select QUOTACTL
 	help
 	  If you say Y here, you will be able to set limits for disk usage on
 	  a per user and/or a per group basis under XFS.  XFS considers quota
-- 
1.6.4.2

Re: [PATCH -next] quota: fix QUOTACTL kconfig for gfs2

[Randy Dunlap]

On 08/17/10 03:22, Jan Kara wrote:
> On Thu 12-08-10 08:39:45, Randy Dunlap wrote:
>> On 08/11/10 19:46, Jan Kara wrote:
>>> On Wed 11-08-10 16:24:35, Randy Dunlap wrote:
>>>> From: Randy Dunlap <randy.dunlap@oracle.com>
>>>>
>>>> warning: (GFS2_FS && BLOCK && EXPERIMENTAL && (64BIT || LBDAF)) selects QUOTACTL which has unmet direct dependencies (XFS_QUOTA || QUOTA)
>>>>
>>>> Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
>>>> Cc:	Steven Whitehouse <swhiteho@redhat.com>
>>>> Cc:	cluster-devel@redhat.com
>>>> ---
>>>>  fs/quota/Kconfig |    2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> However, the "depends on" line here seems like overkill to me.
>>>   OK, so do you mean that making QUOTACTL default to 'n', removing the
>>> depends on line and letting QUOTA, XFS_QUOTA, and GFS2_QUOTA select
>>> QUOTACTL would be better?
>>
>> Yes, I think that would be better.
>   OK, I've pushed the patch below to my tree.

Thanks.  Looks good.

-- 
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***