Re: [PATCH RESEND] block: Don't check QUEUE_FLAG_SAME_COMP in __blk_complete_request.

[Tao Ma]

On 09/15/2011 09:05 AM, Shaohua Li wrote:
> 2011/9/14 Tao Ma <tm@xxxxxx>:
>> From: Tao Ma <boyu.mt@xxxxxxxxxx>
>>
>> In __blk_complete_request, we check both QUEUE_FLAG_SAME_COMP and req->cpu
>> to decide whether we should use req->cpu. Actually the user can also
>> select the complete cpu by either setting BIO_CPU_AFFINE or by calling
>> bio_set_completion_cpu. Current solution makes these 2 ways don't work
>> any more. So we'd better just check req->cpu.
>>
>> Cc: Jens Axboe <jaxboe@xxxxxxxxxxxx>
>> Signed-off-by: Tao Ma <boyu.mt@xxxxxxxxxx>
>> ---
>>  block/blk-softirq.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/block/blk-softirq.c b/block/blk-softirq.c
>> index 58340d0..1366a89 100644
>> --- a/block/blk-softirq.c
>> +++ b/block/blk-softirq.c
>> @@ -115,7 +115,7 @@ void __blk_complete_request(struct request *req)
>>        /*
>>         * Select completion CPU
>>         */
>> -       if (test_bit(QUEUE_FLAG_SAME_COMP, &q->queue_flags) && req->cpu != -1) {
>> +       if (req->cpu != -1) {
>>                ccpu = req->cpu;
>>                if (!test_bit(QUEUE_FLAG_SAME_FORCE, &q->queue_flags)) {
>>                        ccpu = blk_cpu_to_group(ccpu);
> why not delete bio_set_completion_cpu()?
Not sure. It is added in commit c7c22e4d from the very beginning, and in
the commit log Jens described it as:
A bio helper (bio_set_completion_cpu()) is also added, so that queuers
can ask for completion on that specific CPU. Maybe it is obsolete.
Anyway, I am fine to generate a patch to remove it if you prefer.
> 1. nobody uses it in my search
yes, but it may be designed for someone to use it. The same goes with
BIO_CPU_AFFINE.
> 2. it's misleading. even setting the completion cpu, the bio isn't
> always to run finish in the cpu,
> it's just run in the group of the cpu.
Without this patch, it works as you said.
But with this patch, it the user uses bio_set_completion_cpu and
QUEUE_FLAG_SAME_FORCE isn't set, the completion will happen in the
req->cpu the user specified.

Jens,
	Do you have any option for it? Whether we should preserve it and make
it work or just totally remove it?

Thanks
Tao
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/