All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] exfat: improve performance of exfat_free_cluster when using dirsync mount option
@ 2021-01-06  4:39 ` Hyeongseok Kim
  2021-01-07  6:23   ` Sungjong Seo
  0 siblings, 1 reply; 4+ messages in thread
From: Hyeongseok Kim @ 2021-01-06  4:39 UTC (permalink / raw)
  To: namjae.jeon, sj1557.seo; +Cc: linux-fsdevel, linux-kernel, Hyeongseok Kim

There are stressful update of cluster allocation bitmap when using
dirsync mount option which is doing sync buffer on every cluster bit
clearing. This could result in performance degradation when deleting
big size file.
Fix to update only when the bitmap buffer index is changed would make
less disk access, improving performance especially for truncate operation.

Testing with Samsung 256GB sdcard, mounted with dirsync option
(mount -t exfat /dev/block/mmcblk0p1 /temp/mount -o dirsync)

Remove 4GB file, blktrace result.
[Before] : 39 secs.
Total (blktrace):
 Reads Queued:      0,        0KiB	 Writes Queued:      32775,    16387KiB
 Read Dispatches:   0,        0KiB	 Write Dispatches:   32775,    16387KiB
 Reads Requeued:    0		         Writes Requeued:        0
 Reads Completed:   0,        0KiB	 Writes Completed:   32775,    16387KiB
 Read Merges:       0,        0KiB	 Write Merges:           0,        0KiB
 IO unplugs:        2        	     Timer unplugs:          0

[After] : 1 sec.
Total (blktrace):
 Reads Queued:      0,        0KiB	 Writes Queued:         13,        6KiB
 Read Dispatches:   0,        0KiB	 Write Dispatches:      13,        6KiB
 Reads Requeued:    0		         Writes Requeued:        0
 Reads Completed:   0,        0KiB	 Writes Completed:      13,        6KiB
 Read Merges:       0,        0KiB	 Write Merges:           0,        0KiB
 IO unplugs:        1        	     Timer unplugs:          0

Signed-off-by: Hyeongseok Kim <hyeongseok@gmail.com>
---
 fs/exfat/balloc.c   |  4 ++--
 fs/exfat/exfat_fs.h |  2 +-
 fs/exfat/fatent.c   | 42 ++++++++++++++++++++++++++++++++++++------
 3 files changed, 39 insertions(+), 9 deletions(-)

diff --git a/fs/exfat/balloc.c b/fs/exfat/balloc.c
index a987919686c0..761c79c3a4ba 100644
--- a/fs/exfat/balloc.c
+++ b/fs/exfat/balloc.c
@@ -166,7 +166,7 @@ int exfat_set_bitmap(struct inode *inode, unsigned int clu)
  * If the value of "clu" is 0, it means cluster 2 which is the first cluster of
  * the cluster heap.
  */
-void exfat_clear_bitmap(struct inode *inode, unsigned int clu)
+void exfat_clear_bitmap(struct inode *inode, unsigned int clu, bool sync)
 {
 	int i, b;
 	unsigned int ent_idx;
@@ -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(sbi->vol_amap[i], IS_DIRSYNC(inode));
+	exfat_update_bh(sbi->vol_amap[i], sync);
 
 	if (opts->discard) {
 		int ret_discard;
diff --git a/fs/exfat/exfat_fs.h b/fs/exfat/exfat_fs.h
index b8f0e829ecbd..764bc645241e 100644
--- a/fs/exfat/exfat_fs.h
+++ b/fs/exfat/exfat_fs.h
@@ -408,7 +408,7 @@ int exfat_count_num_clusters(struct super_block *sb,
 int exfat_load_bitmap(struct super_block *sb);
 void exfat_free_bitmap(struct exfat_sb_info *sbi);
 int exfat_set_bitmap(struct inode *inode, unsigned int clu);
-void exfat_clear_bitmap(struct inode *inode, unsigned int clu);
+void exfat_clear_bitmap(struct inode *inode, unsigned int clu, bool sync);
 unsigned int exfat_find_free_bitmap(struct super_block *sb, unsigned int clu);
 int exfat_count_used_clusters(struct super_block *sb, unsigned int *ret_count);
 
diff --git a/fs/exfat/fatent.c b/fs/exfat/fatent.c
index c3c9afee7418..b0118ad53845 100644
--- a/fs/exfat/fatent.c
+++ b/fs/exfat/fatent.c
@@ -157,6 +157,7 @@ int exfat_free_cluster(struct inode *inode, struct exfat_chain *p_chain)
 	unsigned int clu;
 	struct super_block *sb = inode->i_sb;
 	struct exfat_sb_info *sbi = EXFAT_SB(sb);
+	int cur_cmap_i, next_cmap_i;
 
 	/* invalid cluster number */
 	if (p_chain->dir == EXFAT_FREE_CLUSTER ||
@@ -176,21 +177,50 @@ int exfat_free_cluster(struct inode *inode, struct exfat_chain *p_chain)
 
 	clu = p_chain->dir;
 
+	cur_cmap_i = BITMAP_OFFSET_SECTOR_INDEX(sb, CLUSTER_TO_BITMAP_ENT(clu));
+
 	if (p_chain->flags == ALLOC_NO_FAT_CHAIN) {
+		unsigned int last_cluster = p_chain->dir + p_chain->size - 1;
 		do {
-			exfat_clear_bitmap(inode, clu);
-			clu++;
+			bool sync = false;
+
+			if (clu < last_cluster)
+				next_cmap_i =
+				  BITMAP_OFFSET_SECTOR_INDEX(sb, CLUSTER_TO_BITMAP_ENT(clu+1));
 
+			/* flush bitmap only if index would be changed or for last cluster */
+			if (clu == last_cluster || cur_cmap_i != next_cmap_i) {
+				sync = true;
+				cur_cmap_i = next_cmap_i;
+			}
+
+			exfat_clear_bitmap(inode, clu, (sync && IS_DIRSYNC(inode)));
+			clu++;
 			num_clusters++;
 		} while (num_clusters < p_chain->size);
 	} else {
 		do {
-			exfat_clear_bitmap(inode, clu);
-
-			if (exfat_get_next_cluster(sb, &clu))
-				goto dec_used_clus;
+			bool sync = false;
+			unsigned int n_clu = clu;
+			int err = exfat_get_next_cluster(sb, &n_clu);
+
+			if (err || n_clu == EXFAT_EOF_CLUSTER)
+				sync = true;
+			else
+				next_cmap_i =
+				  BITMAP_OFFSET_SECTOR_INDEX(sb, CLUSTER_TO_BITMAP_ENT(n_clu));
+
+			if (cur_cmap_i != next_cmap_i) {
+				sync = true;
+				cur_cmap_i = next_cmap_i;
+			}
 
+			exfat_clear_bitmap(inode, clu, (sync && IS_DIRSYNC(inode)));
+			clu = n_clu;
 			num_clusters++;
+
+			if (err)
+				goto dec_used_clus;
 		} while (clu != EXFAT_EOF_CLUSTER);
 	}
 
-- 
2.27.0.83.g0313f36


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

* RE: [PATCH] exfat: improve performance of exfat_free_cluster when using dirsync mount option
  2021-01-06  4:39 ` [PATCH] exfat: improve performance of exfat_free_cluster when using dirsync mount option Hyeongseok Kim
@ 2021-01-07  6:23   ` Sungjong Seo
  2021-01-08  0:19     ` Namjae Jeon
  0 siblings, 1 reply; 4+ messages in thread
From: Sungjong Seo @ 2021-01-07  6:23 UTC (permalink / raw)
  To: 'Hyeongseok Kim', namjae.jeon; +Cc: linux-fsdevel, linux-kernel

> There are stressful update of cluster allocation bitmap when using dirsync
> mount option which is doing sync buffer on every cluster bit clearing.
> This could result in performance degradation when deleting big size file.
> Fix to update only when the bitmap buffer index is changed would make less
> disk access, improving performance especially for truncate operation.
> 
> Testing with Samsung 256GB sdcard, mounted with dirsync option (mount -t
> exfat /dev/block/mmcblk0p1 /temp/mount -o dirsync)
> 
> Remove 4GB file, blktrace result.
> [Before] : 39 secs.
> Total (blktrace):
>  Reads Queued:      0,        0KiB	 Writes Queued:      32775,
16387KiB
>  Read Dispatches:   0,        0KiB	 Write Dispatches:   32775,
16387KiB
>  Reads Requeued:    0		         Writes Requeued:        0
>  Reads Completed:   0,        0KiB	 Writes Completed:   32775,
16387KiB
>  Read Merges:       0,        0KiB	 Write Merges:           0,
0KiB
>  IO unplugs:        2        	     Timer unplugs:          0
> 
> [After] : 1 sec.
> Total (blktrace):
>  Reads Queued:      0,        0KiB	 Writes Queued:         13,
6KiB
>  Read Dispatches:   0,        0KiB	 Write Dispatches:      13,
6KiB
>  Reads Requeued:    0		         Writes Requeued:        0
>  Reads Completed:   0,        0KiB	 Writes Completed:      13,
6KiB
>  Read Merges:       0,        0KiB	 Write Merges:           0,
0KiB
>  IO unplugs:        1        	     Timer unplugs:          0
> 
> Signed-off-by: Hyeongseok Kim <hyeongseok@gmail.com>

Looks good.
Thanks for your work!

Acked-by: Sungjong Seo <sj1557.seo@samsung.com>


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

* RE: [PATCH] exfat: improve performance of exfat_free_cluster when using dirsync mount option
  2021-01-07  6:23   ` Sungjong Seo
@ 2021-01-08  0:19     ` Namjae Jeon
  0 siblings, 0 replies; 4+ messages in thread
From: Namjae Jeon @ 2021-01-08  0:19 UTC (permalink / raw)
  To: 'Sungjong Seo', 'Hyeongseok Kim'
  Cc: linux-fsdevel, linux-kernel

> > There are stressful update of cluster allocation bitmap when using
> > dirsync mount option which is doing sync buffer on every cluster bit clearing.
> > This could result in performance degradation when deleting big size file.
> > Fix to update only when the bitmap buffer index is changed would make
> > less disk access, improving performance especially for truncate operation.
> >
> > Testing with Samsung 256GB sdcard, mounted with dirsync option (mount
> > -t exfat /dev/block/mmcblk0p1 /temp/mount -o dirsync)
> >
> > Remove 4GB file, blktrace result.
> > [Before] : 39 secs.
> > Total (blktrace):
> >  Reads Queued:      0,        0KiB	 Writes Queued:      32775,
> 16387KiB
> >  Read Dispatches:   0,        0KiB	 Write Dispatches:   32775,
> 16387KiB
> >  Reads Requeued:    0		         Writes Requeued:        0
> >  Reads Completed:   0,        0KiB	 Writes Completed:   32775,
> 16387KiB
> >  Read Merges:       0,        0KiB	 Write Merges:           0,
> 0KiB
> >  IO unplugs:        2        	     Timer unplugs:          0
> >
> > [After] : 1 sec.
> > Total (blktrace):
> >  Reads Queued:      0,        0KiB	 Writes Queued:         13,
> 6KiB
> >  Read Dispatches:   0,        0KiB	 Write Dispatches:      13,
> 6KiB
> >  Reads Requeued:    0		         Writes Requeued:        0
> >  Reads Completed:   0,        0KiB	 Writes Completed:      13,
> 6KiB
> >  Read Merges:       0,        0KiB	 Write Merges:           0,
> 0KiB
> >  IO unplugs:        1        	     Timer unplugs:          0
> >
> > Signed-off-by: Hyeongseok Kim <hyeongseok@gmail.com>
> 
> Looks good.
> Thanks for your work!
> 
> Acked-by: Sungjong Seo <sj1557.seo@samsung.com>
Applied. Thanks!



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

* Re: [PATCH] exfat: improve performance of exfat_free_cluster when using dirsync mount option
@ 2021-01-06  7:51 kernel test robot
  0 siblings, 0 replies; 4+ messages in thread
From: kernel test robot @ 2021-01-06  7:51 UTC (permalink / raw)
  To: kbuild

[-- Attachment #1: Type: text/plain, Size: 7677 bytes --]

CC: kbuild-all(a)lists.01.org
In-Reply-To: <20210106043945.36546-1-hyeongseok@gmail.com>
References: <20210106043945.36546-1-hyeongseok@gmail.com>
TO: Hyeongseok Kim <hyeongseok@gmail.com>
TO: namjae.jeon(a)samsung.com
TO: sj1557.seo(a)samsung.com
CC: linux-fsdevel(a)vger.kernel.org
CC: linux-kernel(a)vger.kernel.org
CC: Hyeongseok Kim <hyeongseok@gmail.com>

Hi Hyeongseok,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v5.11-rc2 next-20210104]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Hyeongseok-Kim/exfat-improve-performance-of-exfat_free_cluster-when-using-dirsync-mount-option/20210106-124440
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git e71ba9452f0b5b2e8dc8aa5445198cd9214a6a62
:::::: branch date: 3 hours ago
:::::: commit date: 3 hours ago
config: arc-randconfig-m031-20210106 (attached as .config)
compiler: arc-elf-gcc (GCC) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

New smatch warnings:
fs/exfat/fatent.c:192 exfat_free_cluster() error: uninitialized symbol 'next_cmap_i'.

Old smatch warnings:
fs/exfat/fatent.c:194 exfat_free_cluster() error: uninitialized symbol 'next_cmap_i'.
fs/exfat/fatent.c:213 exfat_free_cluster() error: uninitialized symbol 'next_cmap_i'.

vim +/next_cmap_i +192 fs/exfat/fatent.c

31023864e67a5f39 Namjae Jeon    2020-03-02  153  
31023864e67a5f39 Namjae Jeon    2020-03-02  154  int exfat_free_cluster(struct inode *inode, struct exfat_chain *p_chain)
31023864e67a5f39 Namjae Jeon    2020-03-02  155  {
31023864e67a5f39 Namjae Jeon    2020-03-02  156  	unsigned int num_clusters = 0;
31023864e67a5f39 Namjae Jeon    2020-03-02  157  	unsigned int clu;
31023864e67a5f39 Namjae Jeon    2020-03-02  158  	struct super_block *sb = inode->i_sb;
31023864e67a5f39 Namjae Jeon    2020-03-02  159  	struct exfat_sb_info *sbi = EXFAT_SB(sb);
c37357510a2be482 Hyeongseok Kim 2021-01-06  160  	int cur_cmap_i, next_cmap_i;
31023864e67a5f39 Namjae Jeon    2020-03-02  161  
31023864e67a5f39 Namjae Jeon    2020-03-02  162  	/* invalid cluster number */
31023864e67a5f39 Namjae Jeon    2020-03-02  163  	if (p_chain->dir == EXFAT_FREE_CLUSTER ||
31023864e67a5f39 Namjae Jeon    2020-03-02  164  	    p_chain->dir == EXFAT_EOF_CLUSTER ||
31023864e67a5f39 Namjae Jeon    2020-03-02  165  	    p_chain->dir < EXFAT_FIRST_CLUSTER)
31023864e67a5f39 Namjae Jeon    2020-03-02  166  		return 0;
31023864e67a5f39 Namjae Jeon    2020-03-02  167  
31023864e67a5f39 Namjae Jeon    2020-03-02  168  	/* no cluster to truncate */
31023864e67a5f39 Namjae Jeon    2020-03-02  169  	if (p_chain->size == 0)
31023864e67a5f39 Namjae Jeon    2020-03-02  170  		return 0;
31023864e67a5f39 Namjae Jeon    2020-03-02  171  
31023864e67a5f39 Namjae Jeon    2020-03-02  172  	/* check cluster validation */
a949824f01f3b39f hyeongseok.kim 2020-06-04  173  	if (!is_valid_cluster(sbi, p_chain->dir)) {
d1727d55c0327efd Joe Perches    2020-04-24  174  		exfat_err(sb, "invalid start cluster (%u)", p_chain->dir);
31023864e67a5f39 Namjae Jeon    2020-03-02  175  		return -EIO;
31023864e67a5f39 Namjae Jeon    2020-03-02  176  	}
31023864e67a5f39 Namjae Jeon    2020-03-02  177  
31023864e67a5f39 Namjae Jeon    2020-03-02  178  	clu = p_chain->dir;
31023864e67a5f39 Namjae Jeon    2020-03-02  179  
c37357510a2be482 Hyeongseok Kim 2021-01-06  180  	cur_cmap_i = BITMAP_OFFSET_SECTOR_INDEX(sb, CLUSTER_TO_BITMAP_ENT(clu));
c37357510a2be482 Hyeongseok Kim 2021-01-06  181  
31023864e67a5f39 Namjae Jeon    2020-03-02  182  	if (p_chain->flags == ALLOC_NO_FAT_CHAIN) {
c37357510a2be482 Hyeongseok Kim 2021-01-06  183  		unsigned int last_cluster = p_chain->dir + p_chain->size - 1;
31023864e67a5f39 Namjae Jeon    2020-03-02  184  		do {
c37357510a2be482 Hyeongseok Kim 2021-01-06  185  			bool sync = false;
c37357510a2be482 Hyeongseok Kim 2021-01-06  186  
c37357510a2be482 Hyeongseok Kim 2021-01-06  187  			if (clu < last_cluster)
c37357510a2be482 Hyeongseok Kim 2021-01-06  188  				next_cmap_i =
c37357510a2be482 Hyeongseok Kim 2021-01-06  189  				  BITMAP_OFFSET_SECTOR_INDEX(sb, CLUSTER_TO_BITMAP_ENT(clu+1));
31023864e67a5f39 Namjae Jeon    2020-03-02  190  
c37357510a2be482 Hyeongseok Kim 2021-01-06  191  			/* flush bitmap only if index would be changed or for last cluster */
c37357510a2be482 Hyeongseok Kim 2021-01-06 @192  			if (clu == last_cluster || cur_cmap_i != next_cmap_i) {
c37357510a2be482 Hyeongseok Kim 2021-01-06  193  				sync = true;
c37357510a2be482 Hyeongseok Kim 2021-01-06  194  				cur_cmap_i = next_cmap_i;
c37357510a2be482 Hyeongseok Kim 2021-01-06  195  			}
c37357510a2be482 Hyeongseok Kim 2021-01-06  196  
c37357510a2be482 Hyeongseok Kim 2021-01-06  197  			exfat_clear_bitmap(inode, clu, (sync && IS_DIRSYNC(inode)));
c37357510a2be482 Hyeongseok Kim 2021-01-06  198  			clu++;
31023864e67a5f39 Namjae Jeon    2020-03-02  199  			num_clusters++;
31023864e67a5f39 Namjae Jeon    2020-03-02  200  		} while (num_clusters < p_chain->size);
31023864e67a5f39 Namjae Jeon    2020-03-02  201  	} else {
31023864e67a5f39 Namjae Jeon    2020-03-02  202  		do {
c37357510a2be482 Hyeongseok Kim 2021-01-06  203  			bool sync = false;
c37357510a2be482 Hyeongseok Kim 2021-01-06  204  			unsigned int n_clu = clu;
c37357510a2be482 Hyeongseok Kim 2021-01-06  205  			int err = exfat_get_next_cluster(sb, &n_clu);
31023864e67a5f39 Namjae Jeon    2020-03-02  206  
c37357510a2be482 Hyeongseok Kim 2021-01-06  207  			if (err || n_clu == EXFAT_EOF_CLUSTER)
c37357510a2be482 Hyeongseok Kim 2021-01-06  208  				sync = true;
c37357510a2be482 Hyeongseok Kim 2021-01-06  209  			else
c37357510a2be482 Hyeongseok Kim 2021-01-06  210  				next_cmap_i =
c37357510a2be482 Hyeongseok Kim 2021-01-06  211  				  BITMAP_OFFSET_SECTOR_INDEX(sb, CLUSTER_TO_BITMAP_ENT(n_clu));
c37357510a2be482 Hyeongseok Kim 2021-01-06  212  
c37357510a2be482 Hyeongseok Kim 2021-01-06  213  			if (cur_cmap_i != next_cmap_i) {
c37357510a2be482 Hyeongseok Kim 2021-01-06  214  				sync = true;
c37357510a2be482 Hyeongseok Kim 2021-01-06  215  				cur_cmap_i = next_cmap_i;
c37357510a2be482 Hyeongseok Kim 2021-01-06  216  			}
31023864e67a5f39 Namjae Jeon    2020-03-02  217  
c37357510a2be482 Hyeongseok Kim 2021-01-06  218  			exfat_clear_bitmap(inode, clu, (sync && IS_DIRSYNC(inode)));
c37357510a2be482 Hyeongseok Kim 2021-01-06  219  			clu = n_clu;
31023864e67a5f39 Namjae Jeon    2020-03-02  220  			num_clusters++;
c37357510a2be482 Hyeongseok Kim 2021-01-06  221  
c37357510a2be482 Hyeongseok Kim 2021-01-06  222  			if (err)
c37357510a2be482 Hyeongseok Kim 2021-01-06  223  				goto dec_used_clus;
31023864e67a5f39 Namjae Jeon    2020-03-02  224  		} while (clu != EXFAT_EOF_CLUSTER);
31023864e67a5f39 Namjae Jeon    2020-03-02  225  	}
31023864e67a5f39 Namjae Jeon    2020-03-02  226  
31023864e67a5f39 Namjae Jeon    2020-03-02  227  dec_used_clus:
31023864e67a5f39 Namjae Jeon    2020-03-02  228  	sbi->used_clusters -= num_clusters;
31023864e67a5f39 Namjae Jeon    2020-03-02  229  	return 0;
31023864e67a5f39 Namjae Jeon    2020-03-02  230  }
31023864e67a5f39 Namjae Jeon    2020-03-02  231  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 43531 bytes --]

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

end of thread, other threads:[~2021-01-08  0:19 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20210106044038epcas1p2d3488531b0a63c122f7401d4d56b03a8@epcas1p2.samsung.com>
2021-01-06  4:39 ` [PATCH] exfat: improve performance of exfat_free_cluster when using dirsync mount option Hyeongseok Kim
2021-01-07  6:23   ` Sungjong Seo
2021-01-08  0:19     ` Namjae Jeon
2021-01-06  7:51 kernel test robot

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.