block: Add secure discard
in [Kernel]
|
Prev: Removing dead MACH_U300_BS26
Next: Filename case conflict Documentation/io-mapping.txt and Documentation/IO-mapping.txt
From: Andrew Morton on 30 Jun 2010 18:20 On Thu, 24 Jun 2010 11:44:23 +0300 Adrian Hunter <adrian.hunter(a)nokia.com> wrote: > >From b25b9a499f255ee5999c219525d82ef40382318c Mon Sep 17 00:00:00 2001 > From: Adrian Hunter <adrian.hunter(a)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-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Adrian Hunter on 1 Jul 2010 06:20 Andrew Morton wrote: > On Thu, 24 Jun 2010 11:44:23 +0300 > Adrian Hunter <adrian.hunter(a)nokia.com> wrote: > >> >From b25b9a499f255ee5999c219525d82ef40382318c Mon Sep 17 00:00:00 2001 >> From: Adrian Hunter <adrian.hunter(a)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-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Adrian Hunter on 13 Jul 2010 02:50 Hunter Adrian (Nokia-MS/Helsinki) wrote: >>From 1cdd5b576bfafcd53857ed4d37830446e96e8206 Mon Sep 17 00:00:00 2001 > From: Adrian Hunter <adrian.hunter(a)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.hunter(a)nokia.com> Christoph will you comment on this patch? Is it doing what you asked? > --- > block/blk-core.c | 5 ++++- > block/blk-lib.c | 6 ++++++ > block/compat_ioctl.c | 1 + > block/elevator.c | 6 ++++++ > block/ioctl.c | 15 ++++++++++----- > include/linux/bio.h | 2 ++ > include/linux/blkdev.h | 7 ++++++- > include/linux/fs.h | 2 ++ > kernel/trace/blktrace.c | 8 ++++++++ > 9 files changed, 45 insertions(+), 7 deletions(-) > > diff --git a/block/blk-core.c b/block/blk-core.c > index 3c37894..7931081 100644 > --- a/block/blk-core.c > +++ b/block/blk-core.c > @@ -1513,7 +1513,10 @@ static inline void __generic_make_request(struct bio *bio) > if (bio_check_eod(bio, nr_sectors)) > goto end_io; > > - if ((bio->bi_rw & REQ_DISCARD) && !blk_queue_discard(q)) { > + if ((bio->bi_rw & REQ_DISCARD) && > + (!blk_queue_discard(q) || > + ((bio->bi_rw & REQ_SECURE) && > + !blk_queue_secdiscard(q)))) { > err = -EOPNOTSUPP; > goto end_io; > } > diff --git a/block/blk-lib.c b/block/blk-lib.c > index e16185b..ba8c0f9 100644 > --- a/block/blk-lib.c > +++ b/block/blk-lib.c > @@ -50,6 +50,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 max_discard_sectors = > min(q->limits.max_discard_sectors, UINT_MAX >> 9); > 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 816a7c8..ec585c9 100644 > --- a/block/elevator.c > +++ b/block/elevator.c > @@ -83,6 +83,12 @@ int elv_rq_merge_ok(struct request *rq, struct bio *bio) > return 0; > > /* > + * Don't merge discard requests and secure discard requests > + */ > + if ((bio->bi_rw & REQ_SECURE) != (rq->bio->bi_rw & REQ_SECURE)) > + return 0; > + > + /* > * different data direction or already started, don't merge > */ > if (bio_data_dir(bio) != rq_data_dir(rq)) > diff --git a/block/ioctl.c b/block/ioctl.c > index 6f3db6f..afcfe48 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))) > return -EFAULT; > > - return blk_ioctl_discard(bdev, range[0], range[1]); > + return blk_ioctl_discard(bdev, range[0], range[1], > + cmd == BLKSECDISCARD); > } > > case HDIO_GETGEO: { > diff --git a/include/linux/bio.h b/include/linux/bio.h > index 4d379c8..147e631 100644 > --- a/include/linux/bio.h > +++ b/include/linux/bio.h > @@ -176,6 +176,7 @@ enum rq_flag_bits { > __REQ_INTEGRITY, /* integrity metadata has been remapped */ > __REQ_IO_STAT, /* account I/O stat */ > __REQ_MIXED_MERGE, /* merge of different types, fail separately */ > + __REQ_SECURE, /* secure discard (used with __REQ_DISCARD) */ > __REQ_NR_BITS, /* stops here */ > }; > > @@ -215,6 +216,7 @@ enum rq_flag_bits { > #define REQ_INTEGRITY (1 << __REQ_INTEGRITY) > #define REQ_IO_STAT (1 << __REQ_IO_STAT) > #define REQ_MIXED_MERGE (1 << __REQ_MIXED_MERGE) > +#define REQ_SECURE (1 << __REQ_SECURE) > > /* > * upper 16 bits of bi_rw define the io priority of this bio > diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h > index 204fbe2..da643f4 100644 > --- a/include/linux/blkdev.h > +++ b/include/linux/blkdev.h > @@ -404,6 +404,7 @@ struct request_queue > #define QUEUE_FLAG_DISCARD 16 /* supports DISCARD */ > #define QUEUE_FLAG_NOXMERGES 17 /* No extended merges */ > #define QUEUE_FLAG_ADD_RANDOM 18 /* Contributes to random pool */ > +#define QUEUE_FLAG_SECDISCARD 19 /* supports SECDISCARD */ > > #define QUEUE_FLAG_DEFAULT ((1 << QUEUE_FLAG_IO_STAT) | \ > (1 << QUEUE_FLAG_CLUSTER) | \ > @@ -539,6 +540,8 @@ enum { > #define blk_queue_stackable(q) \ > test_bit(QUEUE_FLAG_STACKABLE, &(q)->queue_flags) > #define blk_queue_discard(q) test_bit(QUEUE_FLAG_DISCARD, &(q)->queue_flags) > +#define blk_queue_secdiscard(q) (blk_queue_discard(q) && \ > + test_bit(QUEUE_FLAG_SECDISCARD, &(q)->queue_flags)) > > #define blk_noretry_request(rq) \ > ((rq)->cmd_flags & (REQ_FAILFAST_DEV|REQ_FAILFAST_TRANSPORT| \ > @@ -931,10 +934,12 @@ static inline struct request *blk_map_queue_find_tag(struct blk_queue_tag *bqt, > } > enum{ > BLKDEV_WAIT, /* wait for completion */ > - BLKDEV_BARRIER, /*issue request with barrier */ > + BLKDEV_BARRIER, /* issue request with barrier */ > + BLKDEV_SECURE, /* secure discard */ > }; > #define BLKDEV_IFL_WAIT (1 << BLKDEV_WAIT) > #define BLKDEV_IFL_BARRIER (1 << BLKDEV_BARRIER) > +#define BLKDEV_IFL_SECURE (1 << BLKDEV_SECURE) > extern int blkdev_issue_flush(struct block_device *, gfp_t, sector_t *, > unsigned long); > extern int blkdev_issue_discard(struct block_device *bdev, sector_t sector, > diff --git a/include/linux/fs.h b/include/linux/fs.h > index dc9d185..2f5df1f 100644 > --- a/include/linux/fs.h > +++ b/include/linux/fs.h > @@ -176,6 +176,7 @@ struct inodes_stat_t { > */ > #define DISCARD_NOBARRIER (WRITE | REQ_DISCARD) > #define DISCARD_BARRIER (WRITE | REQ_DISCARD | REQ_HARDBARRIER) > +#define DISCARD_SECURE (DISCARD_NOBARRIER | REQ_SECURE) > > #define SEL_IN 1 > #define SEL_OUT 2 > @@ -318,6 +319,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 */ > diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c > index 3b4a695..a711b47 100644 > --- a/kernel/trace/blktrace.c > +++ b/kernel/trace/blktrace.c > @@ -667,6 +667,9 @@ static void blk_add_trace_rq(struct request_queue *q, struct request *rq, > if (rq->cmd_flags & REQ_DISCARD) > rw |= REQ_DISCARD; > > + if (rq->cmd_flags & REQ_SECURE) > + rw |= REQ_SECURE; > + > if (rq->cmd_type == REQ_TYPE_BLOCK_PC) { > what |= BLK_TC_ACT(BLK_TC_PC); > __blk_add_trace(bt, 0, blk_rq_bytes(rq), rw, > @@ -1773,6 +1776,8 @@ void blk_fill_rwbs(char *rwbs, u32 rw, int bytes) > rwbs[i++] = 'S'; > if (rw & REQ_META) > rwbs[i++] = 'M'; > + if (rw & REQ_SECURE) > + rwbs[i++] = 'E'; > > rwbs[i] = '\0'; > } > @@ -1785,6 +1790,9 @@ void blk_fill_rwbs_rq(char *rwbs, struct request *rq) > if (rq->cmd_flags & REQ_DISCARD) > rw |= REQ_DISCARD; > > + if (rq->cmd_flags & REQ_SECURE) > + rw |= REQ_SECURE; > + > bytes = blk_rq_bytes(rq); > > blk_fill_rwbs(rwbs, rw, bytes); -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Christoph Hellwig on 13 Jul 2010 23:20 The patches look good to me, but they'll need some work to apply against Jens' block tree. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Andrew Morton on 15 Jul 2010 15:30
On Wed, 14 Jul 2010 13:50:39 +0300 Adrian Hunter <adrian.hunter(a)nokia.com> wrote: > >From 9ba7750fb96d9994e8715b14d8091c37b832b3e6 Mon Sep 17 00:00:00 2001 > From: Adrian Hunter <adrian.hunter(a)nokia.com> > Date: Wed, 23 Jun 2010 15:41:38 +0300 > Subject: [PATCH] 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.hunter(a)nokia.com> > --- > > > > Here is "[PATCH V4 4/5] block: Add secure discard" applied to > git://git.kernel.dk/linux-2.6-block.git for-next. > > If Jens takes this patch (originally 4th of 5), Andrew takes patches 1 to 3, > and patch 5 waits for this one to propagate, then there will be no dependency > problem. hm, complicated. I merged all five into -mm. If Jens sends along some acks then I send all five along to Linus, nice and simple. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |