nvme: Do not test the read-ahead flag in cmd_flags

RFC PATCH New Bart Van Assche 3 participants 5 messages Sep 14, 2016 NVMe

Share & tools

Bart Van Assche

author

2016-09-14 10:02 UTC #0 lore raw

RFC PATCH 1/1 nvme: Do not test the read-ahead flag in cmd_flags

REQ_RAHEAD is a bio-only flag (bio.bi_opf) and hence is never set in request.cmd_flags. Hence remove the cmd_flags & REQ_RAHEAD tests. Compile-tested only.

Signed-off-by Bart Van Assche <bart.vanassche at sandisk.com>

Cc Jens Axboe <axboe at fb.com>

Cc Christoph Hellwig <hch at lst.de>

Cc Ming Lin <ming.l at ssi.samsung.com>

---
 drivers/nvme/host/core.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

drivers/nvme/host/core.c +1 -4

index 2feacc7..c920b5f 100644
@@ -285,12 +285,9 @@ static inline void nvme_setup_rw(struct nvme_ns *ns, struct request *req,
 
 	if (req->cmd_flags & REQ_FUA)
 		control |= NVME_RW_FUA;
-	if (req->cmd_flags & (REQ_FAILFAST_DEV | REQ_RAHEAD))
+	if (req->cmd_flags & REQ_FAILFAST_DEV)
 		control |= NVME_RW_LR;
 
-	if (req->cmd_flags & REQ_RAHEAD)
-		dsmgmt |= NVME_RW_DSM_FREQ_PREFETCH;
-
 	memset(cmnd, 0, sizeof(*cmnd));
 	cmnd->rw.opcode = (rq_data_dir(req) ? nvme_cmd_write : nvme_cmd_read);
 	cmnd->rw.command_id = req->tag;

Signature · 1 line

2.10.0

Christoph Hellwig

maintainer

2016-09-14 14:23 UTC #1 lore raw

RFC PATCH 1/1 nvme: Do not test the read-ahead flag in cmd_flags

Quoted 3 lines · Bart Van Assche

REQ_RAHEAD is a bio-only flag (bio.bi_opf) and hence is never set in request.cmd_flags. Hence remove the cmd_flags & REQ_RAHEAD tests. Compile-tested only.

It's indeed a bug, but the questions is if why we shouldn't pass REQ_RAHEAD down to the request layer. It's a useful attribute, and NVMe devices could make use of it.

Jens Axboe

2016-09-14 14:43 UTC #2 lore raw

RFC PATCH 1/1 nvme: Do not test the read-ahead flag in cmd_flags

Quoted 8 lines · Christoph Hellwig

Quoted 3 lines · Bart Van Assche
REQ_RAHEAD is a bio-only flag (bio.bi_opf) and hence is never set in request.cmd_flags. Hence remove the cmd_flags & REQ_RAHEAD tests. Compile-tested only.
It's indeed a bug, but the questions is if why we shouldn't pass REQ_RAHEAD down to the request layer. It's a useful attribute, and NVMe devices could make use of it.

Indeed, seems a shame to lose this hint.

Signature · 1 line

Jens Axboe

Christoph Hellwig

maintainer

2016-09-14 14:47 UTC #3 lore raw

RFC PATCH 1/1 nvme: Do not test the read-ahead flag in cmd_flags

Quoted 5 lines · Jens Axboe

Quoted 3 lines
It's indeed a bug, but the questions is if why we shouldn't pass REQ_RAHEAD down to the request layer. It's a useful attribute, and NVMe devices could make use of it.
Indeed, seems a shame to lose this hint.

It currently doesn't work. But it would be useful to actually pass it through. That being said we currently have a complete mess with the common vs request only flags, both about their placement in the enum and inclusion in the common mask. Making it worse it seems like REQ_NOMERGE actually is used in both currently and not actually expecting propagation.

Jens Axboe

2016-09-14 14:54 UTC #4 lore raw

RFC PATCH 1/1 nvme: Do not test the read-ahead flag in cmd_flags

Quoted 8 lines · Christoph Hellwig

Quoted 5 lines · Jens Axboe
Quoted 3 lines
It's indeed a bug, but the questions is if why we shouldn't pass REQ_RAHEAD down to the request layer. It's a useful attribute, and NVMe devices could make use of it.
Indeed, seems a shame to lose this hint.
It currently doesn't work. But it would be useful to actually pass

Right, hence we lose it when mapping from a bio to a request.

Quoted 5 lines

it through. That being said we currently have a complete mess with the common vs request only flags, both about their placement in the enum and inclusion in the common mask. Making it worse it seems like REQ_NOMERGE actually is used in both currently and not actually expecting propagation.

It would be nice to harden that.

Signature · 1 line

Jens Axboe