[PATCH v3 1/2] exfat: add exfat_update

linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

* [PATCH v3 1/2] exfat: add exfat_update_inode()
@ 2020-10-02  6:05 ` Tetsuhiro Kohada
  2020-10-06  8:23   ` Namjae Jeon
  0 siblings, 1 reply; 3+ messages in thread
From: Tetsuhiro Kohada @ 2020-10-02  6:05 UTC (permalink / raw)
  To: kohada.t2
  Cc: kohada.tetsuhiro, mori.takahiro, Namjae Jeon, Sungjong Seo,
	linux-fsdevel, linux-kernel

Integrate exfat_sync_inode() and mark_inode_dirty() as exfat_update_inode()
Also, return the result of _exfat_write_inode () when sync is specified.

Signed-off-by: Tetsuhiro Kohada <kohada.t2@gmail.com>
---
Changes in v3
 - no change
Changes in v2
 - no change

 fs/exfat/exfat_fs.h |  2 +-
 fs/exfat/file.c     |  5 +----
 fs/exfat/inode.c    |  9 +++++++--
 fs/exfat/namei.c    | 35 +++++++----------------------------
 4 files changed, 16 insertions(+), 35 deletions(-)

diff --git a/fs/exfat/exfat_fs.h b/fs/exfat/exfat_fs.h
index b8f0e829ecbd..ec0ee516aee2 100644
--- a/fs/exfat/exfat_fs.h
+++ b/fs/exfat/exfat_fs.h
@@ -466,7 +466,7 @@ int exfat_count_dir_entries(struct super_block *sb, struct exfat_chain *p_dir);
 
 /* inode.c */
 extern const struct inode_operations exfat_file_inode_operations;
-void exfat_sync_inode(struct inode *inode);
+int exfat_update_inode(struct inode *inode);
 struct inode *exfat_build_inode(struct super_block *sb,
 		struct exfat_dir_entry *info, loff_t i_pos);
 void exfat_hash_inode(struct inode *inode, loff_t i_pos);
diff --git a/fs/exfat/file.c b/fs/exfat/file.c
index a92478eabfa4..e510b95dbf77 100644
--- a/fs/exfat/file.c
+++ b/fs/exfat/file.c
@@ -245,10 +245,7 @@ void exfat_truncate(struct inode *inode, loff_t size)
 		goto write_size;
 
 	inode->i_ctime = inode->i_mtime = current_time(inode);
-	if (IS_DIRSYNC(inode))
-		exfat_sync_inode(inode);
-	else
-		mark_inode_dirty(inode);
+	exfat_update_inode(inode);
 
 	inode->i_blocks = ((i_size_read(inode) + (sbi->cluster_size - 1)) &
 			~(sbi->cluster_size - 1)) >> inode->i_blkbits;
diff --git a/fs/exfat/inode.c b/fs/exfat/inode.c
index 730373e0965a..5a55303e1f65 100644
--- a/fs/exfat/inode.c
+++ b/fs/exfat/inode.c
@@ -91,10 +91,15 @@ int exfat_write_inode(struct inode *inode, struct writeback_control *wbc)
 	return ret;
 }
 
-void exfat_sync_inode(struct inode *inode)
+int exfat_update_inode(struct inode *inode)
 {
 	lockdep_assert_held(&EXFAT_SB(inode->i_sb)->s_lock);
-	__exfat_write_inode(inode, 1);
+
+	if (IS_DIRSYNC(inode))
+		return __exfat_write_inode(inode, 1);
+
+	mark_inode_dirty(inode);
+	return 0;
 }
 
 /*
diff --git a/fs/exfat/namei.c b/fs/exfat/namei.c
index 676094f2abe2..4eb7cb528e97 100644
--- a/fs/exfat/namei.c
+++ b/fs/exfat/namei.c
@@ -561,10 +561,7 @@ static int exfat_create(struct inode *dir, struct dentry *dentry, umode_t mode,
 
 	inode_inc_iversion(dir);
 	dir->i_ctime = dir->i_mtime = current_time(dir);
-	if (IS_DIRSYNC(dir))
-		exfat_sync_inode(dir);
-	else
-		mark_inode_dirty(dir);
+	exfat_update_inode(dir);
 
 	i_pos = exfat_make_i_pos(&info);
 	inode = exfat_build_inode(sb, &info, i_pos);
@@ -812,10 +809,7 @@ static int exfat_unlink(struct inode *dir, struct dentry *dentry)
 	inode_inc_iversion(dir);
 	dir->i_mtime = dir->i_atime = current_time(dir);
 	exfat_truncate_atime(&dir->i_atime);
-	if (IS_DIRSYNC(dir))
-		exfat_sync_inode(dir);
-	else
-		mark_inode_dirty(dir);
+	exfat_update_inode(dir);
 
 	clear_nlink(inode);
 	inode->i_mtime = inode->i_atime = current_time(inode);
@@ -846,10 +840,7 @@ static int exfat_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
 
 	inode_inc_iversion(dir);
 	dir->i_ctime = dir->i_mtime = current_time(dir);
-	if (IS_DIRSYNC(dir))
-		exfat_sync_inode(dir);
-	else
-		mark_inode_dirty(dir);
+	exfat_update_inode(dir);
 	inc_nlink(dir);
 
 	i_pos = exfat_make_i_pos(&info);
@@ -976,10 +967,7 @@ static int exfat_rmdir(struct inode *dir, struct dentry *dentry)
 	inode_inc_iversion(dir);
 	dir->i_mtime = dir->i_atime = current_time(dir);
 	exfat_truncate_atime(&dir->i_atime);
-	if (IS_DIRSYNC(dir))
-		exfat_sync_inode(dir);
-	else
-		mark_inode_dirty(dir);
+	exfat_update_inode(dir);
 	drop_nlink(dir);
 
 	clear_nlink(inode);
@@ -1352,19 +1340,13 @@ static int exfat_rename(struct inode *old_dir, struct dentry *old_dentry,
 	new_dir->i_ctime = new_dir->i_mtime = new_dir->i_atime =
 		EXFAT_I(new_dir)->i_crtime = current_time(new_dir);
 	exfat_truncate_atime(&new_dir->i_atime);
-	if (IS_DIRSYNC(new_dir))
-		exfat_sync_inode(new_dir);
-	else
-		mark_inode_dirty(new_dir);
+	exfat_update_inode(new_dir);
 
 	i_pos = ((loff_t)EXFAT_I(old_inode)->dir.dir << 32) |
 		(EXFAT_I(old_inode)->entry & 0xffffffff);
 	exfat_unhash_inode(old_inode);
 	exfat_hash_inode(old_inode, i_pos);
-	if (IS_DIRSYNC(new_dir))
-		exfat_sync_inode(old_inode);
-	else
-		mark_inode_dirty(old_inode);
+	exfat_update_inode(old_inode);
 
 	if (S_ISDIR(old_inode->i_mode) && old_dir != new_dir) {
 		drop_nlink(old_dir);
@@ -1374,10 +1356,7 @@ static int exfat_rename(struct inode *old_dir, struct dentry *old_dentry,
 
 	inode_inc_iversion(old_dir);
 	old_dir->i_ctime = old_dir->i_mtime = current_time(old_dir);
-	if (IS_DIRSYNC(old_dir))
-		exfat_sync_inode(old_dir);
-	else
-		mark_inode_dirty(old_dir);
+	exfat_update_inode(old_dir);
 
 	if (new_inode) {
 		exfat_unhash_inode(new_inode);
-- 
2.25.1


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

* RE: [PATCH v3 1/2] exfat: add exfat_update_inode()
  2020-10-02  6:05 ` [PATCH v3 1/2] exfat: add exfat_update_inode() Tetsuhiro Kohada
@ 2020-10-06  8:23   ` Namjae Jeon
  2020-10-07  8:13     ` Tetsuhiro Kohada
  0 siblings, 1 reply; 3+ messages in thread
From: Namjae Jeon @ 2020-10-06  8:23 UTC (permalink / raw)
  To: 'Tetsuhiro Kohada'
  Cc: kohada.tetsuhiro, mori.takahiro, 'Sungjong Seo',
	linux-fsdevel, linux-kernel

> @@ -1352,19 +1340,13 @@ static int exfat_rename(struct inode *old_dir, struct dentry *old_dentry,
>  	new_dir->i_ctime = new_dir->i_mtime = new_dir->i_atime =
>  		EXFAT_I(new_dir)->i_crtime = current_time(new_dir);
>  	exfat_truncate_atime(&new_dir->i_atime);
> -	if (IS_DIRSYNC(new_dir))
> -		exfat_sync_inode(new_dir);
> -	else
> -		mark_inode_dirty(new_dir);
> +	exfat_update_inode(new_dir);
> 
>  	i_pos = ((loff_t)EXFAT_I(old_inode)->dir.dir << 32) |
>  		(EXFAT_I(old_inode)->entry & 0xffffffff);
>  	exfat_unhash_inode(old_inode);
>  	exfat_hash_inode(old_inode, i_pos);
> -	if (IS_DIRSYNC(new_dir))
> -		exfat_sync_inode(old_inode);
> -	else
> -		mark_inode_dirty(old_inode);
> +	exfat_update_inode(old_inode);
This is checking if old_inode is IS_DIRSYNC, not new_dir.
Is there any reason ?


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

* Re: [PATCH v3 1/2] exfat: add exfat_update_inode()
  2020-10-06  8:23   ` Namjae Jeon
@ 2020-10-07  8:13     ` Tetsuhiro Kohada
  0 siblings, 0 replies; 3+ messages in thread
From: Tetsuhiro Kohada @ 2020-10-07  8:13 UTC (permalink / raw)
  To: Namjae Jeon
  Cc: kohada.tetsuhiro, mori.takahiro, 'Sungjong Seo',
	linux-fsdevel, linux-kernel

Thank you for your reply.

>>   	new_dir->i_ctime = new_dir->i_mtime = new_dir->i_atime =
>>   		EXFAT_I(new_dir)->i_crtime = current_time(new_dir);
>>   	exfat_truncate_atime(&new_dir->i_atime);
>> -	if (IS_DIRSYNC(new_dir))
>> -		exfat_sync_inode(new_dir);
>> -	else
>> -		mark_inode_dirty(new_dir);
>> +	exfat_update_inode(new_dir);
>>
>>   	i_pos = ((loff_t)EXFAT_I(old_inode)->dir.dir << 32) |
>>   		(EXFAT_I(old_inode)->entry & 0xffffffff);
>>   	exfat_unhash_inode(old_inode);
>>   	exfat_hash_inode(old_inode, i_pos);
>> -	if (IS_DIRSYNC(new_dir))
>> -		exfat_sync_inode(old_inode);
>> -	else
>> -		mark_inode_dirty(old_inode);
>> +	exfat_update_inode(old_inode);
> This is checking if old_inode is IS_DIRSYNC, not new_dir.
> Is there any reason ?

To eliminate meaningless usage and simplify it.

Th exfat does not have an attribute that indicates whether each file/dir should be synced(such as ext4).
Therefore, sync necessity cannot be set for each inode, so sync necessity of the whole FS setting(sb-> s_flags) is inherited.
As a result, the following values are all the same.
  IS_DIRSYNC (new_dir)
  IS_DIRSYNC (old_dir)
  IS_DIRSYNC (old_inode)
  sb-> s_flags & SB_SYNCHRONOUS | SB_DIRSYNC

In exfat, IS_DIRSYNC only works as a shortcut to sb->s_flags.

Even if S_SYNC or S_DIRSYNC were set to inode->i_flags, the current implementation is inappropriate.
Whether to sync or not should be determined by "IS_DIRSYNC(new_dir)||IS_DIRSYNC(old_dir)", I think.
(Syncing only old_dir is a high risk of losing file)

Whatever, no one sets S_SYNC and S_DIRSYNC in exfat, so the behavior is no different.

***
Please tell me your opinion about "aggregate dir-entry updates into __exfat_write_inode()"

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

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

end of thread, other threads:[~2020-10-07  8:13 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20201002060529epcas1p2e05b4f565283969f4f2adc337f23a0d2@epcas1p2.samsung.com>
2020-10-02  6:05 ` [PATCH v3 1/2] exfat: add exfat_update_inode() Tetsuhiro Kohada
2020-10-06  8:23   ` Namjae Jeon
2020-10-07  8:13     ` Tetsuhiro Kohada

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