Re: Preempt & smp_processor_id in __make_request

[Jens Axboe <axboe@kernel.dk>]

On 2011-07-26 13:43, Christoph Hellwig wrote:
> On Tue, Jul 26, 2011 at 10:32:06AM +0100, Steven Whitehouse wrote:
>> diff --git a/block/blk-core.c b/block/blk-core.c
>> index f8cb099..f925581 100644
>> --- a/block/blk-core.c
>> +++ b/block/blk-core.c
>> @@ -1283,7 +1283,7 @@ get_rq:
>>  
>>  	if (test_bit(QUEUE_FLAG_SAME_COMP, &q->queue_flags) ||
>>  	    bio_flagged(bio, BIO_CPU_AFFINE))
>> -		req->cpu = smp_processor_id();
>> +		req->cpu = raw_smp_processor_id();
>>  
>>  	plug = current->plug;
>>  	if (plug) {
>>
>> However this fixes the symptoms, rather than the cause, so I'm not at
>> all sure that this is the correct solution,
> 
> It doesn't fix the symptoms - the warning is one of those 98% percent
> right types of warnings.  In this case get_cpu/put_cpu doesn't buy you
> anthing - we want to stash away the cpu id that we submitted the I/O
> from, to compare it when we later process the I/O completion from
> a different context.  With some PREEMPT options we might actually get
> rescheduled to a different CPU during the rest of this function, but
> as we submit the plugged requests at this point we'll still be correct.
> 
> Even if wasn't we would already thrash the cache badly enough for this
> optimization not to matter in that case.

This was reported earlier today as well, and I suggested the same fix.
raw_smp_processor_id() is fine, since it's just a hint.

-- 
Jens Axboe