linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH] exfat: remove EXFAT_SB_DIRTY flag
@ 2020-06-12  8:48 Markus Elfring
  2020-06-12 10:01 ` Greg KH
  0 siblings, 1 reply; 7+ messages in thread
From: Markus Elfring @ 2020-06-12  8:48 UTC (permalink / raw)
  To: Tetsuhiro Kohada, linux-fsdevel
  Cc: Tetsuhiro Kohada, Takahiro Mori, Hirotaka Motai, Namjae Jeon,
	Sungjong Seo, linux-kernel

> remove EXFAT_SB_DIRTY flag and related codes.

I suggest to omit this sentence because a similar information
is provided a bit later again for this change description.


> If performe 'sync' in this state, VOL_DIRTY will not be cleared.

Please improve this wording.


Would you like to add the tag “Fixes” to the commit message?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=b791d1bdf9212d944d749a5c7ff6febdba241771#n183

Regards,
Markus

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] exfat: remove EXFAT_SB_DIRTY flag
  2020-06-12  8:48 [PATCH] exfat: remove EXFAT_SB_DIRTY flag Markus Elfring
@ 2020-06-12 10:01 ` Greg KH
  0 siblings, 0 replies; 7+ messages in thread
From: Greg KH @ 2020-06-12 10:01 UTC (permalink / raw)
  To: Markus Elfring
  Cc: Tetsuhiro Kohada, linux-fsdevel, Tetsuhiro Kohada, Takahiro Mori,
	Hirotaka Motai, Namjae Jeon, Sungjong Seo, linux-kernel

On Fri, Jun 12, 2020 at 10:48:20AM +0200, Markus Elfring wrote:
> > remove EXFAT_SB_DIRTY flag and related codes.
> 
> I suggest to omit this sentence because a similar information
> is provided a bit later again for this change description.
> 
> 
> > If performe 'sync' in this state, VOL_DIRTY will not be cleared.
> 
> Please improve this wording.
> 
> 
> Would you like to add the tag “Fixes” to the commit message?
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=b791d1bdf9212d944d749a5c7ff6febdba241771#n183
> 
> Regards,
> Markus

Hi,

This is the semi-friendly patch-bot of Greg Kroah-Hartman.

Markus, you seem to have sent a nonsensical or otherwise pointless
review comment to a patch submission on a Linux kernel developer mailing
list.  I strongly suggest that you not do this anymore.  Please do not
bother developers who are actively working to produce patches and
features with comments that, in the end, are a waste of time.

Patch submitter, please ignore Markus's suggestion; you do not need to
follow it at all.  The person/bot/AI that sent it is being ignored by
almost all Linux kernel maintainers for having a persistent pattern of
behavior of producing distracting and pointless commentary, and
inability to adapt to feedback.  Please feel free to also ignore emails
from them.

thanks,

greg k-h's patch email bot

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH] exfat: remove EXFAT_SB_DIRTY flag
  2020-06-15  2:59       ` Sungjong Seo
@ 2020-06-15  8:31         ` Kohada.Tetsuhiro
  0 siblings, 0 replies; 7+ messages in thread
From: Kohada.Tetsuhiro @ 2020-06-15  8:31 UTC (permalink / raw)
  To: 'sj1557.seo@samsung.com'
  Cc: Mori.Takahiro, Motai.Hirotaka, 'Namjae Jeon',
	'linux-fsdevel@vger.kernel.org',
	'linux-kernel@vger.kernel.org'

> > On 2020/06/12 17:34, Sungjong Seo wrote:
> > >> remove EXFAT_SB_DIRTY flag and related codes.
> > >>
> > >> This flag is set/reset in exfat_put_super()/exfat_sync_fs() to
> > >> avoid sync_blockdev().
> > >> However ...
> > >> - exfat_put_super():
> > >> Before calling this, the VFS has already called sync_filesystem(),
> > >> so sync is never performed here.
> > >> - exfat_sync_fs():
> > >> After calling this, the VFS calls sync_blockdev(), so, it is
> > >> meaningless to check EXFAT_SB_DIRTY or to bypass sync_blockdev() here.
> > >> Not only that, but in some cases can't clear VOL_DIRTY.
> > >> ex:
> > >> VOL_DIRTY is set when rmdir starts, but when non-empty-dir is
> > >> detected, return error without setting EXFAT_SB_DIRTY.
> > >> If performe 'sync' in this state, VOL_DIRTY will not be cleared.
> > >>
> > >> Remove the EXFAT_SB_DIRTY check to ensure synchronization.
> > >> And, remove the code related to the flag.
> > >>
> > >> Signed-off-by: Tetsuhiro Kohada <kohada.t2@gmail.com>
> > >> ---
> > >>   fs/exfat/balloc.c   |  4 ++--
> > >>   fs/exfat/dir.c      | 16 ++++++++--------
> > >>   fs/exfat/exfat_fs.h |  5 +----
> > >>   fs/exfat/fatent.c   |  7 ++-----
> > >>   fs/exfat/misc.c     |  3 +--
> > >>   fs/exfat/namei.c    | 12 ++++++------
> > >>   fs/exfat/super.c    | 11 +++--------
> > >>   7 files changed, 23 insertions(+), 35 deletions(-)
> > >>
> > > [snip]
> > >>
> > >> @@ -62,11 +59,9 @@ static int exfat_sync_fs(struct super_block *sb,
> > >> int
> > >> wait)
> > >>
> > >>   	/* If there are some dirty buffers in the bdev inode */
> > >>   	mutex_lock(&sbi->s_lock);
> > >> -	if (test_and_clear_bit(EXFAT_SB_DIRTY, &sbi->s_state)) {
> > >> -		sync_blockdev(sb->s_bdev);
> > >> -		if (exfat_set_vol_flags(sb, VOL_CLEAN))
> > >> -			err = -EIO;
> > >> -	}
> > >
> > > I looked through most codes related to EXFAT_SB_DIRTY and VOL_DIRTY.
> > > And your approach looks good because all of them seem to be
> > > protected by s_lock.
> > >
> > > BTW, as you know, sync_filesystem() calls sync_fs() with 'nowait'
> > > first, and then calls it again with 'wait' twice. No need to sync
> > > with
> > lock twice.
> > > If so, isn't it okay to do nothing when wait is 0?
> >
> > I also think  ‘do nothing when wait is 0’ as you say, but I'm still
> > not sure.
> >
> > Some other Filesystems do nothing with nowait and just return.
> > However, a few Filesystems always perform sync.
> >
> > sync_blockdev() waits for completion, so it may be inappropriate to
> > call with  nowait. (But it was called in the original code)
> >
> > I'm still not sure, so I excluded it in this patch.
> > Is it okay to include it?
> >
> 
> Yes, I think so. sync_filesystem() will call __sync_blockdev() without 'wait' first.
> So, it's enough to call sync_blockdev() with s_lock just one time.

OK.
I will repost v2-patch with the 'wait' check added.
Thanks for your comment.


> > >> +	sync_blockdev(sb->s_bdev);
> > >> +	if (exfat_set_vol_flags(sb, VOL_CLEAN))
> > >> +		err = -EIO;
> > >>   	mutex_unlock(&sbi->s_lock);
> > >>   	return err;
> > >>   }
> > >> --
> > >> 2.25.1

BR
---
Kohada Tetsuhiro <Kohada.Tetsuhiro@dc.MitsubishiElectric.co.jp>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH] exfat: remove EXFAT_SB_DIRTY flag
  2020-06-12 10:22     ` Tetsuhiro Kohada
@ 2020-06-15  2:59       ` Sungjong Seo
  2020-06-15  8:31         ` Kohada.Tetsuhiro
  0 siblings, 1 reply; 7+ messages in thread
From: Sungjong Seo @ 2020-06-15  2:59 UTC (permalink / raw)
  To: 'Tetsuhiro Kohada'
  Cc: kohada.tetsuhiro, mori.takahiro, motai.hirotaka,
	'Namjae Jeon',
	linux-fsdevel, linux-kernel

> On 2020/06/12 17:34, Sungjong Seo wrote:
> >> remove EXFAT_SB_DIRTY flag and related codes.
> >>
> >> This flag is set/reset in exfat_put_super()/exfat_sync_fs() to avoid
> >> sync_blockdev().
> >> However ...
> >> - exfat_put_super():
> >> Before calling this, the VFS has already called sync_filesystem(), so
> >> sync is never performed here.
> >> - exfat_sync_fs():
> >> After calling this, the VFS calls sync_blockdev(), so, it is
> >> meaningless to check EXFAT_SB_DIRTY or to bypass sync_blockdev() here.
> >> Not only that, but in some cases can't clear VOL_DIRTY.
> >> ex:
> >> VOL_DIRTY is set when rmdir starts, but when non-empty-dir is
> >> detected, return error without setting EXFAT_SB_DIRTY.
> >> If performe 'sync' in this state, VOL_DIRTY will not be cleared.
> >>
> >> Remove the EXFAT_SB_DIRTY check to ensure synchronization.
> >> And, remove the code related to the flag.
> >>
> >> Signed-off-by: Tetsuhiro Kohada <kohada.t2@gmail.com>
> >> ---
> >>   fs/exfat/balloc.c   |  4 ++--
> >>   fs/exfat/dir.c      | 16 ++++++++--------
> >>   fs/exfat/exfat_fs.h |  5 +----
> >>   fs/exfat/fatent.c   |  7 ++-----
> >>   fs/exfat/misc.c     |  3 +--
> >>   fs/exfat/namei.c    | 12 ++++++------
> >>   fs/exfat/super.c    | 11 +++--------
> >>   7 files changed, 23 insertions(+), 35 deletions(-)
> >>
> > [snip]
> >>
> >> @@ -62,11 +59,9 @@ static int exfat_sync_fs(struct super_block *sb,
> >> int
> >> wait)
> >>
> >>   	/* If there are some dirty buffers in the bdev inode */
> >>   	mutex_lock(&sbi->s_lock);
> >> -	if (test_and_clear_bit(EXFAT_SB_DIRTY, &sbi->s_state)) {
> >> -		sync_blockdev(sb->s_bdev);
> >> -		if (exfat_set_vol_flags(sb, VOL_CLEAN))
> >> -			err = -EIO;
> >> -	}
> >
> > I looked through most codes related to EXFAT_SB_DIRTY and VOL_DIRTY.
> > And your approach looks good because all of them seem to be protected
> > by s_lock.
> >
> > BTW, as you know, sync_filesystem() calls sync_fs() with 'nowait'
> > first, and then calls it again with 'wait' twice. No need to sync with
> lock twice.
> > If so, isn't it okay to do nothing when wait is 0?
> 
> I also think  ‘do nothing when wait is 0’ as you say, but I'm still not
> sure.
> 
> Some other Filesystems do nothing with nowait and just return.
> However, a few Filesystems always perform sync.
> 
> sync_blockdev() waits for completion, so it may be inappropriate to call
> with  nowait. (But it was called in the original code)
> 
> I'm still not sure, so I excluded it in this patch.
> Is it okay to include it?
> 

Yes, I think so. sync_filesystem() will call __sync_blockdev() without 'wait' first.
So, it's enough to call sync_blockdev() with s_lock just one time.

> 
> >> +	sync_blockdev(sb->s_bdev);
> >> +	if (exfat_set_vol_flags(sb, VOL_CLEAN))
> >> +		err = -EIO;
> >>   	mutex_unlock(&sbi->s_lock);
> >>   	return err;
> >>   }
> >> --
> >> 2.25.1
> >
> >
> 
> BR
> ---
> Tetsuhiro Kohada <kohada.t2@gmail.com>




^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] exfat: remove EXFAT_SB_DIRTY flag
  2020-06-12  8:34   ` Sungjong Seo
@ 2020-06-12 10:22     ` Tetsuhiro Kohada
  2020-06-15  2:59       ` Sungjong Seo
  0 siblings, 1 reply; 7+ messages in thread
From: Tetsuhiro Kohada @ 2020-06-12 10:22 UTC (permalink / raw)
  To: Sungjong Seo
  Cc: kohada.tetsuhiro, mori.takahiro, motai.hirotaka,
	'Namjae Jeon',
	linux-fsdevel, linux-kernel

On 2020/06/12 17:34, Sungjong Seo wrote:
>> remove EXFAT_SB_DIRTY flag and related codes.
>>
>> This flag is set/reset in exfat_put_super()/exfat_sync_fs() to avoid
>> sync_blockdev().
>> However ...
>> - exfat_put_super():
>> Before calling this, the VFS has already called sync_filesystem(), so sync
>> is never performed here.
>> - exfat_sync_fs():
>> After calling this, the VFS calls sync_blockdev(), so, it is meaningless
>> to check EXFAT_SB_DIRTY or to bypass sync_blockdev() here.
>> Not only that, but in some cases can't clear VOL_DIRTY.
>> ex:
>> VOL_DIRTY is set when rmdir starts, but when non-empty-dir is detected,
>> return error without setting EXFAT_SB_DIRTY.
>> If performe 'sync' in this state, VOL_DIRTY will not be cleared.
>>
>> Remove the EXFAT_SB_DIRTY check to ensure synchronization.
>> And, remove the code related to the flag.
>>
>> Signed-off-by: Tetsuhiro Kohada <kohada.t2@gmail.com>
>> ---
>>   fs/exfat/balloc.c   |  4 ++--
>>   fs/exfat/dir.c      | 16 ++++++++--------
>>   fs/exfat/exfat_fs.h |  5 +----
>>   fs/exfat/fatent.c   |  7 ++-----
>>   fs/exfat/misc.c     |  3 +--
>>   fs/exfat/namei.c    | 12 ++++++------
>>   fs/exfat/super.c    | 11 +++--------
>>   7 files changed, 23 insertions(+), 35 deletions(-)
>>
> [snip]
>>
>> @@ -62,11 +59,9 @@ static int exfat_sync_fs(struct super_block *sb, int
>> wait)
>>
>>   	/* If there are some dirty buffers in the bdev inode */
>>   	mutex_lock(&sbi->s_lock);
>> -	if (test_and_clear_bit(EXFAT_SB_DIRTY, &sbi->s_state)) {
>> -		sync_blockdev(sb->s_bdev);
>> -		if (exfat_set_vol_flags(sb, VOL_CLEAN))
>> -			err = -EIO;
>> -	}
> 
> I looked through most codes related to EXFAT_SB_DIRTY and VOL_DIRTY.
> And your approach looks good because all of them seem to be protected by
> s_lock.
> 
> BTW, as you know, sync_filesystem() calls sync_fs() with 'nowait' first,
> and then calls it again with 'wait' twice. No need to sync with lock twice.
> If so, isn't it okay to do nothing when wait is 0?

I also think  ‘do nothing when wait is 0’ as you say, but I'm still not sure.

Some other Filesystems do nothing with nowait and just return.
However, a few Filesystems always perform sync.

sync_blockdev() waits for completion, so it may be inappropriate to call with  nowait. (But it was called in the original code)

I'm still not sure, so I excluded it in this patch.
Is it okay to include it?


>> +	sync_blockdev(sb->s_bdev);
>> +	if (exfat_set_vol_flags(sb, VOL_CLEAN))
>> +		err = -EIO;
>>   	mutex_unlock(&sbi->s_lock);
>>   	return err;
>>   }
>> --
>> 2.25.1
> 
> 

BR
---
Tetsuhiro Kohada <kohada.t2@gmail.com>


^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH] exfat: remove EXFAT_SB_DIRTY flag
  2020-06-12  1:28 ` Tetsuhiro Kohada
@ 2020-06-12  8:34   ` Sungjong Seo
  2020-06-12 10:22     ` Tetsuhiro Kohada
  0 siblings, 1 reply; 7+ messages in thread
From: Sungjong Seo @ 2020-06-12  8:34 UTC (permalink / raw)
  To: 'Tetsuhiro Kohada'
  Cc: kohada.tetsuhiro, mori.takahiro, motai.hirotaka,
	'Namjae Jeon',
	linux-fsdevel, linux-kernel

> remove EXFAT_SB_DIRTY flag and related codes.
> 
> This flag is set/reset in exfat_put_super()/exfat_sync_fs() to avoid
> sync_blockdev().
> However ...
> - exfat_put_super():
> Before calling this, the VFS has already called sync_filesystem(), so sync
> is never performed here.
> - exfat_sync_fs():
> After calling this, the VFS calls sync_blockdev(), so, it is meaningless
> to check EXFAT_SB_DIRTY or to bypass sync_blockdev() here.
> Not only that, but in some cases can't clear VOL_DIRTY.
> ex:
> VOL_DIRTY is set when rmdir starts, but when non-empty-dir is detected,
> return error without setting EXFAT_SB_DIRTY.
> If performe 'sync' in this state, VOL_DIRTY will not be cleared.
> 
> Remove the EXFAT_SB_DIRTY check to ensure synchronization.
> And, remove the code related to the flag.
> 
> Signed-off-by: Tetsuhiro Kohada <kohada.t2@gmail.com>
> ---
>  fs/exfat/balloc.c   |  4 ++--
>  fs/exfat/dir.c      | 16 ++++++++--------
>  fs/exfat/exfat_fs.h |  5 +----
>  fs/exfat/fatent.c   |  7 ++-----
>  fs/exfat/misc.c     |  3 +--
>  fs/exfat/namei.c    | 12 ++++++------
>  fs/exfat/super.c    | 11 +++--------
>  7 files changed, 23 insertions(+), 35 deletions(-)
> 
[snip]
> 
> @@ -62,11 +59,9 @@ static int exfat_sync_fs(struct super_block *sb, int
> wait)
> 
>  	/* If there are some dirty buffers in the bdev inode */
>  	mutex_lock(&sbi->s_lock);
> -	if (test_and_clear_bit(EXFAT_SB_DIRTY, &sbi->s_state)) {
> -		sync_blockdev(sb->s_bdev);
> -		if (exfat_set_vol_flags(sb, VOL_CLEAN))
> -			err = -EIO;
> -	}

I looked through most codes related to EXFAT_SB_DIRTY and VOL_DIRTY.
And your approach looks good because all of them seem to be protected by
s_lock.

BTW, as you know, sync_filesystem() calls sync_fs() with 'nowait' first,
and then calls it again with 'wait' twice. No need to sync with lock twice.
If so, isn't it okay to do nothing when wait is 0?

> +	sync_blockdev(sb->s_bdev);
> +	if (exfat_set_vol_flags(sb, VOL_CLEAN))
> +		err = -EIO;
>  	mutex_unlock(&sbi->s_lock);
>  	return err;
>  }
> --
> 2.25.1



^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH] exfat: remove EXFAT_SB_DIRTY flag
@ 2020-06-12  1:28 ` Tetsuhiro Kohada
  2020-06-12  8:34   ` Sungjong Seo
  0 siblings, 1 reply; 7+ messages in thread
From: Tetsuhiro Kohada @ 2020-06-12  1:28 UTC (permalink / raw)
  To: kohada.t2
  Cc: kohada.tetsuhiro, mori.takahiro, motai.hirotaka, Namjae Jeon,
	Sungjong Seo, linux-fsdevel, linux-kernel

remove EXFAT_SB_DIRTY flag and related codes.

This flag is set/reset in exfat_put_super()/exfat_sync_fs()
to avoid sync_blockdev().
However ...
- exfat_put_super():
Before calling this, the VFS has already called sync_filesystem(),
so sync is never performed here.
- exfat_sync_fs():
After calling this, the VFS calls sync_blockdev(), so, it is meaningless
to check EXFAT_SB_DIRTY or to bypass sync_blockdev() here.
Not only that, but in some cases can't clear VOL_DIRTY.
ex:
VOL_DIRTY is set when rmdir starts, but when non-empty-dir is detected,
return error without setting EXFAT_SB_DIRTY.
If performe 'sync' in this state, VOL_DIRTY will not be cleared.

Remove the EXFAT_SB_DIRTY check to ensure synchronization.
And, remove the code related to the flag.

Signed-off-by: Tetsuhiro Kohada <kohada.t2@gmail.com>
---
 fs/exfat/balloc.c   |  4 ++--
 fs/exfat/dir.c      | 16 ++++++++--------
 fs/exfat/exfat_fs.h |  5 +----
 fs/exfat/fatent.c   |  7 ++-----
 fs/exfat/misc.c     |  3 +--
 fs/exfat/namei.c    | 12 ++++++------
 fs/exfat/super.c    | 11 +++--------
 7 files changed, 23 insertions(+), 35 deletions(-)

diff --git a/fs/exfat/balloc.c b/fs/exfat/balloc.c
index 4055eb00ea9b..a987919686c0 100644
--- a/fs/exfat/balloc.c
+++ b/fs/exfat/balloc.c
@@ -158,7 +158,7 @@ int exfat_set_bitmap(struct inode *inode, unsigned int clu)
 	b = BITMAP_OFFSET_BIT_IN_SECTOR(sb, ent_idx);
 
 	set_bit_le(b, sbi->vol_amap[i]->b_data);
-	exfat_update_bh(sb, sbi->vol_amap[i], IS_DIRSYNC(inode));
+	exfat_update_bh(sbi->vol_amap[i], IS_DIRSYNC(inode));
 	return 0;
 }
 
@@ -180,7 +180,7 @@ void exfat_clear_bitmap(struct inode *inode, unsigned int clu)
 	b = BITMAP_OFFSET_BIT_IN_SECTOR(sb, ent_idx);
 
 	clear_bit_le(b, sbi->vol_amap[i]->b_data);
-	exfat_update_bh(sb, sbi->vol_amap[i], IS_DIRSYNC(inode));
+	exfat_update_bh(sbi->vol_amap[i], IS_DIRSYNC(inode));
 
 	if (opts->discard) {
 		int ret_discard;
diff --git a/fs/exfat/dir.c b/fs/exfat/dir.c
index 8e775bd5d523..02acbb6ddf02 100644
--- a/fs/exfat/dir.c
+++ b/fs/exfat/dir.c
@@ -470,7 +470,7 @@ int exfat_init_dir_entry(struct inode *inode, struct exfat_chain *p_dir,
 			&ep->dentry.file.access_date,
 			NULL);
 
-	exfat_update_bh(sb, bh, IS_DIRSYNC(inode));
+	exfat_update_bh(bh, IS_DIRSYNC(inode));
 	brelse(bh);
 
 	ep = exfat_get_dentry(sb, p_dir, entry + 1, &bh, &sector);
@@ -480,7 +480,7 @@ int exfat_init_dir_entry(struct inode *inode, struct exfat_chain *p_dir,
 	exfat_init_stream_entry(ep,
 		(type == TYPE_FILE) ? ALLOC_FAT_CHAIN : ALLOC_NO_FAT_CHAIN,
 		start_clu, size);
-	exfat_update_bh(sb, bh, IS_DIRSYNC(inode));
+	exfat_update_bh(bh, IS_DIRSYNC(inode));
 	brelse(bh);
 
 	return 0;
@@ -516,7 +516,7 @@ int exfat_update_dir_chksum(struct inode *inode, struct exfat_chain *p_dir,
 	}
 
 	fep->dentry.file.checksum = cpu_to_le16(chksum);
-	exfat_update_bh(sb, fbh, IS_DIRSYNC(inode));
+	exfat_update_bh(fbh, IS_DIRSYNC(inode));
 release_fbh:
 	brelse(fbh);
 	return ret;
@@ -538,7 +538,7 @@ int exfat_init_ext_entry(struct inode *inode, struct exfat_chain *p_dir,
 		return -EIO;
 
 	ep->dentry.file.num_ext = (unsigned char)(num_entries - 1);
-	exfat_update_bh(sb, bh, sync);
+	exfat_update_bh(bh, sync);
 	brelse(bh);
 
 	ep = exfat_get_dentry(sb, p_dir, entry + 1, &bh, &sector);
@@ -547,7 +547,7 @@ int exfat_init_ext_entry(struct inode *inode, struct exfat_chain *p_dir,
 
 	ep->dentry.stream.name_len = p_uniname->name_len;
 	ep->dentry.stream.name_hash = cpu_to_le16(p_uniname->name_hash);
-	exfat_update_bh(sb, bh, sync);
+	exfat_update_bh(bh, sync);
 	brelse(bh);
 
 	for (i = EXFAT_FIRST_CLUSTER; i < num_entries; i++) {
@@ -556,7 +556,7 @@ int exfat_init_ext_entry(struct inode *inode, struct exfat_chain *p_dir,
 			return -EIO;
 
 		exfat_init_name_entry(ep, uniname);
-		exfat_update_bh(sb, bh, sync);
+		exfat_update_bh(bh, sync);
 		brelse(bh);
 		uniname += EXFAT_FILE_NAME_LEN;
 	}
@@ -580,7 +580,7 @@ int exfat_remove_entries(struct inode *inode, struct exfat_chain *p_dir,
 			return -EIO;
 
 		exfat_set_entry_type(ep, TYPE_DELETED);
-		exfat_update_bh(sb, bh, IS_DIRSYNC(inode));
+		exfat_update_bh(bh, IS_DIRSYNC(inode));
 		brelse(bh);
 	}
 
@@ -610,7 +610,7 @@ void exfat_free_dentry_set(struct exfat_entry_set_cache *es, int sync)
 
 	for (i = 0; i < es->num_bh; i++) {
 		if (es->modified)
-			exfat_update_bh(es->sb, es->bh[i], sync);
+			exfat_update_bh(es->bh[i], sync);
 		brelse(es->bh[i]);
 	}
 	kfree(es);
diff --git a/fs/exfat/exfat_fs.h b/fs/exfat/exfat_fs.h
index 595f3117f492..84664024e51e 100644
--- a/fs/exfat/exfat_fs.h
+++ b/fs/exfat/exfat_fs.h
@@ -13,8 +13,6 @@
 #define EXFAT_SUPER_MAGIC       0x2011BAB0UL
 #define EXFAT_ROOT_INO		1
 
-#define EXFAT_SB_DIRTY		0
-
 #define EXFAT_CLUSTERS_UNTRACKED (~0u)
 
 /*
@@ -238,7 +236,6 @@ struct exfat_sb_info {
 	unsigned int clu_srch_ptr; /* cluster search pointer */
 	unsigned int used_clusters; /* number of used clusters */
 
-	unsigned long s_state;
 	struct mutex s_lock; /* superblock lock */
 	struct exfat_mount_options options;
 	struct nls_table *nls_io; /* Charset used for input and display */
@@ -514,7 +511,7 @@ void exfat_set_entry_time(struct exfat_sb_info *sbi, struct timespec64 *ts,
 		u8 *tz, __le16 *time, __le16 *date, u8 *time_cs);
 u16 exfat_calc_chksum16(void *data, int len, u16 chksum, int type);
 u32 exfat_calc_chksum32(void *data, int len, u32 chksum, int type);
-void exfat_update_bh(struct super_block *sb, struct buffer_head *bh, int sync);
+void exfat_update_bh(struct buffer_head *bh, int sync);
 void exfat_chain_set(struct exfat_chain *ec, unsigned int dir,
 		unsigned int size, unsigned char flags);
 void exfat_chain_dup(struct exfat_chain *dup, struct exfat_chain *ec);
diff --git a/fs/exfat/fatent.c b/fs/exfat/fatent.c
index 4e5c5c9c0f2d..82ee8246c080 100644
--- a/fs/exfat/fatent.c
+++ b/fs/exfat/fatent.c
@@ -75,7 +75,7 @@ int exfat_ent_set(struct super_block *sb, unsigned int loc,
 
 	fat_entry = (__le32 *)&(bh->b_data[off]);
 	*fat_entry = cpu_to_le32(content);
-	exfat_update_bh(sb, bh, sb->s_flags & SB_SYNCHRONOUS);
+	exfat_update_bh(bh, sb->s_flags & SB_SYNCHRONOUS);
 	exfat_mirror_bh(sb, sec, bh);
 	brelse(bh);
 	return 0;
@@ -174,7 +174,6 @@ int exfat_free_cluster(struct inode *inode, struct exfat_chain *p_chain)
 		return -EIO;
 	}
 
-	set_bit(EXFAT_SB_DIRTY, &sbi->s_state);
 	clu = p_chain->dir;
 
 	if (p_chain->flags == ALLOC_NO_FAT_CHAIN) {
@@ -274,7 +273,7 @@ int exfat_zeroed_cluster(struct inode *dir, unsigned int clu)
 			goto release_bhs;
 		}
 		memset(bhs[n]->b_data, 0, sb->s_blocksize);
-		exfat_update_bh(sb, bhs[n], 0);
+		exfat_update_bh(bhs[n], 0);
 
 		n++;
 		blknr++;
@@ -358,8 +357,6 @@ int exfat_alloc_cluster(struct inode *inode, unsigned int num_alloc,
 		}
 	}
 
-	set_bit(EXFAT_SB_DIRTY, &sbi->s_state);
-
 	p_chain->dir = EXFAT_EOF_CLUSTER;
 
 	while ((new_clu = exfat_find_free_bitmap(sb, hint_clu)) !=
diff --git a/fs/exfat/misc.c b/fs/exfat/misc.c
index 17d41f3d3709..8a3dde59052b 100644
--- a/fs/exfat/misc.c
+++ b/fs/exfat/misc.c
@@ -163,9 +163,8 @@ u32 exfat_calc_chksum32(void *data, int len, u32 chksum, int type)
 	return chksum;
 }
 
-void exfat_update_bh(struct super_block *sb, struct buffer_head *bh, int sync)
+void exfat_update_bh(struct buffer_head *bh, int sync)
 {
-	set_bit(EXFAT_SB_DIRTY, &EXFAT_SB(sb)->s_state);
 	set_buffer_uptodate(bh);
 	mark_buffer_dirty(bh);
 
diff --git a/fs/exfat/namei.c b/fs/exfat/namei.c
index edd8023865a0..5eef2217fcf2 100644
--- a/fs/exfat/namei.c
+++ b/fs/exfat/namei.c
@@ -387,7 +387,7 @@ static int exfat_find_empty_entry(struct inode *inode,
 			ep->dentry.stream.valid_size = cpu_to_le64(size);
 			ep->dentry.stream.size = ep->dentry.stream.valid_size;
 			ep->dentry.stream.flags = p_dir->flags;
-			exfat_update_bh(sb, bh, IS_DIRSYNC(inode));
+			exfat_update_bh(bh, IS_DIRSYNC(inode));
 			brelse(bh);
 			if (exfat_update_dir_chksum(inode, &(ei->dir),
 			    ei->entry))
@@ -1071,7 +1071,7 @@ static int exfat_rename_file(struct inode *inode, struct exfat_chain *p_dir,
 			epnew->dentry.file.attr |= cpu_to_le16(ATTR_ARCHIVE);
 			ei->attr |= ATTR_ARCHIVE;
 		}
-		exfat_update_bh(sb, new_bh, sync);
+		exfat_update_bh(new_bh, sync);
 		brelse(old_bh);
 		brelse(new_bh);
 
@@ -1087,7 +1087,7 @@ static int exfat_rename_file(struct inode *inode, struct exfat_chain *p_dir,
 		}
 
 		memcpy(epnew, epold, DENTRY_SIZE);
-		exfat_update_bh(sb, new_bh, sync);
+		exfat_update_bh(new_bh, sync);
 		brelse(old_bh);
 		brelse(new_bh);
 
@@ -1104,7 +1104,7 @@ static int exfat_rename_file(struct inode *inode, struct exfat_chain *p_dir,
 			epold->dentry.file.attr |= cpu_to_le16(ATTR_ARCHIVE);
 			ei->attr |= ATTR_ARCHIVE;
 		}
-		exfat_update_bh(sb, old_bh, sync);
+		exfat_update_bh(old_bh, sync);
 		brelse(old_bh);
 		ret = exfat_init_ext_entry(inode, p_dir, oldentry,
 			num_new_entries, p_uniname);
@@ -1159,7 +1159,7 @@ static int exfat_move_file(struct inode *inode, struct exfat_chain *p_olddir,
 		epnew->dentry.file.attr |= cpu_to_le16(ATTR_ARCHIVE);
 		ei->attr |= ATTR_ARCHIVE;
 	}
-	exfat_update_bh(sb, new_bh, IS_DIRSYNC(inode));
+	exfat_update_bh(new_bh, IS_DIRSYNC(inode));
 	brelse(mov_bh);
 	brelse(new_bh);
 
@@ -1175,7 +1175,7 @@ static int exfat_move_file(struct inode *inode, struct exfat_chain *p_olddir,
 	}
 
 	memcpy(epnew, epmov, DENTRY_SIZE);
-	exfat_update_bh(sb, new_bh, IS_DIRSYNC(inode));
+	exfat_update_bh(new_bh, IS_DIRSYNC(inode));
 	brelse(mov_bh);
 	brelse(new_bh);
 
diff --git a/fs/exfat/super.c b/fs/exfat/super.c
index e650e65536f8..b2611050b671 100644
--- a/fs/exfat/super.c
+++ b/fs/exfat/super.c
@@ -45,9 +45,6 @@ static void exfat_put_super(struct super_block *sb)
 	struct exfat_sb_info *sbi = EXFAT_SB(sb);
 
 	mutex_lock(&sbi->s_lock);
-	if (test_and_clear_bit(EXFAT_SB_DIRTY, &sbi->s_state))
-		sync_blockdev(sb->s_bdev);
-	exfat_set_vol_flags(sb, VOL_CLEAN);
 	exfat_free_bitmap(sbi);
 	brelse(sbi->boot_bh);
 	mutex_unlock(&sbi->s_lock);
@@ -62,11 +59,9 @@ static int exfat_sync_fs(struct super_block *sb, int wait)
 
 	/* If there are some dirty buffers in the bdev inode */
 	mutex_lock(&sbi->s_lock);
-	if (test_and_clear_bit(EXFAT_SB_DIRTY, &sbi->s_state)) {
-		sync_blockdev(sb->s_bdev);
-		if (exfat_set_vol_flags(sb, VOL_CLEAN))
-			err = -EIO;
-	}
+	sync_blockdev(sb->s_bdev);
+	if (exfat_set_vol_flags(sb, VOL_CLEAN))
+		err = -EIO;
 	mutex_unlock(&sbi->s_lock);
 	return err;
 }
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-06-15  8:33 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-12  8:48 [PATCH] exfat: remove EXFAT_SB_DIRTY flag Markus Elfring
2020-06-12 10:01 ` Greg KH
     [not found] <CGME20200612012902epcas1p4194d6fa3b3f7c46a8becb9bb6ce23d56@epcas1p4.samsung.com>
2020-06-12  1:28 ` Tetsuhiro Kohada
2020-06-12  8:34   ` Sungjong Seo
2020-06-12 10:22     ` Tetsuhiro Kohada
2020-06-15  2:59       ` Sungjong Seo
2020-06-15  8:31         ` Kohada.Tetsuhiro

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).