2010-06-03 21:15:00

by Adrian Hunter

[permalink] [raw]
Subject: [PATCH 1/4] block: Allow drivers to implement BLKDISCARD and add BLKSECDISCARD

>From 328a858c01be1e0d08f4a4b1f43d9fd117a295e5 Mon Sep 17 00:00:00 2001
From: Adrian Hunter <[email protected]>
Date: Thu, 3 Jun 2010 10:46:04 +0300
Subject: [PATCH 1/4] block: Allow drivers to implement BLKDISCARD and add BLKSECDISCARD

SD/MMC cards provide an erase operation that is too inefficient to
allow file systems to use it, however it is still useful for
userspace tools because it is still 10 - 100 times faster than
writing zeroes. Allow the MMC block driver to provide its own
BLKDISCARD implementation for this purpose.

In addition, eMMC v4.4 cards can provide a secure erase operation
which guarantees that all copies of the discarded sectors (for
example created by garbage collection) will also be erased. For
this a new ioctl BLKSECDISCARD is added.

Signed-off-by: Adrian Hunter <[email protected]>
---
block/compat_ioctl.c | 1 +
block/ioctl.c | 9 +++++++++
include/linux/fs.h | 1 +
3 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/block/compat_ioctl.c b/block/compat_ioctl.c
index f26051f..24a146d 100644
--- a/block/compat_ioctl.c
+++ b/block/compat_ioctl.c
@@ -753,6 +753,7 @@ long compat_blkdev_ioctl(struct file *file, unsigned cmd, unsigned long arg)
case BLKFLSBUF:
case BLKROSET:
case BLKDISCARD:
+ case BLKSECDISCARD:
/*
* the ones below are implemented in blkdev_locked_ioctl,
* but we call blkdev_ioctl, which gets the lock for us
diff --git a/block/ioctl.c b/block/ioctl.c
index e8eb679..985aa88 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -232,12 +232,21 @@ int blkdev_ioctl(struct block_device *bdev, fmode_t mode, unsigned cmd,
if (!(mode & FMODE_WRITE))
return -EBADF;

+ ret = __blkdev_driver_ioctl(bdev, mode, cmd, arg);
+ if (ret != -ENOTTY)
+ return ret;
+
if (copy_from_user(range, (void __user *)arg, sizeof(range)))
return -EFAULT;

return blk_ioctl_discard(bdev, range[0], range[1]);
}

+ case BLKSECDISCARD:
+ if (!(mode & FMODE_WRITE))
+ return -EBADF;
+ return __blkdev_driver_ioctl(bdev, mode, cmd, arg);
+
case HDIO_GETGEO: {
struct hd_geometry geo;

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 3428393..25a6058 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -309,6 +309,7 @@ struct inodes_stat_t {
#define BLKALIGNOFF _IO(0x12,122)
#define BLKPBSZGET _IO(0x12,123)
#define BLKDISCARDZEROES _IO(0x12,124)
+#define BLKSECDISCARD _IO(0x12,125)

#define BMAP_IOCTL 1 /* obsolete - kept for compatibility */
#define FIBMAP _IO(0x00,1) /* bmap access */
--
1.6.3.3


2010-06-10 07:00:08

by Adrian Hunter

[permalink] [raw]
Subject: Re: [PATCH 1/4] block: Allow drivers to implement BLKDISCARD and add BLKSECDISCARD

Adrian Hunter wrote:
> From 328a858c01be1e0d08f4a4b1f43d9fd117a295e5 Mon Sep 17 00:00:00 2001
> From: Adrian Hunter <[email protected]>
> Date: Thu, 3 Jun 2010 10:46:04 +0300
> Subject: [PATCH 1/4] block: Allow drivers to implement BLKDISCARD and add BLKSECDISCARD
>
> SD/MMC cards provide an erase operation that is too inefficient to
> allow file systems to use it, however it is still useful for
> userspace tools because it is still 10 - 100 times faster than
> writing zeroes. Allow the MMC block driver to provide its own
> BLKDISCARD implementation for this purpose.
>
> In addition, eMMC v4.4 cards can provide a secure erase operation
> which guarantees that all copies of the discarded sectors (for
> example created by garbage collection) will also be erased. For
> this a new ioctl BLKSECDISCARD is added.

Ping?

>
> Signed-off-by: Adrian Hunter <[email protected]>
> ---
> block/compat_ioctl.c | 1 +
> block/ioctl.c | 9 +++++++++
> include/linux/fs.h | 1 +
> 3 files changed, 11 insertions(+), 0 deletions(-)
>
> diff --git a/block/compat_ioctl.c b/block/compat_ioctl.c
> index f26051f..24a146d 100644
> --- a/block/compat_ioctl.c
> +++ b/block/compat_ioctl.c
> @@ -753,6 +753,7 @@ long compat_blkdev_ioctl(struct file *file, unsigned cmd, unsigned long arg)
> case BLKFLSBUF:
> case BLKROSET:
> case BLKDISCARD:
> + case BLKSECDISCARD:
> /*
> * the ones below are implemented in blkdev_locked_ioctl,
> * but we call blkdev_ioctl, which gets the lock for us
> diff --git a/block/ioctl.c b/block/ioctl.c
> index e8eb679..985aa88 100644
> --- a/block/ioctl.c
> +++ b/block/ioctl.c
> @@ -232,12 +232,21 @@ int blkdev_ioctl(struct block_device *bdev, fmode_t mode, unsigned cmd,
> if (!(mode & FMODE_WRITE))
> return -EBADF;
>
> + ret = __blkdev_driver_ioctl(bdev, mode, cmd, arg);
> + if (ret != -ENOTTY)
> + return ret;
> +
> if (copy_from_user(range, (void __user *)arg, sizeof(range)))
> return -EFAULT;
>
> return blk_ioctl_discard(bdev, range[0], range[1]);
> }
>
> + case BLKSECDISCARD:
> + if (!(mode & FMODE_WRITE))
> + return -EBADF;
> + return __blkdev_driver_ioctl(bdev, mode, cmd, arg);
> +
> case HDIO_GETGEO: {
> struct hd_geometry geo;
>
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 3428393..25a6058 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -309,6 +309,7 @@ struct inodes_stat_t {
> #define BLKALIGNOFF _IO(0x12,122)
> #define BLKPBSZGET _IO(0x12,123)
> #define BLKDISCARDZEROES _IO(0x12,124)
> +#define BLKSECDISCARD _IO(0x12,125)
>
> #define BMAP_IOCTL 1 /* obsolete - kept for compatibility */
> #define FIBMAP _IO(0x00,1) /* bmap access */

2010-06-10 08:20:05

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 1/4] block: Allow drivers to implement BLKDISCARD and add BLKSECDISCARD

On Fri, Jun 04, 2010 at 12:13:16AM +0300, Adrian Hunter wrote:
> SD/MMC cards provide an erase operation that is too inefficient to
> allow file systems to use it, however it is still useful for
> userspace tools because it is still 10 - 100 times faster than
> writing zeroes. Allow the MMC block driver to provide its own
> BLKDISCARD implementation for this purpose.

That's no good reason. Almost all current SSD TRIM implementations
have the same issue, that's why filesustems require the -o discard
option to enabled it. In addition I've not actually seen any numbers
from you on a current kernel.

2010-06-11 07:52:14

by Kyungmin Park

[permalink] [raw]
Subject: Re: [PATCH 1/4] block: Allow drivers to implement BLKDISCARD and add BLKSECDISCARD

On Thu, Jun 10, 2010 at 5:19 PM, Christoph Hellwig <[email protected]> wrote:
> On Fri, Jun 04, 2010 at 12:13:16AM +0300, Adrian Hunter wrote:
>> SD/MMC cards provide an erase operation that is too inefficient to
>> allow file systems to use it, however it is still useful for
>> userspace tools because it is still 10 - 100 times faster than
>> writing zeroes. ?Allow the MMC block driver to provide its own
>> BLKDISCARD implementation for this purpose.

I heard if we give trim command without proper align, it writes some
data to align its data.
then user data can be corrupt.
The main problem is we don't know exact used block at filesystem. so
user can corrupt their data easily.

And as you know each manufacturers use their own superpage and align.

>
> That's no good reason. ?Almost all current SSD TRIM implementations
> have the same issue, that's why filesustems require the -o discard
> option to enabled it. ?In addition I've not actually seen any numbers
> from you on a current kernel.

I hope to see the performance results. In my test, no improvement.

Thank you,
Kyungmin Park

>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>