Re: [PATCH] hpfs: add fstrim support
On Sun, Jun 28, 2015 at 1:59 PM, Al Viro 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-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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
> 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-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] hpfs: add fstrim support
On Sun, Jun 28, 2015 at 6:16 AM, Mikulas Patocka 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-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] hpfs: add fstrim support
This patch adds support for fstrim to the HPFS filesystem.
Signed-off-by: Mikulas Patocka
---
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.c2014-07-01 18:49:25.0 +0200
+++ linux-4.1-rc1/fs/hpfs/dir.c 2015-05-01 15:52:29.0 +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.0 +0200
+++ linux-4.1-rc1/fs/hpfs/file.c2015-05-01 15:52:29.0 +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.0 +0200
+++ linux-4.1-rc1/fs/hpfs/alloc.c 2015-05-01 15:52:29.0 +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, ))) {
+ err = -EIO;
+ goto unlock_1;
+ }
+ idx = 0;
+ while ((len = find_run(bmp, )) && !err) {
+ err = do_trim(s, sbi->sb_dirband_start + idx * 4, len *
4, start, end, minlen, result);
+ idx += len;
+ }
+ hpfs_brelse4();
+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, , "trim"))) {
+ err = -EIO;
+ goto unlock_2;
+ }
+ idx = 0;
+ while ((len = find_run(bmp, )) && !err) {
+ err = do_trim(s, (start_bmp << 14) + idx, len, start,
end, minlen, result);
+ idx += len;
+ }
+ hpfs_brelse4();
+unlock_2:
+ hpfs_unlock(s);
+
[PATCH] hpfs: add fstrim support
This patch adds support for fstrim to the HPFS filesystem.
Signed-off-by: Mikulas Patocka miku...@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(+)
Index: linux-4.1-rc1/fs/hpfs/dir.c
===
--- linux-4.1-rc1.orig/fs/hpfs/dir.c2014-07-01 18:49:25.0 +0200
+++ linux-4.1-rc1/fs/hpfs/dir.c 2015-05-01 15:52:29.0 +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.0 +0200
+++ linux-4.1-rc1/fs/hpfs/file.c2015-05-01 15:52:29.0 +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.0 +0200
+++ linux-4.1-rc1/fs/hpfs/alloc.c 2015-05-01 15:52:29.0 +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);
+
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 miku...@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-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] hpfs: add fstrim support
On Sun, Jun 28, 2015 at 1:59 PM, Al Viro v...@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-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] hpfs: add fstrim support
On Sun, Jun 28, 2015 at 6:16 AM, Mikulas Patocka miku...@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-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
