Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1362885 > unrolled thread

[PATCH 1/6] block: ensure we don't truncate top bits of the request command flags

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 1/6] block: ensure we don't truncate top bits of the request command flags Jens Axboe <axboe@fb.com> - 2016-03-22 19:00 +0100
    Re: [PATCH 1/6] block: ensure we don't truncate top bits of the  request command flags Christoph Hellwig <hch@infradead.org> - 2016-03-22 20:00 +0100
      Re: [PATCH 1/6] block: ensure we don't truncate top bits of the  request command flags Jens Axboe <axboe@fb.com> - 2016-03-22 20:10 +0100
        Re: [PATCH 1/6] block: ensure we don't truncate top bits of the  request command flags Mike Christie <mchristi@redhat.com> - 2016-03-25 03:10 +0100
          Re: [PATCH 1/6] block: ensure we don't truncate top bits of the  request command flags Jens Axboe <axboe@fb.com> - 2016-03-25 05:20 +0100

#1362885 — [PATCH 1/6] block: ensure we don't truncate top bits of the request command flags

Some of the flags that we want to use from the make_request_fn path
are now larger than 32-bit, so change the functions involved to
accept an u64 instead of an unsigned int.

Signed-off-by: Jens Axboe <axboe@fb.com>
---
 block/blk-core.c         | 7 ++++---
 block/cfq-iosched.c      | 2 +-
 block/elevator.c         | 6 +++---
 include/linux/blkdev.h   | 2 +-
 include/linux/elevator.h | 4 ++--
 5 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 827f8badd143..a9fe3d88af99 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1065,7 +1065,7 @@ static struct io_context *rq_ioc(struct bio *bio)
  * Returns ERR_PTR on failure, with @q->queue_lock held.
  * Returns request pointer on success, with @q->queue_lock *not held*.
  */
-static struct request *__get_request(struct request_list *rl, int rw_flags,
+static struct request *__get_request(struct request_list *rl, u64 rw_flags,
 				     struct bio *bio, gfp_t gfp_mask)
 {
 	struct request_queue *q = rl->q;
@@ -1237,7 +1237,7 @@ rq_starved:
  * Returns ERR_PTR on failure, with @q->queue_lock held.
  * Returns request pointer on success, with @q->queue_lock *not held*.
  */
-static struct request *get_request(struct request_queue *q, int rw_flags,
+static struct request *get_request(struct request_queue *q, u64 rw_flags,
 				   struct bio *bio, gfp_t gfp_mask)
 {
 	const bool is_sync = rw_is_sync(rw_flags) != 0;
@@ -1711,9 +1711,10 @@ static blk_qc_t blk_queue_bio(struct request_queue *q, struct bio *bio)
 {
 	const bool sync = !!(bio->bi_rw & REQ_SYNC);
 	struct blk_plug *plug;
-	int el_ret, rw_flags, where = ELEVATOR_INSERT_SORT;
+	int el_ret, where = ELEVATOR_INSERT_SORT;
 	struct request *req;
 	unsigned int request_count = 0;
+	u64 rw_flags;
 
 	/*
 	 * low level driver can indicate that it wants pages above a
diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
index e3c591dd8f19..3e3b47b6a72f 100644
--- a/block/cfq-iosched.c
+++ b/block/cfq-iosched.c
@@ -4285,7 +4285,7 @@ static inline int __cfq_may_queue(struct cfq_queue *cfqq)
 	return ELV_MQUEUE_MAY;
 }
 
-static int cfq_may_queue(struct request_queue *q, int rw)
+static int cfq_may_queue(struct request_queue *q, u64 rw)
 {
 	struct cfq_data *cfqd = q->elevator->elevator_data;
 	struct task_struct *tsk = current;
diff --git a/block/elevator.c b/block/elevator.c
index c3555c9c672f..5b9a615fd2df 100644
--- a/block/elevator.c
+++ b/block/elevator.c
@@ -352,7 +352,7 @@ void elv_dispatch_sort(struct request_queue *q, struct request *rq)
 {
 	sector_t boundary;
 	struct list_head *entry;
-	int stop_flags;
+	u64 stop_flags;
 
 	if (q->last_merge == rq)
 		q->last_merge = NULL;
@@ -511,7 +511,7 @@ void elv_merge_requests(struct request_queue *q, struct request *rq,
 			     struct request *next)
 {
 	struct elevator_queue *e = q->elevator;
-	const int next_sorted = next->cmd_flags & REQ_SORTED;
+	const bool next_sorted = (next->cmd_flags & REQ_SORTED) != 0;
 
 	if (next_sorted && e->type->ops.elevator_merge_req_fn)
 		e->type->ops.elevator_merge_req_fn(q, rq, next);
@@ -717,7 +717,7 @@ void elv_put_request(struct request_queue *q, struct request *rq)
 		e->type->ops.elevator_put_req_fn(rq);
 }
 
-int elv_may_queue(struct request_queue *q, int rw)
+int elv_may_queue(struct request_queue *q, u64 rw)
 {
 	struct elevator_queue *e = q->elevator;
 
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 7e5d7e018bea..930bd4c5b7ff 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -615,7 +615,7 @@ static inline unsigned int blk_queue_cluster(struct request_queue *q)
 /*
  * We regard a request as sync, if either a read or a sync write
  */
-static inline bool rw_is_sync(unsigned int rw_flags)
+static inline bool rw_is_sync(u64 rw_flags)
 {
 	return !(rw_flags & REQ_WRITE) || (rw_flags & REQ_SYNC);
 }
diff --git a/include/linux/elevator.h b/include/linux/elevator.h
index 638b324f0291..a06cca4d0f1a 100644
--- a/include/linux/elevator.h
+++ b/include/linux/elevator.h
@@ -26,7 +26,7 @@ typedef int (elevator_dispatch_fn) (struct request_queue *, int);
 typedef void (elevator_add_req_fn) (struct request_queue *, struct request *);
 typedef struct request *(elevator_request_list_fn) (struct request_queue *, struct request *);
 typedef void (elevator_completed_req_fn) (struct request_queue *, struct request *);
-typedef int (elevator_may_queue_fn) (struct request_queue *, int);
+typedef int (elevator_may_queue_fn) (struct request_queue *, u64);
 
 typedef void (elevator_init_icq_fn) (struct io_cq *);
 typedef void (elevator_exit_icq_fn) (struct io_cq *);
@@ -134,7 +134,7 @@ extern struct request *elv_former_request(struct request_queue *, struct request
 extern struct request *elv_latter_request(struct request_queue *, struct request *);
 extern int elv_register_queue(struct request_queue *q);
 extern void elv_unregister_queue(struct request_queue *q);
-extern int elv_may_queue(struct request_queue *, int);
+extern int elv_may_queue(struct request_queue *, u64);
 extern void elv_completed_request(struct request_queue *, struct request *);
 extern int elv_set_request(struct request_queue *q, struct request *rq,
 			   struct bio *bio, gfp_t gfp_mask);
-- 
2.4.1.168.g1ea28e1

[toc] | [next] | [standalone]

#1362914 — Re: [PATCH 1/6] block: ensure we don't truncate top bits of the request command flags

On Tue, Mar 22, 2016 at 11:55:15AM -0600, Jens Axboe wrote:
> Some of the flags that we want to use from the make_request_fn path
> are now larger than 32-bit, so change the functions involved to
> accept an u64 instead of an unsigned int.

When did we start doing that?  We really should merge Mike's split
of the operation style flags into the cmd_type before making things
even worse in the flags area.

[toc] | [prev] | [next] | [standalone]

#1362920 — Re: [PATCH 1/6] block: ensure we don't truncate top bits of the request command flags

On 03/22/2016 12:59 PM, Christoph Hellwig wrote:
> On Tue, Mar 22, 2016 at 11:55:15AM -0600, Jens Axboe wrote:
>> Some of the flags that we want to use from the make_request_fn path
>> are now larger than 32-bit, so change the functions involved to
>> accept an u64 instead of an unsigned int.
>
> When did we start doing that?  We really should merge Mike's split
> of the operation style flags into the cmd_type before making things
> even worse in the flags area.

Just now, and I ran into it last week as well, for a test patch on cfq 
that passed in higher flags for get_request -> may_queue() as well. We 
can do Mike's split first, I think it's a good cleanup. As a standalone 
series, I needed it though.

-- 
Jens Axboe

[toc] | [prev] | [next] | [standalone]

#1364519 — Re: [PATCH 1/6] block: ensure we don't truncate top bits of the request command flags

On 03/22/2016 02:01 PM, Jens Axboe wrote:
> On 03/22/2016 12:59 PM, Christoph Hellwig wrote:
>> On Tue, Mar 22, 2016 at 11:55:15AM -0600, Jens Axboe wrote:
>>> Some of the flags that we want to use from the make_request_fn path
>>> are now larger than 32-bit, so change the functions involved to
>>> accept an u64 instead of an unsigned int.
>>
>> When did we start doing that?  We really should merge Mike's split
>> of the operation style flags into the cmd_type before making things
>> even worse in the flags area.
> 
> Just now, and I ran into it last week as well, for a test patch on cfq
> that passed in higher flags for get_request -> may_queue() as well. We
> can do Mike's split first, I think it's a good cleanup. As a standalone
> series, I needed it though.
> 

Hey, did you want any changes on that patchset? I was going to repost it
with the kbuild fix against linux-next, but I can make any changes you
wanted first.

[toc] | [prev] | [next] | [standalone]

#1364557 — Re: [PATCH 1/6] block: ensure we don't truncate top bits of the request command flags

On 03/24/2016 08:08 PM, Mike Christie wrote:
> On 03/22/2016 02:01 PM, Jens Axboe wrote:
>> On 03/22/2016 12:59 PM, Christoph Hellwig wrote:
>>> On Tue, Mar 22, 2016 at 11:55:15AM -0600, Jens Axboe wrote:
>>>> Some of the flags that we want to use from the make_request_fn path
>>>> are now larger than 32-bit, so change the functions involved to
>>>> accept an u64 instead of an unsigned int.
>>>
>>> When did we start doing that?  We really should merge Mike's split
>>> of the operation style flags into the cmd_type before making things
>>> even worse in the flags area.
>>
>> Just now, and I ran into it last week as well, for a test patch on cfq
>> that passed in higher flags for get_request -> may_queue() as well. We
>> can do Mike's split first, I think it's a good cleanup. As a standalone
>> series, I needed it though.
>>
>
> Hey, did you want any changes on that patchset? I was going to repost it
> with the kbuild fix against linux-next, but I can make any changes you
> wanted first.

I don't believe I've ever been CC'ed on the posting, or it even being 
posted on the block list? If so, I don't see it... I did become aware of 
it since Christoph CC'ed me in. In general, I think it looks good, at 
least the end results. It's a bit murky in the middle, and the commit 
messages need some help. So go over everything, sanitize it, and repost 
it. I don't like the current pure flag based scheme we have, it's a mess 
of ops and modifiers. So splitting that up is definitely a good thing.

-- 
Jens Axboe

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web