2015-06-28 13:27:19

by Mikulas Patocka

[permalink] [raw]
Subject: [PATCH] hpfs: add fstrim support

This patch adds support for fstrim to the HPFS filesystem.

Signed-off-by: Mikulas Patocka <[email protected]>

---
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(+)

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)


2015-06-28 19:52:20

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH] hpfs: add fstrim support

On Sun, Jun 28, 2015 at 6:16 AM, Mikulas Patocka <[email protected]> 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

2015-06-28 20:59:37

by Al Viro

[permalink] [raw]
Subject: Re: [PATCH] hpfs: add fstrim support

On Sun, Jun 28, 2015 at 12:52:11PM -0700, Linus Torvalds wrote:
> On Sun, Jun 28, 2015 at 6:16 AM, Mikulas Patocka <[email protected]> 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...

2015-06-28 21:46:18

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH] hpfs: add fstrim support

On Sun, Jun 28, 2015 at 1:59 PM, Al Viro <[email protected]> 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

2015-07-09 16:05:34

by Mikulas Patocka

[permalink] [raw]
Subject: [PATCH] ioctl_compat: handle FITRIM



On Sun, 28 Jun 2015, Linus Torvalds wrote:

> On Sun, Jun 28, 2015 at 6:16 AM, Mikulas Patocka <[email protected]> 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

Here I'm sending a patch that handles FITRIM in a generic way?

BTW. what happened with the other HPFS patches? They haven't been included
in Linux 4.2-rc1.


From: Mikulas Patocka <[email protected]>

The FITRIM ioctl has the same arguments on 32-bit and 64-bit
architectures, so we can add it to the list of compatible ioctls and drop
it from compat_ioctl method of various filesystems.

Signed-off-by: Mikulas Patocka <[email protected]>

---
fs/compat_ioctl.c | 1 +
fs/ecryptfs/file.c | 1 -
fs/ext4/ioctl.c | 1 -
fs/hpfs/dir.c | 3 ---
fs/hpfs/file.c | 3 ---
fs/hpfs/super.c | 7 -------
fs/jfs/ioctl.c | 4 ----
fs/nilfs2/ioctl.c | 1 -
fs/ocfs2/ioctl.c | 1 -
9 files changed, 1 insertion(+), 21 deletions(-)

Index: linux-4.2-rc1/fs/compat_ioctl.c
===================================================================
--- linux-4.2-rc1.orig/fs/compat_ioctl.c 2015-07-09 16:17:16.000000000 +0200
+++ linux-4.2-rc1/fs/compat_ioctl.c 2015-07-09 16:18:50.000000000 +0200
@@ -896,6 +896,7 @@ COMPATIBLE_IOCTL(FIGETBSZ)
/* 'X' - originally XFS but some now in the VFS */
COMPATIBLE_IOCTL(FIFREEZE)
COMPATIBLE_IOCTL(FITHAW)
+COMPATIBLE_IOCTL(FITRIM)
COMPATIBLE_IOCTL(KDGETKEYCODE)
COMPATIBLE_IOCTL(KDSETKEYCODE)
COMPATIBLE_IOCTL(KDGKBTYPE)
Index: linux-4.2-rc1/fs/ecryptfs/file.c
===================================================================
--- linux-4.2-rc1.orig/fs/ecryptfs/file.c 2015-07-09 16:13:54.000000000 +0200
+++ linux-4.2-rc1/fs/ecryptfs/file.c 2015-07-09 16:14:05.000000000 +0200
@@ -325,7 +325,6 @@ ecryptfs_compat_ioctl(struct file *file,
return rc;

switch (cmd) {
- case FITRIM:
case FS_IOC32_GETFLAGS:
case FS_IOC32_SETFLAGS:
case FS_IOC32_GETVERSION:
Index: linux-4.2-rc1/fs/ext4/ioctl.c
===================================================================
--- linux-4.2-rc1.orig/fs/ext4/ioctl.c 2015-07-09 16:16:38.000000000 +0200
+++ linux-4.2-rc1/fs/ext4/ioctl.c 2015-07-09 16:16:45.000000000 +0200
@@ -755,7 +755,6 @@ long ext4_compat_ioctl(struct file *file
return err;
}
case EXT4_IOC_MOVE_EXT:
- case FITRIM:
case EXT4_IOC_RESIZE_FS:
case EXT4_IOC_PRECACHE_EXTENTS:
case EXT4_IOC_SET_ENCRYPTION_POLICY:
Index: linux-4.2-rc1/fs/hpfs/dir.c
===================================================================
--- linux-4.2-rc1.orig/fs/hpfs/dir.c 2015-07-09 16:15:22.000000000 +0200
+++ linux-4.2-rc1/fs/hpfs/dir.c 2015-07-09 16:15:26.000000000 +0200
@@ -328,7 +328,4 @@ const struct file_operations hpfs_dir_op
.release = hpfs_dir_release,
.fsync = hpfs_file_fsync,
.unlocked_ioctl = hpfs_ioctl,
-#ifdef CONFIG_COMPAT
- .compat_ioctl = hpfs_compat_ioctl,
-#endif
};
Index: linux-4.2-rc1/fs/hpfs/file.c
===================================================================
--- linux-4.2-rc1.orig/fs/hpfs/file.c 2015-07-09 16:15:30.000000000 +0200
+++ linux-4.2-rc1/fs/hpfs/file.c 2015-07-09 16:15:33.000000000 +0200
@@ -204,9 +204,6 @@ const struct file_operations hpfs_file_o
.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.2-rc1/fs/hpfs/super.c
===================================================================
--- linux-4.2-rc1.orig/fs/hpfs/super.c 2015-07-09 16:14:53.000000000 +0200
+++ linux-4.2-rc1/fs/hpfs/super.c 2015-07-09 16:15:12.000000000 +0200
@@ -228,13 +228,6 @@ long hpfs_ioctl(struct file *file, unsig
}
}

-#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;

Index: linux-4.2-rc1/fs/jfs/ioctl.c
===================================================================
--- linux-4.2-rc1.orig/fs/jfs/ioctl.c 2015-07-09 16:16:52.000000000 +0200
+++ linux-4.2-rc1/fs/jfs/ioctl.c 2015-07-09 16:17:03.000000000 +0200
@@ -180,10 +180,6 @@ long jfs_compat_ioctl(struct file *filp,
case JFS_IOC_SETFLAGS32:
cmd = JFS_IOC_SETFLAGS;
break;
- case FITRIM:
- cmd = FITRIM;
- break;
- }
return jfs_ioctl(filp, cmd, arg);
}
#endif
Index: linux-4.2-rc1/fs/nilfs2/ioctl.c
===================================================================
--- linux-4.2-rc1.orig/fs/nilfs2/ioctl.c 2015-07-09 16:13:37.000000000 +0200
+++ linux-4.2-rc1/fs/nilfs2/ioctl.c 2015-07-09 16:13:47.000000000 +0200
@@ -1369,7 +1369,6 @@ long nilfs_compat_ioctl(struct file *fil
case NILFS_IOCTL_SYNC:
case NILFS_IOCTL_RESIZE:
case NILFS_IOCTL_SET_ALLOC_RANGE:
- case FITRIM:
break;
default:
return -ENOIOCTLCMD;
Index: linux-4.2-rc1/fs/ocfs2/ioctl.c
===================================================================
--- linux-4.2-rc1.orig/fs/ocfs2/ioctl.c 2015-07-09 16:14:20.000000000 +0200
+++ linux-4.2-rc1/fs/ocfs2/ioctl.c 2015-07-09 16:14:25.000000000 +0200
@@ -980,7 +980,6 @@ long ocfs2_compat_ioctl(struct file *fil
case OCFS2_IOC_GROUP_EXTEND:
case OCFS2_IOC_GROUP_ADD:
case OCFS2_IOC_GROUP_ADD64:
- case FITRIM:
break;
case OCFS2_IOC_REFLINK:
if (copy_from_user(&args, argp, sizeof(args)))