support ioctl for tunable user request

RFC PATCH In review Jaehoon Chung 5 participants 14 messages Aug 29, 2011 – Sep 9, 2011

Subsystems (2)

VFS Block layer

lore

Share & tools

lore

Jaehoon Chung

author

2011-08-29 08:15 UTC #0 lore raw

RFC PATCH v2 1/1 support ioctl for tunable user request

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 <[email protected]>

Signed-off-by Kyungmin Park <[email protected]>

---
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(-)

block/blk-core.c +1 -1

index 7c59b0f..6d54ca0 100644
@@ -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 {

block/blk-lib.c +38 -0

index 2b461b4..c9824b6 100644
@@ -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

block/ioctl.c +3 -0

index 1124cd2..ca580c4 100644
@@ -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];

include/linux/blk_types.h +3 -1

index 32f0076..7a1ada2 100644
@@ -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)

include/linux/blkdev.h +3 -0

index 84b15d5..dd43aa2 100644
@@ -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,

include/linux/fs.h +1 -0

index f23bcb7..b27ea55 100644
@@ -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 */

Lin Ming

2011-08-29 08:58 UTC #1 lore raw

Quoted 1 line · Jaehoon Chung

This patch is added the ioctl for tunable user request.

Hi,

What's the use case for this new ioctl?

Thanks,
Lin Ming

Jaehoon Chung

author

2011-08-29 10:11 UTC #2 lore raw

Quoted 7 lines · Lin Ming

Quoted 1 line · Jaehoon Chung
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

Kyungmin Park

2011-08-29 14:02 UTC #3 lore raw

Quoted 9 lines · Jaehoon Chung

Hi
Quoted 6 lines · Lin Ming
Quoted 1 line · Jaehoon Chung
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

Quoted 20 lines

Quoted 1 line
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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Lin Ming

2011-08-29 15:17 UTC #4 lore raw

Quoted 11 lines · Kyungmin Park

Quoted 9 lines · Jaehoon Chung
Hi
Quoted 6 lines · Lin Ming
Quoted 1 line · Jaehoon Chung
This patch is added the ioctl for tunable user request.
Hi,
What's the use case for this new ioctl?

Quoted 8 lines

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.

Quoted 3 lines

Thank you,
Kyungmin Park

Kyungmin Park

2011-09-05 08:49 UTC #5 lore raw

Hi Jens,

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

Thank you,
Kyungmin Park

Quoted 39 lines · Lin Ming

Quoted 11 lines · Kyungmin Park
Quoted 9 lines · Jaehoon Chung
Hi
Quoted 6 lines · Lin Ming
Quoted 1 line · Jaehoon Chung
This patch is added the ioctl for tunable user request.
Hi,
What's the use case for this new ioctl?
Quoted 8 lines
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.
Quoted 3 lines
Thank you,
Kyungmin Park
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Christoph Hellwig

2011-09-05 09:15 UTC #6 lore raw

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

Kyungmin Park

2011-09-05 11:52 UTC #7 lore raw

Hi Christoph,

Quoted 2 lines · Christoph Hellwig

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

Christoph Hellwig

2011-09-05 12:18 UTC #8 lore raw

Quoted 7 lines · Kyungmin Park

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.

Quoted 3 lines

Thank you,
Kyungmin Park

---end quoted text---

Kyungmin Park

2011-09-05 12:40 UTC #9 lore raw

Quoted 11 lines · Christoph Hellwig

Quoted 7 lines · Kyungmin Park
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.

Quoted 1 line

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

Arnd Bergmann

2011-09-05 15:04 UTC #10 lore raw

Quoted 10 lines · Kyungmin Park

Quoted 9 lines · Christoph Hellwig
Quoted 7 lines · Kyungmin Park
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.

Quoted 6 lines

Quoted 2 lines
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

Kyungmin Park

2011-09-06 04:24 UTC #11 lore raw

Quoted 18 lines · Arnd Bergmann

Quoted 10 lines · Kyungmin Park
Quoted 9 lines · Christoph Hellwig
Quoted 7 lines · Kyungmin Park
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.
Quoted 6 lines
Quoted 2 lines
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.

Quoted 5 lines

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

Arnd Bergmann

2011-09-09 01:40 UTC #12 lore raw

Quoted 8 lines · Kyungmin Park

Quoted 5 lines
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

Christoph Hellwig

2011-09-08 23:34 UTC #13 lore raw

Quoted 2 lines · Arnd Bergmann

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.