Re: [PATCH 4/6] mtip32xx: convert internal command issue to block IO path

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 05/02/2017 09:42 AM, Bart Van Assche wrote:
> On Tue, 2017-05-02 at 09:25 -0600, Jens Axboe wrote:
>> On 05/02/2017 09:16 AM, Christoph Hellwig wrote:
>>> Any reason for the move from ->end_io_data to ->special?  I thought
>>> that ->special was something we'd get rid of sooner or later now
>>> that we can have additional per-cmd data even for !mq.
>>
>> With the switch to blk_execute_rq(), we can't be using end_io_data
>> and end_io, as we use that internally for the wakeup. So I have to
>> stuff it somewhere.
>>
>> The obvious option would be to move it to mtip_cmd, but we can't
>> safely access that prior to having a driver tag assigned, which doesn't
>> happen until we end up in our ->queue_rq(). So we need to stuff it
>> somewhere.
> 
> Hello Jens,
> 
> Do you think it would be a good idea to allow blk_get_request() callers
> to specify that a driver tag has to be allocated even if a scheduler has
> been configured? That would make it possible to store completion data in
> mtip_cmd for the mtip driver.

Honestly, I think it's fine that we can stuff it _somewhere_ in the
request. I'd rather not expose the driver vs internal tag anymore
than we absolutely have, it's just prone to work on one configuration
and then break if someone turns scheduling on or off.

The real fix here, to avoid using ->special, would be to map the
request before insertion. Then we don't have to pass any info at all.
But that will require touching the callers of the internal command
path for mtip32xx. Left as an exercise to the reader...

-- 
Jens Axboe




[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux