Re: [dm-devel] [PATCH v4 6/6] dm rq: Avoid that request processing stalls sporadically

[Bart Van Assche]

On Fri, 2017-04-14 at 09:13 +0800, Ming Lei wrote:
> On Thu, Apr 13, 2017 at 09:59:57AM -0700, Bart Van Assche wrote:
> > On 04/12/17 19:20, Ming Lei wrote:
> > > On Wed, Apr 12, 2017 at 06:38:07PM +, Bart Van Assche wrote:
> > > > If the blk-mq core would always rerun a hardware queue if a block driver
> > > > returns BLK_MQ_RQ_QUEUE_BUSY then that would cause 100% of a single CPU 
> > > > core
> > > 
> > > It won't casue 100% CPU utilization since we restart queue in completion
> > > path and at that time at least one tag is available, then progress can be
> > > made.
> > 
> > Hello Ming,
> > 
> > Sorry but you are wrong. If .queue_rq() returns BLK_MQ_RQ_QUEUE_BUSY
> > then it's likely that calling .queue_rq() again after only a few
> > microseconds will cause it to return BLK_MQ_RQ_QUEUE_BUSY again. If you
> > don't believe me, change "if (!blk_mq_sched_needs_restart(hctx) &&
> > !test_bit(BLK_MQ_S_TAG_WAITING, &hctx->state)) blk_mq_run_hw_queue(hctx,
> > true);" into "blk_mq_run_hw_queue(hctx, true);", trigger a busy
> 
> Yes, that can be true, but I mean it is still OK to run the queue again
> with
> 
>   if (!blk_mq_sched_needs_restart(hctx) &&
>   !test_bit(BLK_MQ_S_TAG_WAITING, &hctx->state))
>   blk_mq_run_hw_queue(hctx, true);
> 
> and restarting queue in __blk_mq_finish_request() when
> BLK_MQ_RQ_QUEUE_BUSY is returned from .queue_rq(). And both are in current
> blk-mq implementation.
> 
> Then why do we need blk_mq_delay_run_hw_queue(hctx, 100/*ms*/) in dm?

Because if dm_mq_queue_rq() returns BLK_MQ_RQ_QUEUE_BUSY that there is no
guarantee that __blk_mq_finish_request() will be called later on for the
same queue. dm_mq_queue_rq() can e.g. return BLK_MQ_RQ_QUEUE_BUSY while no
dm requests are in progress because the SCSI error handler is active for
all underlying paths. See also scsi_lld_busy() and scsi_host_in_recovery().

Bart.

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH] libmultipath: fix skip_kpartx support for removing maps

[Christophe Varoqui]

Merged,
Thanks.

On Thu, Apr 13, 2017 at 3:05 PM, Martin Wilck  wrote:

> Commit 79a05a4e inadvertently removed the code necessary
> to support multipath maps without partitions, as introduced
> in b73a34ea "libmultipath: add skip_kpartx option". Revert
> this part of the commit, so that skip_kpartx works again.
>
> Fixes: 79a05a4e libmultipath: move suspend logic to _dm_flush_map
> Signed-off-by: Martin Wilck 
> ---
>  libmultipath/devmapper.c | 14 +-
>  1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
> index 026418f8..5fb9d9ac 100644
> --- a/libmultipath/devmapper.c
> +++ b/libmultipath/devmapper.c
> @@ -761,6 +761,12 @@ out:
>  }
>
>  static int
> +has_partmap(const char *name, void *data)
> +{
> +   return 1;
> +}
> +
> +static int
>  partmap_in_use(const char *name, void *data)
>  {
> int part_count, *ret_count = (int *)data;
> @@ -785,12 +791,18 @@ int _dm_flush_map (const char * mapname, int
> need_sync, int deferred_remove,
>  {
> int r;
> int queue_if_no_path = 0;
> +   int udev_flags = 0;
> unsigned long long mapsize;
> char params[PARAMS_SIZE] = {0};
>
> if (!dm_is_mpath(mapname))
> return 0; /* nothing to do */
>
> +   /* if the device currently has no partitions, do not
> +  run kpartx on it if you fail to delete it */
> +   if (do_foreach_partmaps(mapname, has_partmap, NULL) == 0)
> +   udev_flags |= MPATH_UDEV_NO_KPARTX_FLAG;
> +
> /* If you aren't doing a deferred remove, make sure that no
>  * devices are in use */
> if (!do_deferred(deferred_remove) && partmap_in_use(mapname, NULL))
> @@ -834,7 +846,7 @@ int _dm_flush_map (const char * mapname, int
> need_sync, int deferred_remove,
> mapname);
> if (need_suspend && queue_if_no_path != -1) {
> dm_simplecmd_noflush(DM_DEVICE_RESUME,
> -mapname, 0);
> +mapname, udev_flags);
> }
> }
> if (retries)
> --
> 2.12.2
>
>
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH 0/1] Missing RDAC patch

[Christophe Varoqui]

Merged,
and version bumped to 0.7.1

The tags are now pushed too.

On Fri, Apr 14, 2017 at 1:52 AM, Xose Vazquez Perez 
wrote:

> On 04/13/2017 05:39 PM, Martin Wilck wrote:
>
> > I'm sending this on Hannes' behalf now. Sorry we didn't make it for
> 0.7.0.
>
> You're lucky, 0.7.0 release wasn't tagged yet.
>
> > Hannes Reinecke (1):
> >   libmultipath/propsel: Do not select sysfs prioritizer for RDAC arrays
> >
> >  libmultipath/discovery.c |  2 +-
> >  libmultipath/discovery.h |  1 +
> >  libmultipath/propsel.c   | 24 ++--
> >  3 files changed, 24 insertions(+), 3 deletions(-)
>
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH] multipath-tools: replace multipath configuration output

[Christophe Varoqui]

Right,
pushed now.

Thanks,
Christophe.

On Fri, Apr 14, 2017 at 1:56 AM, Xose Vazquez Perez 
wrote:

> On 04/12/2017 09:35 AM, Christophe Varoqui wrote:
>
> > Merged,
>
> No, it's missing.
>
> > On Sat, Apr 8, 2017 at 3:30 PM, Xose Vazquez Perez <
> xose.vazq...@gmail.com > wrote:
> >
> > Cc: Christophe Varoqui  christophe.varo...@opensvc.com>>
> > Cc: device-mapper development  dm-devel@redhat.com>>
> > Signed-off-by: Xose Vazquez Perez  xose.vazq...@gmail.com>>
> > ---
> >  libmultipath/propsel.c | 12 ++--
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/libmultipath/propsel.c b/libmultipath/propsel.c
> > index dd10ceb..5f70d86 100644
> > --- a/libmultipath/propsel.c
> > +++ b/libmultipath/propsel.c
> > @@ -162,7 +162,7 @@ int select_pgpolicy(struct config *conf, struct
> multipath * mp)
> >
> > if (conf->pgpolicy_flag > 0) {
> > mp->pgpolicy = conf->pgpolicy_flag;
> > -   origin = "(cmd line flag)";
> > +   origin = "(setting: multipath command line [-p]
> flag)";
> > goto out;
> > }
> > mp_set_mpe(pgpolicy);
> > @@ -251,18 +251,18 @@ int select_alias(struct config *conf, struct
> multipath * mp)
> > mp->alias_old, mp->alias_prefix,
> > conf->bindings_read_only);
> > memset (mp->alias_old, 0, WWID_SIZE);
> > -   origin = "(using existing alias)";
> > +   origin = "(setting: using existing alias)";
> > }
> >
> > if (mp->alias == NULL) {
> > mp->alias = get_user_friendly_alias(mp->wwid,
> > conf->bindings_file,
> mp->alias_prefix, conf->bindings_read_only);
> > -   origin = "(user_friendly_name)";
> > +   origin = "(setting: user_friendly_name)";
> > }
> >  out:
> > if (mp->alias == NULL) {
> > mp->alias = STRDUP(mp->wwid);
> > -   origin = "(default to wwid)";
> > +   origin = "(setting: default to WWID)";
> > }
> > if (mp->alias)
> > condlog(3, "%s: alias = %s %s", mp->wwid, mp->alias,
> origin);
> > @@ -565,7 +565,7 @@ int select_retain_hwhandler(struct config *conf,
> struct multipath *mp)
> >
> > if (!VERSION_GE(conf->version, minv_dm_retain)) {
> > mp->retain_hwhandler = RETAIN_HWHANDLER_OFF;
> > -   origin = "(requires kernel version >= 1.5.0)";
> > +   origin = "(setting: WARNING, requires kernel version
> >= 1.5.0)";
> > goto out;
> > }
> > mp_set_ovr(retain_hwhandler);
> > @@ -614,7 +614,7 @@ int select_deferred_remove(struct config *conf,
> struct multipath *mp)
> >
> >  #ifndef LIBDM_API_DEFERRED
> > mp->deferred_remove = DEFERRED_REMOVE_OFF;
> > -   origin = "(not compiled with support)";
> > +   origin = "(setting: WARNING, not compiled with support)";
> > goto out;
> >  #endif
> > if (mp->deferred_remove == DEFERRED_REMOVE_IN_PROGRESS) {
> > --
> > 2.12.2
> >
> >
>
>
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH 02/25] block: remove the blk_execute_rq return value

[h...@lst.de]

On Thu, Apr 13, 2017 at 08:03:22PM +, Bart Van Assche wrote:
> That blk_execute_rq() call can only be reached if a few lines above 0 was
> assigned to the "error" variable. Since nfsd4_scsi_identify_device() returns
> the value of the "error" variable I think -EIO should be assigned to that
> variable before the "goto out_put_request" statement is reached.

You're right!  I'll fix it up.

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH 01/25] remove the mg_disk driver

[h...@lst.de]

On Thu, Apr 13, 2017 at 07:58:13PM +, Bart Van Assche wrote:
> Should the person who submitted this driver be CC-ed for this patch (unsik
> Kim )?

Yes, he should.  And in fact he was when I sent this patch out separately
a little earlier, I just included it in this series for reference.

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel