[RFC PATCH v2] support ioctl for tunable user request

[RFC PATCH v2] support ioctl for tunable user request

[Jaehoon Chung]

This patch is added the ioctl for tunable user request.

First, We defined TUNE request (REQ_TUNE). TUNE request means tunable request.
(For example, TUNE request is background request.)

That request should be defined by device.
User has only to do TUNE-request trigger. Then device should be run its specific request.

In other words, user notify the request to device. then user's responsibility is done.
(i think quite similar to DISCARD request)

I want to get mailing's review.

Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
Change-log v2:
- removed wait completion code.(This request has only to do trigger interrupt)
- based-on for-next tree in linux-block git repository
---
 block/blk-core.c          |    2 +-
 block/blk-lib.c           |   38 ++++++++++++++++++++++++++++++++++++++
 block/ioctl.c             |    3 +++
 include/linux/blk_types.h |    4 +++-
 include/linux/blkdev.h    |    3 +++
 include/linux/fs.h        |    1 +
 6 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 7c59b0f..6d54ca0 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1621,7 +1621,7 @@ 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) || (rw & REQ_TUNE))) {
 		if (rw & WRITE) {
 			count_vm_events(PGPGOUT, count);
 		} else {
diff --git a/block/blk-lib.c b/block/blk-lib.c
index 2b461b4..c9824b6 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -27,6 +27,44 @@ static void bio_batch_end_io(struct bio *bio, int err)
 }
 
 /**
+ * blkdev_issue_tune - queue a tune request
+ * @bdev:	blockdev to issue discard for
+ * @gfp_mask:	memory allocation flags (for bio_alloc)
+ *
+ * Description:
+ *    Issue a tunable request from user.
+ */
+int blkdev_issue_tune(struct block_device *bdev, gfp_t gfp_mask)
+{
+	struct request_queue *q = bdev_get_queue(bdev);
+	struct bio *bio;
+	struct bio_batch bb;
+	int type = REQ_TUNE;
+
+	if (!blk_queue_tune(q))
+		return -ENXIO;
+
+	bio = bio_alloc(gfp_mask, 1);
+	if (bio == NULL)
+		return -ENOMEM;
+
+	/*
+	 * TUNE request didn't wait for completion.
+	 * Because this request didn't include any data
+	 * (Just triger request.)
+	 */
+	atomic_set(&bb.done, 0);
+	bio->bi_end_io = bio_batch_end_io;
+	bio->bi_bdev = bdev;
+	bio->bi_private = &bb;
+
+	submit_bio(type, bio);
+
+	return 0;
+}
+EXPORT_SYMBOL(blkdev_issue_tune);
+
+/**
  * blkdev_issue_discard - queue a discard
  * @bdev:	blockdev to issue discard for
  * @sector:	start sector
diff --git a/block/ioctl.c b/block/ioctl.c
index 1124cd2..ca580c4 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -215,6 +215,9 @@ int blkdev_ioctl(struct block_device *bdev, fmode_t mode, unsigned cmd,
 		set_device_ro(bdev, n);
 		return 0;
 
+	case BLKTUNE:
+		return blkdev_issue_tune(bdev, GFP_KERNEL);
+
 	case BLKDISCARD:
 	case BLKSECDISCARD: {
 		uint64_t range[2];
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index 32f0076..7a1ada2 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -126,6 +126,7 @@ enum rq_flag_bits {
 	__REQ_META,		/* metadata io request */
 	__REQ_DISCARD,		/* request to discard sectors */
 	__REQ_SECURE,		/* secure discard (used with __REQ_DISCARD) */
+	__REQ_TUNE,		/* tunable request */
 
 	__REQ_NOIDLE,		/* don't anticipate more IO after this one */
 	__REQ_FUA,		/* forced unit access */
@@ -162,13 +163,14 @@ enum rq_flag_bits {
 #define REQ_SYNC		(1 << __REQ_SYNC)
 #define REQ_META		(1 << __REQ_META)
 #define REQ_DISCARD		(1 << __REQ_DISCARD)
+#define REQ_TUNE		(1 << __REQ_TUNE)
 #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_DISCARD | \
-	 REQ_NOIDLE | REQ_FLUSH | REQ_FUA | REQ_SECURE)
+	 REQ_NOIDLE | REQ_FLUSH | REQ_FUA | REQ_SECURE | REQ_TUNE)
 #define REQ_CLONE_MASK		REQ_COMMON_MASK
 
 #define REQ_RAHEAD		(1 << __REQ_RAHEAD)
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 84b15d5..dd43aa2 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -408,6 +408,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_TUNE        19	/* support tunable request */
 
 #define QUEUE_FLAG_DEFAULT	((1 << QUEUE_FLAG_IO_STAT) |		\
 				 (1 << QUEUE_FLAG_STACKABLE)	|	\
@@ -492,6 +493,7 @@ static inline void queue_flag_clear(unsigned int flag, struct request_queue *q)
 #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_queue_tune(q)	test_bit(QUEUE_FLAG_TUNE, &(q)->queue_flags)
 
 #define blk_noretry_request(rq) \
 	((rq)->cmd_flags & (REQ_FAILFAST_DEV|REQ_FAILFAST_TRANSPORT| \
@@ -934,6 +936,7 @@ static inline struct request *blk_map_queue_find_tag(struct blk_queue_tag *bqt,
 #define BLKDEV_DISCARD_SECURE  0x01    /* secure discard */
 
 extern int blkdev_issue_flush(struct block_device *, gfp_t, sector_t *);
+extern int blkdev_issue_tune(struct block_device *bdev, gfp_t gfp_mask);
 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_zeroout(struct block_device *bdev, sector_t sector,
diff --git a/include/linux/fs.h b/include/linux/fs.h
index f23bcb7..b27ea55 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -320,6 +320,7 @@ struct inodes_stat_t {
 #define BLKPBSZGET _IO(0x12,123)
 #define BLKDISCARDZEROES _IO(0x12,124)
 #define BLKSECDISCARD _IO(0x12,125)
+#define BLKTUNE _IO(0x12,126)
 
 #define BMAP_IOCTL 1		/* obsolete - kept for compatibility */
 #define FIBMAP	   _IO(0x00,1)	/* bmap access */

Re: [RFC PATCH v2] support ioctl for tunable user request

[Lin Ming]

On Mon, Aug 29, 2011 at 4:15 PM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> This patch is added the ioctl for tunable user request.

Hi,

What's the use case for this new ioctl?

Thanks,
Lin Ming

Re: [RFC PATCH v2] support ioctl for tunable user request

[Jaehoon Chung]

Hi

Lin Ming wrote:
> On Mon, Aug 29, 2011 at 4:15 PM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>> This patch is added the ioctl for tunable user request.
> 
> Hi,
> 
> What's the use case for this new ioctl?
> 
In my case, this ioctl is used the background operation for eMMC.
In eMMC's case, background operation(BKOPS) is supported.
(but that is running with only card notifier, so i added new ioctl for user request)

if we send the BKOPS request, card is notified to need the BKOPS. 
then running background operation. (Then user's responsibility is done)

In future, i think that other block device should be used this ioctl.
(if supported like BKOPS)

Regard,
Jaehoon Chung

Re: [RFC PATCH v2] support ioctl for tunable user request

[Kyungmin Park]

On Mon, Aug 29, 2011 at 7:11 PM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> Hi
>
> Lin Ming wrote:
>> On Mon, Aug 29, 2011 at 4:15 PM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>>> This patch is added the ioctl for tunable user request.
>>
>> Hi,
>>
>> What's the use case for this new ioctl?

The below is the just mmc example.

The main reason is that there's no way to know system or I/O idle, so
user provides the hint. Using user based I/O, device can TUNE the
device itself. In case of MMC, background operation can be used.

and another difference from other user request, it's just trigger the
device tune request, no need to wait the request is done.

Thank you,
Kyungmin Park
>>
> In my case, this ioctl is used the background operation for eMMC.
> In eMMC's case, background operation(BKOPS) is supported.
> (but that is running with only card notifier, so i added new ioctl for user request)
>
> if we send the BKOPS request, card is notified to need the BKOPS.
> then running background operation. (Then user's responsibility is done)
>
> In future, i think that other block device should be used this ioctl.
> (if supported like BKOPS)
>
> Regard,
> Jaehoon Chung
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@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: [RFC PATCH v2] support ioctl for tunable user request

[Lin Ming]

On Mon, Aug 29, 2011 at 10:01 PM, Kyungmin Park <kmpark@infradead.org> wrote:
> On Mon, Aug 29, 2011 at 7:11 PM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>> Hi
>>
>> Lin Ming wrote:
>>> On Mon, Aug 29, 2011 at 4:15 PM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>>>> This patch is added the ioctl for tunable user request.
>>>
>>> Hi,
>>>
>>> What's the use case for this new ioctl?
>
> The below is the just mmc example.
>
> The main reason is that there's no way to know system or I/O idle, so
> user provides the hint. Using user based I/O, device can TUNE the
> device itself. In case of MMC, background operation can be used.
>
> and another difference from other user request, it's just trigger the
> device tune request, no need to wait the request is done.

OK.

After read the previous patch at:
http://marc.info/?l=linux-mmc&m=131407402925988&w=2

I now get better understanding what "tunable user request" mean.

Thanks.

>
> Thank you,
> Kyungmin Park

Re: [RFC PATCH v2] support ioctl for tunable user request

[Kyungmin Park]

Hi Jens,

Any comments on tunable user request concept for flash based storage?

Thank you,
Kyungmin Park

On Tue, Aug 30, 2011 at 12:17 AM, Lin Ming <lxy@ss.pku.edu.cn> wrote:
> On Mon, Aug 29, 2011 at 10:01 PM, Kyungmin Park <kmpark@infradead.org> wrote:
>> On Mon, Aug 29, 2011 at 7:11 PM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>>> Hi
>>>
>>> Lin Ming wrote:
>>>> On Mon, Aug 29, 2011 at 4:15 PM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>>>>> This patch is added the ioctl for tunable user request.
>>>>
>>>> Hi,
>>>>
>>>> What's the use case for this new ioctl?
>>
>> The below is the just mmc example.
>>
>> The main reason is that there's no way to know system or I/O idle, so
>> user provides the hint. Using user based I/O, device can TUNE the
>> device itself. In case of MMC, background operation can be used.
>>
>> and another difference from other user request, it's just trigger the
>> device tune request, no need to wait the request is done.
>
> OK.
>
> After read the previous patch at:
> http://marc.info/?l=linux-mmc&m=131407402925988&w=2
>
> I now get better understanding what "tunable user request" mean.
>
> Thanks.
>
>>
>> Thank you,
>> Kyungmin Park
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@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: [RFC PATCH v2] support ioctl for tunable user request

[Christoph Hellwig]

So what the heck is a tunable request supposed to be?  How does it
qualify for beeing a generic request/bio type?

Re: [RFC PATCH v2] support ioctl for tunable user request

[Kyungmin Park]

Hi Christoph,

On Mon, Sep 5, 2011 at 6:15 PM, Christoph Hellwig <hch@infradead.org> wrote:
> So what the heck is a tunable request supposed to be?  How does it
> qualify for beeing a generic request/bio type?

The concept is similar with DISCARD request except no-wait for request complete

 #define REQ_DISCARD            (1 << __REQ_DISCARD)
+#define REQ_TUNE               (1 << __REQ_TUNE)

As it's hard to detect the idle time at device level, user send the
tune request to device when idle.

Thank you,
Kyungmin Park

Re: [RFC PATCH v2] support ioctl for tunable user request

[Christoph Hellwig]

On Mon, Sep 05, 2011 at 08:47:58PM +0900, Kyungmin Park wrote:
> The concept is similar with DISCARD request except no-wait for request complete
> 
>  #define REQ_DISCARD            (1 << __REQ_DISCARD)
> +#define REQ_TUNE               (1 << __REQ_TUNE)
> 
> As it's hard to detect the idle time at device level, user send the
> tune request to device when idle.

But what does a "tune" request actually do?  It's an overly generic
term, but I still can't even think of what it would do to a storage
device, nor why it would be a generic block layer concept.

> 
> Thank you,
> Kyungmin Park
---end quoted text---

Re: [RFC PATCH v2] support ioctl for tunable user request

[Kyungmin Park]

On Mon, Sep 5, 2011 at 9:18 PM, Christoph Hellwig <hch@infradead.org> wrote:
> On Mon, Sep 05, 2011 at 08:47:58PM +0900, Kyungmin Park wrote:
>> The concept is similar with DISCARD request except no-wait for request complete
>>
>>  #define REQ_DISCARD            (1 << __REQ_DISCARD)
>> +#define REQ_TUNE               (1 << __REQ_TUNE)
>>
>> As it's hard to detect the idle time at device level, user send the
>> tune request to device when idle.
>
> But what does a "tune" request actually do?  It's an overly generic
> term, but I still can't even think of what it would do to a storage

Yes, tune is generic term, that's reason to send the RFC patch, we
consider the several names, but not found the proper name, if you
suggest the proper name, it can change the proper one.

and you can find the prototype at below URL
http://marc.info/?l=linux-mmc&m=131407402925988&w=2

In case of MMC, it supports the background operation (BKOPS) and these
features can be extended at Universal Flash Storage (UFS) later.

> device, nor why it would be a generic block layer concept.
Do you mean the mmc specific ioctl is proper or other place? in case
of ioctl, it's considered but hope to make it the generic request to
use other storage devices.

Thank you,
Kyungmin Park

Re: [RFC PATCH v2] support ioctl for tunable user request

[Arnd Bergmann]

On Monday 05 September 2011, Kyungmin Park wrote:
> On Mon, Sep 5, 2011 at 9:18 PM, Christoph Hellwig <hch@infradead.org> wrote:
> > On Mon, Sep 05, 2011 at 08:47:58PM +0900, Kyungmin Park wrote:
> >> The concept is similar with DISCARD request except no-wait for request complete
> >>
> >>  #define REQ_DISCARD            (1 << __REQ_DISCARD)
> >> +#define REQ_TUNE               (1 << __REQ_TUNE)
> >>
> >> As it's hard to detect the idle time at device level, user send the
> >> tune request to device when idle.
> >
> > But what does a "tune" request actually do?  It's an overly generic
> > term, but I still can't even think of what it would do to a storage
> 
> Yes, tune is generic term, that's reason to send the RFC patch, we
> consider the several names, but not found the proper name, if you
> suggest the proper name, it can change the proper one.

Would __REQ_GC as a shortcut for garbage collection fit? Right now,
I also think TUNE is not at all describing what we expect the drive
to do, but it's hard to come up with a term that is generic enough
to cover similar concepts in other hardware while still describing
what the drive does.

	Arnd

Re: [RFC PATCH v2] support ioctl for tunable user request

[Kyungmin Park]

On Tue, Sep 6, 2011 at 12:04 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Monday 05 September 2011, Kyungmin Park wrote:
>> On Mon, Sep 5, 2011 at 9:18 PM, Christoph Hellwig <hch@infradead.org> wrote:
>> > On Mon, Sep 05, 2011 at 08:47:58PM +0900, Kyungmin Park wrote:
>> >> The concept is similar with DISCARD request except no-wait for request complete
>> >>
>> >>  #define REQ_DISCARD            (1 << __REQ_DISCARD)
>> >> +#define REQ_TUNE               (1 << __REQ_TUNE)
>> >>
>> >> As it's hard to detect the idle time at device level, user send the
>> >> tune request to device when idle.
>> >
>> > But what does a "tune" request actually do?  It's an overly generic
>> > term, but I still can't even think of what it would do to a storage
>>
>> Yes, tune is generic term, that's reason to send the RFC patch, we
>> consider the several names, but not found the proper name, if you
>> suggest the proper name, it can change the proper one.
>
> Would __REQ_GC as a shortcut for garbage collection fit? Right now,
> I also think TUNE is not at all describing what we expect the drive
> to do, but it's hard to come up with a term that is generic enough
> to cover similar concepts in other hardware while still describing
> what the drive does.
No problem to use the REQ_GC. BTW, does it acceptable to GC request? I
hope each devices can do own optimization if REQ_GC is requested, if
no need to these one, just ignore it at driver level.

Thank you,
Kyungmin Park

Re: [RFC PATCH v2] support ioctl for tunable user request

[Arnd Bergmann]

On Tuesday 06 September 2011, Kyungmin Park wrote:
> > Would __REQ_GC as a shortcut for garbage collection fit? Right now,
> > I also think TUNE is not at all describing what we expect the drive
> > to do, but it's hard to come up with a term that is generic enough
> > to cover similar concepts in other hardware while still describing
> > what the drive does.
> No problem to use the REQ_GC. BTW, does it acceptable to GC request? I
> hope each devices can do own optimization if REQ_GC is requested, if
> no need to these one, just ignore it at driver level.

I would suggest that you specify exactly what you mean with REQ_GC at the
point where it is defined, e.g.

/*
 * REQ_GC -- force garbage collection on the device 
 *
 * The drive is forced to perform a "garbage collection" on data that it
 * has recently written. This will keep the device busy within a short
 * time span (up to 60 seconds) during which its performance may be
 * significantly reduced. After the garbage collection has finished,
 * the device is expected to provide optimum write performance again.
 *
 * User applications that expect expect high continious write throughput
 * (such as video recording) should issue a REQ_GC before they start
 * recording. A system daemon may occasionally call this during times
 * of relative inactivity in order to improve overall performance.
 *
 * Examples for hardware that should support this include
 * - eMMC 4.6 (background operations)
 * - SD 3.01 (speed class recording)
 * - PCIe based SSD 
 * - Shingled Hard drives
 *
 * Drivers that does not require or support garbage collection will
 * silently ignore this request.
 */

I'm not sure if that's the definition you need, but I think it should
be at this level of detail.

	Arnd

Re: [RFC PATCH v2] support ioctl for tunable user request

[Christoph Hellwig]

On Thu, Sep 08, 2011 at 05:47:52PM +0200, Arnd Bergmann wrote:
> I would suggest that you specify exactly what you mean with REQ_GC at the
> point where it is defined, e.g.

Why would this be a request?  This seems like global action for the
device to me, and should have its own method.