Bug 107041 - Support FITRIM in vfat to allow fstrim to work on FAT32 partitions
Summary: Support FITRIM in vfat to allow fstrim to work on FAT32 partitions
Status: CLOSED PATCH_ALREADY_AVAILABLE
Alias: None
Product: File System
Classification: Unclassified
Component: FAT/VFAT/MSDOS (show other bugs)
Hardware: All Linux
: P1 enhancement
Assignee: OGAWA Hirofumi
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-11-02 16:32 UTC by Mark
Modified: 2018-08-24 00:24 UTC (History)
4 users (show)

See Also:
Kernel Version: 4.2.5
Subsystem:
Regression: No
Bisected commit-id:


Attachments
Propose to implement FITRIM ioctl (149.77 KB, patch)
2018-03-30 16:15 UTC, Matt Wang
Details | Diff
Correct file (4.01 KB, patch)
2018-03-30 16:22 UTC, Matt Wang
Details | Diff
Latest file under review (6.41 KB, patch)
2018-06-01 02:52 UTC, Matt Wang
Details | Diff
Patch under testing (5.20 KB, patch)
2018-06-06 08:59 UTC, Matt Wang
Details | Diff

Description Mark 2015-11-02 16:32:51 UTC
This is an enhancement request: add FITRIM support to vfat, to enable fstrim to operate on FAT32 partitions.
Comment 1 Oleksandr 2018-02-04 11:05:52 UTC
I am going to implement this
Comment 2 Matt Wang 2018-03-30 16:15:45 UTC
Created attachment 275015 [details]
Propose to implement FITRIM ioctl

This is a possible implementation, please help to review. Thanks.
Comment 3 Matt Wang 2018-03-30 16:21:33 UTC
Comment on attachment 275015 [details]
Propose to implement FITRIM ioctl

From d6525ea4e4b39100432e09f0a31e37af81454b9b Mon Sep 17 00:00:00 2001
From: Wentao Wang <matt.w.wang@emc.com>
Date: Fri, 30 Mar 2018 00:49:32 -0700
Subject: [PATCH] Add new FITRIM ioctl for fstrim to trim FAT file system


diff --git a/fs/fat/fat.h b/fs/fat/fat.h
index 8fc1093..154ae54 100644
--- a/fs/fat/fat.h
+++ b/fs/fat/fat.h
@@ -357,6 +357,7 @@ extern int fat_alloc_clusters(struct inode *inode, int *cluster,
 			      int nr_cluster);
 extern int fat_free_clusters(struct inode *inode, int cluster);
 extern int fat_count_free_clusters(struct super_block *sb);
+extern int fat_trim_fs(struct inode *inode, struct fstrim_range *range);
 
 /* fat/file.c */
 extern long fat_generic_ioctl(struct file *filp, unsigned int cmd,
diff --git a/fs/fat/fatent.c b/fs/fat/fatent.c
index bac10de..59269e7 100644
--- a/fs/fat/fatent.c
+++ b/fs/fat/fatent.c
@@ -459,6 +459,67 @@ static void fat_collect_bhs(struct buffer_head **bhs, int *nr_bhs,
 	}
 }
 
+int fat_trim_fs(struct inode *inode, struct fstrim_range *range)
+{
+	struct super_block *sb = inode->i_sb;
+	struct msdos_sb_info *sbi = MSDOS_SB(sb);
+	struct fat_entry fatent;
+	int next;
+	u64 start_blk, len_blk, minlen_blk;
+	u64 ent_start, ent_end, ent;
+	u32 count = 0, trimmed = 0, minlen;
+
+	/*
+	 * FAT data is organized as clusters, trim at the granulary of cluster.
+	 *
+	 * fstrim_range is in byte, convert vaules to cluster index.
+	 * Treat sectors before data region as all used, not to trim them.
+	 */
+	start_blk = range->start >> sb->s_blocksize_bits;
+	len_blk = range->len >> sb->s_blocksize_bits;
+	minlen_blk = range->minlen >> sb->s_blocksize_bits;
+
+	ent_start = start_blk >> ffs(sbi->sec_per_clus);
+	ent_start = ent_start < FAT_START_ENT? FAT_START_ENT: ent_start;
+
+	ent_end = (ent_start + len_blk) >> ffs(sbi->sec_per_clus);
+	if (ent_end >= sbi->max_cluster)
+		return -EINVAL;
+
+	minlen = minlen_blk >> ffs(sbi->sec_per_clus);
+	if (ent_start + minlen > ent_end)
+		return -EINVAL;
+
+	fatent_init(&fatent);
+	lock_fat(sbi);
+	ent = ent_start;
+	while (ent < ent_end) {
+		next = fat_ent_read(inode, &fatent, ent);
+
+		if (next == FAT_ENT_FREE) {
+			count++;
+		} else {
+			if (count != 0 && count >= minlen) {
+				sb_issue_discard(sb,
+					fat_clus_to_blknr(sbi, ent - count),
+					count * sbi->sec_per_clus,
+					GFP_NOFS, 0);
+
+				trimmed += count;
+			}
+			count = 0;
+		}
+
+		ent++;
+	}
+	fatent_brelse(&fatent);
+	unlock_fat(sbi);
+
+	range->len = (trimmed * sbi->sec_per_clus) << sb->s_blocksize_bits;
+
+	return 0;
+}
+
 int fat_alloc_clusters(struct inode *inode, int *cluster, int nr_cluster)
 {
 	struct super_block *sb = inode->i_sb;
diff --git a/fs/fat/file.c b/fs/fat/file.c
index 4724cc9..f2ec441 100644
--- a/fs/fat/file.c
+++ b/fs/fat/file.c
@@ -121,6 +121,35 @@ static int fat_ioctl_get_volume_id(struct inode *inode, u32 __user *user_attr)
 	return put_user(sbi->vol_id, user_attr);
 }
 
+static int fat_ioctl_fitrim(struct inode *inode, u32 __user *arg)
+{
+	struct super_block *sb = inode->i_sb;
+	struct fstrim_range range;
+	struct request_queue *q = bdev_get_queue(sb->s_bdev);
+	int ret = 0;
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EPERM;
+
+	if (!blk_queue_discard(q))
+		return -EOPNOTSUPP;
+
+	if (copy_from_user(&range, (struct fstrim_range __user *)arg, sizeof(range)))
+		return -EFAULT;
+
+	range.minlen = max_t(unsigned int, range.minlen,
+			q->limits.discard_granularity);
+
+	ret = fat_trim_fs(inode, &range);
+	if (ret < 0)
+		return ret;
+
+	if (copy_to_user((struct fstrim_range __user *)arg, &range, sizeof(range)))
+		return -EFAULT;
+
+	return 0;
+}
+
 long fat_generic_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 {
 	struct inode *inode = file_inode(filp);
@@ -133,6 +162,8 @@ long fat_generic_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 		return fat_ioctl_set_attributes(filp, user_attr);
 	case FAT_IOCTL_GET_VOLUME_ID:
 		return fat_ioctl_get_volume_id(inode, user_attr);
+	case FITRIM:
+		return fat_ioctl_fitrim(inode, user_attr);
 	default:
 		return -ENOTTY;	/* Inappropriate ioctl for device */
 	}
-- 
2.7.4
Comment 4 Matt Wang 2018-03-30 16:22:23 UTC
Created attachment 275017 [details]
Correct file
Comment 5 Matt Wang 2018-06-01 02:52:13 UTC
Created attachment 276287 [details]
Latest file under review
Comment 6 Matt Wang 2018-06-06 08:59:54 UTC
Created attachment 276339 [details]
Patch under testing

This patch is enhanced/reviewed by OGAWA Hirofumi, I am doing test.
Comment 7 Matt Wang 2018-07-01 04:40:27 UTC
Testing Done, here is the info

1. My mount info
root@matt-ubuntu:/home/matt# mount
/dev/sdb6 on /media/test type vfat .....

2. Change fat_trim_cluster
add debuginfo:     printk("TRIM issued! clus %d, nr_clus %d, blknr %ld, sec_per_clus %d\n",
                clus, nr_clus, blknr, sbi->sec_per_clus);

################### No files on partition test #############################
# test 1
a. Use fstrim to trim whole partition (no files on partition)
fstrim /media/test

b. Kernel log showed:
[ 1684.525043] TRIM issued! clus 3, nr_clus 230967, blknr 3650, sec_per_clus 8

# test 2
a. Trim range (no files on partition) start at 10MB, len 4096B.
fstrim -o 10485760 -l 4096 /media/test/

b. Kernel log showed:
[ 1918.714012] TRIM issued! clus 2560, nr_clus 1, blknr 24106, sec_per_clus 8

################### With files on partition test #############################
# test 3
a. Create a 4kB file 
matt@matt-ubuntu:/media/test$ sudo dd if=/dev/zero of=./a.test bs=1024 count=4
4+0 records in
4+0 records out
4096 bytes (4.1 kB, 4.0 KiB) copied, 0.000188191 s, 21.8 MB/s

b. Trim whole partition
fstrim /media/test/

c. Kernel log showed:
[ 2623.243549] TRIM issued! clus 4, nr_clus 230966, blknr 3658, sec_per_clus 8

# test 4
a. Trim with --minimum option
fstrim  -l 40960 -m 40960 /media/test/

b. No Trim issueed.

# test 5 (file integrity test)
a. Keep 10 random files after random create/delete.
matt@matt-ubuntu:/media/test$ ls
alternatives.log  auth.log       c.test    fontconfig.log   kern.log
a.test            bootstrap.log  dpkg.log  gpu-manager.log  Xorg.0.log

b. Trim partition
[ 4393.712792] TRIM issued! clus 4, nr_clus 4, blknr 3658, sec_per_clus 8
[ 4393.725997] TRIM issued! clus 9, nr_clus 2, blknr 3698, sec_per_clus 8
[ 4393.744997] TRIM issued! clus 47, nr_clus 230923, blknr 4002, sec_per_clus 8

c. Files are all the same with before
Comment 8 Matt Wang 2018-07-02 14:42:39 UTC
filefrag has something wrong to handle fat:
root@matt-ubuntu:/media/matt/F1E6-E9AE# filefrag  -b512 Xorg.0.log 
Xorg.0.log: 1 extent found

############################################################################
So we choose a hard way:
fdisk -l:
/dev/sdb6       349798400 351649791   1851392  904M  b W95 FAT32

############################################################################
root@matt-ubuntu:/media/matt/F1E6-E9AE# ls
bootstrap.log  fontconfig.log  kern.log  Xorg.0.log

root@matt-ubuntu:/media/matt/F1E6-E9AE# hdparm --fibmap ./bootstrap.log

./bootstrap.log:
 filesystem blocksize 512, begins at LBA 349798400; assuming 512 byte sectors.
 byte_offset  begin_LBA    end_LBA    sectors
           0  349802130  349802242        113
root@matt-ubuntu:/media/matt/F1E6-E9AE# hdparm --fibmap ./fontconfig.log 

./fontconfig.log:
 filesystem blocksize 512, begins at LBA 349798400; assuming 512 byte sectors.
 byte_offset  begin_LBA    end_LBA    sectors
           0  349802250  349802258          9
root@matt-ubuntu:/media/matt/F1E6-E9AE# hdparm --fibmap ./kern.log 

./kern.log:
 filesystem blocksize 512, begins at LBA 349798400; assuming 512 byte sectors.
 byte_offset  begin_LBA    end_LBA    sectors
           0  349802274  349802487        214
root@matt-ubuntu:/media/matt/F1E6-E9AE# hdparm --fibmap ./Xorg.0.log 

./Xorg.0.log:
 filesystem blocksize 512, begins at LBA 349798400; assuming 512 byte sectors.
 byte_offset  begin_LBA    end_LBA    sectors
           0  349802490  349802617        128


root@matt-ubuntu:/media/matt# fstrim /media/matt/F1E6-E9AE/
root@matt-ubuntu:/media/matt# dmesg 
[ 1967.573430] TRIM issued! clus 3, nr_clus 10, blknr 3650, sec_per_clus 8
[ 1967.749874] TRIM issued! clus 30, nr_clus 1, blknr 3866, sec_per_clus 8
[ 1967.765649] TRIM issued! clus 74, nr_clus 230896, blknr 4218, sec_per_clus 8


We can calculate: 
bootstrap.log  occupied [3730 - 3842] logical sector
fontconfig.log occupied [3850 - 3858] logical sector
kern.log       occupied [3874 - 4087] logical sector
Xorg.0.log     occupied [4090 - 4217] logical sector

From dmesg we can see we trimed:
3650 - 3730 sector
3866 - 3874 sector
4218 - end  sector

That's what we suppose to do.
Comment 9 OGAWA Hirofumi 2018-08-24 00:24:35 UTC
The code was merged into for v4.19.

Note You need to log in before you can comment on or make changes to this bug.