On Wed, Jan 10 2018 at 2:55am -0500, Ming Lei <tom.leiming@xxxxxxxxx> wrote: > On Wed, Jan 10, 2018 at 12:21 PM, Mike Snitzer <snitzer@xxxxxxxxxx> wrote: > > On Tue, Jan 09 2018 at 10:46pm -0500, > > Ming Lei <tom.leiming@xxxxxxxxx> wrote: > > > >> Another related issue is that blk-mq debugfs can't work yet for dm-mpath. > > > > Not sure how that is a related issue to this thread. > > But can you expand on that? I'm not familiar with "blk-mq debugfs". > > Any pointer to code that activates it? Could be that my changes make it > > work now... > > Hi, > > blk_mq_debugfs_register() is called in blk_register_queue() from add_disk(), > and blk_mq_debugfs_register() need queue's information to setup mq debugfs > stuff. > > But your patch does fix this issue, cool! Great, thought it might. > >> So I am just wondering if it is possible to follow the usual way to add disk > >> after setting up queue for DM? If it is possible, it may avoid lots of trouble > >> for us. > > > > As I explained in the header (quoted above actually) it is _not_ > > possible. It is the gendisk that enables the device stack to be > > constructed (at least how DM's stacking is currently implemented with > > bd_link_disk_holder() and taking references on devices found in a DM > > device's table). And from that gendisk I can then walk the devices to > > establish the request_queue as needed, its stacked limits, etc. > > > > The approach I've taken in these patches is the closest I've gotten to > > make DM _much_ more sane about how its request_queue is established. > > But its still off the beaten path due to needing "block: cope with > > gendisk's 'queue' being added later". > > OK, thanks for your explanation. > > After taking close look at your patches, I think this approach is clean. > But there are still corner cases which need to be addressed: > > 1) in the following disk attributes, queue is referenced, so we have > to add check in their show/store operations since the attributes > are shown up just after disk is added. > > disk_alignment_offset_show > disk_discard_alignment_show > part_timeout_show/part_timeout_store > > 2) same issue on /proc/diskstats: see diskstats_show() > > 3) in IO path, looks we already checked disk->queue in > generic_make_request_checks(), so it should be fine, and you set > disk->queue after the queue is fully initialized in the 3rd patch. I'll work through these and see what I can do. Will likely deal with each independent of the 2/3 patch (otherwise if I just keep folding changes in to the original patch review will get too hard). So hopefully I'll have a v3 to share later today. Thanks, Mike
