diff mbox

hpfs: add fstrim support

Message ID alpine.DEB.2.00.1506281506450.29365@leontynka
State New, archived
Headers show

Commit Message

Mikulas Patocka June 28, 2015, 1:16 p.m. UTC
This patch adds support for fstrim to the HPFS filesystem.

Signed-off-by: Mikulas Patocka <mikulas@twibright.com>

---
 fs/hpfs/alloc.c   |   95 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/hpfs/dir.c     |    4 ++
 fs/hpfs/file.c    |    4 ++
 fs/hpfs/hpfs_fn.h |    7 +++
 fs/hpfs/super.c   |   35 +++++++++++++++++++
 5 files changed, 145 insertions(+)

--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Linus Torvalds June 28, 2015, 7:52 p.m. UTC | #1
On Sun, Jun 28, 2015 at 6:16 AM, Mikulas Patocka <mikulas@twibright.com> wrote:
> This patch adds support for fstrim to the HPFS filesystem.
...
> +#ifdef CONFIG_COMPAT
> +       .compat_ioctl   = hpfs_compat_ioctl,
> +#endif
...
> +#ifdef CONFIG_COMPAT
> +       .compat_ioctl   = hpfs_compat_ioctl,
> +#endif
...
> +#ifdef CONFIG_COMPAT
> +long hpfs_compat_ioctl(struct file *file, unsigned cmd, unsigned long arg)
> +{
> +       return hpfs_ioctl(file, cmd, (unsigned long)compat_ptr(arg));
> +}
> +#endif

Hmm. You've clearly copied this pattern from other filesystems, and so
I can't really blame you, but this thing annoys me a lot.

Why isn't FITRIM just marked as a COMPATIBLE_IOCTL(), at which point
the generic ioctl layer will do exactly the above translation for us?

Am I missing something?

                     Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Al Viro June 28, 2015, 8:59 p.m. UTC | #2
On Sun, Jun 28, 2015 at 12:52:11PM -0700, Linus Torvalds wrote:
> On Sun, Jun 28, 2015 at 6:16 AM, Mikulas Patocka <mikulas@twibright.com> wrote:
> > This patch adds support for fstrim to the HPFS filesystem.
> ...
> > +#ifdef CONFIG_COMPAT
> > +       .compat_ioctl   = hpfs_compat_ioctl,
> > +#endif
> ...
> > +#ifdef CONFIG_COMPAT
> > +       .compat_ioctl   = hpfs_compat_ioctl,
> > +#endif
> ...
> > +#ifdef CONFIG_COMPAT
> > +long hpfs_compat_ioctl(struct file *file, unsigned cmd, unsigned long arg)
> > +{
> > +       return hpfs_ioctl(file, cmd, (unsigned long)compat_ptr(arg));
> > +}
> > +#endif
> 
> Hmm. You've clearly copied this pattern from other filesystems, and so
> I can't really blame you, but this thing annoys me a lot.
> 
> Why isn't FITRIM just marked as a COMPATIBLE_IOCTL(), at which point
> the generic ioctl layer will do exactly the above translation for us?
> 
> Am I missing something?

More to the point, why bother with ->ioctl() at all?  Why not make
->fitrim() a super_block method and let do_vfs_ioctl() handle all
marshalling?  As in
	(int *)fitrim(struct super_block *, struct fstrim_range *);
guaranteed to be called only on a filesystem kept active by caller...
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Linus Torvalds June 28, 2015, 9:46 p.m. UTC | #3
On Sun, Jun 28, 2015 at 1:59 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> More to the point, why bother with ->ioctl() at all?  Why not make
> ->fitrim() a super_block method and let do_vfs_ioctl() handle all
> marshalling?  As in
>         (int *)fitrim(struct super_block *, struct fstrim_range *);
> guaranteed to be called only on a filesystem kept active by caller...

I'd be ok with that, but that's a bigger issue and I think would be a
separate second step from removing the whole compat mess anyway.

                   Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

Index: linux-4.1-rc1/fs/hpfs/dir.c
===================================================================
--- linux-4.1-rc1.orig/fs/hpfs/dir.c	2014-07-01 18:49:25.000000000 +0200
+++ linux-4.1-rc1/fs/hpfs/dir.c	2015-05-01 15:52:29.000000000 +0200
@@ -327,4 +327,8 @@  const struct file_operations hpfs_dir_op
 	.iterate	= hpfs_readdir,
 	.release	= hpfs_dir_release,
 	.fsync		= hpfs_file_fsync,
+	.unlocked_ioctl	= hpfs_ioctl,
+#ifdef CONFIG_COMPAT
+	.compat_ioctl	= hpfs_compat_ioctl,
+#endif
 };
Index: linux-4.1-rc1/fs/hpfs/file.c
===================================================================
--- linux-4.1-rc1.orig/fs/hpfs/file.c	2015-05-01 12:31:01.000000000 +0200
+++ linux-4.1-rc1/fs/hpfs/file.c	2015-05-01 15:52:29.000000000 +0200
@@ -203,6 +203,10 @@  const struct file_operations hpfs_file_o
 	.release	= hpfs_file_release,
 	.fsync		= hpfs_file_fsync,
 	.splice_read	= generic_file_splice_read,
+	.unlocked_ioctl	= hpfs_ioctl,
+#ifdef CONFIG_COMPAT
+	.compat_ioctl	= hpfs_compat_ioctl,
+#endif
 };
 
 const struct inode_operations hpfs_file_iops =
Index: linux-4.1-rc1/fs/hpfs/alloc.c
===================================================================
--- linux-4.1-rc1.orig/fs/hpfs/alloc.c	2014-07-01 18:49:25.000000000 +0200
+++ linux-4.1-rc1/fs/hpfs/alloc.c	2015-05-01 15:52:29.000000000 +0200
@@ -484,3 +484,98 @@  struct anode *hpfs_alloc_anode(struct su
 	a->btree.first_free = cpu_to_le16(8);
 	return a;
 }
+
+static unsigned find_run(__le32 *bmp, unsigned *idx)
+{
+	unsigned len;
+	while (tstbits(bmp, *idx, 1)) {
+		(*idx)++;
+		if (unlikely(*idx >= 0x4000))
+			return 0;
+	}
+	len = 1;
+	while (!tstbits(bmp, *idx + len, 1))
+		len++;
+	return len;
+}
+
+static int do_trim(struct super_block *s, secno start, unsigned len, secno limit_start, secno limit_end, unsigned minlen, unsigned *result)
+{
+	int err;
+	secno end;
+	if (fatal_signal_pending(current))
+		return -EINTR;
+	end = start + len;
+	if (start < limit_start)
+		start = limit_start;
+	if (end > limit_end)
+		end = limit_end;
+	if (start >= end)
+		return 0;
+	if (end - start < minlen)
+		return 0;
+	err = sb_issue_discard(s, start, end - start, GFP_NOFS, 0);
+	if (err)
+		return err;
+	*result += end - start;
+	return 0;
+}
+
+int hpfs_trim_fs(struct super_block *s, u64 start, u64 end, u64 minlen, unsigned *result)
+{
+	int err = 0;
+	struct hpfs_sb_info *sbi = hpfs_sb(s);
+	unsigned idx, len, start_bmp, end_bmp;
+	__le32 *bmp;
+	struct quad_buffer_head qbh;
+
+	*result = 0;
+	if (!end || end > sbi->sb_fs_size)
+		end = sbi->sb_fs_size;
+	if (start >= sbi->sb_fs_size)
+		return 0;
+	if (minlen > 0x4000)
+		return 0;
+	if (start < sbi->sb_dirband_start + sbi->sb_dirband_size && end > sbi->sb_dirband_start) {
+		hpfs_lock(s);
+		if (s->s_flags & MS_RDONLY) {
+			err = -EROFS;
+			goto unlock_1;
+		}
+		if (!(bmp = hpfs_map_dnode_bitmap(s, &qbh))) {
+			err = -EIO;
+			goto unlock_1;
+		}
+		idx = 0;
+		while ((len = find_run(bmp, &idx)) && !err) {
+			err = do_trim(s, sbi->sb_dirband_start + idx * 4, len * 4, start, end, minlen, result);
+			idx += len;
+		}
+		hpfs_brelse4(&qbh);
+unlock_1:
+		hpfs_unlock(s);
+	}
+	start_bmp = start >> 14;
+	end_bmp = (end + 0x3fff) >> 14;
+	while (start_bmp < end_bmp && !err) {
+		hpfs_lock(s);
+		if (s->s_flags & MS_RDONLY) {
+			err = -EROFS;
+			goto unlock_2;
+		}
+		if (!(bmp = hpfs_map_bitmap(s, start_bmp, &qbh, "trim"))) {
+			err = -EIO;
+			goto unlock_2;
+		}
+		idx = 0;
+		while ((len = find_run(bmp, &idx)) && !err) {
+			err = do_trim(s, (start_bmp << 14) + idx, len, start, end, minlen, result);
+			idx += len;
+		}
+		hpfs_brelse4(&qbh);
+unlock_2:
+		hpfs_unlock(s);
+		start_bmp++;
+	}
+	return err;
+}
Index: linux-4.1-rc1/fs/hpfs/hpfs_fn.h
===================================================================
--- linux-4.1-rc1.orig/fs/hpfs/hpfs_fn.h	2014-07-01 18:49:25.000000000 +0200
+++ linux-4.1-rc1/fs/hpfs/hpfs_fn.h	2015-05-01 15:52:29.000000000 +0200
@@ -18,6 +18,8 @@ 
 #include <linux/pagemap.h>
 #include <linux/buffer_head.h>
 #include <linux/slab.h>
+#include <linux/sched.h>
+#include <linux/blkdev.h>
 #include <asm/unaligned.h>
 
 #include "hpfs.h"
@@ -200,6 +202,7 @@  void hpfs_free_dnode(struct super_block 
 struct dnode *hpfs_alloc_dnode(struct super_block *, secno, dnode_secno *, struct quad_buffer_head *);
 struct fnode *hpfs_alloc_fnode(struct super_block *, secno, fnode_secno *, struct buffer_head **);
 struct anode *hpfs_alloc_anode(struct super_block *, secno, anode_secno *, struct buffer_head **);
+int hpfs_trim_fs(struct super_block *, u64, u64, u64, unsigned *);
 
 /* anode.c */
 
@@ -318,6 +321,10 @@  __printf(2, 3)
 void hpfs_error(struct super_block *, const char *, ...);
 int hpfs_stop_cycles(struct super_block *, int, int *, int *, char *);
 unsigned hpfs_get_free_dnodes(struct super_block *);
+long hpfs_ioctl(struct file *file, unsigned cmd, unsigned long arg);
+#ifdef CONFIG_COMPAT
+long hpfs_compat_ioctl(struct file *file, unsigned cmd, unsigned long arg);
+#endif
 
 /*
  * local time (HPFS) to GMT (Unix)
Index: linux-4.1-rc1/fs/hpfs/super.c
===================================================================
--- linux-4.1-rc1.orig/fs/hpfs/super.c	2015-05-01 12:58:39.000000000 +0200
+++ linux-4.1-rc1/fs/hpfs/super.c	2015-05-01 16:02:53.000000000 +0200
@@ -15,6 +15,7 @@ 
 #include <linux/sched.h>
 #include <linux/bitmap.h>
 #include <linux/slab.h>
+#include <linux/compat.h>
 
 /* Mark the filesystem dirty, so that chkdsk checks it when os/2 booted */
 
@@ -198,6 +199,40 @@  static int hpfs_statfs(struct dentry *de
 	return 0;
 }
 
+
+long hpfs_ioctl(struct file *file, unsigned cmd, unsigned long arg)
+{
+	switch (cmd) {
+		case FITRIM: {
+			struct fstrim_range range;
+			secno n_trimmed;
+			int r;
+			if (!capable(CAP_SYS_ADMIN))
+				return -EPERM;
+			if (copy_from_user(&range, (struct fstrim_range __user *)arg, sizeof(range)))
+				return -EFAULT;
+			r = hpfs_trim_fs(file_inode(file)->i_sb, range.start >> 9, (range.start + range.len) >> 9, (range.minlen + 511) >> 9, &n_trimmed);
+			if (r)
+				return r;
+			range.len = (u64)n_trimmed << 9;
+			if (copy_to_user((struct fstrim_range __user *)arg, &range, sizeof(range)))
+				return -EFAULT;
+			return 0;
+		}
+		default: {
+			return -ENOIOCTLCMD;
+		}
+	}
+}
+
+#ifdef CONFIG_COMPAT
+long hpfs_compat_ioctl(struct file *file, unsigned cmd, unsigned long arg)
+{
+	return hpfs_ioctl(file, cmd, (unsigned long)compat_ptr(arg));
+}
+#endif
+
+
 static struct kmem_cache * hpfs_inode_cachep;
 
 static struct inode *hpfs_alloc_inode(struct super_block *sb)