* [PATCH] block: fix a type conversion error in __get_request()
@ 2016-07-04 13:03 Wei Fang
2016-07-05 17:33 ` Jens Axboe
0 siblings, 1 reply; 3+ messages in thread
From: Wei Fang @ 2016-07-04 13:03 UTC (permalink / raw)
To: axboe; +Cc: linux-block, Wei Fang
Theoretically, request only flags in enum rq_flag_bits can bigger
than 31 after we add some new flags in the future, so we can't
store REQ_IO_STAT in op_flags which is a int type in __get_request().
Actually, when REQ_IO_STAT become 31, the most-significant bit of
op_flags will be 1, and OR it to ->cmd_flags will cause the top 32
bits of ->cmd_flags become 1.
Fix it by using a u64-type object to store flags.
Signed-off-by: Wei Fang <fangwei1@huawei.com>
---
block/blk-core.c | 11 ++++++-----
1 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index c94c7ad..3860b7d 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1077,6 +1077,7 @@ static struct request *__get_request(struct request_list *rl, int op,
struct io_cq *icq = NULL;
const bool is_sync = rw_is_sync(op, op_flags) != 0;
int may_queue;
+ u64 cmd_flags = (u64)(unsigned int)op_flags;
if (unlikely(blk_queue_dying(q)))
return ERR_PTR(-ENODEV);
@@ -1125,7 +1126,7 @@ static struct request *__get_request(struct request_list *rl, int op,
/*
* Decide whether the new request will be managed by elevator. If
- * so, mark @op_flags and increment elvpriv. Non-zero elvpriv will
+ * so, mark @cmd_flags and increment elvpriv. Non-zero elvpriv will
* prevent the current elevator from being destroyed until the new
* request is freed. This guarantees icq's won't be destroyed and
* makes creating new ones safe.
@@ -1134,14 +1135,14 @@ static struct request *__get_request(struct request_list *rl, int op,
* it will be created after releasing queue_lock.
*/
if (blk_rq_should_init_elevator(bio) && !blk_queue_bypass(q)) {
- op_flags |= REQ_ELVPRIV;
+ cmd_flags |= REQ_ELVPRIV;
q->nr_rqs_elvpriv++;
if (et->icq_cache && ioc)
icq = ioc_lookup_icq(ioc, q);
}
if (blk_queue_io_stat(q))
- op_flags |= REQ_IO_STAT;
+ cmd_flags |= REQ_IO_STAT;
spin_unlock_irq(q->queue_lock);
/* allocate and init request */
@@ -1151,10 +1152,10 @@ static struct request *__get_request(struct request_list *rl, int op,
blk_rq_init(q, rq);
blk_rq_set_rl(rq, rl);
- req_set_op_attrs(rq, op, op_flags | REQ_ALLOCED);
+ req_set_op_attrs(rq, op, cmd_flags | REQ_ALLOCED);
/* init elvpriv */
- if (op_flags & REQ_ELVPRIV) {
+ if (cmd_flags & REQ_ELVPRIV) {
if (unlikely(et->icq_cache && !icq)) {
if (ioc)
icq = ioc_create_icq(ioc, q, gfp_mask);
--
1.7.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] block: fix a type conversion error in __get_request()
2016-07-04 13:03 [PATCH] block: fix a type conversion error in __get_request() Wei Fang
@ 2016-07-05 17:33 ` Jens Axboe
2016-07-06 2:05 ` Wei Fang
0 siblings, 1 reply; 3+ messages in thread
From: Jens Axboe @ 2016-07-05 17:33 UTC (permalink / raw)
To: Wei Fang; +Cc: linux-block
On 07/04/2016 07:03 AM, Wei Fang wrote:
> Theoretically, request only flags in enum rq_flag_bits can bigger
> than 31 after we add some new flags in the future, so we can't
> store REQ_IO_STAT in op_flags which is a int type in __get_request().
> Actually, when REQ_IO_STAT become 31, the most-significant bit of
> op_flags will be 1, and OR it to ->cmd_flags will cause the top 32
> bits of ->cmd_flags become 1.
>
> Fix it by using a u64-type object to store flags.
Why not change op_flags to a 64-bit type, if the flags are already
overflowing?
Either that, or we need a BUILD_BUG_ON() for the flags not being > 32 bit.
--
Jens Axboe
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] block: fix a type conversion error in __get_request()
2016-07-05 17:33 ` Jens Axboe
@ 2016-07-06 2:05 ` Wei Fang
0 siblings, 0 replies; 3+ messages in thread
From: Wei Fang @ 2016-07-06 2:05 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-block
Hi, Jens,
On 2016/7/6 1:33, Jens Axboe wrote:
> On 07/04/2016 07:03 AM, Wei Fang wrote:
>> Theoretically, request only flags in enum rq_flag_bits can bigger
>> than 31 after we add some new flags in the future, so we can't
>> store REQ_IO_STAT in op_flags which is a int type in __get_request().
>> Actually, when REQ_IO_STAT become 31, the most-significant bit of
>> op_flags will be 1, and OR it to ->cmd_flags will cause the top 32
>> bits of ->cmd_flags become 1.
>>
>> Fix it by using a u64-type object to store flags.
>
> Why not change op_flags to a 64-bit type, if the flags are already overflowing?
>
> Either that, or we need a BUILD_BUG_ON() for the flags not being > 32 bit.
>
op_flags passed into __get_request() is used to store bio flags which won't
be > 31 bit, so it can not be overflowing when it passed in. But request
only flags, e.g. REQ_IO_STAT, that op_flags try to store in __get_request()
may be > 31 bit in the future, so op_flags will be overflowing in that
case. (I already met that problem after I added some new common flags.)
Converting int to u64 can be problematic when the most-significant bit of
op_flags is 1 when it passed in (it may happen theoretically?), so I add
a explicit cast between op_flags and cmd_flags.
Thanks,
Wei
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-07-06 2:05 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-04 13:03 [PATCH] block: fix a type conversion error in __get_request() Wei Fang
2016-07-05 17:33 ` Jens Axboe
2016-07-06 2:05 ` Wei Fang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.