[dm-devel] [RFC PATCH v3] dm mpath: add a queue_if_no_path timeout
Mike Snitzer
snitzer at redhat.com
Tue Nov 5 16:53:26 UTC 2013
On Tue, Nov 05 2013 at 11:02am -0500,
Frank Mayhar <fmayhar at google.com> wrote:
> This is the patch submitted by Jun'ichi Nomura, originally based on
> Mike's patch with some small changes by me. Jun'ichi's description
> follows, along with my changes:
>
> On Tue, 2013-11-05 at 07:18 -0800, Frank Mayhar wrote:
> > On Fri, 2013-11-01 at 04:17 +0000, Junichi Nomura wrote:
> > > I slightly modified the patch:
> > > - fixed the timeout handler to correctly find
> > > clone request and "struct multipath"
> > > - the timeout handler just disables "queue_if_no_path"
> > > instead of killing the request directly
> > > - "dmsetup status" to show the parameter
> > > - changed an interface between dm core and target
> > > - added some debugging printk (you can remove them)
> > > and checked the timeout occurs, at least.
> > >
> > > I'm not sure if this feature is good or not though.
> > > (The timer behavior is not intuitive, I think)
> > Thanks! I integrated your new patch and tested it. Sure enough, it
> > seems to work. I've made a few tweaks (added a module tunable and
> > support for setting the timer in multipath_message(), removed your debug
> > printks) and will submit the modified patch for discussion shortly.
>
> Comments?
My primary concern is that this patch is always establishing a timed_out
method via blk_queue_rq_timed_out() regardless of whether the mpath
device needs it. I'm also not a fan of adding such a specialized
rq-based only hook (dm_rq_timed_out_fn timed_out).
Could be we'll need to do other things to the queue (be it bio-based or
rq-based) in the future.
So I prefer the approach I took to arming the queue (via new .init_queue
hook) in this patch: https://patchwork.kernel.org/patch/3070391/
More information about the dm-devel
mailing list