Re: [PATCH V2] mmc: card: IOCTL support for Sanitize feature of eMMC v4.5
From: Kyungmin Park
Date: Tue Dec 27 2011 - 08:54:33 EST
Hi Yaniv,
You maybe split the patch into block and mmc parts.
Please see the comments inlined.
Thank you,
Kyungmin Park
On Tue, Dec 27, 2011 at 10:33 PM, Yaniv Gardi <ygardi@xxxxxxxxxxxxxx> wrote:
> Signed-off-by: Yaniv Gardi <ygardi@xxxxxxxxxxxxxx>
>
> ---
> block/blk-core.c | 15 ++++++++++--
> block/blk-lib.c | 44 +++++++++++++++++++++++++++++++++++++++
> block/blk-merge.c | 6 +++++
> block/elevator.c | 8 ++++++-
> block/ioctl.c | 9 ++++++++
> drivers/mmc/card/block.c | 50 ++++++++++++++++++++++++++++++++++++--------
> drivers/mmc/card/queue.c | 7 +++++-
> include/linux/blk_types.h | 7 ++++-
> include/linux/blkdev.h | 3 ++
> include/linux/fs.h | 1 +
> include/linux/mmc/card.h | 7 ++++++
> kernel/trace/blktrace.c | 2 +
> 12 files changed, 143 insertions(+), 16 deletions(-)
>
> diff --git a/block/blk-core.c b/block/blk-core.c
> index e6c05a9..e3512a4 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -1544,7 +1544,7 @@ generic_make_request_checks(struct bio *bio)
> goto end_io;
> }
>
> - if (unlikely(!(bio->bi_rw & REQ_DISCARD) &&
> + if (unlikely(!(bio->bi_rw & (REQ_DISCARD | REQ_SANITIZE)) &&
> nr_sectors > queue_max_hw_sectors(q))) {
> printk(KERN_ERR "bio too big device %s (%u > %u)\n",
> bdevname(bio->bi_bdev, b),
> @@ -1592,6 +1592,14 @@ generic_make_request_checks(struct bio *bio)
> goto end_io;
> }
>
> + if ((bio->bi_rw & REQ_SANITIZE) &&
> + (!blk_queue_sanitize(q))) {
> + pr_err("%s - SANITIZE request but queue "
> + "doesn't support sanitize", __func__);
> + err = -EOPNOTSUPP;
> + goto end_io;
> + }
> +
> if (blk_throtl_bio(q, bio))
> return false; /* throttled, will be resubmitted later */
>
> @@ -1697,7 +1705,8 @@ void submit_bio(int rw, struct bio *bio)
> * If it's a regular read/write or a barrier with data attached,
> * go through the normal accounting stuff before submission.
> */
> - if (bio_has_data(bio) && !(rw & REQ_DISCARD)) {
> + if (bio_has_data(bio) &&
> + (!(rw & (REQ_DISCARD | REQ_SANITIZE)))) {
> if (rw & WRITE) {
> count_vm_events(PGPGOUT, count);
> } else {
> @@ -1743,7 +1752,7 @@ EXPORT_SYMBOL(submit_bio);
> */
> int blk_rq_check_limits(struct request_queue *q, struct request *rq)
> {
> - if (rq->cmd_flags & REQ_DISCARD)
> + if ((rq->cmd_flags & REQ_DISCARD) || (rq->cmd_flags & REQ_SANITIZE))
(rq->cmd_flags & (REQ_DISCARD | REA_SANITIZE))?
> return 0;
>
> if (blk_rq_sectors(rq) > queue_max_sectors(q) ||
> diff --git a/block/blk-lib.c b/block/blk-lib.c
> index 2b461b4..76ed930 100644
> --- a/block/blk-lib.c
> +++ b/block/blk-lib.c
> @@ -114,6 +114,50 @@ int blkdev_issue_discard(struct block_device *bdev, sector_t sector,
> }
> EXPORT_SYMBOL(blkdev_issue_discard);
>
> +int blkdev_issue_sanitize(struct block_device *bdev, gfp_t gfp_mask)
> +{
> + DECLARE_COMPLETION_ONSTACK(wait);
> + struct request_queue *q = bdev_get_queue(bdev);
> + int type = REQ_WRITE | REQ_SANITIZE;
> +
> + struct bio_batch bb;
> + struct bio *bio;
> + int ret = 0;
> +
> + if (!q)
> + return -ENXIO;
> +
> + bio = bio_alloc(gfp_mask, 1);
> + if (!bio)
> + return -ENOMEM;
> +
> + if (!blk_queue_sanitize(q)) {
> + pr_err("%s - card doesn't support sanitize", __func__);
You should add the bio_free-like at here?
> + return -EOPNOTSUPP;
> + }
> +
> + atomic_set(&bb.done, 1);
> + bb.flags = 1 << BIO_UPTODATE;
> + bb.wait = &wait;
> +
> + bio->bi_end_io = bio_batch_end_io;
> + bio->bi_bdev = bdev;
> + bio->bi_private = &bb;
> +
> + atomic_inc(&bb.done);
> + submit_bio(type, bio);
> +
> + /* Wait for bios in-flight */
> + if (!atomic_dec_and_test(&bb.done))
> + wait_for_completion(&wait);
> +
> + if (!test_bit(BIO_UPTODATE, &bb.flags))
> + ret = -EIO;
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(blkdev_issue_sanitize);
> +
> /**
> * blkdev_issue_zeroout - generate number of zero filed write bios
> * @bdev: blockdev to issue
> diff --git a/block/blk-merge.c b/block/blk-merge.c
> index cfcc37c..f3ed15b 100644
> --- a/block/blk-merge.c
> +++ b/block/blk-merge.c
> @@ -383,6 +383,12 @@ static int attempt_merge(struct request_queue *q, struct request *req,
> return 0;
>
> /*
> + * Don't merge file system requests and sanitize requests
> + */
> + if ((req->cmd_flags & REQ_SANITIZE) != (next->cmd_flags & REQ_SANITIZE))
> + return 0;
> +
> + /*
> * not contiguous
> */
> if (blk_rq_pos(req) + blk_rq_sectors(req) != blk_rq_pos(next))
> diff --git a/block/elevator.c b/block/elevator.c
> index 99838f4..4255dd9 100644
> --- a/block/elevator.c
> +++ b/block/elevator.c
> @@ -88,6 +88,12 @@ int elv_rq_merge_ok(struct request *rq, struct bio *bio)
> return 0;
>
> /*
> + * Don't merge sanitize requests
> + */
> + if ((bio->bi_rw & REQ_SANITIZE) != (rq->bio->bi_rw & REQ_SANITIZE))
> + return 0;
> +
> + /*
> * different data direction or already started, don't merge
> */
> if (bio_data_dir(bio) != rq_data_dir(rq))
> @@ -647,7 +653,7 @@ void __elv_add_request(struct request_queue *q, struct request *rq, int where)
> if (rq->cmd_flags & REQ_SOFTBARRIER) {
> /* barriers are scheduling boundary, update end_sector */
> if (rq->cmd_type == REQ_TYPE_FS ||
> - (rq->cmd_flags & REQ_DISCARD)) {
> + (rq->cmd_flags & (REQ_DISCARD | REQ_SANITIZE))) {
> q->end_sector = rq_end_sector(rq);
> q->boundary_rq = rq;
> }
> diff --git a/block/ioctl.c b/block/ioctl.c
> index ca939fc..112215d 100644
> --- a/block/ioctl.c
> +++ b/block/ioctl.c
> @@ -132,6 +132,11 @@ static int blk_ioctl_discard(struct block_device *bdev, uint64_t start,
> return blkdev_issue_discard(bdev, start, len, GFP_KERNEL, flags);
> }
>
> +static int blk_ioctl_sanitize(struct block_device *bdev)
> +{
> + return blkdev_issue_sanitize(bdev, GFP_KERNEL);
> +}
> +
> static int put_ushort(unsigned long arg, unsigned short val)
> {
> return put_user(val, (unsigned short __user *)arg);
> @@ -216,6 +221,10 @@ int blkdev_ioctl(struct block_device *bdev, fmode_t mode, unsigned cmd,
> set_device_ro(bdev, n);
> return 0;
>
> + case BLKSANITIZE:
> + ret = blk_ioctl_sanitize(bdev);
> + break;
> +
> case BLKDISCARD:
> case BLKSECDISCARD: {
> uint64_t range[2];
> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
> index 0c959c9..06561b9 100644
> --- a/drivers/mmc/card/block.c
> +++ b/drivers/mmc/card/block.c
> @@ -868,18 +868,11 @@ static int mmc_blk_issue_secdiscard_rq(struct mmc_queue *mq,
> unsigned int from, nr, arg;
> int err = 0, type = MMC_BLK_SECDISCARD;
>
> - if (!(mmc_can_secure_erase_trim(card) || mmc_can_sanitize(card))) {
> + if (!(mmc_can_secure_erase_trim(card))) {
> err = -EOPNOTSUPP;
> goto out;
> }
>
> - /* The sanitize operation is supported at v4.5 only */
> - if (mmc_can_sanitize(card)) {
> - err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> - EXT_CSD_SANITIZE_START, 1, 0);
> - goto out;
> - }
> -
> from = blk_rq_pos(req);
> nr = blk_rq_sectors(req);
>
> @@ -922,6 +915,40 @@ out:
> return err ? 0 : 1;
> }
>
> +int mmc_blk_issue_sanitize_rq(struct mmc_queue *mq,
> + struct request *req)
> +{
> + struct mmc_blk_data *md = mq->data;
> + struct mmc_card *card = md->queue.card;
> + int err = 0;
> +
> + if (!(mmc_can_sanitize(card))) {
> + pr_err("%s: SANITIZE is not supported", __func__);
> + err = -EOPNOTSUPP;
> + goto out;
> + }
> +
> + pr_debug("%s: SANITIZE is supported", __func__);
> +
> + mmc_sd_card_set_sanitize_in_progress(card);
> +
> + err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> + EXT_CSD_SANITIZE_START, 1, 0);
> +
> + if (err)
> + pr_err("%s: mmcblk: mmc_switch() failed. err=%d\n",
> + __func__, err);
> +
> + mmc_sd_card_set_sanitize_completed(card);
> +
> +out:
> + spin_lock_irq(&md->lock);
> + __blk_end_request(req, err, blk_rq_bytes(req));
> + spin_unlock_irq(&md->lock);
> +
> + return err ? 0 : 1;
> +}
> +
> static int mmc_blk_issue_flush(struct mmc_queue *mq, struct request *req)
> {
> struct mmc_blk_data *md = mq->data;
> @@ -1379,7 +1406,12 @@ static int mmc_blk_issue_rq(struct mmc_queue *mq, struct request *req)
> goto out;
> }
>
> - if (req && req->cmd_flags & REQ_DISCARD) {
> + if (req && req->cmd_flags & REQ_SANITIZE) {
> + /* complete ongoing async transfer before issuing discard */
> + if (card->host && card->host->areq)
> + mmc_blk_issue_rw_rq(mq, NULL);
> + ret = mmc_blk_issue_sanitize_rq(mq, req);
> + } else if (req && req->cmd_flags & REQ_DISCARD) {
> /* complete ongoing async transfer before issuing discard */
> if (card->host->areq)
> mmc_blk_issue_rw_rq(mq, NULL);
> diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c
> index dcad59c..642a80d 100644
> --- a/drivers/mmc/card/queue.c
> +++ b/drivers/mmc/card/queue.c
> @@ -140,7 +140,7 @@ static void mmc_queue_setup_discard(struct request_queue *q,
> /* granularity must not be greater than max. discard */
> if (card->pref_erase > max_discard)
> q->limits.discard_granularity = 0;
> - if (mmc_can_secure_erase_trim(card) || mmc_can_sanitize(card))
> + if (mmc_can_secure_erase_trim(card))
> queue_flag_set_unlocked(QUEUE_FLAG_SECDISCARD, q);
Please add the QUEUE_FLAG_SANITIZE at here as other.
> }
>
> @@ -181,6 +181,11 @@ int mmc_init_queue(struct mmc_queue *mq, struct mmc_card *card,
> if (mmc_can_erase(card))
> mmc_queue_setup_discard(mq->queue, card);
>
> + if (mmc_can_sanitize(card))
> + queue_flag_set_unlocked(QUEUE_FLAG_SANITIZE, mq->queue);
> + else
> + pr_info("%s - card doesn't support sanitize", __func__);
Please move to it as above
> +
> #ifdef CONFIG_MMC_BLOCK_BOUNCE
> if (host->max_segs == 1) {
> unsigned int bouncesz;
> diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
> index 4053cbd..826c724 100644
> --- a/include/linux/blk_types.h
> +++ b/include/linux/blk_types.h
> @@ -150,6 +150,7 @@ enum rq_flag_bits {
> __REQ_FLUSH_SEQ, /* request for flush sequence */
> __REQ_IO_STAT, /* account I/O stat */
> __REQ_MIXED_MERGE, /* merge of different types, fail separately */
> + __REQ_SANITIZE, /* sanitize */
> __REQ_NR_BITS, /* stops here */
> };
>
> @@ -161,13 +162,15 @@ enum rq_flag_bits {
> #define REQ_META (1 << __REQ_META)
> #define REQ_PRIO (1 << __REQ_PRIO)
> #define REQ_DISCARD (1 << __REQ_DISCARD)
> +#define REQ_SANITIZE (1 << __REQ_SANITIZE)
> #define REQ_NOIDLE (1 << __REQ_NOIDLE)
>
> #define REQ_FAILFAST_MASK \
> (REQ_FAILFAST_DEV | REQ_FAILFAST_TRANSPORT | REQ_FAILFAST_DRIVER)
> #define REQ_COMMON_MASK \
> (REQ_WRITE | REQ_FAILFAST_MASK | REQ_SYNC | REQ_META | REQ_PRIO | \
> - REQ_DISCARD | REQ_NOIDLE | REQ_FLUSH | REQ_FUA | REQ_SECURE)
> + REQ_DISCARD | REQ_NOIDLE | REQ_FLUSH | REQ_FUA | REQ_SECURE | \
> + REQ_SANITIZE)
> #define REQ_CLONE_MASK REQ_COMMON_MASK
>
> #define REQ_RAHEAD (1 << __REQ_RAHEAD)
> @@ -192,4 +195,4 @@ enum rq_flag_bits {
> #define REQ_MIXED_MERGE (1 << __REQ_MIXED_MERGE)
> #define REQ_SECURE (1 << __REQ_SECURE)
>
> -#endif /* __LINUX_BLK_TYPES_H */
> +#endif /* __LINUX_REQ__TYPES_H */
Why do you change this one?
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index 7084318..1197c31 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -423,6 +423,7 @@ struct request_queue {
> #define QUEUE_FLAG_ADD_RANDOM 16 /* Contributes to random pool */
> #define QUEUE_FLAG_SECDISCARD 17 /* supports SECDISCARD */
> #define QUEUE_FLAG_SAME_FORCE 18 /* force complete on same CPU */
> +#define QUEUE_FLAG_SANITIZE 19 /* supports SANITIZE */
>
> #define QUEUE_FLAG_DEFAULT ((1 << QUEUE_FLAG_IO_STAT) | \
> (1 << QUEUE_FLAG_STACKABLE) | \
> @@ -506,6 +507,7 @@ static inline void queue_flag_clear(unsigned int flag, struct request_queue *q)
> #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_sanitize(q) test_bit(QUEUE_FLAG_SANITIZE, &(q)->queue_flags)
> #define blk_queue_secdiscard(q) (blk_queue_discard(q) && \
> test_bit(QUEUE_FLAG_SECDISCARD, &(q)->queue_flags))
>
> @@ -955,6 +957,7 @@ static inline struct request *blk_map_queue_find_tag(struct blk_queue_tag *bqt,
> extern int blkdev_issue_flush(struct block_device *, gfp_t, sector_t *);
> extern int blkdev_issue_discard(struct block_device *bdev, sector_t sector,
> sector_t nr_sects, gfp_t gfp_mask, unsigned long flags);
> +extern int blkdev_issue_sanitize(struct block_device *bdev, gfp_t gfp_mask);
> extern int blkdev_issue_zeroout(struct block_device *bdev, sector_t sector,
> sector_t nr_sects, gfp_t gfp_mask);
> static inline int sb_issue_discard(struct super_block *sb, sector_t block,
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 057434a..3680eef 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -319,6 +319,7 @@ struct inodes_stat_t {
> #define BLKPBSZGET _IO(0x12,123)
> #define BLKDISCARDZEROES _IO(0x12,124)
> #define BLKSECDISCARD _IO(0x12,125)
> +#define BLKSANITIZE _IO(0x12, 126)
>
> #define BMAP_IOCTL 1 /* obsolete - kept for compatibility */
> #define FIBMAP _IO(0x00,1) /* bmap access */
> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
> index 9478a6b..0890e9c 100644
> --- a/include/linux/mmc/card.h
> +++ b/include/linux/mmc/card.h
> @@ -236,6 +236,9 @@ struct mmc_card {
> #define MMC_POWEROFF_SHORT 2
> #define MMC_POWEROFF_LONG 3
>
> +/* card is in sanitize progress */
> +#define MMC_STATE_SANITIZE_IN_PROGRESS (1<<9)
> +
> unsigned int erase_size; /* erase size in sectors */
> unsigned int erase_shift; /* if erase unit is power 2 */
> unsigned int pref_erase; /* in sectors */
> @@ -388,6 +391,10 @@ static inline void __maybe_unused remove_quirk(struct mmc_card *card, int data)
> #define mmc_card_set_ddr_mode(c) ((c)->state |= MMC_STATE_HIGHSPEED_DDR)
> #define mmc_card_set_uhs(c) ((c)->state |= MMC_STATE_ULTRAHIGHSPEED)
> #define mmc_sd_card_set_uhs(c) ((c)->state |= MMC_STATE_ULTRAHIGHSPEED)
> +#define mmc_sd_card_set_sanitize_in_progress(c) ((c)->state |= \
> + MMC_STATE_SANITIZE_IN_PROGRESS)
> +#define mmc_sd_card_set_sanitize_completed(c) ((c)->state &= \
> + ~MMC_STATE_SANITIZE_IN_PROGRESS)
> #define mmc_card_set_ext_capacity(c) ((c)->state |= MMC_CARD_SDXC)
> #define mmc_card_set_removed(c) ((c)->state |= MMC_CARD_REMOVED)
>
> diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c
> index 16fc34a..2c2efc6 100644
> --- a/kernel/trace/blktrace.c
> +++ b/kernel/trace/blktrace.c
> @@ -1802,6 +1802,8 @@ void blk_fill_rwbs(char *rwbs, u32 rw, int bytes)
> rwbs[i++] = 'W';
> else if (rw & REQ_DISCARD)
> rwbs[i++] = 'D';
> + else if (rw & REQ_SANITIZE)
> + rwbs[i++] = 'Z';
> else if (bytes)
> rwbs[i++] = 'R';
> else
> --
> 1.7.6
> --
> Sent by a consultant of the Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/