[PATCH] fs: Rename the parameter of mnt_get_write_access()

[PATCH] fs: Rename the parameter of mnt_get_write_access()

[Zizhi Wo]

From: Zizhi Wo <wozizhi@huawei.com>

Rename the parameter in mnt_get_write_access() from "m" to "mnt" for
consistency between declaration and implementation.

Signed-off-by: Zizhi Wo <wozizhi@huawei.com>
---
 fs/namespace.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/fs/namespace.c b/fs/namespace.c
index 1b466c54a357..b1b679433ab3 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -483,7 +483,7 @@ static int mnt_is_readonly(struct vfsmount *mnt)
  */
 /**
  * mnt_get_write_access - get write access to a mount without freeze protection
- * @m: the mount on which to take a write
+ * @mnt: the mount on which to take a write
  *
  * This tells the low-level filesystem that a write is about to be performed to
  * it, and makes sure that writes are allowed (mnt it read-write) before
@@ -491,13 +491,13 @@ static int mnt_is_readonly(struct vfsmount *mnt)
  * frozen. When the write operation is finished, mnt_put_write_access() must be
  * called. This is effectively a refcount.
  */
-int mnt_get_write_access(struct vfsmount *m)
+int mnt_get_write_access(struct vfsmount *mnt)
 {
-	struct mount *mnt = real_mount(m);
+	struct mount *m = real_mount(mnt);
 	int ret = 0;
 
 	preempt_disable();
-	mnt_inc_writers(mnt);
+	mnt_inc_writers(m);
 	/*
 	 * The store to mnt_inc_writers must be visible before we pass
 	 * MNT_WRITE_HOLD loop below, so that the slowpath can see our
@@ -505,7 +505,7 @@ int mnt_get_write_access(struct vfsmount *m)
 	 */
 	smp_mb();
 	might_lock(&mount_lock.lock);
-	while (READ_ONCE(mnt->mnt.mnt_flags) & MNT_WRITE_HOLD) {
+	while (READ_ONCE(m->mnt.mnt_flags) & MNT_WRITE_HOLD) {
 		if (!IS_ENABLED(CONFIG_PREEMPT_RT)) {
 			cpu_relax();
 		} else {
@@ -530,8 +530,8 @@ int mnt_get_write_access(struct vfsmount *m)
 	 * read-only.
 	 */
 	smp_rmb();
-	if (mnt_is_readonly(m)) {
-		mnt_dec_writers(mnt);
+	if (mnt_is_readonly(mnt)) {
+		mnt_dec_writers(m);
 		ret = -EROFS;
 	}
 	preempt_enable();
-- 
2.39.2

Re: [PATCH] fs: Rename the parameter of mnt_get_write_access()

[Jan Kara]

On Fri 16-05-25 11:21:47, Zizhi Wo wrote:
> From: Zizhi Wo <wozizhi@huawei.com>
> 
> Rename the parameter in mnt_get_write_access() from "m" to "mnt" for
> consistency between declaration and implementation.
> 
> Signed-off-by: Zizhi Wo <wozizhi@huawei.com>

I'm sorry but this is just a pointless churn. I agree the declaration and
implementation should better be consistent (although in this particular
case it isn't too worrying) but it's much easier (and with much lower
chance to cause conflicts) to just fixup the declaration.

								Honza

> ---
>  fs/namespace.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/namespace.c b/fs/namespace.c
> index 1b466c54a357..b1b679433ab3 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -483,7 +483,7 @@ static int mnt_is_readonly(struct vfsmount *mnt)
>   */
>  /**
>   * mnt_get_write_access - get write access to a mount without freeze protection
> - * @m: the mount on which to take a write
> + * @mnt: the mount on which to take a write
>   *
>   * This tells the low-level filesystem that a write is about to be performed to
>   * it, and makes sure that writes are allowed (mnt it read-write) before
> @@ -491,13 +491,13 @@ static int mnt_is_readonly(struct vfsmount *mnt)
>   * frozen. When the write operation is finished, mnt_put_write_access() must be
>   * called. This is effectively a refcount.
>   */
> -int mnt_get_write_access(struct vfsmount *m)
> +int mnt_get_write_access(struct vfsmount *mnt)
>  {
> -	struct mount *mnt = real_mount(m);
> +	struct mount *m = real_mount(mnt);
>  	int ret = 0;
>  
>  	preempt_disable();
> -	mnt_inc_writers(mnt);
> +	mnt_inc_writers(m);
>  	/*
>  	 * The store to mnt_inc_writers must be visible before we pass
>  	 * MNT_WRITE_HOLD loop below, so that the slowpath can see our
> @@ -505,7 +505,7 @@ int mnt_get_write_access(struct vfsmount *m)
>  	 */
>  	smp_mb();
>  	might_lock(&mount_lock.lock);
> -	while (READ_ONCE(mnt->mnt.mnt_flags) & MNT_WRITE_HOLD) {
> +	while (READ_ONCE(m->mnt.mnt_flags) & MNT_WRITE_HOLD) {
>  		if (!IS_ENABLED(CONFIG_PREEMPT_RT)) {
>  			cpu_relax();
>  		} else {
> @@ -530,8 +530,8 @@ int mnt_get_write_access(struct vfsmount *m)
>  	 * read-only.
>  	 */
>  	smp_rmb();
> -	if (mnt_is_readonly(m)) {
> -		mnt_dec_writers(mnt);
> +	if (mnt_is_readonly(mnt)) {
> +		mnt_dec_writers(m);
>  		ret = -EROFS;
>  	}
>  	preempt_enable();
> -- 
> 2.39.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH] fs: Rename the parameter of mnt_get_write_access()

[Zizhi Wo]

在 2025/5/16 18:31, Jan Kara 写道:
> On Fri 16-05-25 11:21:47, Zizhi Wo wrote:
>> From: Zizhi Wo <wozizhi@huawei.com>
>>
>> Rename the parameter in mnt_get_write_access() from "m" to "mnt" for
>> consistency between declaration and implementation.
>>
>> Signed-off-by: Zizhi Wo <wozizhi@huawei.com>
> 
> I'm sorry but this is just a pointless churn. I agree the declaration and
> implementation should better be consistent (although in this particular
> case it isn't too worrying) but it's much easier (and with much lower
> chance to cause conflicts) to just fixup the declaration.
> 
> 								Honza

Yes, I had considered simply fixing the declaration earlier. However, in
the include/linux/mount.h file, similar functions like
"mnt_put_write_access" use "mnt" as the parameter name rather than "m",
just like "mnt_get_write_access". So I chose to modify the function
implementation directly, although this resulted in a larger amount of
changes. So as you can see, for simplicity, I will directly update the
parameter name in the function declaration in the second version.

Thanks,
Zizhi Wo

> 
>> ---
>>   fs/namespace.c | 14 +++++++-------
>>   1 file changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/fs/namespace.c b/fs/namespace.c
>> index 1b466c54a357..b1b679433ab3 100644
>> --- a/fs/namespace.c
>> +++ b/fs/namespace.c
>> @@ -483,7 +483,7 @@ static int mnt_is_readonly(struct vfsmount *mnt)
>>    */
>>   /**
>>    * mnt_get_write_access - get write access to a mount without freeze protection
>> - * @m: the mount on which to take a write
>> + * @mnt: the mount on which to take a write
>>    *
>>    * This tells the low-level filesystem that a write is about to be performed to
>>    * it, and makes sure that writes are allowed (mnt it read-write) before
>> @@ -491,13 +491,13 @@ static int mnt_is_readonly(struct vfsmount *mnt)
>>    * frozen. When the write operation is finished, mnt_put_write_access() must be
>>    * called. This is effectively a refcount.
>>    */
>> -int mnt_get_write_access(struct vfsmount *m)
>> +int mnt_get_write_access(struct vfsmount *mnt)
>>   {
>> -	struct mount *mnt = real_mount(m);
>> +	struct mount *m = real_mount(mnt);
>>   	int ret = 0;
>>   
>>   	preempt_disable();
>> -	mnt_inc_writers(mnt);
>> +	mnt_inc_writers(m);
>>   	/*
>>   	 * The store to mnt_inc_writers must be visible before we pass
>>   	 * MNT_WRITE_HOLD loop below, so that the slowpath can see our
>> @@ -505,7 +505,7 @@ int mnt_get_write_access(struct vfsmount *m)
>>   	 */
>>   	smp_mb();
>>   	might_lock(&mount_lock.lock);
>> -	while (READ_ONCE(mnt->mnt.mnt_flags) & MNT_WRITE_HOLD) {
>> +	while (READ_ONCE(m->mnt.mnt_flags) & MNT_WRITE_HOLD) {
>>   		if (!IS_ENABLED(CONFIG_PREEMPT_RT)) {
>>   			cpu_relax();
>>   		} else {
>> @@ -530,8 +530,8 @@ int mnt_get_write_access(struct vfsmount *m)
>>   	 * read-only.
>>   	 */
>>   	smp_rmb();
>> -	if (mnt_is_readonly(m)) {
>> -		mnt_dec_writers(mnt);
>> +	if (mnt_is_readonly(mnt)) {
>> +		mnt_dec_writers(m);
>>   		ret = -EROFS;
>>   	}
>>   	preempt_enable();
>> -- 
>> 2.39.2
>>

Re: [PATCH] fs: Rename the parameter of mnt_get_write_access()

[Al Viro]

On Sat, May 17, 2025 at 07:54:55AM +0800, Zizhi Wo wrote:
> 
> 
> 在 2025/5/16 18:31, Jan Kara 写道:
> > On Fri 16-05-25 11:21:47, Zizhi Wo wrote:
> > > From: Zizhi Wo <wozizhi@huawei.com>
> > > 
> > > Rename the parameter in mnt_get_write_access() from "m" to "mnt" for
> > > consistency between declaration and implementation.
> > > 
> > > Signed-off-by: Zizhi Wo <wozizhi@huawei.com>
> > 
> > I'm sorry but this is just a pointless churn. I agree the declaration and
> > implementation should better be consistent (although in this particular
> > case it isn't too worrying) but it's much easier (and with much lower
> > chance to cause conflicts) to just fixup the declaration.
> > 
> > 								Honza
> 
> Yes, I had considered simply fixing the declaration earlier. However, in
> the include/linux/mount.h file, similar functions like
> "mnt_put_write_access" use "mnt" as the parameter name rather than "m",
> just like "mnt_get_write_access". So I chose to modify the function
> implementation directly, although this resulted in a larger amount of
> changes. So as you can see, for simplicity, I will directly update the
> parameter name in the function declaration in the second version.

FWIW, "mnt for vfsmount, m for mount" is an informal convention in that
area, so I'd say go for it if there had been any change in the function
in question.  Same as with coding style, really...

Re: [PATCH] fs: Rename the parameter of mnt_get_write_access()

[Zizhi Wo]

在 2025/5/17 7:57, Al Viro 写道:
> On Sat, May 17, 2025 at 07:54:55AM +0800, Zizhi Wo wrote:
>>
>>
>> 在 2025/5/16 18:31, Jan Kara 写道:
>>> On Fri 16-05-25 11:21:47, Zizhi Wo wrote:
>>>> From: Zizhi Wo <wozizhi@huawei.com>
>>>>
>>>> Rename the parameter in mnt_get_write_access() from "m" to "mnt" for
>>>> consistency between declaration and implementation.
>>>>
>>>> Signed-off-by: Zizhi Wo <wozizhi@huawei.com>
>>>
>>> I'm sorry but this is just a pointless churn. I agree the declaration and
>>> implementation should better be consistent (although in this particular
>>> case it isn't too worrying) but it's much easier (and with much lower
>>> chance to cause conflicts) to just fixup the declaration.
>>>
>>> 								Honza
>>
>> Yes, I had considered simply fixing the declaration earlier. However, in
>> the include/linux/mount.h file, similar functions like
>> "mnt_put_write_access" use "mnt" as the parameter name rather than "m",
>> just like "mnt_get_write_access". So I chose to modify the function
>> implementation directly, although this resulted in a larger amount of
>> changes. So as you can see, for simplicity, I will directly update the
>> parameter name in the function declaration in the second version.
> 
> FWIW, "mnt for vfsmount, m for mount" is an informal convention in that
> area, so I'd say go for it if there had been any change in the function
> in question.  Same as with coding style, really...

Thanks for the additional information. Based on informal conventions, it
seems that this is the only way to modify it... ?

Thanks,
Zizhi Wo

Re: [PATCH] fs: Rename the parameter of mnt_get_write_access()

[Zizhi Wo]

Hello!

There are currently two possible approaches to this patch.
The first is to directly change the declaration, which would be
straightforward and involve minimal modifications.

However, per Al Viro's suggestion — that "mnt for vfsmount, m for mount" 
is an informal convention. This is in line with what the current
patch does, although I understand Jan Kara might feel that the scope of 
the changes is a bit large.

I would appreciate any suggestions or guidance on how to proceed. So
friendly ping...

在 2025/5/16 11:21, Zizhi Wo 写道:
> From: Zizhi Wo <wozizhi@huawei.com>
> 
> Rename the parameter in mnt_get_write_access() from "m" to "mnt" for
> consistency between declaration and implementation.
> 
> Signed-off-by: Zizhi Wo <wozizhi@huawei.com>
> ---
>   fs/namespace.c | 14 +++++++-------
>   1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/namespace.c b/fs/namespace.c
> index 1b466c54a357..b1b679433ab3 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -483,7 +483,7 @@ static int mnt_is_readonly(struct vfsmount *mnt)
>    */
>   /**
>    * mnt_get_write_access - get write access to a mount without freeze protection
> - * @m: the mount on which to take a write
> + * @mnt: the mount on which to take a write
>    *
>    * This tells the low-level filesystem that a write is about to be performed to
>    * it, and makes sure that writes are allowed (mnt it read-write) before
> @@ -491,13 +491,13 @@ static int mnt_is_readonly(struct vfsmount *mnt)
>    * frozen. When the write operation is finished, mnt_put_write_access() must be
>    * called. This is effectively a refcount.
>    */
> -int mnt_get_write_access(struct vfsmount *m)
> +int mnt_get_write_access(struct vfsmount *mnt)
>   {
> -	struct mount *mnt = real_mount(m);
> +	struct mount *m = real_mount(mnt);
>   	int ret = 0;
>   
>   	preempt_disable();
> -	mnt_inc_writers(mnt);
> +	mnt_inc_writers(m);
>   	/*
>   	 * The store to mnt_inc_writers must be visible before we pass
>   	 * MNT_WRITE_HOLD loop below, so that the slowpath can see our
> @@ -505,7 +505,7 @@ int mnt_get_write_access(struct vfsmount *m)
>   	 */
>   	smp_mb();
>   	might_lock(&mount_lock.lock);
> -	while (READ_ONCE(mnt->mnt.mnt_flags) & MNT_WRITE_HOLD) {
> +	while (READ_ONCE(m->mnt.mnt_flags) & MNT_WRITE_HOLD) {
>   		if (!IS_ENABLED(CONFIG_PREEMPT_RT)) {
>   			cpu_relax();
>   		} else {
> @@ -530,8 +530,8 @@ int mnt_get_write_access(struct vfsmount *m)
>   	 * read-only.
>   	 */
>   	smp_rmb();
> -	if (mnt_is_readonly(m)) {
> -		mnt_dec_writers(mnt);
> +	if (mnt_is_readonly(mnt)) {
> +		mnt_dec_writers(m);
>   		ret = -EROFS;
>   	}
>   	preempt_enable();

Re: [PATCH] fs: Rename the parameter of mnt_get_write_access()

[Amir Goldstein]

On Thu, May 22, 2025 at 3:02 AM Zizhi Wo <wozizhi@huaweicloud.com> wrote:
>
> Hello!
>
> There are currently two possible approaches to this patch.
> The first is to directly change the declaration, which would be
> straightforward and involve minimal modifications.
>
> However, per Al Viro's suggestion — that "mnt for vfsmount, m for mount"
> is an informal convention. This is in line with what the current
> patch does, although I understand Jan Kara might feel that the scope of
> the changes is a bit large.
>
> I would appreciate any suggestions or guidance on how to proceed. So
> friendly ping...

Hi Zizhi,

I guess you are not familiar with kernel lingo so I will translate:
"...so I'd say go for it if there had been any change in the function
in question.  Same as with coding style, really..."

It means that your change is correct, but maintainers are
not interested in taking "style only" changes because it
creates undesired git history noise called "churn".

Should anyone be going to make logic changes in
mnt_get_write_access() in the future, the style change
can be applied along in the same patch.

One observation I have is -
If this was the only case that deviates from the standard
the change might have been justified.
From a quick grep, I see that the reality in the code is very far
from this standard.

FWIW, wholeheartedly I agree that the ambiguity of the type of
an 'mnt' arg is annoying, but IMO 'm' is not making that very clear.
To me, 'mount' arg is very clear when it appears in the code.

Thanks,
Amir.

Re: [PATCH] fs: Rename the parameter of mnt_get_write_access()

[Zizhi Wo]

在 2025/5/22 15:41, Amir Goldstein 写道:
> On Thu, May 22, 2025 at 3:02 AM Zizhi Wo <wozizhi@huaweicloud.com> wrote:
>>
>> Hello!
>>
>> There are currently two possible approaches to this patch.
>> The first is to directly change the declaration, which would be
>> straightforward and involve minimal modifications.
>>
>> However, per Al Viro's suggestion — that "mnt for vfsmount, m for mount"
>> is an informal convention. This is in line with what the current
>> patch does, although I understand Jan Kara might feel that the scope of
>> the changes is a bit large.
>>
>> I would appreciate any suggestions or guidance on how to proceed. So
>> friendly ping...
> 
> Hi Zizhi,
> 
> I guess you are not familiar with kernel lingo so I will translate:
> "...so I'd say go for it if there had been any change in the function
> in question.  Same as with coding style, really...
> 
> It means that your change is correct, but maintainers are
> not interested in taking "style only" changes because it
> creates undesired git history noise called "churn".

Thank you for your patient explanation! I'm indeed a newcomer to the
Linux kernel. Now I understand what everyone means.

> 
> Should anyone be going to make logic changes in
> mnt_get_write_access() in the future, the style change
> can be applied along in the same patch.
> 
> One observation I have is -
> If this was the only case that deviates from the standard
> the change might have been justified.
>>From a quick grep, I see that the reality in the code is very far
> from this standard.

Yes, I noticed that as well. However, for consistency with the later use
of mnt_put_write_access(), I chose to go with the modification in this
patch...

Thanks,
Zizhi Wo

> 
> FWIW, wholeheartedly I agree that the ambiguity of the type of
> an 'mnt' arg is annoying, but IMO 'm' is not making that very clear.
> To me, 'mount' arg is very clear when it appears in the code

> 
> Thanks,
> Amir.
>