On 6/14/22 23:08, Christoph Hellwig wrote:

On Wed, Jun 15, 2022 at 08:31:21AM +0800, Ming Lei wrote:

quoted

quoted

-	struct blk_mq_hw_ctx *hctx = blk_mq_map_queue(q, 0, ctx);
+	struct blk_mq_hw_ctx *hctx = blk_mq_map_queue(q, REQ_OP_WRITE, ctx);

The change itself doesn't make a difference, since both results in choosing
HCTX_TYPE_DEFAULT, but passing REQ_OP_WRITE is a bit misleading.

Well, the argument is an operationm so we better pass in a correct
operation (at some point we should look into a __bitwise annotation
or similar to make it clean).  And as 0 is REQ_OP_READ, we will end
up with the HCTX_TYPE_READ hctx IFF someone configures read queues
and uses an sq only schedule.  Which is a completely stupid but
possible setup.

Hi Christoph,

I looked into adding a __bitwise annotation for request flags by 
introducing a new type that is called blk_mq_opf_t. Introducing such a 
type without modifying a lot of code seems difficult to me. This is what 
I ran into:
* If the type of the operation type constants (REQ_OP_READ etc.) is 
modified into blk_mq_opf_t then their type changes from 'enum req_opf' 
into type blk_mq_opf_t and sparse complains when passing e.g. 
REQ_OP_READ to a function that accepts an argument with type enum req_opf.
* If the type of the operation type constants is not modified then 
sparse complains about bitwise or-ing the operation type and a request 
flag, e.g. REQ_OP_WRITE | REQ_FUA.

I'm not sure how to solve this other than by modifying the functions 
that accept an 'opf' argument into accepting an additional argument 
(enum req_opf op + blk_mq_opf_t op_flags).

Thanks,

Bart.