Re: [PATCH v4 09/10] blk-mq: Add blk_mq_ops.show_rq()

[Jens Axboe <axboe@kernel.dk>]

On 04/25/2017 08:16 AM, Hannes Reinecke wrote:
> On 04/24/2017 11:51 PM, Bart Van Assche wrote:
>> On Mon, 2017-04-24 at 09:32 +0200, Hannes Reinecke wrote:
>>> On 04/22/2017 01:40 AM, Bart Van Assche wrote:
>>>> --- a/include/linux/blk-mq.h
>>>> +++ b/include/linux/blk-mq.h
>>>> @@ -121,6 +121,12 @@ struct blk_mq_ops {
>>>>  	softirq_done_fn		*complete;
>>>>  
>>>>  	/*
>>>> +	 * Used by the debugfs implementation to show driver-specific
>>>> +	 * information about a request.
>>>> +	 */
>>>> +	void (*show_rq)(struct seq_file *m, struct request *rq);
>>>> +
>>>> +	/*
>>>>  	 * Called when the block layer side of a hardware queue has been
>>>>  	 * set up, allowing the driver to allocate/init matching structures.
>>>>  	 * Ditto for exit/teardown.
>>>>
>>>
>>> I don't really like this; what does happen if someone disabled
>>> CONFIG_BLK_DEBUGFS? Won't we end up with a stale callback?
>>
>> Hello Hannes,
>>
>> How about surrounding (*show_rq)() function pointer with #ifdef CONFIG_BLK_DEBUGFS /
>> #endif?
>>
> Nope.
> 
> Then you'll end up with different offsets in the structures, depending
> on how the kernel is compiled. Making debugging a nightmare.

That's nonsense, we deal with this all the time already.

-- 
Jens Axboe