Re: [dm-devel] [PATCH 1/7] multipath: fix tur checker locking

2018-02-08 Thread Benjamin Marzinski
On Thu, Feb 08, 2018 at 09:49:20AM +0100, Martin Wilck wrote: > Hello Ben, > > On Wed, 2018-02-07 at 16:49 -0600, Benjamin Marzinski wrote: > > Commit 6e2423fd fixed a bug where the tur checker could cancel a > > detached thread after it had exitted. However in fixing this, the new > > code

Re: [dm-devel] [PATCH 3/7] multipathd: remove coalesce_paths from ev_add_map

2018-02-08 Thread Benjamin Marzinski
On Thu, Feb 08, 2018 at 10:10:38AM +0100, Martin Wilck wrote: > Hi Ben, > > On Wed, 2018-02-07 at 16:49 -0600, Benjamin Marzinski wrote: > > If ev_add_map is called for a multipath device that doesn't exist in > > device-mapper, it will call coalesce_paths to add it. This doesn't > > work > >

Re: [dm-devel] [PATCH 1/7] multipath: fix tur checker locking

2018-02-08 Thread Bart Van Assche
On Wed, 2018-02-07 at 16:49 -0600, Benjamin Marzinski wrote: > Commit 6e2423fd fixed a bug where the tur checker could cancel a > detached thread after it had exitted. However in fixing this, the new > code grabbed a mutex (to call condlog) while holding a spin_lock. To > deal with that, and to

Re: [dm-devel] [PATCH 5/7] Fix set_no_path_retry() regression

2018-02-08 Thread Benjamin Marzinski
On Thu, Feb 08, 2018 at 10:21:45AM +0100, Martin Wilck wrote: > Hi Ben, > > On Wed, 2018-02-07 at 16:49 -0600, Benjamin Marzinski wrote: > > commit 0f850db7fceb6b2bf4968f3831efd250c17c6138 "multipathd: clean up > > set_no_path_retry" has a bug in it. It made set_no_path_retry > > never reset

Re: [dm-devel] [PATCH 7/7] multipath: print sysfs state in fast list mode

2018-02-08 Thread Benjamin Marzinski
On Thu, Feb 08, 2018 at 10:32:06AM +0100, Martin Wilck wrote: > Hi Ben, > > On Wed, 2018-02-07 at 16:49 -0600, Benjamin Marzinski wrote: > > commit b123e711ea2a4b471a98ff5e26815df3773636b5 "libmultipath: > > cleanup > > orphan device states" stopped multipathd from showing old state for > >

Re: [dm-devel] [PATCH 1/7] multipath: fix tur checker locking

2018-02-08 Thread Bart Van Assche
On Thu, 2018-02-08 at 11:52 -0600, Benjamin Marzinski wrote: > On Thu, Feb 08, 2018 at 09:49:20AM +0100, Martin Wilck wrote: > > On Wed, 2018-02-07 at 16:49 -0600, Benjamin Marzinski wrote: > > > + } else { > > > + /* locking here solely to make static > > > analyzers happy

Re: [dm-devel] [PATCH 1/7] multipath: fix tur checker locking

2018-02-08 Thread Benjamin Marzinski
On Thu, Feb 08, 2018 at 06:19:32PM +, Bart Van Assche wrote: > On Wed, 2018-02-07 at 16:49 -0600, Benjamin Marzinski wrote: > > Commit 6e2423fd fixed a bug where the tur checker could cancel a > > detached thread after it had exitted. However in fixing this, the new > > code grabbed a mutex

Re: [dm-devel] [PATCH 1/7] multipath: fix tur checker locking

2018-02-08 Thread Bart Van Assche
On Thu, 2018-02-08 at 13:27 -0600, Benjamin Marzinski wrote: > On Thu, Feb 08, 2018 at 06:19:32PM +, Bart Van Assche wrote: > > On Wed, 2018-02-07 at 16:49 -0600, Benjamin Marzinski wrote: > > > Commit 6e2423fd fixed a bug where the tur checker could cancel a > > > detached thread after it had

[dm-devel] [PATCH v2 3/7] multipathd: remove coalesce_paths from ev_add_map

2018-02-08 Thread Benjamin Marzinski
If ev_add_map is called for a multipath device that doesn't exist in device-mapper, it will call coalesce_paths to add it. This doesn't work and hasn't for years. It doesn't add the map to the mpvec, or start up waiters, or do any of the necessary things that do get done when you call ev_add_map

[dm-devel] [PATCH v2 2/7] multipath: fix DEF_TIMEOUT use

2018-02-08 Thread Benjamin Marzinski
DEF_TIMEOUT is specified in seconds. The io_hdr timeout is specified in milliseconds, so we need to convert it. Multipath should be waiting longer than 30 milliseconds here. If there are concerns that 30 seconds may be too long, we could always make this configurable, using conf->checker_timeout

[dm-devel] [PATCH v2 7/7] multipath: print sysfs state in fast list mode

2018-02-08 Thread Benjamin Marzinski
commit b123e711ea2a4b471a98ff5e26815df3773636b5 "libmultipath: cleanup orphan device states" stopped multipathd from showing old state for orphan paths by checking if pp->mpp was set, and only printing the state if it was. Unfortunately, when "multipath -l" is run, pp->mpp isn't set. While the

Re: [dm-devel] [PATCH 7/7] multipath: print sysfs state in fast list mode

2018-02-08 Thread Martin Wilck
Hi Ben, On Wed, 2018-02-07 at 16:49 -0600, Benjamin Marzinski wrote: > commit b123e711ea2a4b471a98ff5e26815df3773636b5 "libmultipath: > cleanup > orphan device states" stopped multipathd from showing old state for > orphan paths by checking if pp->mpp was set, and only printing the > state > if

Re: [dm-devel] [PATCH 4/7] multipathd: remove unused configure parameter

2018-02-08 Thread Martin Wilck
On Wed, 2018-02-07 at 16:49 -0600, Benjamin Marzinski wrote: > configure() is always called with start_waiters=1, so there is no > point > in having the parameter. Remove it. > > Signed-off-by: Benjamin Marzinski > --- > multipathd/main.c | 12 +--- > 1 file

Re: [dm-devel] [PATCH v2 18/20] libmultipath: path latency: log threshold with p2

2018-02-08 Thread Guan Junxiong
Looks Good. Reviewed-by: Guan Junxiong On 2018/1/14 5:19, Martin Wilck wrote: > This is not a critical error. It just means that the path in > question will have low priority (rightly so, if it has >100s latency). > --- > libmultipath/prioritizers/path_latency.c | 2 +-

Re: [dm-devel] [PATCH v2 20/20] libmultipath: path latency: remove warnings

2018-02-08 Thread Guan Junxiong
Looks Good. Thanks for your improvement. Reviewed-by Guan Junxiong On 2018/1/14 5:19, Martin Wilck wrote: > The warnings at here are pointless. We are looking at a single > path only. Firstly, the standdard deviation for this measurement > can't be "too low" - the