Re: [PATCH V3 4/5] block: Add secure discard

2010-07-01 Thread Adrian Hunter

Andrew Morton wrote:

On Thu, 24 Jun 2010 11:44:23 +0300
Adrian Hunter adrian.hun...@nokia.com wrote:


From b25b9a499f255ee5999c219525d82ef40382318c Mon Sep 17 00:00:00 2001
From: Adrian Hunter adrian.hun...@nokia.com
Date: Wed, 23 Jun 2010 15:41:38 +0300
Subject: [PATCH 4/5] block: Add secure discard

Secure discard is the same as discard except that all copies
of the discarded sectors (perhaps created by garbage collection)
must also be erased.


That's not an awfully informative changelog.


From a quick peek at the code it seems that you took your earlier

design sketch:

On Mon, Jun 14, 2010 at 02:10:06PM +0300, Adrian Hunter wrote:

Needs a bio flag, a request flag, setup the request flag based on the
bio flag, prevent merging secure and non-secure discards, prevent drivers
doing non-secure discards for secure discards.

Seems like a lot of little changes for something that no one wants.
Shouldn't it wait for someone to need it first?


and changed your mind and implemented it.

Is that a correct interpretation?



Yes.  It does allude to that in [PATCH V3 0/5] Add MMC erase and secure erase 
V3
i.e.

Changes from V2

- move the addition of BLKSECDISCARD to a separate patch and implement it
using I/O requests
- move the MMC support of secure discard to a separate patch and support
the secure discard I/O request
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH V3 4/5] block: Add secure discard

2010-06-30 Thread Andrew Morton
On Thu, 24 Jun 2010 11:44:23 +0300
Adrian Hunter adrian.hun...@nokia.com wrote:

 From b25b9a499f255ee5999c219525d82ef40382318c Mon Sep 17 00:00:00 2001
 From: Adrian Hunter adrian.hun...@nokia.com
 Date: Wed, 23 Jun 2010 15:41:38 +0300
 Subject: [PATCH 4/5] block: Add secure discard
 
 Secure discard is the same as discard except that all copies
 of the discarded sectors (perhaps created by garbage collection)
 must also be erased.

That's not an awfully informative changelog.

From a quick peek at the code it seems that you took your earlier
design sketch:

On Mon, Jun 14, 2010 at 02:10:06PM +0300, Adrian Hunter wrote:
 Needs a bio flag, a request flag, setup the request flag based on the
 bio flag, prevent merging secure and non-secure discards, prevent drivers
 doing non-secure discards for secure discards.
 
 Seems like a lot of little changes for something that no one wants.
 Shouldn't it wait for someone to need it first?

and changed your mind and implemented it.

Is that a correct interpretation?
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH V3 4/5] block: Add secure discard

2010-06-24 Thread Adrian Hunter
From b25b9a499f255ee5999c219525d82ef40382318c Mon Sep 17 00:00:00 2001
From: Adrian Hunter adrian.hun...@nokia.com
Date: Wed, 23 Jun 2010 15:41:38 +0300
Subject: [PATCH 4/5] block: Add secure discard

Secure discard is the same as discard except that all copies
of the discarded sectors (perhaps created by garbage collection)
must also be erased.

Signed-off-by: Adrian Hunter adrian.hun...@nokia.com
---
 block/blk-core.c|6 +-
 block/blk-lib.c |6 ++
 block/compat_ioctl.c|1 +
 block/elevator.c|4 
 block/ioctl.c   |   15 ++-
 include/linux/bio.h |7 ++-
 include/linux/blkdev.h  |   10 +-
 include/linux/fs.h  |2 ++
 kernel/trace/blktrace.c |8 
 9 files changed, 51 insertions(+), 8 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index f84cce4..6f9c6df 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1153,6 +1153,8 @@ void init_request_from_bio(struct request *req, struct 
bio *bio)
req-cmd_flags |= REQ_DISCARD;
if (bio_rw_flagged(bio, BIO_RW_BARRIER))
req-cmd_flags |= REQ_SOFTBARRIER;
+   if (bio_rw_flagged(bio, BIO_RW_SECURE))
+   req-cmd_flags |= REQ_SECURE;
} else if (unlikely(bio_rw_flagged(bio, BIO_RW_BARRIER)))
req-cmd_flags |= REQ_HARDBARRIER;
 
@@ -1501,7 +1503,9 @@ static inline void __generic_make_request(struct bio *bio)
goto end_io;
 
if (bio_rw_flagged(bio, BIO_RW_DISCARD) 
-   !blk_queue_discard(q)) {
+   (!blk_queue_discard(q) ||
+(bio_rw_flagged(bio, BIO_RW_SECURE) 
+ !blk_queue_secdiscard(q {
err = -EOPNOTSUPP;
goto end_io;
}
diff --git a/block/blk-lib.c b/block/blk-lib.c
index d0216b9..dc56682 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -52,6 +52,12 @@ int blkdev_issue_discard(struct block_device *bdev, sector_t 
sector,
if (!blk_queue_discard(q))
return -EOPNOTSUPP;
 
+   if (flags  BLKDEV_IFL_SECURE) {
+   if (!blk_queue_secdiscard(q))
+   return -EOPNOTSUPP;
+   type |= DISCARD_SECURE;
+   }
+
while (nr_sects  !ret) {
unsigned int sector_size = q-limits.logical_block_size;
unsigned int max_discard_sectors =
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/elevator.c b/block/elevator.c
index 923a913..23b5f3d 100644
--- a/block/elevator.c
+++ b/block/elevator.c
@@ -83,6 +83,10 @@ int elv_rq_merge_ok(struct request *rq, struct bio *bio)
bio_rw_flagged(rq-bio, BIO_RW_DISCARD))
return 0;
 
+   if (bio_rw_flagged(bio, BIO_RW_SECURE) !=
+   bio_rw_flagged(rq-bio, BIO_RW_SECURE))
+   return 0;
+
/*
 * different data direction or already started, don't merge
 */
diff --git a/block/ioctl.c b/block/ioctl.c
index e8eb679..1fba55f 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -114,8 +114,10 @@ static int blkdev_reread_part(struct block_device *bdev)
 }
 
 static int blk_ioctl_discard(struct block_device *bdev, uint64_t start,
-uint64_t len)
+uint64_t len, int secure)
 {
+   unsigned long flags = BLKDEV_IFL_WAIT;
+
if (start  511)
return -EINVAL;
if (len  511)
@@ -125,8 +127,9 @@ static int blk_ioctl_discard(struct block_device *bdev, 
uint64_t start,
 
if (start + len  (bdev-bd_inode-i_size  9))
return -EINVAL;
-   return blkdev_issue_discard(bdev, start, len, GFP_KERNEL,
-   BLKDEV_IFL_WAIT);
+   if (secure)
+   flags |= BLKDEV_IFL_SECURE;
+   return blkdev_issue_discard(bdev, start, len, GFP_KERNEL, flags);
 }
 
 static int put_ushort(unsigned long arg, unsigned short val)
@@ -226,7 +229,8 @@ int blkdev_ioctl(struct block_device *bdev, fmode_t mode, 
unsigned cmd,
unlock_kernel();
return 0;
 
-   case BLKDISCARD: {
+   case BLKDISCARD:
+   case BLKSECDISCARD: {
uint64_t range[2];
 
if (!(mode  FMODE_WRITE))
@@ -235,7 +239,8 @@ int blkdev_ioctl(struct block_device *bdev, fmode_t mode, 
unsigned cmd,
if (copy_from_user(range, (void __user *)arg, sizeof(range)))