Re: [PATCH 4/4] blk-mq: enable checking two part inflight counts at the same time

[Jens Axboe <axboe@kernel.dk>]

On 08/03/2017 03:29 PM, Bart Van Assche wrote:
> On Thu, 2017-08-03 at 14:01 -0600, Jens Axboe wrote:
>>  static void blk_mq_check_inflight(struct blk_mq_hw_ctx *hctx,
>> @@ -97,17 +98,25 @@ static void blk_mq_check_inflight(struct blk_mq_hw_ctx *hctx,
>>  {
>>  	struct mq_inflight *mi = priv;
>>  
>> -	if (rq->part == mi->part)
>> -		mi->inflight++;
>> +	if (!test_bit(REQ_ATOM_STARTED, &rq->atomic_flags))
>> +		return;
> 
> Should the REQ_ATOM_STARTED test perhaps have been introduced in patch 3/4
> instead of in this patch?

It should, moved.

>> +	if (rq->part == mi->part1) {
>> +		mi->inflight[0]++;
>> +		if (mi->part1->partno &&
>> +		    &part_to_disk(mi->part1)->part0 == mi->part2)
>> +			mi->inflight[1]++;
>> +	} else if (rq->part == mi->part2)
>> +		mi->inflight[1]++;
>>  }
> 
> So mi->part2 may represent part0 but mi->part1 not? Does that deserve a comment?
> 
> Additionally, shouldn't the mi->part2 == part0 test be moved out of the if-statement
> such that all requests are counted for part0 instead of storing the same count in
> inflight[0] and inflight[1] if mi->part2 == part0?

I think I'll just clean up the whole thing and get rid of part1/part2. The two
can only exist together, if part1 is a partition. So will be easier to just
unconditionally sum part+root, if part is a partition. Then we only need to
pass in 'part', not part1/2.

>> -unsigned int blk_mq_in_flight(struct request_queue *q,
>> -			       struct hd_struct *part)
>> +void blk_mq_in_flight(struct request_queue *q, struct hd_struct *part1,
>> +		      struct hd_struct *part2, unsigned int *inflight)
>>  {
> 
> Should inflight be declared as an array to make it clear that it is a pointer to
> an array with two elements?

Sure, I can do that.

-- 
Jens Axboe