On Tue, 2017-04-25 at 17:16 +0200, 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?
> >
> > How about surrounding (*show_rq)() function pointer with #ifdef
> > CONFIG_BLK_DEBUGFS /
> > #endif?
>
> Then you'll end up with different offsets in the structures, depending
> on how the kernel is compiled. Making debugging a nightmare.
Hello Hannes,
How about moving the .show_rq function pointer to the end such that the
offset of other members of struct blk_mq_ops does not depend on whether or
not CONFIG_BLK_DEBUGFS has been defined?
Bart.
