* [PATCH 1/3] cifs: add new module parameter 'enable_oplocks'
@ 2011-10-11 15:58 Suresh Jayaraman
[not found] ` <4E946788.3060101-l3A5Bk7waGM@public.gmane.org>
0 siblings, 1 reply; 7+ messages in thread
From: Suresh Jayaraman @ 2011-10-11 15:58 UTC (permalink / raw)
To: Steve French; +Cc: linux-cifs, Jeff Layton, Alexander Swen
Thus spake Jeff Layton:
"Making that a module parm would allow you to set that parameter at boot
time without needing to add special startup scripts. IMO, all of the
procfile "switches" under /proc/fs/cifs should be module parms
instead."
This patch doesn't alter the default behavior (Oplocks are enabled by
default).
To disable oplocks when loading the module, use
modprobe cifs enable_oplocks=0
(any of '0' or 'n' or 'N' conventions can be used).
To disable oplocks at runtime using the new interface, use
echo 0 > /sys/module/cifs/parameters/enable_oplocks
The older /proc/fs/cifs/OplockEnabled interface will be deprecated
after two releases. A subsequent patch will add an warning message
about the deprecation.
Reported-by: Alexander Swen <alex-UE4+9DRHXtc@public.gmane.org>
Cc: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org>
---
fs/cifs/cifsfs.c | 5 +++++
fs/cifs/cifsglob.h | 3 +++
fs/cifs/dir.c | 2 +-
fs/cifs/file.c | 4 ++--
4 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 3e29899..675ab40 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -81,6 +81,11 @@ module_param(echo_retries, ushort, 0644);
MODULE_PARM_DESC(echo_retries, "Number of echo attempts before giving up and "
"reconnecting server. Default: 5. 0 means "
"never reconnect.");
+unsigned int enable_oplocks = 1;
+module_param(enable_oplocks, bool, 0644);
+MODULE_PARM_DESC(enable_oplocks, "Enable or disable oplocks (bool). Default:"
+ "y/Y/1");
+
extern mempool_t *cifs_sm_req_poolp;
extern mempool_t *cifs_req_poolp;
extern mempool_t *cifs_mid_poolp;
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 6255fa8..b3e229c 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -936,6 +936,9 @@ GLOBAL_EXTERN unsigned int cifs_max_pending; /* MAX requests at once to server*/
/* reconnect after this many failed echo attempts */
GLOBAL_EXTERN unsigned short echo_retries;
+/* enable or disable oplocks */
+GLOBAL_EXTERN unsigned int enable_oplocks;
+
GLOBAL_EXTERN struct rb_root uidtree;
GLOBAL_EXTERN struct rb_root gidtree;
GLOBAL_EXTERN spinlock_t siduidlock;
diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
index 81914df..6529b69 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -165,7 +165,7 @@ cifs_create(struct inode *inode, struct dentry *direntry, int mode,
}
tcon = tlink_tcon(tlink);
- if (oplockEnabled)
+ if (oplockEnabled || enable_oplocks)
oplock = REQ_OPLOCK;
if (nd && (nd->flags & LOOKUP_OPEN))
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index bb71471..b76c31d 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -369,7 +369,7 @@ int cifs_open(struct inode *inode, struct file *file)
cFYI(1, "inode = 0x%p file flags are 0x%x for %s",
inode, file->f_flags, full_path);
- if (oplockEnabled)
+ if (oplockEnabled || enable_oplocks)
oplock = REQ_OPLOCK;
else
oplock = 0;
@@ -493,7 +493,7 @@ static int cifs_reopen_file(struct cifsFileInfo *pCifsFile, bool can_flush)
cFYI(1, "inode = 0x%p file flags 0x%x for %s",
inode, pCifsFile->f_flags, full_path);
- if (oplockEnabled)
+ if (oplockEnabled || enable_oplocks)
oplock = REQ_OPLOCK;
else
oplock = 0;
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] cifs: add new module parameter 'enable_oplocks'
[not found] ` <4E946788.3060101-l3A5Bk7waGM@public.gmane.org>
@ 2011-10-11 16:44 ` Jeff Layton
[not found] ` <20111011124405.37e09487-4QP7MXygkU+dMjc06nkz3ljfA9RmPOcC@public.gmane.org>
2011-10-11 17:31 ` Steve French
1 sibling, 1 reply; 7+ messages in thread
From: Jeff Layton @ 2011-10-11 16:44 UTC (permalink / raw)
To: sjayaraman-l3A5Bk7waGM; +Cc: Steve French, linux-cifs, Alexander Swen
On Tue, 11 Oct 2011 21:28:00 +0530
Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org> wrote:
> Thus spake Jeff Layton:
>
> "Making that a module parm would allow you to set that parameter at boot
> time without needing to add special startup scripts. IMO, all of the
> procfile "switches" under /proc/fs/cifs should be module parms
> instead."
>
> This patch doesn't alter the default behavior (Oplocks are enabled by
> default).
>
> To disable oplocks when loading the module, use
>
> modprobe cifs enable_oplocks=0
>
> (any of '0' or 'n' or 'N' conventions can be used).
>
> To disable oplocks at runtime using the new interface, use
>
> echo 0 > /sys/module/cifs/parameters/enable_oplocks
>
> The older /proc/fs/cifs/OplockEnabled interface will be deprecated
> after two releases. A subsequent patch will add an warning message
> about the deprecation.
>
> Reported-by: Alexander Swen <alex-UE4+9DRHXtc@public.gmane.org>
> Cc: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org>
> ---
> fs/cifs/cifsfs.c | 5 +++++
> fs/cifs/cifsglob.h | 3 +++
> fs/cifs/dir.c | 2 +-
> fs/cifs/file.c | 4 ++--
> 4 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> index 3e29899..675ab40 100644
> --- a/fs/cifs/cifsfs.c
> +++ b/fs/cifs/cifsfs.c
> @@ -81,6 +81,11 @@ module_param(echo_retries, ushort, 0644);
> MODULE_PARM_DESC(echo_retries, "Number of echo attempts before giving up and "
> "reconnecting server. Default: 5. 0 means "
> "never reconnect.");
> +unsigned int enable_oplocks = 1;
> +module_param(enable_oplocks, bool, 0644);
> +MODULE_PARM_DESC(enable_oplocks, "Enable or disable oplocks (bool). Default:"
> + "y/Y/1");
> +
> extern mempool_t *cifs_sm_req_poolp;
> extern mempool_t *cifs_req_poolp;
> extern mempool_t *cifs_mid_poolp;
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index 6255fa8..b3e229c 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -936,6 +936,9 @@ GLOBAL_EXTERN unsigned int cifs_max_pending; /* MAX requests at once to server*/
> /* reconnect after this many failed echo attempts */
> GLOBAL_EXTERN unsigned short echo_retries;
>
> +/* enable or disable oplocks */
> +GLOBAL_EXTERN unsigned int enable_oplocks;
> +
Let's not add an extra variable for this. I'd just rename oplockEnabled
to enable_oplocks and fix up all the references to it. It should
probably also be a bool too...
> GLOBAL_EXTERN struct rb_root uidtree;
> GLOBAL_EXTERN struct rb_root gidtree;
> GLOBAL_EXTERN spinlock_t siduidlock;
> diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
> index 81914df..6529b69 100644
> --- a/fs/cifs/dir.c
> +++ b/fs/cifs/dir.c
> @@ -165,7 +165,7 @@ cifs_create(struct inode *inode, struct dentry *direntry, int mode,
> }
> tcon = tlink_tcon(tlink);
>
> - if (oplockEnabled)
> + if (oplockEnabled || enable_oplocks)
> oplock = REQ_OPLOCK;
>
> if (nd && (nd->flags & LOOKUP_OPEN))
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index bb71471..b76c31d 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -369,7 +369,7 @@ int cifs_open(struct inode *inode, struct file *file)
> cFYI(1, "inode = 0x%p file flags are 0x%x for %s",
> inode, file->f_flags, full_path);
>
> - if (oplockEnabled)
> + if (oplockEnabled || enable_oplocks)
> oplock = REQ_OPLOCK;
> else
> oplock = 0;
> @@ -493,7 +493,7 @@ static int cifs_reopen_file(struct cifsFileInfo *pCifsFile, bool can_flush)
> cFYI(1, "inode = 0x%p file flags 0x%x for %s",
> inode, pCifsFile->f_flags, full_path);
>
> - if (oplockEnabled)
> + if (oplockEnabled || enable_oplocks)
> oplock = REQ_OPLOCK;
> else
> oplock = 0;
--
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] cifs: add new module parameter 'enable_oplocks'
[not found] ` <4E946788.3060101-l3A5Bk7waGM@public.gmane.org>
2011-10-11 16:44 ` Jeff Layton
@ 2011-10-11 17:31 ` Steve French
1 sibling, 0 replies; 7+ messages in thread
From: Steve French @ 2011-10-11 17:31 UTC (permalink / raw)
To: sjayaraman-l3A5Bk7waGM; +Cc: linux-cifs, Jeff Layton, Alexander Swen
Does that really work - to disable oplocks you would need to reset both
the old (/proc/fs/cifs/OplockEnabled) and new module parm (oplock_enabled)
to zero.
We should be using one global for this, not two (the /proc/fs/cifs one
could be changed to affect "enable_oplocks")
On Tue, Oct 11, 2011 at 10:58 AM, Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org> wrote:
> Thus spake Jeff Layton:
>
> "Making that a module parm would allow you to set that parameter at boot
> time without needing to add special startup scripts. IMO, all of the
> procfile "switches" under /proc/fs/cifs should be module parms
> instead."
>
> This patch doesn't alter the default behavior (Oplocks are enabled by
> default).
>
> To disable oplocks when loading the module, use
>
> modprobe cifs enable_oplocks=0
>
> (any of '0' or 'n' or 'N' conventions can be used).
>
> To disable oplocks at runtime using the new interface, use
>
> echo 0 > /sys/module/cifs/parameters/enable_oplocks
>
> The older /proc/fs/cifs/OplockEnabled interface will be deprecated
> after two releases. A subsequent patch will add an warning message
> about the deprecation.
>
> Reported-by: Alexander Swen <alex-UE4+9DRHXtc@public.gmane.org>
> Cc: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org>
> ---
> fs/cifs/cifsfs.c | 5 +++++
> fs/cifs/cifsglob.h | 3 +++
> fs/cifs/dir.c | 2 +-
> fs/cifs/file.c | 4 ++--
> 4 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> index 3e29899..675ab40 100644
> --- a/fs/cifs/cifsfs.c
> +++ b/fs/cifs/cifsfs.c
> @@ -81,6 +81,11 @@ module_param(echo_retries, ushort, 0644);
> MODULE_PARM_DESC(echo_retries, "Number of echo attempts before giving up and "
> "reconnecting server. Default: 5. 0 means "
> "never reconnect.");
> +unsigned int enable_oplocks = 1;
> +module_param(enable_oplocks, bool, 0644);
> +MODULE_PARM_DESC(enable_oplocks, "Enable or disable oplocks (bool). Default:"
> + "y/Y/1");
> +
> extern mempool_t *cifs_sm_req_poolp;
> extern mempool_t *cifs_req_poolp;
> extern mempool_t *cifs_mid_poolp;
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index 6255fa8..b3e229c 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -936,6 +936,9 @@ GLOBAL_EXTERN unsigned int cifs_max_pending; /* MAX requests at once to server*/
> /* reconnect after this many failed echo attempts */
> GLOBAL_EXTERN unsigned short echo_retries;
>
> +/* enable or disable oplocks */
> +GLOBAL_EXTERN unsigned int enable_oplocks;
> +
> GLOBAL_EXTERN struct rb_root uidtree;
> GLOBAL_EXTERN struct rb_root gidtree;
> GLOBAL_EXTERN spinlock_t siduidlock;
> diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
> index 81914df..6529b69 100644
> --- a/fs/cifs/dir.c
> +++ b/fs/cifs/dir.c
> @@ -165,7 +165,7 @@ cifs_create(struct inode *inode, struct dentry *direntry, int mode,
> }
> tcon = tlink_tcon(tlink);
>
> - if (oplockEnabled)
> + if (oplockEnabled || enable_oplocks)
> oplock = REQ_OPLOCK;
>
> if (nd && (nd->flags & LOOKUP_OPEN))
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index bb71471..b76c31d 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -369,7 +369,7 @@ int cifs_open(struct inode *inode, struct file *file)
> cFYI(1, "inode = 0x%p file flags are 0x%x for %s",
> inode, file->f_flags, full_path);
>
> - if (oplockEnabled)
> + if (oplockEnabled || enable_oplocks)
> oplock = REQ_OPLOCK;
> else
> oplock = 0;
> @@ -493,7 +493,7 @@ static int cifs_reopen_file(struct cifsFileInfo *pCifsFile, bool can_flush)
> cFYI(1, "inode = 0x%p file flags 0x%x for %s",
> inode, pCifsFile->f_flags, full_path);
>
> - if (oplockEnabled)
> + if (oplockEnabled || enable_oplocks)
> oplock = REQ_OPLOCK;
> else
> oplock = 0;
>
--
Thanks,
Steve
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] cifs: add new module parameter 'enable_oplocks'
[not found] ` <20111011124405.37e09487-4QP7MXygkU+dMjc06nkz3ljfA9RmPOcC@public.gmane.org>
@ 2011-10-11 18:50 ` Steve French
[not found] ` <CAH2r5mv=SruLndtjRjPkpF27n5ekokk7SwMgY+4eOpehbJBpHg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2011-10-12 5:59 ` Suresh Jayaraman
1 sibling, 1 reply; 7+ messages in thread
From: Steve French @ 2011-10-11 18:50 UTC (permalink / raw)
To: Jeff Layton; +Cc: sjayaraman-l3A5Bk7waGM, linux-cifs, Alexander Swen
On Tue, Oct 11, 2011 at 11:44 AM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On Tue, 11 Oct 2011 21:28:00 +0530
> Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org> wrote:
>
>> Thus spake Jeff Layton:
>>
>> "Making that a module parm would allow you to set that parameter at boot
>> time without needing to add special startup scripts. IMO, all of the
>> procfile "switches" under /proc/fs/cifs should be module parms
>> instead."
>>
>> This patch doesn't alter the default behavior (Oplocks are enabled by
>> default).
>>
>> To disable oplocks when loading the module, use
>>
>> modprobe cifs enable_oplocks=0
>>
>> (any of '0' or 'n' or 'N' conventions can be used).
>>
>> To disable oplocks at runtime using the new interface, use
>>
>> echo 0 > /sys/module/cifs/parameters/enable_oplocks
>>
>> The older /proc/fs/cifs/OplockEnabled interface will be deprecated
>> after two releases. A subsequent patch will add an warning message
>> about the deprecation.
>>
>> Reported-by: Alexander Swen <alex-UE4+9DRHXtc@public.gmane.org>
>> Cc: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>> Signed-off-by: Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org>
>> ---
>> fs/cifs/cifsfs.c | 5 +++++
>> fs/cifs/cifsglob.h | 3 +++
>> fs/cifs/dir.c | 2 +-
>> fs/cifs/file.c | 4 ++--
>> 4 files changed, 11 insertions(+), 3 deletions(-)
>>
>> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
>> index 3e29899..675ab40 100644
>> --- a/fs/cifs/cifsfs.c
>> +++ b/fs/cifs/cifsfs.c
>> @@ -81,6 +81,11 @@ module_param(echo_retries, ushort, 0644);
>> MODULE_PARM_DESC(echo_retries, "Number of echo attempts before giving up and "
>> "reconnecting server. Default: 5. 0 means "
>> "never reconnect.");
>> +unsigned int enable_oplocks = 1;
>> +module_param(enable_oplocks, bool, 0644);
>> +MODULE_PARM_DESC(enable_oplocks, "Enable or disable oplocks (bool). Default:"
>> + "y/Y/1");
>> +
>> extern mempool_t *cifs_sm_req_poolp;
>> extern mempool_t *cifs_req_poolp;
>> extern mempool_t *cifs_mid_poolp;
>> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
>> index 6255fa8..b3e229c 100644
>> --- a/fs/cifs/cifsglob.h
>> +++ b/fs/cifs/cifsglob.h
>> @@ -936,6 +936,9 @@ GLOBAL_EXTERN unsigned int cifs_max_pending; /* MAX requests at once to server*/
>> /* reconnect after this many failed echo attempts */
>> GLOBAL_EXTERN unsigned short echo_retries;
>>
>> +/* enable or disable oplocks */
>> +GLOBAL_EXTERN unsigned int enable_oplocks;
>> +
>
> Let's not add an extra variable for this. I'd just rename oplockEnabled
> to enable_oplocks and fix up all the references to it. It should
> probably also be a bool too...
Right, leave as one variable - as I mentioned in the earlier note, it
also looks like adding
another variable broke the logic in Suresh's patch.
--
Thanks,
Steve
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] cifs: add new module parameter 'enable_oplocks'
[not found] ` <CAH2r5mv=SruLndtjRjPkpF27n5ekokk7SwMgY+4eOpehbJBpHg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2011-10-12 5:21 ` Suresh Jayaraman
0 siblings, 0 replies; 7+ messages in thread
From: Suresh Jayaraman @ 2011-10-12 5:21 UTC (permalink / raw)
To: Steve French; +Cc: Jeff Layton, linux-cifs, Alexander Swen
On 10/12/2011 12:20 AM, Steve French wrote:
> On Tue, Oct 11, 2011 at 11:44 AM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>> On Tue, 11 Oct 2011 21:28:00 +0530
>> Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org> wrote:
>>
>>> Thus spake Jeff Layton:
>>>
>>> "Making that a module parm would allow you to set that parameter at boot
>>> time without needing to add special startup scripts. IMO, all of the
>>> procfile "switches" under /proc/fs/cifs should be module parms
>>> instead."
>>>
>>> This patch doesn't alter the default behavior (Oplocks are enabled by
>>> default).
>>>
>>> To disable oplocks when loading the module, use
>>>
>>> modprobe cifs enable_oplocks=0
>>>
>>> (any of '0' or 'n' or 'N' conventions can be used).
>>>
>>> To disable oplocks at runtime using the new interface, use
>>>
>>> echo 0 > /sys/module/cifs/parameters/enable_oplocks
>>>
>>> The older /proc/fs/cifs/OplockEnabled interface will be deprecated
>>> after two releases. A subsequent patch will add an warning message
>>> about the deprecation.
>>>
>>> Reported-by: Alexander Swen <alex-UE4+9DRHXtc@public.gmane.org>
>>> Cc: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>>> Signed-off-by: Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org>
>>> ---
>>> fs/cifs/cifsfs.c | 5 +++++
>>> fs/cifs/cifsglob.h | 3 +++
>>> fs/cifs/dir.c | 2 +-
>>> fs/cifs/file.c | 4 ++--
>>> 4 files changed, 11 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
>>> index 3e29899..675ab40 100644
>>> --- a/fs/cifs/cifsfs.c
>>> +++ b/fs/cifs/cifsfs.c
>>> @@ -81,6 +81,11 @@ module_param(echo_retries, ushort, 0644);
>>> MODULE_PARM_DESC(echo_retries, "Number of echo attempts before giving up and "
>>> "reconnecting server. Default: 5. 0 means "
>>> "never reconnect.");
>>> +unsigned int enable_oplocks = 1;
>>> +module_param(enable_oplocks, bool, 0644);
>>> +MODULE_PARM_DESC(enable_oplocks, "Enable or disable oplocks (bool). Default:"
>>> + "y/Y/1");
>>> +
>>> extern mempool_t *cifs_sm_req_poolp;
>>> extern mempool_t *cifs_req_poolp;
>>> extern mempool_t *cifs_mid_poolp;
>>> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
>>> index 6255fa8..b3e229c 100644
>>> --- a/fs/cifs/cifsglob.h
>>> +++ b/fs/cifs/cifsglob.h
>>> @@ -936,6 +936,9 @@ GLOBAL_EXTERN unsigned int cifs_max_pending; /* MAX requests at once to server*/
>>> /* reconnect after this many failed echo attempts */
>>> GLOBAL_EXTERN unsigned short echo_retries;
>>>
>>> +/* enable or disable oplocks */
>>> +GLOBAL_EXTERN unsigned int enable_oplocks;
>>> +
>>
>> Let's not add an extra variable for this. I'd just rename oplockEnabled
>> to enable_oplocks and fix up all the references to it. It should
>> probably also be a bool too...
Will make this change.
> Right, leave as one variable - as I mentioned in the earlier note, it
> also looks like adding
> another variable broke the logic in Suresh's patch.
>
Bah, indeed it did. Will repost the patches.
-Suresh
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] cifs: add new module parameter 'enable_oplocks'
[not found] ` <20111011124405.37e09487-4QP7MXygkU+dMjc06nkz3ljfA9RmPOcC@public.gmane.org>
2011-10-11 18:50 ` Steve French
@ 2011-10-12 5:59 ` Suresh Jayaraman
[not found] ` <4E952CBD.2010906-l3A5Bk7waGM@public.gmane.org>
1 sibling, 1 reply; 7+ messages in thread
From: Suresh Jayaraman @ 2011-10-12 5:59 UTC (permalink / raw)
To: Jeff Layton; +Cc: Steve French, linux-cifs, Alexander Swen
On 10/11/2011 10:14 PM, Jeff Layton wrote:
> On Tue, 11 Oct 2011 21:28:00 +0530
> Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org> wrote:
>
>> Thus spake Jeff Layton:
>>
>> "Making that a module parm would allow you to set that parameter at boot
>> time without needing to add special startup scripts. IMO, all of the
>> procfile "switches" under /proc/fs/cifs should be module parms
>> instead."
>>
>> This patch doesn't alter the default behavior (Oplocks are enabled by
>> default).
>>
>> To disable oplocks when loading the module, use
>>
>> modprobe cifs enable_oplocks=0
>>
>> (any of '0' or 'n' or 'N' conventions can be used).
>>
>> To disable oplocks at runtime using the new interface, use
>>
>> echo 0 > /sys/module/cifs/parameters/enable_oplocks
>>
>> The older /proc/fs/cifs/OplockEnabled interface will be deprecated
>> after two releases. A subsequent patch will add an warning message
>> about the deprecation.
>>
>> Reported-by: Alexander Swen <alex-UE4+9DRHXtc@public.gmane.org>
>> Cc: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>> Signed-off-by: Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org>
>> ---
>> fs/cifs/cifsfs.c | 5 +++++
>> fs/cifs/cifsglob.h | 3 +++
>> fs/cifs/dir.c | 2 +-
>> fs/cifs/file.c | 4 ++--
>> 4 files changed, 11 insertions(+), 3 deletions(-)
>>
>> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
>> index 3e29899..675ab40 100644
>> --- a/fs/cifs/cifsfs.c
>> +++ b/fs/cifs/cifsfs.c
>> @@ -81,6 +81,11 @@ module_param(echo_retries, ushort, 0644);
>> MODULE_PARM_DESC(echo_retries, "Number of echo attempts before giving up and "
>> "reconnecting server. Default: 5. 0 means "
>> "never reconnect.");
>> +unsigned int enable_oplocks = 1;
>> +module_param(enable_oplocks, bool, 0644);
>> +MODULE_PARM_DESC(enable_oplocks, "Enable or disable oplocks (bool). Default:"
>> + "y/Y/1");
>> +
>> extern mempool_t *cifs_sm_req_poolp;
>> extern mempool_t *cifs_req_poolp;
>> extern mempool_t *cifs_mid_poolp;
>> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
>> index 6255fa8..b3e229c 100644
>> --- a/fs/cifs/cifsglob.h
>> +++ b/fs/cifs/cifsglob.h
>> @@ -936,6 +936,9 @@ GLOBAL_EXTERN unsigned int cifs_max_pending; /* MAX requests at once to server*/
>> /* reconnect after this many failed echo attempts */
>> GLOBAL_EXTERN unsigned short echo_retries;
>>
>> +/* enable or disable oplocks */
>> +GLOBAL_EXTERN unsigned int enable_oplocks;
>> +
>
> Let's not add an extra variable for this. I'd just rename oplockEnabled
> to enable_oplocks and fix up all the references to it. It should
> probably also be a bool too...
>
Need not be I think. Looking in to it further it looks like 'bool' type
of module parameter are stored in variable of type 'int'. Looking into a
few examples comfirms this too (e.g. enable_ino64 and printk_time etc.).
So, I'll make it an int.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] cifs: add new module parameter 'enable_oplocks'
[not found] ` <4E952CBD.2010906-l3A5Bk7waGM@public.gmane.org>
@ 2011-10-12 11:28 ` Jeff Layton
0 siblings, 0 replies; 7+ messages in thread
From: Jeff Layton @ 2011-10-12 11:28 UTC (permalink / raw)
To: sjayaraman-l3A5Bk7waGM; +Cc: Steve French, linux-cifs, Alexander Swen
On Wed, 12 Oct 2011 11:29:25 +0530
Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org> wrote:
> On 10/11/2011 10:14 PM, Jeff Layton wrote:
> > On Tue, 11 Oct 2011 21:28:00 +0530
> > Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org> wrote:
> >
> >> Thus spake Jeff Layton:
> >>
> >> "Making that a module parm would allow you to set that parameter at boot
> >> time without needing to add special startup scripts. IMO, all of the
> >> procfile "switches" under /proc/fs/cifs should be module parms
> >> instead."
> >>
> >> This patch doesn't alter the default behavior (Oplocks are enabled by
> >> default).
> >>
> >> To disable oplocks when loading the module, use
> >>
> >> modprobe cifs enable_oplocks=0
> >>
> >> (any of '0' or 'n' or 'N' conventions can be used).
> >>
> >> To disable oplocks at runtime using the new interface, use
> >>
> >> echo 0 > /sys/module/cifs/parameters/enable_oplocks
> >>
> >> The older /proc/fs/cifs/OplockEnabled interface will be deprecated
> >> after two releases. A subsequent patch will add an warning message
> >> about the deprecation.
> >>
> >> Reported-by: Alexander Swen <alex-UE4+9DRHXtc@public.gmane.org>
> >> Cc: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> >> Signed-off-by: Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org>
> >> ---
> >> fs/cifs/cifsfs.c | 5 +++++
> >> fs/cifs/cifsglob.h | 3 +++
> >> fs/cifs/dir.c | 2 +-
> >> fs/cifs/file.c | 4 ++--
> >> 4 files changed, 11 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> >> index 3e29899..675ab40 100644
> >> --- a/fs/cifs/cifsfs.c
> >> +++ b/fs/cifs/cifsfs.c
> >> @@ -81,6 +81,11 @@ module_param(echo_retries, ushort, 0644);
> >> MODULE_PARM_DESC(echo_retries, "Number of echo attempts before giving up and "
> >> "reconnecting server. Default: 5. 0 means "
> >> "never reconnect.");
> >> +unsigned int enable_oplocks = 1;
> >> +module_param(enable_oplocks, bool, 0644);
> >> +MODULE_PARM_DESC(enable_oplocks, "Enable or disable oplocks (bool). Default:"
> >> + "y/Y/1");
> >> +
> >> extern mempool_t *cifs_sm_req_poolp;
> >> extern mempool_t *cifs_req_poolp;
> >> extern mempool_t *cifs_mid_poolp;
> >> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> >> index 6255fa8..b3e229c 100644
> >> --- a/fs/cifs/cifsglob.h
> >> +++ b/fs/cifs/cifsglob.h
> >> @@ -936,6 +936,9 @@ GLOBAL_EXTERN unsigned int cifs_max_pending; /* MAX requests at once to server*/
> >> /* reconnect after this many failed echo attempts */
> >> GLOBAL_EXTERN unsigned short echo_retries;
> >>
> >> +/* enable or disable oplocks */
> >> +GLOBAL_EXTERN unsigned int enable_oplocks;
> >> +
> >
> > Let's not add an extra variable for this. I'd just rename oplockEnabled
> > to enable_oplocks and fix up all the references to it. It should
> > probably also be a bool too...
> >
>
> Need not be I think. Looking in to it further it looks like 'bool' type
> of module parameter are stored in variable of type 'int'. Looking into a
> few examples comfirms this too (e.g. enable_ino64 and printk_time etc.).
> So, I'll make it an int.
I don't think so...
types.h has this:
typedef _Bool bool;
...and _Bool is an internal type in gcc (and other C9X compilers). I
think gcc actually uses a char for this internally, but that's not
always guaranteed to be the case. In any event, why ask for more memory
than you need here? This is just a single-bit flag.
--
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2011-10-12 11:28 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-10-11 15:58 [PATCH 1/3] cifs: add new module parameter 'enable_oplocks' Suresh Jayaraman
[not found] ` <4E946788.3060101-l3A5Bk7waGM@public.gmane.org>
2011-10-11 16:44 ` Jeff Layton
[not found] ` <20111011124405.37e09487-4QP7MXygkU+dMjc06nkz3ljfA9RmPOcC@public.gmane.org>
2011-10-11 18:50 ` Steve French
[not found] ` <CAH2r5mv=SruLndtjRjPkpF27n5ekokk7SwMgY+4eOpehbJBpHg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2011-10-12 5:21 ` Suresh Jayaraman
2011-10-12 5:59 ` Suresh Jayaraman
[not found] ` <4E952CBD.2010906-l3A5Bk7waGM@public.gmane.org>
2011-10-12 11:28 ` Jeff Layton
2011-10-11 17:31 ` Steve French
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.