[dm-devel] [PATCH] multipath-tools: improve processing efficiency for addition and deletion of multipath devices

[tang . junhui]

From: "tang.junhui" 

This patch used to improve processing efficiency for addition and deletion
of multipath devices.

This patch is tested pass by ZTE multipath automatic testing system.
The modification reduces the system consumption(such as CPU) and shortens
the processing time obviously in scene of massive multipath devices
addition or deletion.

The main processing flow of code is:
1) add uid_attrs configuration in the defaults section:
   It is configured udev attribute which providing a unique path identifier
   for corresponding type of path devices. If this field is configured and
   matched with type of device, it would override any other methods
   providing for device unique identifier in config file, and it would
   activate merging uevents according to the identifier to promote effiecncy
   in processing uevents. Tt has no default value, so defaultly only uevents
   filtering works, and uevents merging does not works, if users want to
   identify path by udev attribute and to activate merging uevents for SCSI
   and DAS device, they can set it's value as:
   "sd:ID_SERIAL dasd:ID_UID"
2) uevents accumulation in uevents burst scene:
   wait one seconds for more uevents in uevent_listen() in uevents burst
   situations
3) uevents preparing, filtering and merging:
   discard unuse uevents and fetch path idendifier from uevents;
   filter uevents;
   merge uevents.
4) uevents proccessing:
   proccess the merged uevents in uev->merge_node list without calling
   domap();
   proccess the last uevents uev with calling domap().

Signed-off-by: tang.junhui 
---
 libmultipath/config.c  |   3 +
 libmultipath/config.h  |   1 +
 libmultipath/dict.c|   3 +
 libmultipath/discovery.c   |   5 +-
 libmultipath/discovery.h   |   2 +-
 libmultipath/list.h|  41 ++
 libmultipath/propsel.c |   7 +
 libmultipath/uevent.c  | 320 +++--
 libmultipath/uevent.h  |   2 +
 libmultipath/util.c|  42 ++
 libmultipath/util.h|   1 +
 multipath/multipath.conf.5 |  18 +++
 multipathd/cli_handlers.c  |   4 +-
 multipathd/main.c  |  93 +
 multipathd/main.h  |   4 +-
 15 files changed, 471 insertions(+), 75 deletions(-)

diff --git a/libmultipath/config.c b/libmultipath/config.c
index 9d3f3e1..bb6619b 100644
--- a/libmultipath/config.c
+++ b/libmultipath/config.c
@@ -493,6 +493,9 @@ free_config (struct config * conf)
if (conf->uid_attribute)
FREE(conf->uid_attribute);
 
+   if (conf->uid_attrs)
+   FREE(conf->uid_attrs);
+
if (conf->getuid)
FREE(conf->getuid);
 
diff --git a/libmultipath/config.h b/libmultipath/config.h
index 9a90745..4f1d596 100644
--- a/libmultipath/config.h
+++ b/libmultipath/config.h
@@ -164,6 +164,7 @@ struct config {
 
char * multipath_dir;
char * selector;
+   char * uid_attrs;
char * uid_attribute;
char * getuid;
char * features;
diff --git a/libmultipath/dict.c b/libmultipath/dict.c
index bababdb..82066f6 100644
--- a/libmultipath/dict.c
+++ b/libmultipath/dict.c
@@ -249,6 +249,8 @@ declare_ovr_snprint(selector, print_str)
 declare_mp_handler(selector, set_str)
 declare_mp_snprint(selector, print_str)
 
+declare_def_handler(uid_attrs, set_str)
+declare_def_snprint(uid_attrs, print_str)
 declare_def_handler(uid_attribute, set_str)
 declare_def_snprint_defstr(uid_attribute, print_str, DEFAULT_UID_ATTRIBUTE)
 declare_ovr_handler(uid_attribute, set_str)
@@ -1396,6 +1398,7 @@ init_keywords(vector keywords)
install_keyword("multipath_dir", _multipath_dir_handler, 
_def_multipath_dir);
install_keyword("path_selector", _selector_handler, 
_def_selector);
install_keyword("path_grouping_policy", _pgpolicy_handler, 
_def_pgpolicy);
+   install_keyword("uid_attrs", _uid_attrs_handler, 
_def_uid_attrs);
install_keyword("uid_attribute", _uid_attribute_handler, 
_def_uid_attribute);
install_keyword("getuid_callout", _getuid_handler, 
_def_getuid);
install_keyword("prio", _prio_name_handler, _def_prio_name);
diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index 4e99845..7398040 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -34,7 +34,7 @@
 
 int
 alloc_path_with_pathinfo (struct config *conf, struct udev_device *udevice,
- int flag, struct path **pp_ptr)
+ char *wwid, int flag, struct path **pp_ptr)
 {
int err = PATHINFO_FAILED;
struct path * pp;
@@ -52,6 +52,9 @@ alloc_path_with_pathinfo (struct config *conf, struct 
udev_device *udevice,
if (!pp)
return PATHINFO_FAILED;
 
+   if(wwid)
+   strncpy(pp->wwid, wwid, sizeof(pp->wwid));
+
if (safe_sprintf(pp->dev, "%s", devname)) {
condlog(0, "pp->dev too small");
} else 

[dm-devel] [PATCH] multipath-tools: improve processing efficiency for addition and deletion of multipath devices

[tang . junhui]

From: "tang.junhui" 

Change-Id: I3f81a55fff389f991f915927000b281d7e263cc5
Signed-off-by: tang.junhui 

This patch used to improve processing efficiency for addition and deletion
of multipath devices.

This patch is tested pass by ZTE multipath automatic testing system.
The modification reduces the system consumption(such as CPU) and shortens
the processing time obviously in scene of massive multipath devices
addition or deletion.

The main processing flow of code is:
1) add uid_attrs configuration in the defaults section:
   It is configured udev attribute which providing a unique path identifier
   for corresponding type of path devices. If this field is configured and
   matched with type of device, it would override any other methods providing
   for device unique identifier in config file, and it would activate merging
   uevents according to the identifier to promote effiecncy in processing
   uevents. Tt has no default value, so defaultly only uevents filtering
   works, and uevents merging does not works, if users want to identify path
   by udev attribute and to activate merging uevents for SCSI and DAS device,
   they can set it's value as:
   "sd:ID_SERIAL dasd:ID_UID"
2) uevents accumulation in uevents burst scene:
   wait one seconds for more uevents in uevent_listen() in uevents burst
   situations
3) uevents preparing, filtering and merging:
   discard unuse uevents and fetch path idendifier from uevents;
   filter uevents;
   merge uevents.
4) uevents proccessing:
   proccess the merged uevents in uev->merge_node list without calling
   domap();
   proccess the last uevents uev with calling domap().
---
 libmultipath/config.c  |   3 +
 libmultipath/config.h  |   1 +
 libmultipath/dict.c|   3 +
 libmultipath/discovery.c   |   5 +-
 libmultipath/discovery.h   |   2 +-
 libmultipath/list.h|  41 ++
 libmultipath/propsel.c |   7 +
 libmultipath/uevent.c  | 320 +++--
 libmultipath/uevent.h  |   2 +
 libmultipath/util.c|  42 ++
 libmultipath/util.h|   1 +
 multipath/multipath.conf.5 |  18 +++
 multipathd/cli_handlers.c  |   4 +-
 multipathd/main.c  |  93 +
 multipathd/main.h  |   4 +-
 15 files changed, 471 insertions(+), 75 deletions(-)

diff --git a/libmultipath/config.c b/libmultipath/config.c
index 15ddbd8..765e91d 100644
--- a/libmultipath/config.c
+++ b/libmultipath/config.c
@@ -488,6 +488,9 @@ free_config (struct config * conf)
if (conf->uid_attribute)
FREE(conf->uid_attribute);
 
+   if (conf->uid_attrs)
+   FREE(conf->uid_attrs);
+
if (conf->getuid)
FREE(conf->getuid);
 
diff --git a/libmultipath/config.h b/libmultipath/config.h
index 9670020..ab85930 100644
--- a/libmultipath/config.h
+++ b/libmultipath/config.h
@@ -153,6 +153,7 @@ struct config {
 
char * multipath_dir;
char * selector;
+   char * uid_attrs;
char * uid_attribute;
char * getuid;
char * features;
diff --git a/libmultipath/dict.c b/libmultipath/dict.c
index dc21846..0a531d1 100644
--- a/libmultipath/dict.c
+++ b/libmultipath/dict.c
@@ -249,6 +249,8 @@ declare_ovr_snprint(selector, print_str)
 declare_mp_handler(selector, set_str)
 declare_mp_snprint(selector, print_str)
 
+declare_def_handler(uid_attrs, set_str)
+declare_def_snprint(uid_attrs, print_str)
 declare_def_handler(uid_attribute, set_str)
 declare_def_snprint_defstr(uid_attribute, print_str, DEFAULT_UID_ATTRIBUTE)
 declare_ovr_handler(uid_attribute, set_str)
@@ -1367,6 +1369,7 @@ init_keywords(vector keywords)
install_keyword("multipath_dir", _multipath_dir_handler, 
_def_multipath_dir);
install_keyword("path_selector", _selector_handler, 
_def_selector);
install_keyword("path_grouping_policy", _pgpolicy_handler, 
_def_pgpolicy);
+   install_keyword("uid_attrs", _uid_attrs_handler, 
_def_uid_attrs);
install_keyword("uid_attribute", _uid_attribute_handler, 
_def_uid_attribute);
install_keyword("getuid_callout", _getuid_handler, 
_def_getuid);
install_keyword("prio", _prio_name_handler, _def_prio_name);
diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index d1aec31..14904f2 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -33,7 +33,7 @@
 
 int
 alloc_path_with_pathinfo (struct config *conf, struct udev_device *udevice,
- int flag, struct path **pp_ptr)
+ char *wwid, int flag, struct path **pp_ptr)
 {
int err = PATHINFO_FAILED;
struct path * pp;
@@ -51,6 +51,9 @@ alloc_path_with_pathinfo (struct config *conf, struct 
udev_device *udevice,
if (!pp)
return PATHINFO_FAILED;
 
+   if(wwid)
+   strncpy(pp->wwid, wwid, sizeof(pp->wwid));
+
if (safe_sprintf(pp->dev, "%s", devname)) {
   

[dm-devel] [PATCH] multipath-tools: improve processing efficiency for addition and deletion of multipath devices

[tang . junhui]

From: "tang.junhui" <tang.jun...@zte.com.cn>

Change-Id: I3f81a55fff389f991f915927000b281d7e263cc5
Signed-off-by: tang.junhui <tang.jun...@zte.com.cn>

This patch used to improve processing efficiency for addition and deletion
of multipath devices.

This patch is tested pass by ZTE multipath automatic testing system.
The modification reduces the system consumption(such as CPU) and shortens
the processing time obviously in scene of massive multipath devices
addition or deletion.

The main processing flow of code is:
1) add uid_attrs configuration in the defaults section:
   It is configured udev attribute which providing a unique path identifier
   for corresponding type of path devices. If this field is configured and
   matched with type of device, it would override any other methods providing
   for device unique identifier in config file, and it would activate merging
   uevents according to the identifier to promote effiecncy in processing
   uevents. Tt has no default value, so defaultly only uevents filtering
   works, and uevents merging does not works, if users want to identify path
   by udev attribute and to activate merging uevents for SCSI and DAS device,
   they can set it's value as:
   "sd:ID_SERIAL dasd:ID_UID"
2) uevents accumulation in uevents burst scene:
   wait one seconds for more uevents in uevent_listen() in uevents burst
   situations
3) uevents preparing, filtering and merging:
   discard unuse uevents and fetch path idendifier from uevents;
   filter uevents;
   merge uevents.
4) uevents proccessing:
   proccess the merged uevents in uev->merge_node list without calling
   domap();
   proccess the last uevents uev with calling domap().

Any comment will be welcome, and it would be appreciated if these
patches would be considered for inclusion in the upstream
multipath-tools.

Thank you all,
Tang Junhui
---
 libmultipath/config.c  |   3 +
 libmultipath/config.h  |   1 +
 libmultipath/dict.c|   3 +
 libmultipath/discovery.c   |   5 +-
 libmultipath/discovery.h   |   2 +-
 libmultipath/list.h|  41 ++
 libmultipath/propsel.c |   7 +
 libmultipath/uevent.c  | 319 +++--
 libmultipath/uevent.h  |   2 +
 libmultipath/util.c|  40 ++
 libmultipath/util.h|   1 +
 multipath/multipath.conf.5 |  18 +++
 multipathd/cli_handlers.c  |   4 +-
 multipathd/main.c  |  93 +
 multipathd/main.h  |   4 +-
 15 files changed, 468 insertions(+), 75 deletions(-)

diff --git a/libmultipath/config.c b/libmultipath/config.c
index 15ddbd8..765e91d 100644
--- a/libmultipath/config.c
+++ b/libmultipath/config.c
@@ -488,6 +488,9 @@ free_config (struct config * conf)
if (conf->uid_attribute)
FREE(conf->uid_attribute);
 
+   if (conf->uid_attrs)
+   FREE(conf->uid_attrs);
+
if (conf->getuid)
FREE(conf->getuid);
 
diff --git a/libmultipath/config.h b/libmultipath/config.h
index 9670020..ab85930 100644
--- a/libmultipath/config.h
+++ b/libmultipath/config.h
@@ -153,6 +153,7 @@ struct config {
 
char * multipath_dir;
char * selector;
+   char * uid_attrs;
char * uid_attribute;
char * getuid;
char * features;
diff --git a/libmultipath/dict.c b/libmultipath/dict.c
index dc21846..0a531d1 100644
--- a/libmultipath/dict.c
+++ b/libmultipath/dict.c
@@ -249,6 +249,8 @@ declare_ovr_snprint(selector, print_str)
 declare_mp_handler(selector, set_str)
 declare_mp_snprint(selector, print_str)
 
+declare_def_handler(uid_attrs, set_str)
+declare_def_snprint(uid_attrs, print_str)
 declare_def_handler(uid_attribute, set_str)
 declare_def_snprint_defstr(uid_attribute, print_str, DEFAULT_UID_ATTRIBUTE)
 declare_ovr_handler(uid_attribute, set_str)
@@ -1367,6 +1369,7 @@ init_keywords(vector keywords)
install_keyword("multipath_dir", _multipath_dir_handler, 
_def_multipath_dir);
install_keyword("path_selector", _selector_handler, 
_def_selector);
install_keyword("path_grouping_policy", _pgpolicy_handler, 
_def_pgpolicy);
+   install_keyword("uid_attrs", _uid_attrs_handler, 
_def_uid_attrs);
install_keyword("uid_attribute", _uid_attribute_handler, 
_def_uid_attribute);
install_keyword("getuid_callout", _getuid_handler, 
_def_getuid);
install_keyword("prio", _prio_name_handler, _def_prio_name);
diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index d1aec31..14904f2 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -33,7 +33,7 @@
 
 int
 alloc_path_with_pathinfo (struct config *conf, struct udev_device *udevice,
- int flag, struct path **pp_ptr)
+ char *wwid, int flag, struct path **pp_ptr)
 {
int err = PATHINFO_FAILED;
struc

Re: [dm-devel] [LSF/MM TOPIC][LSF/MM ATTEND] multipath redesign

[tang . junhui]

Hello Hannes:

As a mulitpath developer, I find that multipath is getting bigger
and harder to maintain now, and I'm really looking forward to this
change, and I hope to be able to devote myself to this change too.
I am very interested in any news of the multipath redesign and
hope to see results soon.

I can't imagine what the new multipath looks like, but I suggest
some bad places for the current multipath we should avoid:
1) coarse grained lock;
2) vectors;
3) waiter thread;
4) high coupling;
5) too many configurations;
I really hope we can make a clean, efficient and easy to use
multipath.

Thank you
Tang Junhui



发件人: Hannes Reinecke <h...@suse.de>
收件人: Mike Snitzer <snit...@redhat.com>, 
抄送:   "linux-bl...@vger.kernel.org" <linux-bl...@vger.kernel.org>, 
"lsf...@lists.linux-foundation.org" <lsf...@lists.linux-foundation.org>, 
device-mapper development <dm-devel@redhat.com>, 
"linux-s...@vger.kernel.org" <linux-s...@vger.kernel.org>
日期:   2017/01/14 01:52
主题:   Re: [dm-devel] [LSF/MM TOPIC][LSF/MM ATTEND] multipath redesign
发件人: dm-devel-boun...@redhat.com



On 01/13/2017 05:07 PM, Mike Snitzer wrote:
> On Fri, Jan 13 2017 at 10:56am -0500,
> Hannes Reinecke <h...@suse.de> wrote:
> 
>> On 01/12/2017 06:29 PM, Benjamin Marzinski wrote:
[ .. ]
>>> While I've daydreamed of rewriting the multipath tools multiple times,
>>> and having nothing aginst you doing it in concept, I would be happier
>>> knowing that it won't simply mean that there are two sets of tools, 
that
>>> both need to be supported to deal with all customer configurations.
>>>
>> Sure. I feel the pain of supporting multipath-tools all too strongly.
>> Having two tools for the same thing is always a pain, and I would like
>> to avoid this if at all possible.
> 
> I welcome your work.  Should help us focus on what fat needs to be
> trimmed from both multipath-tools and kernel.
> 
> Might be a good time to branch multipath-tools and get very aggressive
> with trimming stuff that is outdated.
> 
> Things like the event stuff, using select interface, that Andy Grover is
> working on (and Mikulas is taking a stab at finishing/optimizing) is
> something that might help... but your approach described in this thread
> may prove better.
> 
> Point is, everything should be on the table for revitalizing multipath
> userspace (and kernel) to meet new requirements (e.g. NVMEoF, etc).
> 
> And yes, I'd prefer to ultimately see these advances land in terms of DM
> multipath advances but we'll take it as it comes.

I'm fully on board with that.
And it would be good if Ben Marzinski would be present, too;
he might have some insights which both of us might lack
(like the ominous dm-event interface into multipathd where we both
struggle to figure out what it's for ...)

Looking forward to that discussion.
And promising to have some results by then.

Cheers,

Hannes
-- 
Dr. Hannes ReineckezSeries & Storage
h...@suse.de   +49 911 
74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)

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


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

[dm-devel] [PATCH 00/11] multipath-tools: improve processing efficiency for addition and deletion of multipath devices

[tang . junhui]

From: tang.junhui <tang.jun...@zte.com.cn>

Hello Christophe, Ben, Hannes, Martin, Bart, and other guys:

This series of patches are modified base on:
[PATCH 00/12] multipath-tools: improve processing efficiency for addition 
and deletion of multipath devices

Modifications include follows:

Martin suggested me to use named constants rather than explicit numbers
Thanks to Martin. 

And Ben made several good suggestions:
1) Use *_safe list literation in traversing a list while removing elements
2) Do not move uevent_can_discard() to uevents listening thread in avoiding
   of socket's buffer fills up
3) Filter path change uevents while path addition uevent having not ever
   been processed
4) Remove add and change uevents if they are followed by a remove event, 
   regardless of whether or not they have a wwid
5) Do not change how the wwid is set, and call domap() on the device if its 
   wwid is different from its merge_id.
Thanks to Ben too. 

The eleven patches in this series are used to improve processing
efficiency for addition and deletion of multipath devices.

This patches are tested pass by ZTE multipath automatic testing syetem.
The modification reduces the system consumption(such as CPU) and shortens
the processing time obviously in scene of massive multipath devices
addition or deletion.

The main processing flow of code is:
1)uevents accumulation in uevents burst scene:
  --[pacth]libmultipath: wait one seconds for more uevents in
uevent_listen() in uevents burst situations
2)uevents filtering and merging:
  --[pacth]multipathd: merge uevents before proccessing
  --[pacth]libmultipath: filter uevents before proccessing
  --[patch]multipathd: move uev_discard() to uevent.c and change its name
to uevent_can_discard()
  --[patch]multipathd: move calling filter_devnode() from uev_trigger() to
uevent_can_discard()
3)uevents proccessing:
  --[pacth]multipathd: add need_do_map to indicate whether need calling
domap() in ev_add_path()
  --[pacth]multipathd: add need_do_map to indicate whether need calling
domap() in ev_remove_path()
  --[pacth]multipathd: proccess merged uevents

There are some patches providing basic struct members or maros which
would used in above pathes:
  --[patch]libmultipath: add merge_id in "struct uevent" for uevents merging
  --[patch]libmultipath: add merge_node for "struct uevent" to record
nodes of merged uevents
  --[patch]libmultipath: add three list iteration macros

Any comment will be welcome, and it would be appreciated if these
patches would be considered for inclusion in the upstream
multipath-tools.

Thank you all,
Tang Junhui

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

[dm-devel] [PATCH 03/11] libmultipath: add three list iteration macros

[tang . junhui]

From: tang.junhui 

Add three list iteration macros, list_for_each_entry_reverse_safe
is used for safe list iteration, and the other two macros are used
to iterate list forwards or backwards from the given begin node to
the given end node, which would be used in merging uevents.

Change-Id: I8bb53fef9276bb62a5e0f4fdac6455086dc03d9b
Signed-off-by: tang.junhui 
---
 libmultipath/list.h | 41 +
 1 file changed, 41 insertions(+)

diff --git a/libmultipath/list.h b/libmultipath/list.h
index ceaa381..4433167 100644
--- a/libmultipath/list.h
+++ b/libmultipath/list.h
@@ -317,4 +317,45 @@ static inline void list_splice_tail_init(struct list_head 
*list,
 >member != (head);\
 pos = n, n = list_entry(n->member.next, typeof(*n), member))
 
+/**
+ * list_for_each_entry_reverse - iterate backwards over list of given type.
+ * @pos:   the type * to use as a loop counter.
+ * @n: another type * to use as temporary storage
+ * @head:  the head for your list.
+ * @member:the name of the list_struct within the struct.
+ */
+#define list_for_each_entry_reverse_safe(pos, n, head, member)  \
+   for (pos = list_entry((head)->prev, typeof(*pos), member),   \
+n = list_entry(pos->member.prev, typeof(*pos), member); \
+>member != (head);\
+pos = n, n = list_entry(n->member.prev, typeof(*n), member))
+
+/**
+ * list_for_some_entry -   iterate list of given interval
+ * @pos:   the type * to use as a loop counter.
+ * @n: another type * to use as temporary storage
+ * @from:  the begin node of the iteration.
+ * @to:the end node of the iteration.
+ * @member:the name of the list_struct within the struct.
+ */
+#define list_for_some_entry_safe(pos, n, from, to, member)  \
+   for (pos = list_entry((from)->next, typeof(*pos), member),   \
+n = list_entry(pos->member.next, typeof(*pos), member); \
+>member != (to);  
 \
+pos = n, n = list_entry(n->member.next, typeof(*n), member))
+
+/**
+ * list_for_some_entry_reverse -   iterate backwards list of given interval
+ * @pos:   the type * to use as a loop counter.
+ * @n: another type * to use as temporary storage
+ * @from:  the begin node of the iteration.
+ * @to:the end node of the iteration.
+ * @member:the name of the list_struct within the struct.
+ */
+#define list_for_some_entry_reverse_safe(pos, n, from, to, member) \
+   for (pos = list_entry((from)->prev, typeof(*pos), member), \
+n = list_entry(pos->member.prev, typeof(*pos), member);   \
+>member != (to);  
   \
+pos = n, n = list_entry(n->member.prev, typeof(*n), member))
+
 #endif /* _LIST_H */
-- 
2.8.1.windows.1


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

[dm-devel] [PATCH 01/11] libmultipath: add merge_id in "struct uevent" for uevents merging

[tang . junhui]

From: tang.junhui 

merge_id is used for uevents merging, it is expected to be same with wwid,
uevents with the same merge_id will be merged, and if it is same with
wwid, only the last uevent will call domap(). now we only merge uevents
with label ID_SERIAL or ID_UID in uevent, which usually comming from SCSI
or DASD device.

Change-Id: Ie6b076363b3735dc7de10184b27fa799b499af0e
Signed-off-by: tang.junhui 
---
 libmultipath/uevent.c | 16 
 libmultipath/uevent.h |  1 +
 2 files changed, 17 insertions(+)

diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c
index 7edcce1..5bde864 100644
--- a/libmultipath/uevent.c
+++ b/libmultipath/uevent.c
@@ -424,6 +424,22 @@ struct uevent *uevent_from_udev_device(struct udev_device 
*dev)
uev->devpath = uev->envp[i] + 8;
if (strcmp(name, "ACTION") == 0)
uev->action = uev->envp[i] + 7;
+
+   /*
+* merge_id used for uevents merging
+* it is expected to be same with wwid, 
+* uevents with the same merge_id
+* will be merged, and if it is same with wwid
+* only the last uevent will call domap()
+* now we only merge uevents with
+* label ID_SERIAL or ID_UID, which usually comming
+* from SCSI or DASD device
+*/
+   if (strcmp(name, "ID_SERIAL") == 0)
+   uev->merge_id = uev->envp[i] + 10;
+   else if (strcmp(name, "ID_UID") == 0)   
+   uev->merge_id = uev->envp[i] + 7;
+
i++;
if (i == HOTPLUG_NUM_ENVP - 1)
break;
diff --git a/libmultipath/uevent.h b/libmultipath/uevent.h
index 9d22dcd..3f83bab 100644
--- a/libmultipath/uevent.h
+++ b/libmultipath/uevent.h
@@ -22,6 +22,7 @@ struct uevent {
char *devpath;
char *action;
char *kernel;
+   char *merge_id;
unsigned long seqnum;
char *envp[HOTPLUG_NUM_ENVP];
 };
-- 
2.8.1.windows.1


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

[dm-devel] [PATCH 04/11] multipathd: add need_do_map to indicate whether need calling domap() in ev_add_path()

[tang . junhui]

From: tang.junhui 

Usually calling domap() in ev_add_path() is needed, but only last path
need to call domap() in processing for merged uevents to reduce the
count of calling domap() and promote efficiency. So add input parameter
need_do_map to indicate whether need calling domap() in ev_add_path().

Change-Id: I7f33de49a1d89d91180dcb95cd94e4944ae7ff36
Signed-off-by: tang.wenjun 
---
 multipathd/cli_handlers.c |  3 ++-
 multipathd/main.c | 47 ---
 multipathd/main.h |  2 +-
 3 files changed, 39 insertions(+), 13 deletions(-)

diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c
index b0eeca6..3a46c09 100644
--- a/multipathd/cli_handlers.c
+++ b/multipathd/cli_handlers.c
@@ -670,7 +670,8 @@ cli_add_path (void * v, char ** reply, int * len, void * 
data)
pp->checkint = conf->checkint;
}
put_multipath_config(conf);
-   return ev_add_path(pp, vecs);
+   return ev_add_path(pp, vecs, 1);
+
 blacklisted:
*reply = strdup("blacklisted\n");
*len = strlen(*reply) + 1;
diff --git a/multipathd/main.c b/multipathd/main.c
index adc3258..ebd7de1 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -608,7 +608,7 @@ ev_remove_map (char * devname, char * alias, int minor, 
struct vectors * vecs)
 }
 
 static int
-uev_add_path (struct uevent *uev, struct vectors * vecs)
+uev_add_path (struct uevent *uev, struct vectors * vecs, int need_do_map)
 {
struct path *pp;
int ret = 0, i;
@@ -640,9 +640,18 @@ uev_add_path (struct uevent *uev, struct vectors * vecs)
r = pathinfo(pp, conf,
 DI_ALL | DI_BLACKLIST);
put_multipath_config(conf);
-   if (r == PATHINFO_OK)
-   ret = ev_add_path(pp, vecs);
-   else if (r == PATHINFO_SKIPPED) {
+   if (r == PATHINFO_OK) {
+   /*
+* Make sure merging use the correct wwid
+* Othterwise calling domap()
+*/
+   if (!need_do_map &&
+   uev->merge_id &&
+   strcmp(uev->merge_id, pp->wwid))
+   need_do_map = 1;
+
+   ret = ev_add_path(pp, vecs, need_do_map);
+   } else if (r == PATHINFO_SKIPPED) {
condlog(3, "%s: remove blacklisted path",
uev->kernel);
i = find_slot(vecs->pathvec, (void *)pp);
@@ -681,7 +690,16 @@ uev_add_path (struct uevent *uev, struct vectors * vecs)
conf = get_multipath_config();
pp->checkint = conf->checkint;
put_multipath_config(conf);
-   ret = ev_add_path(pp, vecs);
+   /*
+* Make sure merging use the correct wwid
+* Othterwise calling domap()
+*/
+   if (!need_do_map &&
+   uev->merge_id &&
+   strcmp(uev->merge_id, pp->wwid))
+   need_do_map = 1;
+
+   ret = ev_add_path(pp, vecs, need_do_map);
} else {
condlog(0, "%s: failed to store path info, "
"dropping event",
@@ -699,7 +717,7 @@ uev_add_path (struct uevent *uev, struct vectors * vecs)
  * 1: error
  */
 int
-ev_add_path (struct path * pp, struct vectors * vecs)
+ev_add_path (struct path * pp, struct vectors * vecs, int need_do_map)
 {
struct multipath * mpp;
char params[PARAMS_SIZE] = {0};
@@ -767,6 +785,13 @@ rescan:
/* persistent reservation check*/
mpath_pr_event_handle(pp);
 
+   if (!need_do_map)
+   return 0;
+
+   if (!dm_map_present(mpp->alias)) {
+   mpp->action = ACT_CREATE;
+   start_waiter = 1;
+   }
/*
 * push the map to the device-mapper
 */
@@ -995,7 +1020,7 @@ uev_update_path (struct uevent *uev, struct vectors * vecs)
}
 
if (pp->initialized == INIT_REQUESTED_UDEV)
-   retval = uev_add_path(uev, vecs);
+   retval = uev_add_path(uev, vecs, 1);
else if (mpp && ro >= 0) {
condlog(2, "%s: update path write_protect to '%d' 
(uevent)", uev->kernel, ro);
 
@@ -1150,7 +1175,7 @@ uev_trigger (struct uevent * uev, void * trigger_data)
put_multipath_config(conf);
 
if (!strncmp(uev->action, "add", 3)) {
-   r = uev_add_path(uev, vecs);
+   r = uev_add_path(uev, vecs, 1);
goto out;
}
if (!strncmp(uev->action, "remove", 6)) {
@@ 

[dm-devel] [PATCH 06/11] multipathd: move uev_discard() to uevent.c and change its name to uevent_can_discard()

[tang . junhui]

From: tang.junhui 

This patch is mainly done to adjust the code for the preparation of
uevents merging.

Change-Id: Iaac159ffe3930e53c3325d1069c3ed497e440c0c
Signed-off-by: tang.wenjun 
---
 libmultipath/uevent.c | 40 
 multipathd/main.c | 25 -
 2 files changed, 40 insertions(+), 25 deletions(-)

diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c
index 9b6b1d1..d6c02a6 100644
--- a/libmultipath/uevent.c
+++ b/libmultipath/uevent.c
@@ -24,6 +24,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -80,6 +81,44 @@ struct uevent * alloc_uevent (void)
return uev;
 }
 
+bool
+uevent_can_discard(struct uevent *uev)
+{
+   char *tmp;
+   char a[11], b[11];
+
+   /*
+* keep only block devices, discard partitions
+*/
+   tmp = strstr(uev->devpath, "/block/");
+   if (tmp == NULL){
+   condlog(4, "no /block/ in '%s'", uev->devpath);
+   return true;
+   }
+   if (sscanf(tmp, "/block/%10s", a) != 1 ||
+   sscanf(tmp, "/block/%10[^/]/%10s", a, b) == 2) {
+   condlog(4, "discard event on %s", uev->devpath);
+   return true;
+   }
+
+   return false;
+}
+
+void
+uevent_discard(struct list_head *tmpq)
+{
+   struct uevent *uev, *tmp;
+
+   list_for_each_entry_reverse_safe(uev, tmp, tmpq, node) {
+   if (uevent_can_discard(uev)) {
+   list_del_init(>node);
+   if (uev->udev)
+   udev_device_unref(uev->udev);
+   FREE(uev);
+   }
+   }
+}
+
 void
 service_uevq(struct list_head *tmpq)
 {
@@ -144,6 +183,7 @@ int uevent_dispatch(int (*uev_trigger)(struct uevent *, 
void * trigger_data),
pthread_mutex_unlock(uevq_lockp);
if (!my_uev_trigger)
break;
+   uevent_discard(_tmp);
service_uevq(_tmp);
}
condlog(3, "Terminating uev service queue");
diff --git a/multipathd/main.c b/multipathd/main.c
index 718c5e7..66d5c3d 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1115,28 +1115,6 @@ uxsock_trigger (char * str, char ** reply, int * len, 
void * trigger_data)
return r;
 }
 
-static int
-uev_discard(char * devpath)
-{
-   char *tmp;
-   char a[11], b[11];
-
-   /*
-* keep only block devices, discard partitions
-*/
-   tmp = strstr(devpath, "/block/");
-   if (tmp == NULL){
-   condlog(4, "no /block/ in '%s'", devpath);
-   return 1;
-   }
-   if (sscanf(tmp, "/block/%10s", a) != 1 ||
-   sscanf(tmp, "/block/%10[^/]/%10s", a, b) == 2) {
-   condlog(4, "discard event on %s", devpath);
-   return 1;
-   }
-   return 0;
-}
-
 int
 uev_trigger (struct uevent * uev, void * trigger_data)
 {
@@ -1146,9 +1124,6 @@ uev_trigger (struct uevent * uev, void * trigger_data)
 
vecs = (struct vectors *)trigger_data;
 
-   if (uev_discard(uev->devpath))
-   return 0;
-
pthread_cleanup_push(config_cleanup, NULL);
pthread_mutex_lock(_lock);
if (running_state != DAEMON_IDLE &&
-- 
2.8.1.windows.1


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

[dm-devel] [PATCH 08/11] libmultipath: wait one seconds for more uevents in uevent_listen() in uevents burst situations

[tang . junhui]

From: tang.junhui 

The more uevents are merged, the higher efficiency program will performs.
So, do not process uevents after receiving immediately in uevents burst
situations, but continue wait one seconds for more uevents except that
too much uevents (2048) have already been received or too much time
eclipse (30 seconds).

Change-Id: I763d491540e8114a81d12d603281540a81502742
Signed-off-by: tang.junhui 
---
 libmultipath/uevent.c | 42 +-
 1 file changed, 41 insertions(+), 1 deletion(-)

diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c
index b560a50..f231dad 100644
--- a/libmultipath/uevent.c
+++ b/libmultipath/uevent.c
@@ -39,6 +39,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -51,6 +52,10 @@
 #include "config.h"
 #include "blacklist.h"
 
+#define MAX_ACCUMULATION_COUNT 2048
+#define MAX_ACCUMULATION_TIME 30*1000
+#define MIN_BURST_SPEED 10
+
 typedef int (uev_trigger)(struct uevent *, void * trigger_data);
 
 LIST_HEAD(uevq);
@@ -520,11 +525,43 @@ struct uevent *uevent_from_udev_device(struct udev_device 
*dev)
return uev;
 }
 
+bool uevent_burst(struct timeval *start_time, int events)
+{
+   struct timeval diff_time, end_time;
+   unsigned long speed;
+   unsigned long eclipse_ms;
+
+   if(events > MAX_ACCUMULATION_COUNT) {
+   condlog(2, "burst got %u uevents, too much uevents, stopped", 
events);
+   return false;
+   }
+
+   gettimeofday(_time, NULL);
+   timersub(_time, start_time, _time);
+
+   eclipse_ms = diff_time.tv_sec * 1000 + diff_time.tv_usec / 1000;
+
+   if (eclipse_ms == 0)
+   return true;
+
+   if (eclipse_ms > MAX_ACCUMULATION_TIME) {
+   condlog(2, "burst continued %lu ms, too long time, stopped", 
eclipse_ms);
+   return false;
+   }
+
+   speed = (events * 1000) / eclipse_ms;
+   if (speed > MIN_BURST_SPEED)
+   return true;
+
+   return false;
+}
+
 int uevent_listen(struct udev *udev)
 {
int err = 2;
struct udev_monitor *monitor = NULL;
int fd, socket_flags, events;
+   struct timeval start_time;
int need_failback = 1;
int timeout = 30;
LIST_HEAD(uevlisten_tmp);
@@ -578,6 +615,7 @@ int uevent_listen(struct udev *udev)
}
 
events = 0;
+   gettimeofday(_time, NULL);
while (1) {
struct uevent *uev;
struct udev_device *dev;
@@ -592,7 +630,8 @@ int uevent_listen(struct udev *udev)
errno = 0;
fdcount = poll(_poll, 1, poll_timeout);
if (fdcount && ev_poll.revents & POLLIN) {
-   timeout = 0;
+   timeout = uevent_burst(_time, events + 1) ? 1 : 0;
+
dev = udev_monitor_receive_device(monitor);
if (!dev) {
condlog(0, "failed getting udev device");
@@ -625,6 +664,7 @@ int uevent_listen(struct udev *udev)
pthread_mutex_unlock(uevq_lockp);
events = 0;
}
+   gettimeofday(_time, NULL);
timeout = 30;
}
need_failback = 0;
-- 
2.8.1.windows.1


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

[dm-devel] [PATCH 10/11] libmultipath: filter uevents before proccessing

[tang . junhui]

From: tang.junhui 

Before merging uevents, these uevents are going to be filtered:
1) Change or addition uevent of a removed path (it indicate by an
deletion uevent occurred later), and
2) Change uevent if addition uevent of a path has not ever been
proccessed.

Change-Id: If00c2c2e23ea466c1d4643c541ed2d8f9a0c8dea
Signed-off-by: tang.junhui 
---
 libmultipath/uevent.c | 58 +++
 1 file changed, 58 insertions(+)

diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c
index 7282a51..838f128 100644
--- a/libmultipath/uevent.c
+++ b/libmultipath/uevent.c
@@ -144,6 +144,40 @@ uevent_can_discard(struct uevent *uev)
 }
 
 bool
+uevent_can_filter(struct uevent *earlier, struct uevent *later)
+{
+
+   /*
+* filter earlier uvents if path has removed later. Eg:
+* "add path1 |chang path1 |add path2 |remove path1"
+* can filter as:
+* "add path2 |remove path1"
+* uevents "add path1" and "chang path1" are filtered out
+*/
+   if (!strcmp(earlier->kernel, later->kernel) &&
+   !strcmp(later->action, "remove") &&
+   strncmp(later->kernel, "dm-", 3)) {
+   return true;
+   }
+
+   /*
+* filter change uvents if add uevents exist. Eg:
+* "change path1| add path1 |add path2"
+* can filter as:
+* "add path1 |add path2"
+* uevent "chang path1" is filtered out
+*/
+   if (!strcmp(earlier->kernel, later->kernel) &&
+   !strcmp(earlier->action, "change") &&
+   !strcmp(later->action, "add") &&
+   strncmp(later->kernel, "dm-", 3)) {
+   return true;
+   }
+
+   return false;
+}
+
+bool
 merge_need_stop(struct uevent *earlier, struct uevent *later)
 {
/*
@@ -215,6 +249,29 @@ uevent_discard(struct list_head *tmpq)
 }
 
 void
+uevent_filter(struct uevent *later, struct list_head *tmpq)
+{
+   struct uevent *earlier, *tmp;
+
+   list_for_some_entry_reverse_safe(earlier, tmp, >node, tmpq, 
node) {
+   /*
+* filter unnessary earlier uevents 
+* by the later uevent
+*/
+   if (uevent_can_filter(earlier, later)) {
+   condlog(2, "uevent: %s-%s has filtered by uevent: 
%s-%s",
+   earlier->kernel, earlier->action, 
+   later->kernel, later->action);
+
+   list_del_init(>node);
+   if (earlier->udev)
+   udev_device_unref(earlier->udev);
+   FREE(earlier);
+   }
+   }
+}
+
+void
 uevent_merge(struct uevent *later, struct list_head *tmpq)
 {
struct uevent *earlier, *tmp;
@@ -242,6 +299,7 @@ merge_uevq(struct list_head *tmpq)
 
uevent_discard(tmpq);
list_for_each_entry_reverse(later, tmpq, node) {
+   uevent_filter(later, tmpq);
uevent_merge(later, tmpq);
}
 }
-- 
2.8.1.windows.1


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

[dm-devel] [PATCH 07/11] multipathd: move calling filter_devnode() from uev_trigger() to uevent_can_discard()

[tang . junhui]

From: tang.junhui 

Move calling filter_devnode() from uev_trigger() to uevent_can_discard()
since they do the similar work.

Change-Id: I0322443fa551b21aa3211bf1ce3fad7d37aeeab4
Signed-off-by: tang.junhui 
---
 libmultipath/uevent.c | 20 
 multipathd/main.c |  9 -
 2 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c
index d6c02a6..b560a50 100644
--- a/libmultipath/uevent.c
+++ b/libmultipath/uevent.c
@@ -47,6 +47,9 @@
 #include "list.h"
 #include "uevent.h"
 #include "vector.h"
+#include "structs.h"
+#include "config.h"
+#include "blacklist.h"
 
 typedef int (uev_trigger)(struct uevent *, void * trigger_data);
 
@@ -86,6 +89,7 @@ uevent_can_discard(struct uevent *uev)
 {
char *tmp;
char a[11], b[11];
+   struct config * conf;
 
/*
 * keep only block devices, discard partitions
@@ -101,6 +105,22 @@ uevent_can_discard(struct uevent *uev)
return true;
}
 
+   /* 
+* do not filter dm devices by devnode
+*/
+   if (!strncmp(uev->kernel, "dm-", 3))
+   return false;
+   /* 
+* filter paths devices by devnode
+*/
+   conf = get_multipath_config();
+   if (filter_devnode(conf->blist_devnode, conf->elist_devnode,
+  uev->kernel) > 0) {
+   put_multipath_config(conf);
+   return true;
+   }
+   put_multipath_config(conf);
+
return false;
 }
 
diff --git a/multipathd/main.c b/multipathd/main.c
index 66d5c3d..24116e3 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1120,7 +1120,6 @@ uev_trigger (struct uevent * uev, void * trigger_data)
 {
int r = 0;
struct vectors * vecs;
-   struct config *conf;
 
vecs = (struct vectors *)trigger_data;
 
@@ -1154,14 +1153,6 @@ uev_trigger (struct uevent * uev, void * trigger_data)
/*
 * path add/remove event
 */
-   conf = get_multipath_config();
-   if (filter_devnode(conf->blist_devnode, conf->elist_devnode,
-  uev->kernel) > 0) {
-   put_multipath_config(conf);
-   goto out;
-   }
-   put_multipath_config(conf);
-
if (!strncmp(uev->action, "add", 3)) {
r = uev_add_path(uev, vecs, 1);
goto out;
-- 
2.8.1.windows.1


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

[dm-devel] [PATCH 02/11] libmultipath: add merge_node for "struct uevent" to record nodes of merged uevents

[tang . junhui]

From: tang.junhui 

Add merged nodes list to store nodes of merged uevents. By Adding
this member, after merging, the list of uevents would be linked like
this:
uevent
 ---
|struct list_head node  |->list node of un-merged uevents
 ---
|struct list_head merge_node|->list node of merged uevents, which
   moved from the origin un-merged list
|...|
 ---

Change-Id: I5fbfc7656ede77e03ca35c855212e2d2d60706b2
Signed-off-by: tang.junhui 
---
 libmultipath/uevent.c | 4 +++-
 libmultipath/uevent.h | 1 +
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c
index 5bde864..9b6b1d1 100644
--- a/libmultipath/uevent.c
+++ b/libmultipath/uevent.c
@@ -72,8 +72,10 @@ struct uevent * alloc_uevent (void)
 {
struct uevent *uev = MALLOC(sizeof(struct uevent));
 
-   if (uev)
+   if (uev) {
INIT_LIST_HEAD(>node);
+   INIT_LIST_HEAD(>merge_node);
+   }
 
return uev;
 }
diff --git a/libmultipath/uevent.h b/libmultipath/uevent.h
index 3f83bab..9f65327 100644
--- a/libmultipath/uevent.h
+++ b/libmultipath/uevent.h
@@ -17,6 +17,7 @@ struct udev;
 
 struct uevent {
struct list_head node;
+   struct list_head merge_node;
struct udev_device *udev;
char buffer[HOTPLUG_BUFFER_SIZE + OBJECT_SIZE];
char *devpath;
-- 
2.8.1.windows.1


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

[dm-devel] [PATCH 11/11] multipathd: proccess merged uevents

[tang . junhui]

From: "tang.junhui" 

After filtering and merging, then uevents are processed in
uev_trigger(), firstly, each of merged uevents would be processed one
by one with need_do_map in value of 0. Finally, the node “uev” itself
would be processed with need_do_map in value of 1, which would reload
kernel table, and create or remove the dm device.

Change-Id: Ifb4de0ca36180a8af16729c2f70bc250858877bd
Signed-off-by: tang.junhui 
---
 multipathd/main.c | 28 +++-
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/multipathd/main.c b/multipathd/main.c
index 24116e3..5b63577 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1120,6 +1120,7 @@ uev_trigger (struct uevent * uev, void * trigger_data)
 {
int r = 0;
struct vectors * vecs;
+   struct uevent *merge_uev, *tmp;
 
vecs = (struct vectors *)trigger_data;
 
@@ -1151,20 +1152,21 @@ uev_trigger (struct uevent * uev, void * trigger_data)
}
 
/*
-* path add/remove event
+* path add/remove/change event, add/remove maybe merged
 */
-   if (!strncmp(uev->action, "add", 3)) {
-   r = uev_add_path(uev, vecs, 1);
-   goto out;
-   }
-   if (!strncmp(uev->action, "remove", 6)) {
-   r = uev_remove_path(uev, vecs, 1);
-   goto out;
-   }
-   if (!strncmp(uev->action, "change", 6)) {
-   r = uev_update_path(uev, vecs);
-   goto out;
-   }
+   list_for_each_entry_safe(merge_uev, tmp, >merge_node, node) {
+   if (!strncmp(merge_uev->action, "add", 3))
+   r += uev_add_path(merge_uev, vecs, 0);
+   if (!strncmp(merge_uev->action, "remove", 6))
+   r += uev_remove_path(merge_uev, vecs, 0);
+   }
+
+   if (!strncmp(uev->action, "add", 3))
+   r += uev_add_path(uev, vecs, 1);
+   if (!strncmp(uev->action, "remove", 6))
+   r += uev_remove_path(uev, vecs, 1);
+   if (!strncmp(uev->action, "change", 6))
+   r += uev_update_path(uev, vecs);
 
 out:
return r;
-- 
2.8.1.windows.1



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

[dm-devel] [PATCH 05/11] multipathd: add need_do_map to indicate whether need calling domap() in ev_remove_path()

[tang . junhui]

From: tang.junhui 

Usually calling domap() in ev_remove_path() is needed, but only last
path need to call domap() in processing for merged uevents to reduce the
count of calling domap() and promote efficiency. So add input parameter
need_do_map to indicate whether need calling domap() in ev_remove_path().

Change-Id: I0a787330a249608396cc3e655465dc820f940539
Signed-off-by: tang.wenjun 
---
 multipathd/cli_handlers.c |  2 +-
 multipathd/main.c | 23 ++-
 multipathd/main.h |  2 +-
 3 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c
index 3a46c09..302fd02 100644
--- a/multipathd/cli_handlers.c
+++ b/multipathd/cli_handlers.c
@@ -693,7 +693,7 @@ cli_del_path (void * v, char ** reply, int * len, void * 
data)
condlog(0, "%s: path already removed", param);
return 1;
}
-   return ev_remove_path(pp, vecs);
+   return ev_remove_path(pp, vecs, 1);
 }
 
 int
diff --git a/multipathd/main.c b/multipathd/main.c
index ebd7de1..718c5e7 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -858,7 +858,7 @@ fail:
 }
 
 static int
-uev_remove_path (struct uevent *uev, struct vectors * vecs)
+uev_remove_path (struct uevent *uev, struct vectors * vecs, int need_do_map)
 {
struct path *pp;
int ret;
@@ -868,8 +868,18 @@ uev_remove_path (struct uevent *uev, struct vectors * vecs)
lock(>lock);
pthread_testcancel();
pp = find_path_by_dev(vecs->pathvec, uev->kernel);
-   if (pp)
-   ret = ev_remove_path(pp, vecs);
+   if (pp) {
+   /*
+* Make sure merging use the correct wwid
+* Othterwise calling domap()
+*/
+   if (!need_do_map &&
+   uev->merge_id &&
+   strcmp(uev->merge_id, pp->wwid))
+   need_do_map = 1;
+   
+   ret = ev_remove_path(pp, vecs, need_do_map);
+   }
lock_cleanup_pop(vecs->lock);
if (!pp) {
/* Not an error; path might have been purged earlier */
@@ -880,7 +890,7 @@ uev_remove_path (struct uevent *uev, struct vectors * vecs)
 }
 
 int
-ev_remove_path (struct path *pp, struct vectors * vecs)
+ev_remove_path (struct path *pp, struct vectors * vecs, int need_do_map)
 {
struct multipath * mpp;
int i, retval = 0;
@@ -902,6 +912,9 @@ ev_remove_path (struct path *pp, struct vectors * vecs)
if ((i = find_slot(mpp->paths, (void *)pp)) != -1)
vector_del_slot(mpp->paths, i);
 
+   if(!need_do_map)
+   goto out;
+
/*
 * remove the map IFF removing the last path
 */
@@ -1179,7 +1192,7 @@ uev_trigger (struct uevent * uev, void * trigger_data)
goto out;
}
if (!strncmp(uev->action, "remove", 6)) {
-   r = uev_remove_path(uev, vecs);
+   r = uev_remove_path(uev, vecs, 1);
goto out;
}
if (!strncmp(uev->action, "change", 6)) {
diff --git a/multipathd/main.h b/multipathd/main.h
index f810d41..094b04f 100644
--- a/multipathd/main.h
+++ b/multipathd/main.h
@@ -23,7 +23,7 @@ const char * daemon_status(void);
 int need_to_delay_reconfig (struct vectors *);
 int reconfigure (struct vectors *);
 int ev_add_path (struct path *, struct vectors *, int);
-int ev_remove_path (struct path *, struct vectors *);
+int ev_remove_path (struct path *, struct vectors *, int);
 int ev_add_map (char *, char *, struct vectors *);
 int ev_remove_map (char *, char *, int, struct vectors *);
 void sync_map_state (struct multipath *);
-- 
2.8.1.windows.1


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

Re: [dm-devel] [PATCH 01/12] libmultipath: add wwid for "struct uevent" to record wwid of uevent

[tang . junhui]

Hello Ben,


OK, I will modify code to call domap() on the device
if its wwid is different from its merge_id.

I'll resend a serials of patch later,
Thank you for your patience. 

Regards,
Tang Junhui





发件人: "Benjamin Marzinski" <bmarz...@redhat.com>
收件人: tang.jun...@zte.com.cn, 
抄送:   tang.wenj...@zte.com.cn, zhang.ka...@zte.com.cn, 
dm-devel-boun...@redhat.com, dm-devel@redhat.com, 
bart.vanass...@sandisk.com, mwi...@suse.com
日期:   2017/01/07 00:11
主题:   Re: [dm-devel] [PATCH 01/12] libmultipath: add wwid for "struct 
uevent" to record wwid of uevent
发件人: dm-devel-boun...@redhat.com



On Fri, Jan 06, 2017 at 08:59:27AM +0800, tang.jun...@zte.com.cn wrote:
>Hello Ben,
> 
>>>The premise is that, devices whose "merge_id" is the same 
have
>the
>>>same "wwid". I think this premise can be established.
> 
>>So, if you have device where the merge_id doesn't equal the 
wwid, you
>>call domap on that device, as it it wasn't part of a merged set?
> 
>Maybe I did not say clearly. "merge_id" is only used for uevents 
merging,
>and
>it has nothing to do with "wwid". I only primse devices whose 
"merge_id"
>are
>same have same "wwid" (Sorry, maybe I had a misleading statement in
>previous email).
>"merge_id" and "wwid" can be same or not same. The merged uevents are
>still
>processed as bellow patch I send previous:
>[dm-devel] [PATCH 11/12] multipathd: proccess merged uevents

Let me back up. Say you have a batch of merged uevents

"add path1, path2, path3"

where path1 and path2 are on path3's merge_node list. This means that
when you call uev_add_path() for path1 and path2, you will call it with
need_do_map set to 0, and no multipath device will be created. The
multipath device will be created when you call uev_add_path() for path3,
with need_no_map set to 1. Now lets say that when you call pathinfo on
path2, you get a different wwid than your merge_id.  path1 and path3 get
the same wwid as their merge_id. In this situation you would call domap
on path1 and path3, but nothing would create the multipath device for
path2.

Like I said, this is unlikely to happen.  But it could. Imagine that
there was some device that presented itself like a scsi device, but
ID_SERIAL could have the same value for devices that should not be
multipathed together. To figure out which devices actually should be
multipathed together, you need to call pathinfo and get the correct
wwid. In this case you would find that you had incorrectly batched
devices with different wwids together.

To solve this, you would need to make sure to call domap on the device
if its wwid ended up being different from its merge_id.  The issue
that my last email pointed out was that if the main uevent, path3 in the
above example, was the only one to have a different wwid, then you would
need to make sure to call domap on the other devices as well. Otherwise
you would create a multipath device for path3, but not path1 and path2.

If you don't handle these cases, you will only work correctly on the
(admittedly vastly more common) case where even if the wwid and merge_id
don't match, the wwid is still the same for all the batched devices.
But if you are already assuming that the merge_id will correctly group
the devices, then what is the purpose in fetching the wwid at all? It
may be different from the merge_id, but it will still group the devices
the same way. There's nothing special about the wwid value itself. It is
just a tool to correctly group the devices. So, I would say that you
should either not grab the wwid at all if you got it from the uevent, or
do grab the wwid, but be prepared to deal with the case where not all of
the devices that you have batched having the same wwid.

Does that make more sense?

-Ben

> 
>Regards,
>Tang Junhui
> 
>发件人: "Benjamin Marzinski" <bmarz...@redhat.com>
>收件人: tang.jun...@zte.com.cn,
>抄送:mwi...@suse.com, bart.vanass...@sandisk.com,
>dm-devel@redhat.com, dm-devel-boun...@redhat.com, 
tang.wenj...@zte.com.cn,
>zhang.ka...@zte.com.cn
>日期: 2017/01/06 01:37
>主题:Re: Re: [dm-devel] [PATCH 01/12] libmultipath: add wwid 
for
>"struct uevent" to record wwid of uevent
> 
> 
--
> 
>On Thu, Jan 05, 2017 at 11:10:43AM +0800, tang.jun...@zte.com.cn 
wrote:
>>Hello Ben,
>>
>>I know what you concern about. Maybe we can change the "wwid" 
member
>>in "struct uevent" to another name such as "merge_id" to only
>>ident

Re: [dm-devel] [PATCH 01/12] libmultipath: add wwid for "struct uevent" to record wwid of uevent

[tang . junhui]

Hello Ben,

>>The premise is that, devices whose "merge_id" is the same have 
the
>>same "wwid". I think this premise can be established.

>So, if you have device where the merge_id doesn't equal the wwid, you
>call domap on that device, as it it wasn't part of a merged set?

Maybe I did not say clearly. "merge_id" is only used for uevents merging, 
and
it has nothing to do with "wwid". I only primse devices whose "merge_id" 
are 
same have same "wwid" (Sorry, maybe I had a misleading statement in 
previous email). 
"merge_id" and "wwid" can be same or not same. The merged uevents are 
still 
processed as bellow patch I send previous:
[dm-devel] [PATCH 11/12] multipathd: proccess merged uevents


Regards,
Tang Junhui




发件人: "Benjamin Marzinski" <bmarz...@redhat.com>
收件人: tang.jun...@zte.com.cn, 
抄送:   mwi...@suse.com, bart.vanass...@sandisk.com, dm-devel@redhat.com, 
dm-devel-boun...@redhat.com, tang.wenj...@zte.com.cn, 
zhang.ka...@zte.com.cn
日期:   2017/01/06 01:37
主题:   Re: Re: [dm-devel] [PATCH 01/12] libmultipath: add wwid for 
"struct uevent" to record wwid of uevent



On Thu, Jan 05, 2017 at 11:10:43AM +0800, tang.jun...@zte.com.cn wrote:
>Hello Ben,
> 
>I know what you concern about. Maybe we can change the "wwid" member
>in "struct uevent" to another name such as "merge_id" to only
>identify which uevents can merge together. The value of "merge_id" 
can
>set by "ID_SERIAL" or "ID_UID" in uevent. If we get the "merge_id"
>from uevents, we can merge and process it as merging way, otherwise
>we process it as old way (processed one by one).

 
>And we revert this patch:
>"[dm-devel] [PATCH 12/12] libmultipath: use existing wwid when wwid
>has already been existed in uevent"
>thus wwid of each path will get by methods of configuration as old
>ways.
> 
>The premise is that, devices whose "merge_id" is the same have the
>same "wwid". I think this premise can be established.

So, if you have device where the merge_id doesn't equal the wwid, you
call domap on that device, as it it wasn't part of a merged set?

That would work, but you would have to be careful to deal with the case
where the last uevent in the set, the one that would be used to call
domap for the whole merged set, suddenly gets processed all on it's own,
because it had a different wwid from the rest of the set. In that case
You would need to make sure that something called domap on the other
members of the merged set.  Now, I admit that this is a very unlikely
thing to happen.

I thought about proposing that we revert patch 12, but it does add
complexity if you find that only some of your merged devices change
wwid. Also, if all of them change to the same new wwid, you just skipped
merging when you should be able to. But I'm not against reverting it, as
long as we handle everything o.k. Reverting it would mean that we don't
have to worry about removing a capability that multipath previously had.
There's just a trade-off between supporting a capability (that we don't
know if anyone uses), and maintaining less complex code.

As long as it is possible to configure what uevent variable you check
for by device type, I don't know of a situation where this will cause
real world problems. It's possible that some users are changing
uid_attribute for only certain scsi-based arrays, but I don't know of
any cases. If somebody reading this knows of any cases, please speak up.
But I would like to have the variable you check be configurable so that
it's flexible with regards to new device types or the possiblity of
differing uevent variable names either between distributions or over
time. Does that sound reasonable?

-Ben

> 
>how do you think about the above proposal?
> 
>>The other option would be to not actually merge the uevents, but
>simply
>>run through the filtered but unmerged list of uevents, and skip 
the
>>domap stuff but remember the maps that need pushing to 
device-mapper.
>>Once you are done processing all the uevents, except for 
updating the
>>maps in device-mapper, you go back and update all the maps that 
need
>>updating. There's more code refactoring in this approach, but it
>keeps
>>the uid being set in pathinfo, where you have all the 
information
>>necessary to set it using uid_attribute, getuid, or specialized 
code
>>like rbd uses.
>I think it is ok, but a little complex. and if we can get rid of the
>"wwid" issue, we do not need to do so.
> 
>Regards
>Tang Junhui
> 
> 

Re: [dm-devel] [PATCH 01/12] libmultipath: add wwid for "struct uevent" to record wwid of uevent

[tang . junhui]

Hello Ben,

I know what you concern about. Maybe we can change the "wwid" member
in "struct uevent" to another name such as "merge_id" to only 
identify which uevents can merge together. The value of "merge_id" can
set by "ID_SERIAL" or "ID_UID" in uevent. If we get the "merge_id"
from uevents, we can merge and process it as merging way, otherwise
we process it as old way (processed one by one).

And we revert this patch:
"[dm-devel] [PATCH 12/12] libmultipath: use existing wwid when wwid
has already been existed in uevent"
thus wwid of each path will get by methods of configuration as old
ways.

The premise is that, devices whose "merge_id" is the same have the
same "wwid". I think this premise can be established.

how do you think about the above proposal?

>The other option would be to not actually merge the uevents, but 
simply
>run through the filtered but unmerged list of uevents, and skip the
>domap stuff but remember the maps that need pushing to device-mapper.
>Once you are done processing all the uevents, except for updating the
>maps in device-mapper, you go back and update all the maps that need
>updating. There's more code refactoring in this approach, but it 
keeps
>the uid being set in pathinfo, where you have all the information
>necessary to set it using uid_attribute, getuid, or specialized code
>like rbd uses.
I think it is ok, but a little complex. and if we can get rid of the 
"wwid" issue, we do not need to do so.

Regards
Tang Junhui





发件人: "Benjamin Marzinski" <bmarz...@redhat.com>
收件人: tang.jun...@zte.com.cn, 
抄送:   tang.wenj...@zte.com.cn, zhang.ka...@zte.com.cn, 
dm-devel@redhat.com, bart.vanass...@sandisk.com, mwi...@suse.com
日期:   2017/01/05 02:23
主题:   Re: [dm-devel] [PATCH 01/12] libmultipath: add wwid for "struct 
uevent" to record wwid of uevent
发件人: dm-devel-boun...@redhat.com



On Wed, Jan 04, 2017 at 02:56:46PM +0800, tang.jun...@zte.com.cn wrote:
>Hello Ben,
> 
>> Right now, multipath users are allowed configure devices to set the 
wwid
>> based on any udev environment variable (or even use a callout, 
although
>> this is deprecated). With this patch, that breaks.
>Does WWID obtained by different methods change? If it changes, we 
would
>better to modify code to keep it no change.

Yes. users can pick any udev environment variable (and currently, any
callout program that they write) to use as the wwid.

>> If the udev sets
>> ID_SERIAL for a device, that is its wwid, right?
>Yes
> 
>> Do you know if rbd
>> devices have ID_SERIAL set?
>WWID has different label in uevents for different devices, I only 
test for
>SCSI devices.

Where is that check? I only see a check for whether ID_SERIAL exists. If
it exists on things that are not scsi devices, you will set the wwid for
these devices with it as well, even if it doesn't work for them.

>Now we do not support rbd divice for uevents merging, these
>device process as old way, it has no harm in logic. If we need to 
merge
>rbd uevents for these devices, we can add code to get wwid from 
uevents
>and it can be supported easily.

Look at get_rbd_uid(), and how it's called. rbd devices don't even use
the getuid callout or uid_attribute. They use special code called by
get_uid.

Now you could add explicit checks so we only check ID_SERIAL for scsi
devices, ID_UID for dasd devices, and never set the wwid otherwise.  You
could even make the attribute we check configurable by device type with
an option like

uid_attrs "sd:ID_SERIAL dasd:ID_UID"

in the defaults section, that would set up mappings between device types
and uevent attributes to check for the uid. But this would be on per
device types, not per storage array, like it currently is. uid_attribute
and getuid attribute would only ever be used for device types that
weren't in the uid_attrs list.

The other option would be to not actually merge the uevents, but simply
run through the filtered but unmerged list of uevents, and skip the
domap stuff but remember the maps that need pushing to device-mapper.
Once you are done processing all the uevents, except for updating the
maps in device-mapper, you go back and update all the maps that need
updating. There's more code refactoring in this approach, but it keeps
the uid being set in pathinfo, where you have all the information
necessary to set it using uid_attribute, getuid, or specialized code
like rbd uses.

As long as we make sure that we are only checking specific uevent
attributes for specific device types, I'm o.k. with your way, but we are
losing flexibility that multipath has always had in regards to setting
the wwid. I want to point that out so tha

Re: [dm-devel] [PATCH 08/12] libmultipath: wait one seconds for more uevents in uevent_listen() in uevents burst situations

[tang . junhui]

Hello Ben,

I know the issue of socket's buffer fills up, 
but I think uevent_can_discard() totally process in memory,
it is light-weight and low cpu consumption, and it reduce the 
iteration count in merging, the test result is also good in
massive multipath devices scene. 

But if you still stick to it, I will revert it to uevents 
processing thread, and call it in uevent_dispatch() before 
calling merge_uevq(), actually, I'm going to do that.

Regards
Tang Junhui





发件人: "Benjamin Marzinski" <bmarz...@redhat.com>
收件人: tang.jun...@zte.com.cn, 
抄送:   tang.wenj...@zte.com.cn, zhang.ka...@zte.com.cn, 
dm-devel@redhat.com, bart.vanass...@sandisk.com, mwi...@suse.com
日期:   2017/01/04 06:38
主题:   Re: [dm-devel] [PATCH 08/12] libmultipath: wait one seconds for 
more uevents in uevent_listen() in uevents burst situations
发件人: dm-devel-boun...@redhat.com



On Tue, Dec 27, 2016 at 04:03:25PM +0800, tang.jun...@zte.com.cn wrote:
> From: tang.junhui <tang.jun...@zte.com.cn>
> 
> The more uevents are merged, the higher efficiency program will 
performs.
> So, do not process uevents after receiving immediately in uevents burst
> situations, but continue wait 1 seconds for more uevents except that too
> much uevents (2048) have already been received or too much time eclipse
> (60 seconds).

The issue I have with doing this in uevent_listen is that we seperated
receiving the uevents from servicing the uevents specificially to make
sure what we received them as fast as possible. The udev monitor code
is all based on a NETLINK socket. If our socket's receive buffer fills
up, there is no flow control. Events just start getting dropped, which
does cut down on processing time, but not in a way we would like.

This issue applies to a lesser extent to you previous two patches. I
don't really see the benefit of making sure that we drop the uevents
as soon as possible.  As long as we don't process them, that should
be good enough, right?

Now, maybe you put a lot of thought into your timeouts, and feel
confident that we will start processing well before the receive buffer
fills up. But if so, some comments on that would be reassuring, because
from the commit message, they seem fairly arbitrary to me.

-Ben

> 
> Change-Id: I763d491540e8114a81d12d603281540a81502742
> Signed-off-by: tang.junhui <tang.jun...@zte.com.cn>
> ---
>  libmultipath/uevent.c | 35 +--
>  1 file changed, 33 insertions(+), 2 deletions(-)
> 
> diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c
> index cc10d65..b0b05e9 100644
> --- a/libmultipath/uevent.c
> +++ b/libmultipath/uevent.c
> @@ -39,6 +39,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
> 
> @@ -490,11 +491,37 @@ struct uevent *uevent_from_udev_device(struct 
udev_device *dev)
>return uev;
>  }
> 
> +bool uevent_burst(struct timeval *start_time, int events)
> +{
> +  struct timeval diff_time, end_time;
> +  unsigned long speed;
> +  unsigned long eclipse_ms;
> +
> +  gettimeofday(_time, NULL);
> +  timersub(_time, start_time, _time);
> +
> +  eclipse_ms = diff_time.tv_sec * 1000 + diff_time.tv_usec 
/ 1000;
> +  if (eclipse_ms == 0)
> +  return true;
> +  /* max wait 60s */
> +  if (eclipse_ms > 60*1000) {
> +  condlog(1, "burst continued =%lu ms, 
stoped", eclipse_ms);
> +  return false;
> +  }
> +
> +  speed = (events * 1000) / eclipse_ms;
> +  if (speed > 10)
> +  return true;
> +
> +  return false;
> +}
> +
>  int uevent_listen(struct udev *udev)
>  {
>int err = 2;
>struct udev_monitor *monitor = NULL;
>int fd, socket_flags, events;
> +  struct timeval start_time;
>int need_failback = 1;
>int timeout = 30;
>LIST_HEAD(uevlisten_tmp);
> @@ -548,6 +575,7 @@ int uevent_listen(struct udev *udev)
>}
> 
>events = 0;
> +  gettimeofday(_time, NULL);
>while (1) {
>struct uevent *uev;
>struct udev_device *dev;
> @@ -562,7 +590,8 @@ int uevent_listen(struct udev *udev)
>errno = 0;
>fdcount = poll(_poll, 1, 
poll_timeout);
>if (fdcount && ev_poll.revents & POLLIN) 
{
> -  timeout = 0;
>

Re: [dm-devel] [PATCH 01/12] libmultipath: add wwid for "struct uevent" to record wwid of uevent

[tang . junhui]

Hello Ben,

> Right now, multipath users are allowed configure devices to set the wwid
> based on any udev environment variable (or even use a callout, although
> this is deprecated). With this patch, that breaks.
Does WWID obtained by different methods change? If it changes, we would 
better to modify code to keep it no change. 

> If the udev sets
> ID_SERIAL for a device, that is its wwid, right? 
Yes

> Do you know if rbd
> devices have ID_SERIAL set? 
WWID has different label in uevents for different devices, I only test for
SCSI devices. Now we do not support rbd divice for uevents merging, these 
device process as old way, it has no harm in logic. If we need to merge 
rbd uevents for these devices, we can add code to get wwid from uevents
and it can be supported easily. 


Regards
Tang Junhui





发件人: "Benjamin Marzinski" <bmarz...@redhat.com>
收件人: tang.jun...@zte.com.cn, 
抄送:   christophe.varo...@opensvc.com, h...@suse.de, mwi...@suse.com, 
bart.vanass...@sandisk.com, dm-devel@redhat.com, zhang.ka...@zte.com.cn, 
tang.wenj...@zte.com.cn
日期:   2017/01/04 06:03
主题:   Re: [PATCH 01/12] libmultipath: add wwid for "struct uevent" to 
record wwid of uevent



On Tue, Dec 27, 2016 at 04:03:18PM +0800, tang.jun...@zte.com.cn wrote:
> From: tang.junhui <tang.jun...@zte.com.cn>
> 
> Add "char *wwid" to point WWID of uevent. This member identifies
> the LUN ID which the path belongs to, and it is used for merging
> uevents. WWID possibly did not exist in uevent yet, so ->wwid
> would be NULL, those uevents would not be merged, but be proccessed
> as old way.

Right now, multipath users are allowed configure devices to set the wwid
based on any udev environment variable (or even use a callout, although
this is deprecated). With this patch, that breaks. If the udev sets
ID_SERIAL for a device, that is its wwid, right?  Do you know if rbd
devices have ID_SERIAL set? If so, this change will break them.  Even if
this change doesn't break any devices in their default configurations,
we would need to disallow changing how the wwid is set for this patch
to be safe.

-Ben 

> 
> Change-Id: Ie6b076363b3735dc7de10184b27fa799b499af0e
> Signed-off-by: tang.junhui <tang.jun...@zte.com.cn>
> ---
>  libmultipath/uevent.c | 2 ++
>  libmultipath/uevent.h | 1 +
>  2 files changed, 3 insertions(+)
> 
> diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c
> index 7edcce1..ef1bafe 100644
> --- a/libmultipath/uevent.c
> +++ b/libmultipath/uevent.c
> @@ -424,6 +424,8 @@ struct uevent *uevent_from_udev_device(struct 
udev_device *dev)
>uev->devpath = 
uev->envp[i] + 8;
>if (strcmp(name, "ACTION") == 0)
>uev->action = 
uev->envp[i] + 7;
> +  if (strcmp(name, "ID_SERIAL") == 0)
> +  uev->wwid = uev->envp[i] 
+ 10;
>i++;
>if (i == HOTPLUG_NUM_ENVP - 1)
>break;
> diff --git a/libmultipath/uevent.h b/libmultipath/uevent.h
> index 9d22dcd..7bfccef 100644
> --- a/libmultipath/uevent.h
> +++ b/libmultipath/uevent.h
> @@ -22,6 +22,7 @@ struct uevent {
>char *devpath;
>char *action;
>char *kernel;
> +  char *wwid;
>unsigned long seqnum;
>char *envp[HOTPLUG_NUM_ENVP];
>  };
> -- 
> 2.8.1.windows.1
> 


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

Re: [dm-devel] [PATCH 09/12] multipathd: merge uevents before proccessing

[tang . junhui]

Hello Ben,

Yes, a *_safe list traversal method can meet the needs,
I will modify it and simplify the codes.

Thanks,
Tang Junhui




发件人: "Benjamin Marzinski" <bmarz...@redhat.com>
收件人: tang.jun...@zte.com.cn, 
抄送:   tang.wenj...@zte.com.cn, zhang.ka...@zte.com.cn, 
dm-devel@redhat.com, bart.vanass...@sandisk.com, mwi...@suse.com
日期:   2017/01/04 08:39
主题:   Re: [dm-devel] [PATCH 09/12] multipathd: merge uevents before 
proccessing
发件人: dm-devel-boun...@redhat.com



On Tue, Dec 27, 2016 at 04:03:26PM +0800, tang.jun...@zte.com.cn wrote:
> From: tang.junhui <tang.jun...@zte.com.cn>
> 
> These uevents are going to be merged:
> 1) uevents come from paths and
> 2) uevents type is same and
> 3) uevents type is addition or deletion and
> 4) uevents wwid is same.

This is just a nit, and I might be missing something subtle here, but it
seems like instead of adding list_for_some_entry_reverse, and then
breaking the abstraction to manually get previous entries, you could
have just added list_for_some_entry_reverse_safe in your earlier patch,
and hid the work of traversing a list while removing elements behind the
well understood abstraction of a *_safe list traversal method.

-Ben

> 
> Change-Id: I05ee057391c092aa0c5f989b7a4f9cb550bb4d98
> Signed-off-by: tang.junhui <tang.jun...@zte.com.cn>
> ---
>  libmultipath/uevent.c | 125 
+-
>  1 file changed, 114 insertions(+), 11 deletions(-)
> 
> diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c
> index b0b05e9..114068c 100644
> --- a/libmultipath/uevent.c
> +++ b/libmultipath/uevent.c
> @@ -85,6 +85,20 @@ struct uevent * alloc_uevent (void)
>return uev;
>  }
> 
> +void
> +uevq_cleanup(struct list_head *tmpq)
> +{
> +  struct uevent *uev, *tmp;
> +
> +  list_for_each_entry_safe(uev, tmp, tmpq, node) {
> +  list_del_init(>node);
> +
> +  if (uev->udev)
> + udev_device_unref(uev->udev);
> +  FREE(uev);
> +  }
> +}
> +
>  bool
>  uevent_can_discard(char *devpath, char *kernel)
>  {
> @@ -125,6 +139,103 @@ uevent_can_discard(char *devpath, char *kernel)
>return false;
>  }
> 
> +bool
> +merge_need_stop(struct uevent *earlier, struct uevent *later)
> +{
> +  /*
> +   * dm uevent do not try to merge with left uevents
> +   */
> +  if (!strncmp(later->kernel, "dm-", 3))
> +  return true;
> +
> +  /*
> +   * we can not make a jugement without wwid,
> +   * so it is sensible to stop merging
> +   */
> +  if (!earlier->wwid || !later->wwid)
> +  return true;
> +  /*
> +   * uevents merging stoped
> +   * when we meet an opposite action uevent from the same 
LUN to AVOID
> +   * "add path1 |remove path1 |add path2 |remove path2 |add 
path3"
> +   * to merge as "remove path1, path2" and "add path1, 
path2, path3"
> +   * OR
> +   * "remove path1 |add path1 |remove path2 |add path2 
|remove path3"
> +   * to merge as "add path1, path2" and "remove path1, 
path2, path3"
> +   * SO
> +   * when we meet a non-change uevent from the same LUN
> +   * with the same wwid and different action
> +   * it would be better to stop merging.
> +   */
> +  if (!strcmp(earlier->wwid, later->wwid) &&
> +  strcmp(earlier->action, later->action) &&
> +  strcmp(earlier->action, "change") &&
> +  strcmp(later->action, "change"))
> +  return true;
> +
> +  return false;
> +}
> +
> +bool
> +uevent_can_merge(struct uevent *earlier, struct uevent *later)
> +{
> +  /* merge paths uevents
> +   * whose wwids exsit and are same
> +   * and actions are same,
> +   * and actions are addition or deletion
> +   */
> +  if (earlier->wwid && later->wwid &&
> +  !strcmp(earlier->wwid, later->wwid) &&
> +  !strcmp(earlier->action, later->action) &&
> +  strncmp(earlier->action, "change", 6) &&
> +  strncmp(

Re: [dm-devel] [PATCH 10/12] libmultipath: filter uevents before proccessing

[tang . junhui]

Hello Ben,

I add wwid judgment for safe.
I think twice, and as you say, it is safe enough without
this judgment, I will delete these wwid judgment.

Tanks
Tang Junhui



发件人: "Benjamin Marzinski" <bmarz...@redhat.com>
收件人: tang.jun...@zte.com.cn, 
抄送:   tang.wenj...@zte.com.cn, zhang.ka...@zte.com.cn, 
dm-devel@redhat.com, bart.vanass...@sandisk.com, mwi...@suse.com
日期:   2017/01/04 09:28
主题:   Re: [dm-devel] [PATCH 10/12] libmultipath: filter uevents before 
proccessing
发件人: dm-devel-boun...@redhat.com



On Tue, Dec 27, 2016 at 04:03:27PM +0800, tang.jun...@zte.com.cn wrote:
> From: tang.junhui <tang.jun...@zte.com.cn>
> 
> Before merging uevents, these uevents are going to be filtered:
> Change or addition uevent of a removed path (it indicate by an
> deletion uevent occurred later).
> 

I think it's safe to remove add and change uevents if they are followed
by a remove event, regardless of whether or not they have a wwid, as
long as the kernel name is the same.  We only get the remove event when
the device is gone. Processing the add and change events will never get
us anything in these cases, because there is no device to act on.

-Ben

> Change-Id: If00c2c2e23ea466c1d4643c541ed2d8f9a0c8dea
> Signed-off-by: tang.junhui <tang.jun...@zte.com.cn>
> ---
>  libmultipath/uevent.c | 55 
+++
>  1 file changed, 55 insertions(+)
> 
> diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c
> index 114068c..424f272 100644
> --- a/libmultipath/uevent.c
> +++ b/libmultipath/uevent.c
> @@ -140,6 +140,28 @@ uevent_can_discard(char *devpath, char *kernel)
>  }
> 
>  bool
> +uevent_can_filter(struct uevent *earlier, struct uevent *later)
> +{
> +
> +  /*
> +   * filter earlier uvents if path has removed later, eg:
> +   * "add path3 |chang path3 |add path2 |remove path3"
> +   * can filter as:
> +   * "add path2 |remove path3"
> +   * uevent "add path3" and "chang path3" are filtered out
> +   */
> +  if (earlier->wwid && later->wwid &&
> +  !strcmp(earlier->wwid, later->wwid) &&
> +  strncmp(later->kernel, "dm-", 3) &&
> +  !strcmp(later->action, "remove") &&
> +  !strcmp(earlier->kernel, later->kernel)) 
{
> +  return true;
> +  }
> +
> +  return false;
> +}
> +
> +bool
>  merge_need_stop(struct uevent *earlier, struct uevent *later)
>  {
>/*
> @@ -196,6 +218,38 @@ uevent_can_merge(struct uevent *earlier, struct 
uevent *later)
>  }
> 
>  void
> +uevent_filter(struct uevent *later, struct list_head *tmpq)
> +{
> +  struct uevent *earlier, *temp;
> +  /*
> +   * compare the uevent with earlier uevents
> +   */
> +  list_for_some_entry_reverse(earlier, >node, tmpq, 
node) {
> +next_earlier_node:
> +  /*
> +   * filter unnessary earlier uevents by 
the later uevent
> +   */
> +  if (uevent_can_filter(earlier, later)) {
> +  condlog(2, "uevent: 
%s-%s-%s has removed by uevent: %s-%s-%s, filtered",
> + earlier->action, earlier->kernel, earlier->wwid,
> + later->action, later->kernel, later->wwid);
> +
> +  temp = earlier;
> +  earlier = 
list_entry(earlier->node.prev, typeof(struct uevent), node);
> + list_del_init(>node);
> +  if (temp->udev)
> + udev_device_unref(temp->udev);
> +  FREE(temp);
> +
> +  if (earlier == 
list_entry(tmpq, typeof(struct uevent), node))
> +  break;
> +  else
> +  goto 
next_earlier_node;
> +  }
> +  }
> +}
> +
> +void
>  uevent_merge(struct uevent *later, struct list_head *tmpq)
>  {
>struct uevent *earlier, *temp;
> @@ -232,6 +286,7 @@ merge_uevq(struct list_head *tmpq)
>struct uevent *later;
> 
>list_for_each_entry_reverse(later, tmpq, node) {
> +  uevent_filter(later, tmpq);
>uevent_merge(later, tmpq);
>}
>  }
> -- 
> 2.8.1.windows.1
> 

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


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

Re: [dm-devel] [PATCH 11/12] multipathd: proccess merged uevents

[tang . junhui]

Hello Ben

Good idea,
I will modify the patch to filter these change uevents.

Thanks
Tang Junhui



发件人: "Benjamin Marzinski" <bmarz...@redhat.com>
收件人: tang.jun...@zte.com.cn, 
抄送:   tang.wenj...@zte.com.cn, zhang.ka...@zte.com.cn, 
dm-devel@redhat.com, bart.vanass...@sandisk.com, mwi...@suse.com
日期:   2017/01/04 09:10
主题:   Re: [dm-devel] [PATCH 11/12] multipathd: proccess merged uevents
发件人: dm-devel-boun...@redhat.com



On Tue, Dec 27, 2016 at 04:03:28PM +0800, tang.jun...@zte.com.cn wrote:
> From: "tang.junhui" <tang.jun...@zte.com.cn>
> 
> After filtering and merging, then uevents are processed in
> uev_trigger(), firstly, each of merged uevents would be processed one
> by one with need_do_map in value of 0. Finally, the node “uev” itself
> would be processed with need_do_map in value of 1, which would reload
> kernel table, and create or remove the dm device.

Wait, doesn't this mean that you would change

"add path1 | change path1 | add path2"

to

"change path1 | add path1, path2"

It doesn't make sense to process a change event before an add event.
Looking at uev_update_path, the three things it currently does all can
be safely ignored if you don't process the add event until after you
receive the change event. They are:

disable path on changed wwid: the multipath device hasn't been created
yet, so it will simply be created with the new wwid. That's fine.

attempt to get the pathinfo again if you failed when adding it: Either
you don't have the necessary udev information in the add event, in which
case that uevent won't get merged in the first place and they will
remain in order, or you do have the information, and there's no point in
processing the change event in that case.

change the Read-Only state of the device: since both events (the change
and the add event) have already happened, when you look at the device
for the add event, you will already get the current read-only state.

So, as far as I can tell, if you have change events in your queue as
well as an add event for the same device, you might as well just filter
out the change events, since processing it immediately after the add
event will never actually change anything, and processing it beforehand
makes no sense.

-Ben

> 
> Change-Id: Ifb4de0ca36180a8af16729c2f70bc250858877bd
> Signed-off-by: tang.junhui <tang.jun...@zte.com.cn>
> ---
>  multipathd/main.c | 28 +++-
>  1 file changed, 15 insertions(+), 13 deletions(-)
> 
> diff --git a/multipathd/main.c b/multipathd/main.c
> index 15a6175..0b18f6c 100644
> --- a/multipathd/main.c
> +++ b/multipathd/main.c
> @@ -1092,6 +1092,7 @@ uev_trigger (struct uevent * uev, void * 
trigger_data)
>  {
>int r = 0;
>struct vectors * vecs;
> +  struct uevent *merge_uev, *tmp;
> 
>vecs = (struct vectors *)trigger_data;
> 
> @@ -1123,20 +1124,21 @@ uev_trigger (struct uevent * uev, void * 
trigger_data)
>}
> 
>/*
> -   * path add/remove event
> +   * path add/remove/change event, add/remove maybe merged
> */
> -  if (!strncmp(uev->action, "add", 3)) {
> -  r = uev_add_path(uev, vecs, 1);
> -  goto out;
> -  }
> -  if (!strncmp(uev->action, "remove", 6)) {
> -  r = uev_remove_path(uev, vecs, 1);
> -  goto out;
> -  }
> -  if (!strncmp(uev->action, "change", 6)) {
> -  r = uev_update_path(uev, vecs);
> -  goto out;
> -  }
> +  list_for_each_entry_safe(merge_uev, tmp, 
>merge_node, node) {
> +  if (!strncmp(merge_uev->action, "add", 
3))
> +  r += 
uev_add_path(merge_uev, vecs, 0);
> +  if (!strncmp(merge_uev->action, "remove", 
6))
> +  r += 
uev_remove_path(merge_uev, vecs, 0);
> +  }
> +
> +  if (!strncmp(uev->action, "add", 3))
> +  r += uev_add_path(uev, vecs, 1);
> +  if (!strncmp(uev->action, "remove", 6))
> +  r += uev_remove_path(uev, vecs, 1);
> +  if (!strncmp(uev->action, "change", 6))
> +  r += uev_update_path(uev, vecs);
> 
>  out:
>return r;
> -- 
> 2.8.1.windows.1
> 

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

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

[dm-devel] [PATCH] multipathd: fix SIGUSR2 handling

[tang . junhui]

From: "tang.junhui" 

SIGUSR2 is not blocked by default, it would cause bellow conflict
in removing of the last path:
-
uevent processing thread | waiter thread
-|---
uev_remove_path()|   waiteventloop()
  lock(>lock)  |
  ev_remove_path()   | 
dm_queue_if_no_path()|> dm_task_run()   
flush_map()  |  |
  remove_map_and_stop_waiter()   |  -> 
pthread_cleanup_push(cleanup_lock, >vecs->lock);
_remove_map()| 
lock(>vecs->lock)//wait for the locker   
  stop_waiter_thread()   | 
pthread_cancel(thread)   |  
pthread_kill(thread,SIGUSR2)-|--> sigusr2 (int sig)
 |  condlog()
 |  fprintf() //it has test 
cancel point
 |  cleanup_lock() //thread 
cancel, and pop, which unlock the 
 |   locker of 
uevent processing thread 
--
Since SIGUSR2 is only needed when waiter thread running in dm_task_run(),
and it has already been unblocked before dm_task_run(), so this patch block 
SIGUSR2 for other times.

Change-Id: I8c46292dc954415764ebbe054498b4f7ea53c1c6
Signed-off-by: tang.junhui 
---
 multipathd/main.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/multipathd/main.c b/multipathd/main.c
index adc3258..fe69abd 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -2181,6 +2181,12 @@ sigusr2 (int sig)
 static void
 signal_init(void)
 {
+   sigset_t set;
+
+   sigemptyset();
+   sigaddset(, SIGUSR2);
+   pthread_sigmask(SIG_SETMASK, , NULL);
+
signal_set(SIGHUP, sighup);
signal_set(SIGUSR1, sigusr1);
signal_set(SIGUSR2, sigusr2);
-- 
2.8.1.windows.1


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

[dm-devel] [PATCH 08/12] libmultipath: wait one seconds for more uevents in uevent_listen() in uevents burst situations

[tang . junhui]

From: tang.junhui 

The more uevents are merged, the higher efficiency program will performs.
So, do not process uevents after receiving immediately in uevents burst
situations, but continue wait one seconds for more uevents except that
too much uevents (2048) have already been received or too much time
eclipse (30 seconds).

Change-Id: I763d491540e8114a81d12d603281540a81502742
Signed-off-by: tang.junhui 
---
 libmultipath/uevent.c | 42 +-
 1 file changed, 41 insertions(+), 1 deletion(-)

diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c
index cc10d65..0345e6e 100644
--- a/libmultipath/uevent.c
+++ b/libmultipath/uevent.c
@@ -39,6 +39,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -51,6 +52,10 @@
 #include "config.h"
 #include "blacklist.h"
 
+#define MAX_ACCUMULATION_COUNT 2048
+#define MAX_ACCUMULATION_TIME 30*1000
+#define MIN_BURST_SPEED 10
+
 typedef int (uev_trigger)(struct uevent *, void * trigger_data);
 
 LIST_HEAD(uevq);
@@ -490,11 +495,43 @@ struct uevent *uevent_from_udev_device(struct udev_device 
*dev)
return uev;
 }
 
+bool uevent_burst(struct timeval *start_time, int events)
+{
+   struct timeval diff_time, end_time;
+   unsigned long speed;
+   unsigned long eclipse_ms;
+
+   if(events > MAX_ACCUMULATION_COUNT) {
+   condlog(2, "burst got %u uevents, too much uevents, stopped", 
events);
+   return false;
+   }
+
+   gettimeofday(_time, NULL);
+   timersub(_time, start_time, _time);
+
+   eclipse_ms = diff_time.tv_sec * 1000 + diff_time.tv_usec / 1000;
+
+   if (eclipse_ms == 0)
+   return true;
+
+   if (eclipse_ms > MAX_ACCUMULATION_TIME) {
+   condlog(2, "burst continued %lu ms, too long time, stopped", 
eclipse_ms);
+   return false;
+   }
+
+   speed = (events * 1000) / eclipse_ms;
+   if (speed > MIN_BURST_SPEED)
+   return true;
+
+   return false;
+}
+
 int uevent_listen(struct udev *udev)
 {
int err = 2;
struct udev_monitor *monitor = NULL;
int fd, socket_flags, events;
+   struct timeval start_time;
int need_failback = 1;
int timeout = 30;
LIST_HEAD(uevlisten_tmp);
@@ -548,6 +585,7 @@ int uevent_listen(struct udev *udev)
}
 
events = 0;
+   gettimeofday(_time, NULL);
while (1) {
struct uevent *uev;
struct udev_device *dev;
@@ -562,7 +600,8 @@ int uevent_listen(struct udev *udev)
errno = 0;
fdcount = poll(_poll, 1, poll_timeout);
if (fdcount && ev_poll.revents & POLLIN) {
-   timeout = 0;
+   timeout = uevent_burst(_time, events + 1) ? 1 : 0;
+
dev = udev_monitor_receive_device(monitor);
if (!dev) {
condlog(0, "failed getting udev device");
@@ -600,6 +639,7 @@ int uevent_listen(struct udev *udev)
pthread_mutex_unlock(uevq_lockp);
events = 0;
}
+   gettimeofday(_time, NULL);
timeout = 30;
}
need_failback = 0;
-- 
2.8.1.windows.1


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

[dm-devel] [PATCH 08/12] libmultipath: wait one seconds for more uevents in uevent_listen() in uevents burst situations

[tang . junhui]

From: tang.junhui 

The more uevents are merged, the higher efficiency program will performs.
So, do not process uevents after receiving immediately in uevents burst
situations, but continue wait 1 seconds for more uevents except that too
much uevents (2048) have already been received or too much time eclipse
(60 seconds).

Change-Id: I763d491540e8114a81d12d603281540a81502742
Signed-off-by: tang.junhui 
---
 libmultipath/uevent.c | 35 +--
 1 file changed, 33 insertions(+), 2 deletions(-)

diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c
index cc10d65..b0b05e9 100644
--- a/libmultipath/uevent.c
+++ b/libmultipath/uevent.c
@@ -39,6 +39,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -490,11 +491,37 @@ struct uevent *uevent_from_udev_device(struct udev_device 
*dev)
return uev;
 }
 
+bool uevent_burst(struct timeval *start_time, int events)
+{
+   struct timeval diff_time, end_time;
+   unsigned long speed;
+   unsigned long eclipse_ms;
+
+   gettimeofday(_time, NULL);
+   timersub(_time, start_time, _time);
+
+   eclipse_ms = diff_time.tv_sec * 1000 + diff_time.tv_usec / 1000;
+   if (eclipse_ms == 0)
+   return true;
+   /* max wait 60s */
+   if (eclipse_ms > 60*1000) {
+   condlog(1, "burst continued =%lu ms, stoped", eclipse_ms);
+   return false;
+   }
+
+   speed = (events * 1000) / eclipse_ms;
+   if (speed > 10)
+   return true;
+
+   return false;
+}
+
 int uevent_listen(struct udev *udev)
 {
int err = 2;
struct udev_monitor *monitor = NULL;
int fd, socket_flags, events;
+   struct timeval start_time;
int need_failback = 1;
int timeout = 30;
LIST_HEAD(uevlisten_tmp);
@@ -548,6 +575,7 @@ int uevent_listen(struct udev *udev)
}
 
events = 0;
+   gettimeofday(_time, NULL);
while (1) {
struct uevent *uev;
struct udev_device *dev;
@@ -562,7 +590,8 @@ int uevent_listen(struct udev *udev)
errno = 0;
fdcount = poll(_poll, 1, poll_timeout);
if (fdcount && ev_poll.revents & POLLIN) {
-   timeout = 0;
+   timeout = uevent_burst(_time, events + 1) ? 1:0;
+
dev = udev_monitor_receive_device(monitor);
if (!dev) {
condlog(0, "failed getting udev device");
@@ -578,7 +607,8 @@ int uevent_listen(struct udev *udev)
}
list_add_tail(>node, _tmp);
events++;
-   continue;
+   if(events < 2048)
+   continue;
}
if (fdcount < 0) {
if (errno == EINTR)
@@ -600,6 +630,7 @@ int uevent_listen(struct udev *udev)
pthread_mutex_unlock(uevq_lockp);
events = 0;
}
+   gettimeofday(_time, NULL);
timeout = 30;
}
need_failback = 0;
-- 
2.8.1.windows.1


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

[dm-devel] [PATCH 04/12] multipathd: add need_do_map to indicate whether need calling domap() in ev_add_path()

[tang . junhui]

From: tang.junhui 

Usually calling domap() in ev_add_path() is needed, but only last path
need to call domap() in processing for merged uevents to reduce the
count of calling domap() and promote efficiency. So add input parameter
need_do_map to indicate whether need calling domap() in ev_add_path().

Change-Id: I7f33de49a1d89d91180dcb95cd94e4944ae7ff36
Signed-off-by: tang.wenjun 
---
 multipathd/cli_handlers.c |  3 ++-
 multipathd/main.c | 25 -
 multipathd/main.h |  2 +-
 3 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c
index b0eeca6..3a46c09 100644
--- a/multipathd/cli_handlers.c
+++ b/multipathd/cli_handlers.c
@@ -670,7 +670,8 @@ cli_add_path (void * v, char ** reply, int * len, void * 
data)
pp->checkint = conf->checkint;
}
put_multipath_config(conf);
-   return ev_add_path(pp, vecs);
+   return ev_add_path(pp, vecs, 1);
+
 blacklisted:
*reply = strdup("blacklisted\n");
*len = strlen(*reply) + 1;
diff --git a/multipathd/main.c b/multipathd/main.c
index adc3258..34b1b64 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -608,7 +608,7 @@ ev_remove_map (char * devname, char * alias, int minor, 
struct vectors * vecs)
 }
 
 static int
-uev_add_path (struct uevent *uev, struct vectors * vecs)
+uev_add_path (struct uevent *uev, struct vectors * vecs, int need_do_map)
 {
struct path *pp;
int ret = 0, i;
@@ -641,7 +641,7 @@ uev_add_path (struct uevent *uev, struct vectors * vecs)
 DI_ALL | DI_BLACKLIST);
put_multipath_config(conf);
if (r == PATHINFO_OK)
-   ret = ev_add_path(pp, vecs);
+   ret = ev_add_path(pp, vecs, need_do_map);
else if (r == PATHINFO_SKIPPED) {
condlog(3, "%s: remove blacklisted path",
uev->kernel);
@@ -681,7 +681,7 @@ uev_add_path (struct uevent *uev, struct vectors * vecs)
conf = get_multipath_config();
pp->checkint = conf->checkint;
put_multipath_config(conf);
-   ret = ev_add_path(pp, vecs);
+   ret = ev_add_path(pp, vecs, need_do_map);
} else {
condlog(0, "%s: failed to store path info, "
"dropping event",
@@ -699,7 +699,7 @@ uev_add_path (struct uevent *uev, struct vectors * vecs)
  * 1: error
  */
 int
-ev_add_path (struct path * pp, struct vectors * vecs)
+ev_add_path (struct path * pp, struct vectors * vecs, int need_do_map)
 {
struct multipath * mpp;
char params[PARAMS_SIZE] = {0};
@@ -767,6 +767,13 @@ rescan:
/* persistent reservation check*/
mpath_pr_event_handle(pp);
 
+   if (!need_do_map)
+   return 0;
+
+   if (!dm_map_present(mpp->alias)) {
+   mpp->action = ACT_CREATE;
+   start_waiter = 1;
+   }
/*
 * push the map to the device-mapper
 */
@@ -995,7 +1002,7 @@ uev_update_path (struct uevent *uev, struct vectors * vecs)
}
 
if (pp->initialized == INIT_REQUESTED_UDEV)
-   retval = uev_add_path(uev, vecs);
+   retval = uev_add_path(uev, vecs, 1);
else if (mpp && ro >= 0) {
condlog(2, "%s: update path write_protect to '%d' 
(uevent)", uev->kernel, ro);
 
@@ -1150,7 +1157,7 @@ uev_trigger (struct uevent * uev, void * trigger_data)
put_multipath_config(conf);
 
if (!strncmp(uev->action, "add", 3)) {
-   r = uev_add_path(uev, vecs);
+   r = uev_add_path(uev, vecs, 1);
goto out;
}
if (!strncmp(uev->action, "remove", 6)) {
@@ -1570,7 +1577,7 @@ check_path (struct vectors * vecs, struct path * pp, int 
ticks)
conf = get_multipath_config();
ret = pathinfo(pp, conf, DI_ALL | DI_BLACKLIST);
if (ret == PATHINFO_OK) {
-   ev_add_path(pp, vecs);
+   ev_add_path(pp, vecs, 1);
pp->tick = 1;
} else if (ret == PATHINFO_SKIPPED) {
put_multipath_config(conf);
@@ -1686,7 +1693,7 @@ check_path (struct vectors * vecs, struct path * pp, int 
ticks)
}
if (!disable_reinstate && reinstate_path(pp, add_active)) {
condlog(3, "%s: reload map", pp->dev);
-   ev_add_path(pp, vecs);
+   ev_add_path(pp, vecs, 1);
pp->tick = 1;
return 0;
}
@@ -1709,7 +1716,7 @@ check_path 

[dm-devel] [PATCH 00/12] multipath-tools: improve processing efficiency for addition and deletion of multipath devices

[tang . junhui]

From: tang.junhui <tang.jun...@zte.com.cn>

Hello Christophe, Ben, Hannes, Martin, Bart, and other guys:

The twelve patches in this series are used to improve processing
efficiency for addition and deletion of multipath devices.

These patches are tested pass by ZTE multipath automatic testing syetem.
The modification reduces the system consumption(such as CPU) and shortens
the processing time obviously in scene of massive multipath devices
addition or deletion.

The main processing flow of code is:
1)uevents accumulation in uevents burst scene:
  --[pacth]libmultipath: wait one seconds for more uevents in
uevent_listen() in uevents burst situations
2)uevents filtering and merging:
  --[pacth]multipathd: merge uevents before proccessing
  --[pacth]libmultipath: filter uevents before proccessing
3)uevents proccessing:
  --[pacth]multipathd: add need_do_map to indicate whether need calling
domap() in ev_add_path()
  --[pacth]multipathd: add need_do_map to indicate whether need calling
domap() in ev_remove_path()
  --[pacth]multipathd: proccess merged uevents

There are some patches providing basic struct members or maros which
would used in above pathes:
  --[patch]libmultipath: add wwid for "struct uevent" to record wwid of
uevent
  --[patch]libmultipath: add merge_node for "struct uevent" to record
nodes of merged uevents
  --[patch]libmultipath: add two list iteration macros

There are also some independent optimization pathes for further
improvement:
  --[patch]multipathd: move uev_discard() to uevent.c and change its name
to uevent_can_discard()
  --[patch]multipathd: move calling filter_devnode() from uev_trigger() to
uevent_can_discard()
  --[patch]libmultipath: use existing wwid when wwid has already been
existed in uevent

Any comment will be welcome, and it would be appreciated if these
patches would be considered for inclusion in the upstream
multipath-tools.

Thank you all,
Tang Junhui

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

[dm-devel] [PATCH 11/12] multipathd: proccess merged uevents

[tang . junhui]

From: "tang.junhui" 

After filtering and merging, then uevents are processed in
uev_trigger(), firstly, each of merged uevents would be processed one
by one with need_do_map in value of 0. Finally, the node “uev” itself
would be processed with need_do_map in value of 1, which would reload
kernel table, and create or remove the dm device.

Change-Id: Ifb4de0ca36180a8af16729c2f70bc250858877bd
Signed-off-by: tang.junhui 
---
 multipathd/main.c | 28 +++-
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/multipathd/main.c b/multipathd/main.c
index 15a6175..0b18f6c 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1092,6 +1092,7 @@ uev_trigger (struct uevent * uev, void * trigger_data)
 {
int r = 0;
struct vectors * vecs;
+   struct uevent *merge_uev, *tmp;
 
vecs = (struct vectors *)trigger_data;
 
@@ -1123,20 +1124,21 @@ uev_trigger (struct uevent * uev, void * trigger_data)
}
 
/*
-* path add/remove event
+* path add/remove/change event, add/remove maybe merged
 */
-   if (!strncmp(uev->action, "add", 3)) {
-   r = uev_add_path(uev, vecs, 1);
-   goto out;
-   }
-   if (!strncmp(uev->action, "remove", 6)) {
-   r = uev_remove_path(uev, vecs, 1);
-   goto out;
-   }
-   if (!strncmp(uev->action, "change", 6)) {
-   r = uev_update_path(uev, vecs);
-   goto out;
-   }
+   list_for_each_entry_safe(merge_uev, tmp, >merge_node, node) {
+   if (!strncmp(merge_uev->action, "add", 3))
+   r += uev_add_path(merge_uev, vecs, 0);
+   if (!strncmp(merge_uev->action, "remove", 6))
+   r += uev_remove_path(merge_uev, vecs, 0);
+   }
+
+   if (!strncmp(uev->action, "add", 3))
+   r += uev_add_path(uev, vecs, 1);
+   if (!strncmp(uev->action, "remove", 6))
+   r += uev_remove_path(uev, vecs, 1);
+   if (!strncmp(uev->action, "change", 6))
+   r += uev_update_path(uev, vecs);
 
 out:
return r;
-- 
2.8.1.windows.1



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

[dm-devel] [PATCH 09/12] multipathd: merge uevents before proccessing

[tang . junhui]

From: tang.junhui 

These uevents are going to be merged:
1) uevents come from paths and
2) uevents type is same and
3) uevents type is addition or deletion and
4) uevents wwid is same.

Change-Id: I05ee057391c092aa0c5f989b7a4f9cb550bb4d98
Signed-off-by: tang.junhui 
---
 libmultipath/uevent.c | 125 +-
 1 file changed, 114 insertions(+), 11 deletions(-)

diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c
index b0b05e9..114068c 100644
--- a/libmultipath/uevent.c
+++ b/libmultipath/uevent.c
@@ -85,6 +85,20 @@ struct uevent * alloc_uevent (void)
return uev;
 }
 
+void
+uevq_cleanup(struct list_head *tmpq)
+{
+   struct uevent *uev, *tmp;
+
+   list_for_each_entry_safe(uev, tmp, tmpq, node) {
+   list_del_init(>node);
+
+   if (uev->udev)
+   udev_device_unref(uev->udev);
+   FREE(uev);
+   }
+}
+
 bool
 uevent_can_discard(char *devpath, char *kernel)
 {
@@ -125,6 +139,103 @@ uevent_can_discard(char *devpath, char *kernel)
return false;
 }
 
+bool
+merge_need_stop(struct uevent *earlier, struct uevent *later)
+{
+   /*
+* dm uevent do not try to merge with left uevents
+*/
+   if (!strncmp(later->kernel, "dm-", 3))
+   return true;
+
+   /*
+* we can not make a jugement without wwid,
+* so it is sensible to stop merging
+*/
+   if (!earlier->wwid || !later->wwid)
+   return true;
+   /*
+* uevents merging stoped
+* when we meet an opposite action uevent from the same LUN to AVOID
+* "add path1 |remove path1 |add path2 |remove path2 |add path3"
+* to merge as "remove path1, path2" and "add path1, path2, path3"
+* OR
+* "remove path1 |add path1 |remove path2 |add path2 |remove path3"
+* to merge as "add path1, path2" and "remove path1, path2, path3"
+* SO
+* when we meet a non-change uevent from the same LUN
+* with the same wwid and different action
+* it would be better to stop merging.
+*/
+   if (!strcmp(earlier->wwid, later->wwid) &&
+   strcmp(earlier->action, later->action) &&
+   strcmp(earlier->action, "change") &&
+   strcmp(later->action, "change"))
+   return true;
+
+   return false;
+}
+
+bool
+uevent_can_merge(struct uevent *earlier, struct uevent *later)
+{
+   /* merge paths uevents
+* whose wwids exsit and are same
+* and actions are same,
+* and actions are addition or deletion
+*/
+   if (earlier->wwid && later->wwid &&
+   !strcmp(earlier->wwid, later->wwid) &&
+   !strcmp(earlier->action, later->action) &&
+   strncmp(earlier->action, "change", 6) &&
+   strncmp(earlier->kernel, "dm-", 3)) {
+   return true;
+   }
+
+   return false;
+}
+
+void
+uevent_merge(struct uevent *later, struct list_head *tmpq)
+{
+   struct uevent *earlier, *temp;
+   /*
+* compare the uevent with earlier uevents
+*/
+   list_for_some_entry_reverse(earlier, >node, tmpq, node) {
+next_earlier_node:
+   if (merge_need_stop(earlier, later))
+   break;
+   /*
+* try to merge earlier uevents to the later uevent
+*/
+   if (uevent_can_merge(earlier, later)) {
+   condlog(3, "merged uevent: %s-%s-%s with uevent: 
%s-%s-%s",
+   earlier->action, earlier->kernel, earlier->wwid,
+   later->action, later->kernel, later->wwid);
+   temp = earlier;
+
+   earlier = list_entry(earlier->node.prev, typeof(struct 
uevent), node);
+   list_move(>node, >merge_node);
+
+   if (earlier ==  list_entry(tmpq, typeof(struct uevent), 
node))
+   break;
+   else
+   goto next_earlier_node;
+   }
+   }
+}
+
+void
+merge_uevq(struct list_head *tmpq)
+{
+   struct uevent *later;
+
+   list_for_each_entry_reverse(later, tmpq, node) {
+   uevent_merge(later, tmpq);
+   }
+}
+
 void
 service_uevq(struct list_head *tmpq)
 {
@@ -136,6 +247,8 @@ service_uevq(struct list_head *tmpq)
if (my_uev_trigger && my_uev_trigger(uev, my_trigger_data))
condlog(0, "uevent trigger error");
 
+   uevq_cleanup(>merge_node);
+
if (uev->udev)
udev_device_unref(uev->udev);
FREE(uev);
@@ -150,17 +263,6 @@ static void uevent_cleanup(void *arg)
udev_unref(udev);
 }
 
-void
-uevq_cleanup(struct list_head *tmpq)
-{
-   struct uevent *uev, *tmp;
-
-   

[dm-devel] [PATCH 02/12] libmultipath: add merge_node for "struct uevent" to record nodes of merged uevents

[tang . junhui]

From: tang.junhui 

Add merged nodes list to store nodes of merged uevents. By Adding
this member, after merging, the list of uevents would be linked like
this:
uevent
 ---
|struct list_head node  |->list node of un-merged uevents
 ---
|struct list_head merge_node|->list node of merged uevents, which
   moved from the origin un-merged list
|...|
 ---

Change-Id: I5fbfc7656ede77e03ca35c855212e2d2d60706b2
Signed-off-by: tang.junhui 
---
 libmultipath/uevent.c | 4 +++-
 libmultipath/uevent.h | 1 +
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c
index ef1bafe..181b3b8 100644
--- a/libmultipath/uevent.c
+++ b/libmultipath/uevent.c
@@ -72,8 +72,10 @@ struct uevent * alloc_uevent (void)
 {
struct uevent *uev = MALLOC(sizeof(struct uevent));
 
-   if (uev)
+   if (uev) {
INIT_LIST_HEAD(>node);
+   INIT_LIST_HEAD(>merge_node);
+   }
 
return uev;
 }
diff --git a/libmultipath/uevent.h b/libmultipath/uevent.h
index 7bfccef..61a4207 100644
--- a/libmultipath/uevent.h
+++ b/libmultipath/uevent.h
@@ -17,6 +17,7 @@ struct udev;
 
 struct uevent {
struct list_head node;
+   struct list_head merge_node;
struct udev_device *udev;
char buffer[HOTPLUG_BUFFER_SIZE + OBJECT_SIZE];
char *devpath;
-- 
2.8.1.windows.1


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

[dm-devel] [PATCH 12/12] libmultipath: use existing wwid when wwid has already been existed in uevent

[tang . junhui]

From: tang.junhui 

Use existing wwid when wwid has already been existed in uevent, it
avoids to get wwid again in pathinfo(), and reduces the count of calling
getuid(), which would promote processing efficiency.

Change-Id: Ia0c7273d33a220e5b415ec42d6b72660018cf4d9
Signed-off-by: tang.junhui 
---
 libmultipath/discovery.c | 9 ++---
 libmultipath/discovery.h | 4 +++-
 multipathd/main.c| 4 ++--
 3 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index d1aec31..4985e03 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -32,7 +32,7 @@
 #include "defaults.h"
 
 int
-alloc_path_with_pathinfo (struct config *conf, struct udev_device *udevice,
+alloc_path_with_pathinfo (struct config *conf, struct uevent *uev,
  int flag, struct path **pp_ptr)
 {
int err = PATHINFO_FAILED;
@@ -42,7 +42,7 @@ alloc_path_with_pathinfo (struct config *conf, struct 
udev_device *udevice,
if (pp_ptr)
*pp_ptr = NULL;
 
-   devname = udev_device_get_sysname(udevice);
+   devname = udev_device_get_sysname(uev->udev);
if (!devname)
return PATHINFO_FAILED;
 
@@ -51,10 +51,13 @@ alloc_path_with_pathinfo (struct config *conf, struct 
udev_device *udevice,
if (!pp)
return PATHINFO_FAILED;
 
+   if(uev->wwid)
+   strncpy(pp->wwid, uev->wwid, sizeof(pp->wwid));
+
if (safe_sprintf(pp->dev, "%s", devname)) {
condlog(0, "pp->dev too small");
} else {
-   pp->udev = udev_device_ref(udevice);
+   pp->udev = udev_device_ref(uev->udev);
err = pathinfo(pp, conf, flag | DI_BLACKLIST);
}
 
diff --git a/libmultipath/discovery.h b/libmultipath/discovery.h
index 3039268..df7de0f 100644
--- a/libmultipath/discovery.h
+++ b/libmultipath/discovery.h
@@ -1,6 +1,8 @@
 #ifndef DISCOVERY_H
 #define DISCOVERY_H
 
+#include "uevent.h"
+
 #define SYSFS_PATH_SIZE 255
 #define INQUIRY_CMDLEN  6
 #define INQUIRY_CMD 0x12
@@ -36,7 +38,7 @@ int do_tur (char *);
 int path_offline (struct path *);
 int get_state (struct path * pp, struct config * conf, int daemon);
 int pathinfo (struct path * pp, struct config * conf, int mask);
-int alloc_path_with_pathinfo (struct config *conf, struct udev_device *udevice,
+int alloc_path_with_pathinfo (struct config *conf, struct uevent *uev,
  int flag, struct path **pp_ptr);
 int store_pathinfo (vector pathvec, struct config *conf,
struct udev_device *udevice, int flag,
diff --git a/multipathd/main.c b/multipathd/main.c
index 0b18f6c..7a1cd09 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -664,7 +664,7 @@ uev_add_path (struct uevent *uev, struct vectors * vecs, 
int need_do_map)
 * get path vital state
 */
conf = get_multipath_config();
-   ret = alloc_path_with_pathinfo(conf, uev->udev,
+   ret = alloc_path_with_pathinfo(conf, uev,
   DI_ALL, );
put_multipath_config(conf);
if (!pp) {
@@ -1026,7 +1026,7 @@ out:
int flag = DI_SYSFS | DI_WWID;
 
conf = get_multipath_config();
-   retval = alloc_path_with_pathinfo(conf, uev->udev, 
flag, NULL);
+   retval = alloc_path_with_pathinfo(conf, uev, flag, 
NULL);
put_multipath_config(conf);
 
if (retval == PATHINFO_SKIPPED) {
-- 
2.8.1.windows.1


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

[dm-devel] [PATCH 06/12] multipathd: move uev_discard() to uevent.c and change its name to uevent_can_discard()

[tang . junhui]

From: tang.junhui 

Move uev_discard() form uevent processing thread to uevent listening
thread to discard unnecessary uevents as soon as possible.

Change-Id: Iaac159ffe3930e53c3325d1069c3ed497e440c0c
Signed-off-by: tang.wenjun 
---
 libmultipath/uevent.c | 28 
 multipathd/main.c | 25 -
 2 files changed, 28 insertions(+), 25 deletions(-)

diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c
index 181b3b8..ac49cac 100644
--- a/libmultipath/uevent.c
+++ b/libmultipath/uevent.c
@@ -24,6 +24,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -80,6 +81,28 @@ struct uevent * alloc_uevent (void)
return uev;
 }
 
+bool
+uevent_can_discard(char *devpath)
+{
+   char *tmp;
+   char a[11], b[11];
+
+   /*
+* keep only block devices, discard partitions
+*/
+   tmp = strstr(devpath, "/block/");
+   if (tmp == NULL){
+   condlog(4, "no /block/ in '%s'", devpath);
+   return true;
+   }
+   if (sscanf(tmp, "/block/%10s", a) != 1 ||
+   sscanf(tmp, "/block/%10[^/]/%10s", a, b) == 2) {
+   condlog(4, "discard event on %s", devpath);
+   return true;
+   }
+   return false;
+}
+
 void
 service_uevq(struct list_head *tmpq)
 {
@@ -527,6 +550,11 @@ int uevent_listen(struct udev *udev)
uev = uevent_from_udev_device(dev);
if (!uev)
continue;
+   if (uevent_can_discard(uev->devpath)) {
+udev_device_unref(uev->udev);
+FREE(uev);
+continue;
+   }
list_add_tail(>node, _tmp);
events++;
continue;
diff --git a/multipathd/main.c b/multipathd/main.c
index 68c8e17..51f7b60 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1087,28 +1087,6 @@ uxsock_trigger (char * str, char ** reply, int * len, 
void * trigger_data)
return r;
 }
 
-static int
-uev_discard(char * devpath)
-{
-   char *tmp;
-   char a[11], b[11];
-
-   /*
-* keep only block devices, discard partitions
-*/
-   tmp = strstr(devpath, "/block/");
-   if (tmp == NULL){
-   condlog(4, "no /block/ in '%s'", devpath);
-   return 1;
-   }
-   if (sscanf(tmp, "/block/%10s", a) != 1 ||
-   sscanf(tmp, "/block/%10[^/]/%10s", a, b) == 2) {
-   condlog(4, "discard event on %s", devpath);
-   return 1;
-   }
-   return 0;
-}
-
 int
 uev_trigger (struct uevent * uev, void * trigger_data)
 {
@@ -1118,9 +1096,6 @@ uev_trigger (struct uevent * uev, void * trigger_data)
 
vecs = (struct vectors *)trigger_data;
 
-   if (uev_discard(uev->devpath))
-   return 0;
-
pthread_cleanup_push(config_cleanup, NULL);
pthread_mutex_lock(_lock);
if (running_state != DAEMON_IDLE &&
-- 
2.8.1.windows.1


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

[dm-devel] [PATCH 05/12] multipathd: add need_do_map to indicate whether need calling domap() in ev_remove_path()

[tang . junhui]

From: tang.junhui 

Usually calling domap() in ev_remove_path() is needed, but only last
path need to call domap() in processing for merged uevents to reduce the
count of calling domap() and promote efficiency. So add input parameter
need_do_map to indicate whether need calling domap() in ev_remove_path().

Change-Id: I0a787330a249608396cc3e655465dc820f940539
Signed-off-by: tang.wenjun 
---
 multipathd/cli_handlers.c |  2 +-
 multipathd/main.c | 11 +++
 multipathd/main.h |  2 +-
 3 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c
index 3a46c09..302fd02 100644
--- a/multipathd/cli_handlers.c
+++ b/multipathd/cli_handlers.c
@@ -693,7 +693,7 @@ cli_del_path (void * v, char ** reply, int * len, void * 
data)
condlog(0, "%s: path already removed", param);
return 1;
}
-   return ev_remove_path(pp, vecs);
+   return ev_remove_path(pp, vecs, 1);
 }
 
 int
diff --git a/multipathd/main.c b/multipathd/main.c
index 34b1b64..68c8e17 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -840,7 +840,7 @@ fail:
 }
 
 static int
-uev_remove_path (struct uevent *uev, struct vectors * vecs)
+uev_remove_path (struct uevent *uev, struct vectors * vecs, int need_do_map)
 {
struct path *pp;
int ret;
@@ -851,7 +851,7 @@ uev_remove_path (struct uevent *uev, struct vectors * vecs)
pthread_testcancel();
pp = find_path_by_dev(vecs->pathvec, uev->kernel);
if (pp)
-   ret = ev_remove_path(pp, vecs);
+   ret = ev_remove_path(pp, vecs, need_do_map);
lock_cleanup_pop(vecs->lock);
if (!pp) {
/* Not an error; path might have been purged earlier */
@@ -862,7 +862,7 @@ uev_remove_path (struct uevent *uev, struct vectors * vecs)
 }
 
 int
-ev_remove_path (struct path *pp, struct vectors * vecs)
+ev_remove_path (struct path *pp, struct vectors * vecs, int need_do_map)
 {
struct multipath * mpp;
int i, retval = 0;
@@ -884,6 +884,9 @@ ev_remove_path (struct path *pp, struct vectors * vecs)
if ((i = find_slot(mpp->paths, (void *)pp)) != -1)
vector_del_slot(mpp->paths, i);
 
+   if(!need_do_map)
+   goto out;
+
/*
 * remove the map IFF removing the last path
 */
@@ -1161,7 +1164,7 @@ uev_trigger (struct uevent * uev, void * trigger_data)
goto out;
}
if (!strncmp(uev->action, "remove", 6)) {
-   r = uev_remove_path(uev, vecs);
+   r = uev_remove_path(uev, vecs, 1);
goto out;
}
if (!strncmp(uev->action, "change", 6)) {
diff --git a/multipathd/main.h b/multipathd/main.h
index f810d41..094b04f 100644
--- a/multipathd/main.h
+++ b/multipathd/main.h
@@ -23,7 +23,7 @@ const char * daemon_status(void);
 int need_to_delay_reconfig (struct vectors *);
 int reconfigure (struct vectors *);
 int ev_add_path (struct path *, struct vectors *, int);
-int ev_remove_path (struct path *, struct vectors *);
+int ev_remove_path (struct path *, struct vectors *, int);
 int ev_add_map (char *, char *, struct vectors *);
 int ev_remove_map (char *, char *, int, struct vectors *);
 void sync_map_state (struct multipath *);
-- 
2.8.1.windows.1


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

[dm-devel] [PATCH 10/12] libmultipath: filter uevents before proccessing

[tang . junhui]

From: tang.junhui 

Before merging uevents, these uevents are going to be filtered:
Change or addition uevent of a removed path (it indicate by an
deletion uevent occurred later).

Change-Id: If00c2c2e23ea466c1d4643c541ed2d8f9a0c8dea
Signed-off-by: tang.junhui 
---
 libmultipath/uevent.c | 55 +++
 1 file changed, 55 insertions(+)

diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c
index 114068c..424f272 100644
--- a/libmultipath/uevent.c
+++ b/libmultipath/uevent.c
@@ -140,6 +140,28 @@ uevent_can_discard(char *devpath, char *kernel)
 }
 
 bool
+uevent_can_filter(struct uevent *earlier, struct uevent *later)
+{
+
+   /*
+* filter earlier uvents if path has removed later, eg:
+* "add path3 |chang path3 |add path2 |remove path3"
+* can filter as:
+* "add path2 |remove path3"
+* uevent "add path3" and "chang path3" are filtered out
+*/
+   if (earlier->wwid && later->wwid &&
+   !strcmp(earlier->wwid, later->wwid) &&
+   strncmp(later->kernel, "dm-", 3) &&
+   !strcmp(later->action, "remove") &&
+   !strcmp(earlier->kernel, later->kernel)) {
+   return true;
+   }
+
+   return false;
+}
+
+bool
 merge_need_stop(struct uevent *earlier, struct uevent *later)
 {
/*
@@ -196,6 +218,38 @@ uevent_can_merge(struct uevent *earlier, struct uevent 
*later)
 }
 
 void
+uevent_filter(struct uevent *later, struct list_head *tmpq)
+{
+   struct uevent *earlier, *temp;
+   /*
+* compare the uevent with earlier uevents
+*/
+   list_for_some_entry_reverse(earlier, >node, tmpq, node) {
+next_earlier_node:
+   /*
+* filter unnessary earlier uevents by the later uevent
+*/
+   if (uevent_can_filter(earlier, later)) {
+   condlog(2, "uevent: %s-%s-%s has removed by uevent: 
%s-%s-%s, filtered",
+   earlier->action, earlier->kernel, earlier->wwid,
+   later->action, later->kernel, later->wwid);
+
+   temp = earlier;
+   earlier = list_entry(earlier->node.prev, typeof(struct 
uevent), node);
+   list_del_init(>node);
+   if (temp->udev)
+   udev_device_unref(temp->udev);
+   FREE(temp);
+
+   if (earlier ==  list_entry(tmpq, typeof(struct uevent), 
node))
+   break;
+   else
+   goto next_earlier_node;
+   }
+   }
+}
+
+void
 uevent_merge(struct uevent *later, struct list_head *tmpq)
 {
struct uevent *earlier, *temp;
@@ -232,6 +286,7 @@ merge_uevq(struct list_head *tmpq)
struct uevent *later;
 
list_for_each_entry_reverse(later, tmpq, node) {
+   uevent_filter(later, tmpq);
uevent_merge(later, tmpq);
}
 }
-- 
2.8.1.windows.1


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

[dm-devel] [PATCH 01/12] libmultipath: add wwid for "struct uevent" to record wwid of uevent

[tang . junhui]

From: tang.junhui 

Add "char *wwid" to point WWID of uevent. This member identifies
the LUN ID which the path belongs to, and it is used for merging
uevents. WWID possibly did not exist in uevent yet, so ->wwid
would be NULL, those uevents would not be merged, but be proccessed
as old way.

Change-Id: Ie6b076363b3735dc7de10184b27fa799b499af0e
Signed-off-by: tang.junhui 
---
 libmultipath/uevent.c | 2 ++
 libmultipath/uevent.h | 1 +
 2 files changed, 3 insertions(+)

diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c
index 7edcce1..ef1bafe 100644
--- a/libmultipath/uevent.c
+++ b/libmultipath/uevent.c
@@ -424,6 +424,8 @@ struct uevent *uevent_from_udev_device(struct udev_device 
*dev)
uev->devpath = uev->envp[i] + 8;
if (strcmp(name, "ACTION") == 0)
uev->action = uev->envp[i] + 7;
+   if (strcmp(name, "ID_SERIAL") == 0)
+   uev->wwid = uev->envp[i] + 10;
i++;
if (i == HOTPLUG_NUM_ENVP - 1)
break;
diff --git a/libmultipath/uevent.h b/libmultipath/uevent.h
index 9d22dcd..7bfccef 100644
--- a/libmultipath/uevent.h
+++ b/libmultipath/uevent.h
@@ -22,6 +22,7 @@ struct uevent {
char *devpath;
char *action;
char *kernel;
+   char *wwid;
unsigned long seqnum;
char *envp[HOTPLUG_NUM_ENVP];
 };
-- 
2.8.1.windows.1


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

Re: [dm-devel] Improve processing efficiency for addition and deletion of multipath devices

[tang . junhui]

Hello Ben, Hannes 

I'm sorry for the late reply.

> You can't just get the wwid with no work (look at all work uev_add_path
> does, specifically alloc_path_with_pathinfo). Now you could reorder
> this, but there isn't much point, since it is doing useful things, like
> checking if this is a spurious uevent, and necessary things, like
> figuring out the device type and using that that the configuration to
> figure out HOW to get the wwid.

IMO, WWID can be geted from uevent, since it has a ID_SERIAL filed in 
uevent body.

> It seems like what you want to do is to
> call uev_add_path multiple times, but defer most of the work that
> ev_add_path does (creating or updating the multipath device), until
> you've processed all that paths.

Not exactly, the input parameter "struct uevent *uev" is a batch of 
merged uevents, so uev_add_path() is called once to process the merged 
uevent.

> split off uev_add_path() and
> ev_add_path().
> Then uev_add_path could generate a list of fully-formed paths which
> ev_add_path() would process.
> IE generalize coalesce_paths() to work on a passed-in list rather than
> the normal vecs->pathvec.

Hannes, I think my thoughts are close to your idea now, In uev_add_path(), 

we get all information of the merged paths, and then call ev_add_path() 
to create or update the multipath device. Maybe the different between us 
is "processing all the same type uevents(add, etc.) in ev_add_path()" or 
"processing all the same type uevents(add, etc.) which came from the 
same LUN in ev_add_path()". I think your idea can also reach the goal, and
reduce the DM reload times. So we will try to code as your idea, and 
in list_merger_uevents(_tmp) only merger the same type(add, etc.) 
uevents,
add stop mergering when another type uevents are occured.

Thanks all,
Tang
 



发件人: Hannes Reinecke 
收件人: Benjamin Marzinski , 
tang.jun...@zte.com.cn, 
抄送:   dm-devel@redhat.com, zhang.ka...@zte.com.cn, Martin Wilck 
, Bart Van Assche 
日期:   2016/11/28 23:46
主题:   Re: [dm-devel] Improve processing efficiency for addition and 
deletion of multipath devices
发件人: dm-devel-boun...@redhat.com



On 11/28/2016 04:25 PM, Benjamin Marzinski wrote:
> On Mon, Nov 28, 2016 at 10:19:15AM +0800, tang.jun...@zte.com.cn wrote:
>>Hello Christophe, Ben, Hannes, Martin, Bart,
>>I am a member of host-side software development team of ZXUSP 
storage
>>project
>>in ZTE Corporation. Facing the market demand, our team decides to 
write
>>code to
>>promote multipath efficiency next month. The whole idea is in the 
mail
>>below.We
>>hope to participate in and make progress with the open source 
community,
>>so any
>>suggestion and comment would be welcome.
> 
> Like I mentioned before, I think this is a good idea in general, but the
> devil is in the details here.
> 
>>
>>Thanks,
>>Tang
>>
>> 
--
>> 
--
>>1.Problem
>>In these scenarios, multipath processing efficiency is low:
>>1) Many paths exist in each multipath device,
>>2) Devices addition or deletion during iSCSI login/logout or FC link
>>up/down.
> 
> 
> 
>>4.Proposal
>>Other than processing uevents one by one, uevents which coming from 
the
>>same LUN devices can be mergered to one, and then uevent processing
>>thread only needs to process it once, and it only produces one DM 
addition
>>uevent which could reduce system resource consumption.
>>
>>The example in Chapter 2 is continued to use to explain the 
proposal:
>>1) Multipath receives block device addition uevents from udev:
>>UDEV  [89068.806214] add 
>> /devices/platform/host3/session44/target3:0:0/3:0:0:0/block/sdc 
(block)
>>UDEV  [89068.909457] add 
>> /devices/platform/host3/session44/target3:0:0/3:0:0:2/block/sdg 
(block)
>>UDEV  [89068.944956] add 
>> /devices/platform/host3/session44/target3:0:0/3:0:0:1/block/sde 
(block)
>>UDEV  [89068.959215] add 
>> /devices/platform/host5/session46/target5:0:0/5:0:0:0/block/sdh 
(block)
>>UDEV  [89068.978558] add 
>> /devices/platform/host5/session46/target5:0:0/5:0:0:2/block/sdk 
(block)
>>UDEV  [89069.004217] add 
>> /devices/platform/host5/session46/target5:0:0/5:0:0:1/block/sdj 
(block)
>>UDEV  [89069.486361] add 
>> /devices/platform/host4/session45/target4:0:0/4:0:0:1/block/sdf 
(block)
>>UDEV  [89069.495194] add 
>> /devices/platform/host4/session45/target4:0:0/4:0:0:0/block/sdd 
(block)
>>UDEV  [89069.511628] add 
>> /devices/platform/host4/session45/target4:0:0/4:0:0:2/block/sdi 
(block)
>>UDEV  [89069.716292] add 
>> 

Re: [dm-devel] Improve processing efficiency for addition and deletion of multipath devices

[tang . junhui]

Hello Zdenek

I think you suggestion is constructive，
some unnecessary operations in udev rules waster too much system 
resoure(CPU etc.)
It is another way to promote multipath efficiency,
could you put forward more specific amendment?

Regards,
Tang





发件人: Zdenek Kabelac 
收件人: tang.jun...@zte.com.cn, Benjamin Marzinski 
, 
抄送:   zhang.ka...@zte.com.cn, dm-devel@redhat.com, Martin Wilck 
, Bart Van Assche 
日期:   2016/11/29 17:22
主题:   Re: [dm-devel] Improve processing efficiency for addition and 
deletion of multipath devices
发件人: dm-devel-boun...@redhat.com



Dne 23.11.2016 v 02:08 tang.jun...@zte.com.cn napsal(a):
> So, now we have at least 4 ways to improve mutliapth efficiency:
> 1)  Filtering uevents;
> 2)  Merger uevents;
> 3)  Using separate locks for mpvec and pathvec;
> 4)  Get rid of the gazillion waiter threads.
>
> This is exciting, but how do we achieve this blueprint?
> Can we set up some working groups and develop it in parallel
> to implement each improvement since most of them are independent?
>

I'm missing here the analysis of impact on performance.

Do we have 'perf' traces and other reports where is the CPU wasted ?

While surely solving all issues make sense - there are some tasks which 
have 
major impact on speed - while others user will most not notice at all.


Regards

Zdenek

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


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

[dm-devel] [PATCH 1/2] dm-mpath: Enable hw_handler_params to take effect if hw_handler is the same between new and old

[tang . junhui]

From: "tang.junhui" 

If the hardware handle which a device has already attached is the same
with m->hw_handler_name, m->hw_handler_params should not be ignored, but
be enabled to take effect.

Signed-off-by: tang.junhui 
---
 drivers/md/dm-mpath.c | 14 +-
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
index 0caab4b..b1aba63 100644
--- a/drivers/md/dm-mpath.c
+++ b/drivers/md/dm-mpath.c
@@ -849,19 +849,23 @@ static struct pgpath *parse_path(struct dm_arg_set *as, 
struct path_selector *ps
 retain:
attached_handler_name = scsi_dh_attached_handler_name(q, 
GFP_KERNEL);
if (attached_handler_name) {
+   /* clear any hw_handler_params associated with
+* m->hw_handler_params if a different handler has
+* already been attached.
+*/
+   if(!m->hw_handler_name || strcmp(attached_handler_name, 
m->hw_handler_name))
+   {
+   kfree(m->hw_handler_params);
+   m->hw_handler_params = NULL;
+   }
/*
 * Reset hw_handler_name to match the attached handler
-* and clear any hw_handler_params associated with the
-* ignored handler.
 *
 * NB. This modifies the table line to show the actual
 * handler instead of the original table passed in.
 */
kfree(m->hw_handler_name);
m->hw_handler_name = attached_handler_name;
-
-   kfree(m->hw_handler_params);
-   m->hw_handler_params = NULL;
}
}
 
-- 
2.8.1.windows.1


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

Re: [dm-devel] Improve processing efficiency for addition and deletion of multipath devices

[tang . junhui]

So, now we have at least 4 ways to improve mutliapth efficiency:
1)  Filtering uevents;
2)  Merger uevents;
3)  Using separate locks for mpvec and pathvec;
4)  Get rid of the gazillion waiter threads.

This is exciting, but how do we achieve this blueprint? 
Can we set up some working groups and develop it in parallel 
to implement each improvement since most of them are independent?



发件人: "Benjamin Marzinski" 
收件人: Martin Wilck , 
抄送:   dm-devel@redhat.com, tang.jun...@zte.com.cn
日期:   2016/11/19 06:33
主题:   Re: [dm-devel] Improve processing efficiency for addition and 
deletion of multipath devices
发件人: dm-devel-boun...@redhat.com



My $.02

I'm not sure that we want to wait for a predetermined time to grab the
uevents.  If they are coming in slowly, multipathd is more responsive by
processing them immediately, and if they are coming in a storm, they
will naturally pile up faster than we deal with them. I also don't
really like the idea of waiting when we get an event to see if another
one comes along, for the same reasons. That being said, I wouldn't NACK
an config option (turned off by default) that set a minimum wait time
between uevent dispatches, because I might be wrong here.  However, It
sees like what uevent_dispatch() dispatch already does is the right
thing, which is to pull all oustanding uevents off the uevent queue into
a new list.

The issue is that we simply need to work on processing the whole list at
a time.  Martin's filtering definitely has a place here. He is correct
that if we get a delete uevent for a device, we don't need to process
any of the earlier ones.  We do need to look at both the add and change
uevents individually. For instance, one of the change uevents out of a
group could be for changing the read-only status of a device. If we just
took the most recent, we would miss that information.  The good news is
that we don't need to actually call uev_add_path and uev_update_path
once for each of these events.  All we need to do is read through the
uev environment variables for all of them to find the information we
need (like change in ro status), and then call the function once with
that information.

Hannes pointed out that for adding paths we don't need to do any locking
until we want to add the path to the pathvec. We could grab all the
outstanding add events from the list that gets passed to service_uevq,
and collect the pathinfo for all the paths, add them all into the
pathvec, and then create or update the multipath devices. delete uevents
could work similarly, where we find all the paths for a multipath device
in our list, remove them all, and reload the device once.

I'm not sure how much a separate thread for doing the multipath work
would buy us, however. It's true that we could have a seperate lock for
the mpvec, so we didn't need to hold the pathvec lock while updating the
mpvec, but actually updating the mpvec only takes a moment. The time
consuming part is building the multipath device and passing it to the
kernel. For this, we do need the the paths locked. Otherwise what would
keep a path from getting removed while the multipath device was using it
to set get set up. If working with multipath devices and proccessing
path uevents is happening at the same time, this is a very real
possibility.

But even if we keep one thread processing the uevents, simply avoiding
calling uev_add_path and uev_update_path for repeated add and change
events, and batching multiple add and delete events together could
provide a real speedup, IMHO.

Now, the holy grail of multipathd efficiency would be to totally rework
multipathd's locking into something sensible.  We could have locks for
the vectors that only got held when you were actually traversing or
manipulating the vectors, coupled with reference counts on the
individual paths and multipaths, so that they didn't get removed while
they were in use, and probably locks on the individual paths and
multipaths for just the sections that really needed to be protected.
The problem is that this would take a huge amount of code auditting to
get mostly right, and then a while to flush out all the missed corner
cases.  At least I think it would. But I dismissed decoupling the config
structure from the vecs lock as too painful, and clearly that was
possible.

At any rate, I'd rather get rid of the gazillion waiter threads first.

-Ben

On Thu, Nov 17, 2016 at 11:48:32AM +0100, Martin Wilck wrote:
> Hi Tang,
> 
> > As to process several uevents for the same physical devices, I think
> > the opinions 
> > different between us is "FILTER" or "MERGER". Personally, I think
> > Merger is more 
> > accuracy, for example, we receive 4 paths addition uevent messages
> > from the same 
> > physical devices: 
> > 1)uevent add sdb 
> > 2)uevent add sdc 
> > 3)uevent add sdd 
> > 4)uevent add sde 
> > 
> > We cannot just filter the 1)2)3) uevent messages but only process the
> > 4)uevent message, 

Re: [dm-devel] Improve processing efficiency for addition and deletion of multipath devices

[tang . junhui]

Hi Martin,

〉But once you start merging, you'd rather be prepared for
〉several events for the same phys device, too.

We can base on such a threshold that there is no repeat uevents from the 
same sd device,
otherwise, we pause doing merger, and kick uevent processing thread to 
process the merged uevents.

Regards,
Tang




发件人: Martin Wilck 
收件人: tang.jun...@zte.com.cn, 
抄送:   dm-devel@redhat.com
日期:   2016/11/18 16:38
主题:   Re: [dm-devel] Improve processing efficiency for addition and 
deletion of multipath devices
发件人: dm-devel-boun...@redhat.com



On Fri, 2016-11-18 at 16:24 +0800, tang.jun...@zte.com.cn wrote:
> Hi Martin, 
> 
> In your case, my action is: 
> 1) merger uevents 1) 2) to one uevent "add sda sdb", and process them
> togother 

This will fail because sdb is non-existent at the time you try - no?

> Though the processing efficiency in such scenario is lower than
> yours, but it is simple and reliable,
> more importantly, Martin, you still focus on such special scene,
> which I concerned is like this: 

[...]

I understand what you're concerned with. I just think we need to do
both. I agree that many events for many different devices are more
likely. But once you start merging, you'd rather be prepared for
several events for the same phys device, too.

Martin

-- 
Dr. Martin Wilck , Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

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

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

Re: [dm-devel] Improve processing efficiency for addition and deletion of multipath devices

[tang . junhui]

Hi Martin,

>  This will fail because sdb is non-existent at the time you try - no?
Non-existent paths in merger uevents do not affect the entire algorithm. 
It should be considered at the time of implementation.

Your suggestion is meaningful, but I think we need to focus on the existed 
problem, 
resolve it, and then do more work to make it better in other areas. is not 
it?

Regards,Tang





发件人: Martin Wilck 
收件人: tang.jun...@zte.com.cn, 
抄送:   dm-devel@redhat.com
日期:   2016/11/18 16:31
主题:   Re: Re: Re: [dm-devel] Improve processing efficiency for addition 
and deletion of multipath devices



On Fri, 2016-11-18 at 16:24 +0800, tang.jun...@zte.com.cn wrote:
> Hi Martin, 
> 
> In your case, my action is: 
> 1) merger uevents 1) 2) to one uevent "add sda sdb", and process them
> togother 

This will fail because sdb is non-existent at the time you try - no?

> Though the processing efficiency in such scenario is lower than
> yours, but it is simple and reliable,
> more importantly, Martin, you still focus on such special scene,
> which I concerned is like this: 

[...]

I understand what you're concerned with. I just think we need to do
both. I agree that many events for many different devices are more
likely. But once you start merging, you'd rather be prepared for
several events for the same phys device, too.

Martin

-- 
Dr. Martin Wilck , Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)



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

Re: [dm-devel] Improve processing efficiency for addition and deletion of multipath devices

[tang . junhui]

Hi Martin,

In your case, my action is:
1) merger uevents 1) 2) to one uevent "add sda sdb", and process them 
togother
2) process uevent "del sda"
3) process uevent "add sdc"
4) process uevent "del sdb"
5) process uevent "add sda"

Though the processing efficiency in such scenario is lower than yours, but 
it is simple and reliable,
more importantly, Martin, you still focus on such special scene, which I 
concerned is like this:
UDEV  [19172.014482] add 
/devices/platform/host3/session17/target3:0:0/3:0:0:0/block/sdc (block)
UDEV  [19172.249389] add 
/devices/platform/host4/session18/target4:0:0/4:0:0:0/block/sdd (block)
UDEV  [19172.343791] add 
/devices/platform/host3/session17/target3:0:0/3:0:0:2/block/sdf (block)
UDEV  [19172.364496] add 
/devices/platform/host5/session19/target5:0:0/5:0:0:0/block/sdh (block)
UDEV  [19172.523794] add 
/devices/platform/host4/session18/target4:0:0/4:0:0:2/block/sdi (block)
UDEV  [19172.621333] add 
/devices/platform/host3/session17/target3:0:0/3:0:0:4/block/sdn (block)
UDEV  [19172.699473] add 
/devices/platform/host4/session18/target4:0:0/4:0:0:1/block/sdg (block)
UDEV  [19172.704605] add 
/devices/platform/host3/session17/target3:0:0/3:0:0:1/block/sde (block)
UDEV  [19172.709687] add 
/devices/platform/host3/session17/target3:0:0/3:0:0:3/block/sdj (block)
UDEV  [19172.714919] add 
/devices/platform/host4/session18/target4:0:0/4:0:0:3/block/sdl (block)
UDEV  [19172.728891] add 
/devices/platform/host4/session18/target4:0:0/4:0:0:4/block/sdo (block)
UDEV  [19172.872156] add 
/devices/platform/host3/session17/target3:0:0/3:0:0:6/block/sdt (block)
UDEV  [19172.915542] add 
/devices/platform/host4/session18/target4:0:0/4:0:0:6/block/sdu (block)
UDEV  [19173.086935] add 
/devices/platform/host6/session20/target6:0:0/6:0:0:0/block/sdw (block)
UDEV  [19173.108278] add 
/devices/platform/host6/session20/target6:0:0/6:0:0:1/block/sdz (block)
UDEV  [19173.195153] add 
/devices/platform/host4/session18/target4:0:0/4:0:0:5/block/sdr (block)
UDEV  [19173.228397] add 
/devices/platform/host3/session17/target3:0:0/3:0:0:5/block/sdq (block)
UDEV  [19173.363632] add 
/devices/platform/host5/session19/target5:0:0/5:0:0:2/block/sdm (block)
UDEV  [19173.386560] add 
/devices/platform/host3/session17/target3:0:0/3:0:0:7/block/sdx (block)
UDEV  [19173.394515] add 
/devices/platform/host4/session18/target4:0:0/4:0:0:7/block/sdy (block)
UDEV  [19173.410152] add 
/devices/platform/host5/session19/target5:0:0/5:0:0:1/block/sdk (block)
UDEV  [19173.474286] add 
/devices/platform/host6/session20/target6:0:0/6:0:0:2/block/sdab (block)
UDEV  [19173.508438] add 
/devices/platform/host5/session19/target5:0:0/5:0:0:3/block/sdp (block)
UDEV  [19173.713146] add 
/devices/platform/host5/session19/target5:0:0/5:0:0:4/block/sds (block)
UDEV  [19173.782065] add 
/devices/platform/host5/session19/target5:0:0/5:0:0:5/block/sdv (block)
UDEV  [19173.787447] add 
/devices/platform/host5/session19/target5:0:0/5:0:0:6/block/sdaa (block)
UDEV  [19173.803218] add 
/devices/platform/host6/session20/target6:0:0/6:0:0:3/block/sdad (block)
UDEV  [19173.849411] add 
/devices/platform/host5/session19/target5:0:0/5:0:0:7/block/sdac (block)
UDEV  [19173.918301] add 
/devices/platform/host6/session20/target6:0:0/6:0:0:5/block/sdaf (block)
UDEV  [19173.941005] add 
/devices/platform/host6/session20/target6:0:0/6:0:0:4/block/sdae (block)
UDEV  [19173.987206] add 
/devices/platform/host6/session20/target6:0:0/6:0:0:7/block/sdah (block)
UDEV  [19173.992431] add 
/devices/platform/host6/session20/target6:0:0/6:0:0:6/block/sdag (block)

Or like this:
UDEV  [20712.402631] remove 
/devices/platform/host3/session17/target3:0:0/3:0:0:0/block/sdc (block)
UDEV  [20712.427716] remove 
/devices/platform/host6/session20/target6:0:0/6:0:0:0/block/sdw (block)
UDEV  [20712.459854] remove 
/devices/platform/host4/session18/target4:0:0/4:0:0:0/block/sdd (block)
UDEV  [20712.471124] remove 
/devices/platform/host5/session19/target5:0:0/5:0:0:0/block/sdh (block)
UDEV  [20712.492190] remove 
/devices/platform/host6/session20/target6:0:0/6:0:0:1/block/sdz (block)
UDEV  [20712.495245] remove 
/devices/platform/host4/session18/target4:0:0/4:0:0:1/block/sdg (block)
UDEV  [20712.512007] remove 
/devices/platform/host3/session17/target3:0:0/3:0:0:1/block/sde (block)
UDEV  [20712.522986] remove 
/devices/platform/host5/session19/target5:0:0/5:0:0:1/block/sdk (block)
UDEV  [20712.528676] remove 
/devices/platform/host6/session20/target6:0:0/6:0:0:2/block/sdab (block)
UDEV  [20712.529891] remove 
/devices/platform/host5/session19/target5:0:0/5:0:0:2/block/sdm (block)
UDEV  [20712.536178] remove 
/devices/platform/host4/session18/target4:0:0/4:0:0:2/block/sdi (block)
UDEV  [20712.545444] remove 
/devices/platform/host4/session18/target4:0:0/4:0:0:3/block/sdl (block)
UDEV  [20712.548006] remove 
/devices/platform/host3/session17/target3:0:0/3:0:0:3/block/sdj (block)
UDEV  [20712.551935] remove 

Re: [dm-devel] Improve processing efficiency for addition and deletion of multipath devices

[tang . junhui]

Hi Martin,

For your opinion:

>  My "filtering" idea was meant for cases where several events
>  for the same device are queued, e.g

>  1) add sda
>  2) change sda
>  3) delete sda

>Is it always sufficient to look only at the last event in such a case?

I do not agree with you. The reasons are as follows:

1) It’s risky to filter uevents like that, a SCSI device has been 
generated, 
   May be its life time is very short, but we cannot turn a blind eye on 
it, 
   the system or applications may need multipath device of the SCSI 
device.

2) This scenario you mentioned rarely happens, we are more concerned 
   about the more common situation like addition or deletion devices when 
   iSCSI login/logout or FC link up/down with many iSCSI or FC links in 
   each LUN. At this situation we may receive many uevents from different
   paths of the same LUN device, we want merge these uevents to one and 
   process them together.

Regards,
Tang






发件人: Martin Wilck 
收件人: tang.jun...@zte.com.cn, 
抄送:   dm-devel@redhat.com
日期:   2016/11/17 18:57
主题:   Re: [dm-devel] Improve processing efficiency for addition and 
deletion of multipath devices
发件人: dm-devel-boun...@redhat.com



Hi Tang,

> As to process several uevents for the same physical devices, I think
> the opinions 
> different between us is "FILTER" or "MERGER". Personally, I think
> Merger is more 
> accuracy, for example, we receive 4 paths addition uevent messages
> from the same 
> physical devices: 
> 1)uevent add sdb 
> 2)uevent add sdc 
> 3)uevent add sdd 
> 4)uevent add sde 
> 
> We cannot just filter the 1)2)3) uevent messages but only process the
> 4)uevent message, 
> which would cause losing paths of this multipath devices. 

Of course. My "filtering" idea was meant for cases where several events
for the same device are queued, e.g.

  1) add sda
  2) change sda
  3) delete sda

Is it always sufficient to look only at the last event in such a case?
I think so, but I'm not 100% certain.

Regards
Martin

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

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

Re: [dm-devel] Improve processing efficiency for addition and deletion of multipath devices

[tang . junhui]

Hi Martin,

Nice to see you, May you success in your team and our open source 
community.

You have put forward a good proposal to queued more uevent messages by 
kicking uevent 
processing thread in a predefined time intervals. It is also a good idea 
to have such intervals 
configuration.

As to process several uevents for the same physical devices, I think the 
opinions 
different between us is "FILTER" or "MERGER". Personally, I think Merger 
is more 
accuracy, for example, we receive 4 paths addition uevent messages from 
the same 
physical devices:
1)uevent add sdb
2)uevent add sdc
3)uevent add sdd
4)uevent add sde

We cannot just filter the 1)2)3) uevent messages but only process the 
4)uevent message, 
which would cause losing paths of this multipath devices.

In my opionion, we should MERGE the four uevent messages to one like:
1)uevent add sdb sdb sdd sde

And then uevent processing thread only needs to process one uevent 
message, and it 
only produce one DM addition uevent messages(VS. now: one DM addition 
uevent 
message and 3 DM change uevent messages) which really reduce system 
consumption 
(for example: avoid udev to process every DM uevent messages DM kernel 
produced).

Regards,Tang



发件人: Martin Wilck 
收件人: dm-devel@redhat.com, 
日期:   2016/11/16 17:59
主题:   Re: [dm-devel] Improve processing efficiency for addition and 
deletion of multipath devices
发件人: dm-devel-boun...@redhat.com



Hi Tang,

On Wed, 2016-11-16 at 16:45 +0800, tang.jun...@zte.com.cn wrote:

> I think we can merger most of uevent messages and reduce most of
> unnecessary DM change 
> uevent messages during creation/deletion of multipath devices by this
> way. 
> The method you mentioned I think that it is a little complex, and it
> not reduce the DM 
> addition/change/deletion uevent messages which consumed system high
> resource. 

Let me quickly introduce myself, I'm a new member of Hannes' team at
SUSE and new on this ML.

Apart from Hannes' proposal for more fine-grained locking, I can see
the following options:

 a) instead of kicking the uevent processing thread whenever anything
is queued, kick it in predefined time intervals (1 second, say). The
uevent processor would then accumulate all changes received since it
had been kicked last, and apply DM changes only when necessary. This
may need to be a config option because it could obviously lead to
delayed device setup in some configurations.

 b) the logic of a) could be improved by the uevent listener detecting
"event storms" and waiting for them to end before kicking the
processing thread. The details of the heuristics for "storms" would
need to be worked out, of course.

 c) sometimes several uevents for the same physical path occur in quick
succession. Normally it should be sufficient to filter these such that
only the last event is processed.

Regards,
Martin

-- 
Dr. Martin Wilck , Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

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

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

Re: [dm-devel] Improve processing efficiency for addition and deletion of multipath devices

[tang . junhui]

Hello Hannes:

Since the received uevent messages store in the queue, and the speed 
uevent messages received is 
much faster than the speed uevent messages processed, so, we can merge 
these queued uevent 
message first, and then process it in the next step. Of course, some paths
’ uevent messages of 
multipath device may not be received yet, but we do not need to wait for 
it, since we can deal with 
the left paths in the original way when we received uevent messages of 
these paths later. 

I think we can merger most of uevent messages and reduce most of 
unnecessary DM change 
uevent messages during creation/deletion of multipath devices by this way.

The method you mentioned I think that it is a little complex, and it not 
reduce the DM 
addition/change/deletion uevent messages which consumed system high 
resource.

Sincerely
Tang


On 11/16/2016 02:46 AM, tang.jun...@zte.com.cn wrote:
> In these scenarios, multipath processing efficiency is very low:
> 1) There are many paths in multipath devices,
> 2) Add/delete devices when iSCSI login/logout or FC link up/down.
> 
> Multipath process so slowly that it is not satisfied some applications,
> For example, openstack is often timeout in waiting for the creation of
> multipath devices.
> 
> The reason of the low processing efficiency I think is that multipath
> process uevent message one by one, and each one also produce a new dm
> addition/change/deletion uevent message to increased system resource
> consumption, actually most of these uevent message have no sense at all.
> 
> So, can we find out a way to reduce these uevent messages to promote
> multipath processing efficiency? Personally, I think we can merge
> these uevent messages before processing. For example, during the
> 4 iSCSI session login procedures, we can wait for a moment until
> the addition uevent messages of 4 paths are all arrived, and then we can
> merge these uevent messages to one and deal with it at once. The way to
> deal with deletion uevent messages is the same.
> 
> How do you think about? Any points of view are welcome.

The problem is that we don't know beforehand on how many uevents we
should be waiting for.
And even if we do there still would be a chance of one or several of
these uevents failing to setup the device, so we would be waiting
forever here.

The one possible way out would be to modify the way we're handling
events internally. Event processing really are several steps:
1) Getting information about the attached device (pathinfo() and friends)
2) Store the information in our pathvec
3) Identify and update the mpp structure with the new pathvecs
Currently, we're processing each step for every uevent.
As we have only a single lock protecting both, pathvec and mppvec, we
have to take the lock prior to setup 2 and release it after step 3.
So while we could receive events in parallel, we can only process them
one-by-one, with every event having to re-do step 3.

The idea would be to split off single lock into a pathvec lock and an
mppvec lock, and create a separate thread for updating mppvec.

Then event processing can be streamlined by having the uevent thread
adding the new device to the pathvec and notifying the mppvec thread.
This thread could then check if an pathvec update is in progress, and
only start once this pathvec handling has finished.
With this we would avoid having to issue several similar mppvec updates,
and the entire handling would be far smoother.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke Teamlead Storage & 
Networking
h...@suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

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


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

Re: [dm-devel] dm mpath: remove unusable kmultipathd work queue

[tang . junhui]

Hello Mike,

I used the master branch, the code seems a little older,
It's true that the newer branches such as dm-4.10 have reused 
kmultipathd.

Thanks,
Tang



发件人: Mike Snitzer 
收件人: tang.jun...@zte.com.cn, 
抄送:   zhang.ka...@zte.com.cn, dm-devel@redhat.com, a...@redhat.com
日期:   2016/11/03 22:57
主题:   Re: [dm-devel] dm mpath: remove unusable kmultipathd work queue
发件人: dm-devel-boun...@redhat.com



On Thu, Nov 03 2016 at  6:29am -0400,
tang.jun...@zte.com.cn  wrote:

> From: "tang.junhui" 
> 
> kmultipathd work queue is used to process queued IOs for multipath
> target when IOs need to retry, but now these IOs have been requeued
> to block queue of DM device, hence this work queue is not useful
> anymore, and this patch deletes it.
> 
> Signed-off-by: tang.junhui 

Nack, you must be looking at older code.  bio-based DM multipath makes
use of kmultipathd.

When suggesting changes please always make sure you're working with the
latest upstream Linux code.

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


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

Re: [dm-devel] [PATCH] mpathpersistent: segment faulty occured in mpath_persistent_reserve_in()

[tang . junhui]

Hello Hannes,

Since this issue is introduced by RCU, can you have a review for this 
patch?

Thanks,
Tang





发件人: tang.we...@zte.com.cn
收件人: christophe varoqui , 
抄送:   zhang.ka...@zte.com.cn, dm-devel@redhat.com, 
tang.jun...@zte.com.cn, tang.we...@zte.com.cn
日期:   2016/10/27 17:08
主题:   [dm-devel] [PATCH] mpathpersistent: segment faulty occured in 
mpath_persistent_reserve_in()
发件人: dm-devel-boun...@redhat.com



From: 10111224 

Segment faulty occured when executing "mpathpersist -i -k
/dev/mapper/mpath1" command.The reason is that an uninitialized global 
variable conf is used in mpath_persistent_reserve_in(). The same problem 
also exists in
mpath_persistent_reserve_out().

Signed-off-by: tang.wenji 
---
 libmpathpersist/mpath_persist.c | 21 +++--
 libmpathpersist/mpathpr.h   |  4 
 2 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/libmpathpersist/mpath_persist.c 
b/libmpathpersist/mpath_persist.c
index 7501651..582d4ef 100644
--- a/libmpathpersist/mpath_persist.c
+++ b/libmpathpersist/mpath_persist.c
@@ -78,6 +78,7 @@ updatepaths (struct multipath * mpp)
 int i, j;
 struct pathgroup * pgp;
 struct path * pp;
+struct config *conf;
 
 if (!mpp->pg)
 return 0;
@@ -97,16 +98,24 @@ updatepaths (struct multipath * mpp)
  continue;
 }
 pp->mpp = 
mpp;
+conf = 
get_multipath_config();
 pathinfo(pp, conf, DI_ALL);
+ put_multipath_config(conf);
 continue;
 }
 pp->mpp = mpp;
 if (pp->state == 
PATH_UNCHECKED ||
-  pp->state == PATH_WILD)
+  pp->state == PATH_WILD){
+conf = 
get_multipath_config();
 pathinfo(pp, conf, DI_CHECKER);
+ put_multipath_config(conf);
+}
 
-if (pp->priority == 
PRIO_UNDEF)
+if (pp->priority == 
PRIO_UNDEF){
+conf = 
get_multipath_config();
 pathinfo(pp, conf, DI_PRIO);
+ put_multipath_config(conf);
+}
 }
 }
 return 0;
@@ -159,8 +168,11 @@ int mpath_persistent_reserve_in (int fd, int 
rq_servact,
 int map_present;
 int major, minor;
 int ret;
+struct config *conf;
 
+conf = get_multipath_config();
 conf->verbosity = verbose;
+put_multipath_config( conf);
 
 if (fstat( fd, ) != 0){
 condlog(0, "stat error %d", fd);
@@ -252,8 +264,11 @@ int mpath_persistent_reserve_out ( int fd, int 
rq_servact, int rq_scope,
 int j;
 unsigned char *keyp;
 uint64_t prkey;
+struct config *conf;
 
+conf = get_multipath_config();
 conf->verbosity = verbose;
+put_multipath_config(conf);
 
 if (fstat( fd, ) != 0){
 condlog(0, "stat error fd=%d", fd);
@@ -320,7 +335,9 @@ int mpath_persistent_reserve_out ( int fd, int 
rq_servact, int rq_scope,
 goto out1;
 }
 
+conf = get_multipath_config();
 select_reservation_key(conf, mpp);
+put_multipath_config(conf);
 
 switch(rq_servact)
 {
diff --git a/libmpathpersist/mpathpr.h b/libmpathpersist/mpathpr.h
index cd58201..e6c2ded 100644
--- a/libmpathpersist/mpathpr.h
+++ b/libmpathpersist/mpathpr.h
@@ -25,10 +25,6 @@ struct threadinfo {
 struct prout_param param;
 };
 
-
-struct config * conf;
-
-
 int prin_do_scsi_ioctl(char * dev, int rq_servact, struct prin_resp * 
resp, int noisy);
 int prout_do_scsi_ioctl( char * dev, int rq_servact, int rq_scope,
 unsigned int rq_type, struct 
prout_param_descriptor *paramp, int noisy);
-- 
2.8.1.windows.1

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


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

[dm-devel] [PATCH] md/dm-mpath: Add NULL pointer check in parse_hw_handler()

[tang . junhui]

From: "tang.junhui" 

m->hw_handler_name value should be checked in parse_hw_handler()
after calling kstrdup() to avoid access of NULL pointer.

Signed-off-by: tang.junhui 
---
 drivers/md/dm-mpath.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
index d376dc8..77428fb 100644
--- a/drivers/md/dm-mpath.c
+++ b/drivers/md/dm-mpath.c
@@ -732,6 +732,9 @@ static int parse_hw_handler(struct dm_arg_set *as, struct 
multipath *m)
return 0;
 
m->hw_handler_name = kstrdup(dm_shift_arg(as), GFP_KERNEL);
+   if (!m->hw_handler_name)
+   return -ENOMEM;
+
if (!try_then_request_module(scsi_dh_handler_exist(m->hw_handler_name),
 "scsi_dh_%s", m->hw_handler_name)) {
ti->error = "unknown hardware handler type";
-- 
2.8.1.windows.1

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

Re: [dm-devel] md/dm-table: tgt->type should be putted in dm_table_add_target()

[tang . junhui]

Hello, Mike

Thanks for your quick respond. 
This problem causes failure in "rmmod dm-spp" command,
(dm-spp is a tagert type for our product)
since the reference count is not zero due to 
the failure of DM table loads.

I don't intend to cc stable, I just add the 
"Cc: sta...@vger.kernel.org" after Signed-off-by in
commit message as Bart suggested it to me, then I send email
by "git send-email" command, and it cc to stable automatically,
I think maybe my git is not configured well, I'll research it 
later.

(BTW, I like your name, Mike, Thank you for your accompanying 
with us in our English class for so many years.) 

thanks,

Tang
 



发件人: Mike Snitzer 
收件人: tang.jun...@zte.com.cn, 
抄送:   zhang.ka...@zte.com.cn, dm-devel@redhat.com, 
sta...@vger.kernel.org, a...@redhat.com
日期:   2016/10/21 10:24
主题:   Re: [dm-devel] md/dm-table: tgt->type should be putted in 
dm_table_add_target()
发件人: dm-devel-boun...@redhat.com



On Thu, Oct 20 2016 at  9:35pm -0400,
tang.jun...@zte.com.cn  wrote:

> From: "tang.junhui" 
> 
> tgt->type should be putted in dm_table_add_target()
> when the target do not statisfy the needs of target type,
> otherwise it would cause the module reference count
> of this target type leakage.
> 
> Signed-off-by: tang.junhui 
> Cc: sta...@vger.kernel.org

This issue, missing dm_put_target_type, dates back to 2011.

See these 3 commits:
$ git log --oneline 3791e2fc0^..36a0456fb
36a0456 dm table: add immutable feature
cc6cbe1 dm table: add always writeable feature
3791e2f dm table: add singleton feature

So are you having problems with failed DM table loads (due to the errors
below) resulting in the inability to unload a DM module?

(BTW, you don't need to cc stable when initially proposing a patch for
stable.  The stable@ maintainers will automatically pull such stable@
changes in once they land in Linus' tree.)

Mike

> ---
>  drivers/md/dm-table.c | 24 
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
> index 6554d91..4f56c38 100644
> --- a/drivers/md/dm-table.c
> +++ b/drivers/md/dm-table.c
> @@ -698,30 +698,30 @@ int dm_table_add_target(struct dm_table *t, const 
char *type,
> 
>if (dm_target_needs_singleton(tgt->type)) {
>if (t->num_targets) {
> -  DMERR("%s: target type %s 
must appear alone in table",
> - dm_device_name(t->md), type);
> -  return -EINVAL;
> +  tgt->error = "target type 
must appear alone in table";
> +  r = -EINVAL;
> +  goto bad;
>}
>t->singleton = 1;
>}
> 
>if (dm_target_always_writeable(tgt->type) && !(t->mode & 
FMODE_WRITE)) {
> -  DMERR("%s: target type %s may not be 
included in read-only tables",
> -dm_device_name(t->md), type);
> -  return -EINVAL;
> +  tgt->error = "target type may not be 
included in read-only tables";
> +  r = -EINVAL;
> +  goto bad;
>}
> 
>if (t->immutable_target_type) {
>if (t->immutable_target_type != 
tgt->type) {
> -  DMERR("%s: immutable 
target type %s cannot be mixed with other target types",
> - dm_device_name(t->md), t->immutable_target_type->name);
> -  return -EINVAL;
> +  tgt->error = "immutable 
target type cannot be mixed with other target types";
> +  r = -EINVAL;
> +  goto bad;
>}
>} else if (dm_target_is_immutable(tgt->type)) {
>if (t->num_targets) {
> -  DMERR("%s: immutable 
target type %s cannot be mixed with other target types",
> - dm_device_name(t->md), tgt->type->name);
> -  return -EINVAL;
> +  tgt->error = "immutable 
target type cannot be mixed with other target types";
> +  r = -EINVAL;
> +  goto bad;
>}
>t->immutable_target_type = tgt->type;
>}
> -- 
> 2.8.1.windows.1
> 

--
dm-devel mailing list
dm-devel@redhat.com

[dm-devel] [PATCH 3/3] Treat PATH_TIMEOUT as PATH_DOWN

[tang . junhui]

From: "tang.junhui" 

Treat PATH_TIMEOUT as PATH_DOWN since it is basically the same as
PATH_DOWN, except with a different state name. So change it to PATH_DOWN
immediately if it is PATH_TIMEOUT according to the method in pathinfo().
After the modification all PATH_TIMEOUTs are equal to PATH_DOWN since the
rest of pp->state is assigned all in pathinfo().

Signed-off-by: tang.junhui 
---
 multipathd/main.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/multipathd/main.c b/multipathd/main.c
index e369a79..f715eb5 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1528,6 +1528,10 @@ check_path (struct vectors * vecs, struct path * pp, int 
ticks)
put_multipath_config(conf);
return 1;
}
+
+   if (newstate == PATH_TIMEOUT)
+   newstate = PATH_DOWN;
+
if (!pp->mpp) {
if (!strlen(pp->wwid) && pp->initialized != INIT_MISSING_UDEV &&
(newstate == PATH_UP || newstate == PATH_GHOST)) {
@@ -1601,7 +1605,7 @@ check_path (struct vectors * vecs, struct path * pp, int 
ticks)
pp->checkint = conf->checkint;
put_multipath_config(conf);
 
-   if (newstate == PATH_DOWN || newstate == PATH_SHAKY || newstate 
== PATH_TIMEOUT) {
+   if (newstate == PATH_DOWN || newstate == PATH_SHAKY) {
/*
 * proactively fail path in the DM
 */
-- 
2.8.1.windows.1

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

[dm-devel] [PATCH 1/3] segment faulty occured in dm_get_status()

[tang . junhui]

From: "tang.junhui" 

Signed-off-by: tang.junhui 

segment faulty occured in dm_get_status(), the call trace is:
--
(gdb) bt
0  0x00338ec328a5 in raise () from /lib64/libc.so.6
1  0x00338ec3400d in abort () from /lib64/libc.so.6
2  0x0040596d in sigsegv ()
3  
4  0x00338ec480ac in vfprintf () from /lib64/libc.so.6
5  0x00338ec6f9d2 in vsnprintf () from /lib64/libc.so.6
6  0x00338ec4f4a3 in snprintf () from /lib64/libc.so.6
7  0x7f43fe66bbb9 in dm_get_status (name=0x7f43f0001eb0 "spathe",
outstatus=) at devmapper.c:521
8  0x7f43fe68c058 in update_multipath_status (mpp=0x7f43f000ad60,
pathvec=) at structs_vec.c:465
9  update_multipath_strings (mpp=0x7f43f000ad60, pathvec=) at structs_vec.c:495
10 0x00409056 in check_path ()
11 0x00409c7e in checkerloop ()
12 0x003b27207851 in start_thread () from /lib64/libpthread.so.0
13 0x00338ece890d in clone () from /lib64/libc.so.6
---
we debuged the code, and found that targets information storing in the
list dmt->head which fetching from kernel by executing dm_task_run()
is null.
---
rbx0x7f43c4000d60   139929027874144
(gdb) p *(struct dm_task *)0x7f43c4000d60
$3 = {type = 10, dev_name = 0x7f43c4000d40 "spathe", head = 0x0, tail =
0x0, read_only = 0, event_nr = 0, major = -1, minor = -1,
  allow_default_major_fallback = 1, uid = 0, gid = 0, mode = 384,
  read_ahead = 4294967295, read_ahead_flags = 0, dmi = {
  v4 = 0x7f43c40011e0, v1 = 0x7f43c40011e0}, newname = 0x0, message =
  0x0, geometry = 0x0, sector = 0, no_flush = 0,
no_open_count = 1, skip_lockfs = 0, query_inactive_table = 0,
suppress_identical_reload = 0, existing_table_size = 0,
  cookie_set = 0, uuid = 0x0}
---
since status is not initioned to null, so after calling
dm_get_next_target(dmt, next, , ,_type,
),status becaming wild pointer, which lead snprintf() into the
situation of segment faulty.
---
 libmultipath/devmapper.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
index 5eb1713..89aa5da 100644
--- a/libmultipath/devmapper.c
+++ b/libmultipath/devmapper.c
@@ -534,8 +534,8 @@ dm_get_status(char * name, char * outstatus)
int r = 1;
struct dm_task *dmt;
uint64_t start, length;
-   char *target_type;
-   char *status;
+   char *target_type = NULL;
+   char *status = NULL;
 
if (!(dmt = dm_task_create(DM_DEVICE_STATUS)))
return 1;
@@ -551,6 +551,10 @@ dm_get_status(char * name, char * outstatus)
/* Fetch 1st target */
dm_get_next_target(dmt, NULL, , ,
   _type, );
+   if (!status) {
+   condlog(2, "get null status.");
+   goto out;
+   }
 
if (snprintf(outstatus, PARAMS_SIZE, "%s", status) <= PARAMS_SIZE)
r = 0;
-- 
2.8.1.windows.1

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

[dm-devel] 答复: [RFC] unreleased lock after handler failure

[tang . junhui]

Hi, Michael Wang

This patch looks well for me, cleanup_lock() should be called without
affected by the return value of h->fn().

Regards,
Tang



发件人: Michael Wang 
收件人: christophe.varo...@opensvc.com, dm-devel@redhat.com, 
日期:   2016/10/12 16:11
主题:   [dm-devel] [RFC] unreleased lock after handler failure
发件人: dm-devel-boun...@redhat.com



Hi, folks

While we are testing with the latest multipathd, we encounter issue that 
once
'del map' failed, all the following operation will show 'error -1 
receiving packet'
whatever how long the timeout has been set (we have applied the latest fix 
which
make sure the poll will last for 10 seconds).

After some debugging we found that inside parse_cmd(), the 
pthread_cleanup_pop()
rely on '!r' as indicator for locked or not, while this will be 
overwritten if
the handler return failed (1 in our case), and then the unlock will be 
missed.

After applied below fix the problem got solved, although the del map 
failed, the
other operation can still works.

Could you please help to review whether this is really the problem to be 
fixed
like that?

Regards,
Michael Wang



diff --git a/multipathd/cli.c b/multipathd/cli.c
index e8a9384..50161be 100644
--- a/multipathd/cli.c
+++ b/multipathd/cli.c
@@ -481,6 +481,7 @@ parse_cmd (char * cmd, char ** reply, int * len, void 
* data, int timeout )
tmo.tv_sec = 0;
}
if (h->locked) {
+   int locked = 0;
struct vectors * vecs = (struct vectors *)data;
 
pthread_cleanup_push(cleanup_lock, >lock);
@@ -491,10 +492,11 @@ parse_cmd (char * cmd, char ** reply, int * len, 
void * data, int timeout )
r = 0;
}
if (r == 0) {
+   locked = 1;
pthread_testcancel();
r = h->fn(cmdvec, reply, len, data);
}
-   pthread_cleanup_pop(!r);
+   pthread_cleanup_pop(locked);
} else
r = h->fn(cmdvec, reply, len, data);
free_keys(cmdvec);

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


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

[dm-devel] 答复: Re: [PATCH] libmultipath: fix multipath -q command logic

[tang . junhui]

Hello Hannes,
check_daemon() now is only used to determine whether to set/remove 
queue_if_no_path feature for
mapped devices, which you said maybe another issue, we are looking forward 
for your patch.

Thanks,
Tang



发件人: Hannes Reinecke 
收件人: tang.jun...@zte.com.cn, bmarz...@redhat.com, 
抄送:   Bart Van Assche , device-mapper 
development , zhang.ka...@zte.com.cn
日期:   2016/10/11 18:42
主题:   Re: [dm-devel] [PATCH] libmultipath: fix multipath -q command 
logic
发件人: dm-devel-boun...@redhat.com



On 10/11/2016 11:17 AM, tang.jun...@zte.com.cn wrote:
> Hello Hannes, Ben,
> Could you have a review for this patch, any comment will be highly
> appreciated.
>
> Thanks,
> Tang
>
>
>
>
> 发件人: Christophe Varoqui 
> 收件人: tang.jun...@zte.com.cn,
> 抄送:Bart Van Assche , device-mapper
> development , zhang.ka...@zte.com.cn
> 日期: 2016/10/11 14:59
> 主题:Re: [dm-devel] [PATCH] libmultipath: fix multipath -q
> command logic
> 发件人:dm-devel-boun...@redhat.com
> 
>
>
>
> Hannes, Ben,
>
> are you ok with the solution to these two issues.
> Seems sane to me.
>
This actually is only part of the story.

The whole idea of issuing 'multipath' is to check if a given path 
_should_ be multipathed (as this is typically called from an udev event).
But as it's called from an udev event we cannot rely on the multipath 
daemon to be started; we might just handle an event which came in before 
multipathd got started from systemd.
So checking for the PID file is not enough, we need to check if the 
daemon will be started eventually.
And in fact checking the PID file or calling mpath_connect() is 
equivalent, with the added advantage the mpath_connect() will start the 
daemon _if enabled by systemd_.
So this patch doesn't help much, as it doesn't solve the main problem of 
figuring out if multipathd _should_ be started.

I've done a patch for checking the '.wants' directories from systemd, 
but this obviously will only work if the OS is systemd-based.
And it's not really perfect, as there are corner-cases where just 
checking for the .wants directory is not enough.

Cheers,

Hannes
-- 
Dr. Hannes ReineckezSeries & Storage
h...@suse.de   +49 911 
74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)

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

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

Re: [dm-devel] [PATCH] libmultipath: fix multipath -q command logic

[tang . junhui]

Hello Ben,

In multipath manual document, multipath -q command is described as:
"-q allow device tables with queue_if_no_path when multiathd is not 
running".
However the command does not take effect when we testing it.

About use-case, In my opinion, sometimes we need to stop multipath service 
temporarily (for example: multipath version upgrade), but we do not want 
IO interrupt during those times even path down existed, so we can execute 
“multiath -q” to keep IO queue until we starting service after version 
upgrade.

Cheers,
Tang





发件人: "Benjamin Marzinski" 
收件人: Hannes Reinecke , 
抄送:   Bart Van Assche , device-mapper 
development , tang.jun...@zte.com.cn, 
zhang.ka...@zte.com.cn
日期:   2016/10/12 10:53
主题:   Re: [dm-devel] [PATCH] libmultipath: fix multipath -q command 
logic
发件人: dm-devel-boun...@redhat.com



On Tue, Oct 11, 2016 at 12:33:43PM +0200, Hannes Reinecke wrote:
> On 10/11/2016 11:17 AM, tang.jun...@zte.com.cn wrote:
> >Hello Hannes, Ben,
> >Could you have a review for this patch, any comment will be highly
> >appreciated.
> >
> >Thanks,
> >Tang
> >
> >
> >
> >
> >发件人: Christophe Varoqui 
> >收件人: tang.jun...@zte.com.cn,
> >抄送:Bart Van Assche , 
device-mapper
> >development , zhang.ka...@zte.com.cn
> >日期: 2016/10/11 14:59
> >主题:Re: [dm-devel] [PATCH] libmultipath: fix multipath -q
> >command logic
> >发件人:dm-devel-boun...@redhat.com
> 
>
> >
> >
> >
> >Hannes, Ben,
> >
> >are you ok with the solution to these two issues.
> >Seems sane to me.
> >
> This actually is only part of the story.
> 
> The whole idea of issuing 'multipath' is to check if a given path 
_should_
> be multipathed (as this is typically called from an udev event).
> But as it's called from an udev event we cannot rely on the multipath 
daemon
> to be started; we might just handle an event which came in before 
multipathd
> got started from systemd.
> So checking for the PID file is not enough, we need to check if the 
daemon
> will be started eventually.
> And in fact checking the PID file or calling mpath_connect() is 
equivalent,
> with the added advantage the mpath_connect() will start the daemon _if
> enabled by systemd_.
> So this patch doesn't help much, as it doesn't solve the main problem of
> figuring out if multipathd _should_ be started.
> 
> I've done a patch for checking the '.wants' directories from systemd, 
but
> this obviously will only work if the OS is systemd-based.
> And it's not really perfect, as there are corner-cases where just 
checking
> for the .wants directory is not enough.

I think you are dealing with a different issue.  the "-q" option for
multipath just overrides the default behaviour of not enabling
queue_if_no_path when creating a multipath device is multipathd isn't
running.  For this, we don't care if multipathd will start running in
the future, because if/when multipathd does start up, it will set the
queue_if_no_path flag itself. We only care about now, when we know it's
not running.

Really, I doubt that anyone ever uses the -q option. So your change in
how multipath checks if multipathd is running is fine by me. However,
you also changed what the "-q" option does.  Previously, it just kept
multipath from disabling "queue_if_no_path" on devices that were
configured to have it, when multipathd wasn't running.  With your code,
doesn't it now make all devices set queue_if_no_path if multipathd isn't
running, including devices that weren't configured to set it even when
multipathd is running? Is there a reason for this? In general, setting
"queue_if_no_path" when multiapthd isn't running is a bad idea, since
the paths will never come back, and nothing will ever disable the
queueing.  That's why I said that I doubt anybody uses the option.
Forcing all devices to set "queue_if_no_path", even if they weren't
configured to, seems even less useful.  Or is there actually a use-case
that I'm missing here?

-Ben

> 
> Cheers,
> 
> Hannes
> -- 
> Dr. Hannes Reinecke  zSeries & Storage
> h...@suse.de +49 911 
74053 688
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
> GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)

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

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

Re: [dm-devel] [PATCH] libmultipath: fix multipath -q command logic

[tang . junhui]

Hello Hannes, Ben,
Could you have a review for this patch, any comment will be highly 
appreciated. 

Thanks,
Tang




发件人: Christophe Varoqui 
收件人: tang.jun...@zte.com.cn, 
抄送:   Bart Van Assche , device-mapper 
development , zhang.ka...@zte.com.cn
日期:   2016/10/11 14:59
主题:   Re: [dm-devel] [PATCH] libmultipath: fix multipath -q command 
logic
发件人: dm-devel-boun...@redhat.com



Hannes, Ben,

are you ok with the solution to these two issues.
Seems sane to me.

Thanks,
Christophe

On Tue, Oct 11, 2016 at 8:46 AM,  wrote:
Please have a review for this patch, any comment will be highly 
appreciated.




发件人: tang.jun...@zte.com.cn 
收件人: christophe varoqui , 
抄送:dm-devel@redhat.com, zhang.ka...@zte.com.cn, "tang.junhui" <
tang.jun...@zte.com.cn> 
日期: 2016/08/16 19:33 
主题:[PATCH] libmultipath: fix multipath -q command logic 




From: "tang.junhui" 

multipath judged whether multipathd service in running by check_daemon() 
when executing
mutipath commands, check_daemon() try to connect to the multipathd service 
and execute
"show dameon" command. The expected result is that the command will be 
failed when the
service is not running, however, mpath_connect() will activate the 
multipathd service when
the service is not running, so check_daemon() always return with true. 
Another problem is that
multipath command with -q parameter is not processed in coalesce_paths(). 
This patch fix for
those two problems.

Signed-off-by: tang.junhui 
---
libmultipath/configure.c | 85 
+++-
1 file changed, 48 insertions(+), 37 deletions(-)

diff --git a/libmultipath/configure.c b/libmultipath/configure.c
index 707e6be..d8a17a6 100644
--- a/libmultipath/configure.c
+++ b/libmultipath/configure.c
@@ -715,36 +715,36 @@ deadmap (struct multipath * mpp)
 return 1; /* dead */
}

-int check_daemon(void)
+static int
+check_daemon(void)
{
 int fd;
- char *reply;
- int ret = 0;
- unsigned int timeout;
- struct config *conf;
-
- fd = mpath_connect();
- if (fd == -1)
-  return 0;
+ struct flock lock;

- if (send_packet(fd, "show daemon") != 0)
-  goto out;
- conf = get_multipath_config();
- timeout = conf->uxsock_timeout;
- put_multipath_config(conf);
- if (recv_packet(fd, , timeout) != 0)
-  goto out;
-
- if (strstr(reply, "shutdown"))
-  goto out_free;
-
- ret = 1;
+ fd = open(DEFAULT_PIDFILE, O_RDONLY);
+ if (fd < 0) {
+  if (errno == ENOENT)
+   return 0;
+  if (errno == EMFILE)
+   condlog(0, "failed to 
open file, increase max_fds at %s", DEFAULT_CONFIGFILE);
+  else
+   condlog(0, "can not 
open pidfile %s: %s", DEFAULT_PIDFILE, strerror(errno));
+  return -1;
+ }

-out_free:
- FREE(reply);
-out:
- mpath_disconnect(fd);
- return ret;
+ lock.l_type = F_WRLCK;
+ lock.l_start = 0;
+ lock.l_whence = SEEK_SET;
+ lock.l_len = 0;
+ if (fcntl(fd, F_GETLK, ) < 0) {
+  condlog(0, "can not get file locker, 
%s: %s", DEFAULT_PIDFILE, strerror(errno));
+  close(fd);
+  return -1;
+ }
+ close(fd);
+ if (lock.l_type == F_UNLCK)
+  return 0;
+ return 1;
}

extern int
@@ -873,17 +873,28 @@ coalesce_paths (struct vectors * vecs, vector newmp, 
char * refwwid, int force_r
  if (r == DOMAP_DRY)
   continue;

-  conf = get_multipath_config();
-  allow_queueing = conf->allow_queueing;
-  put_multipath_config(conf);
-  if (!is_daemon && !allow_queueing && 
!check_daemon()) {
-   if (mpp->no_path_retry 
!= NO_PATH_RETRY_UNDEF &&
-   mpp->no_path_retry 
!= 

Re: [dm-devel] [PATCH 07/15] multipathd: Fix a data race

[tang . junhui]

Hello Bart,
This patch solved the data race problem,
but the assignment for paths check interval has no effect,
since there is no path in vecs when calling init_path_check_interval(vecs) 
in child(),
I think it is better to call  init_path_check_interval(vecs) at 
reconfigure() or configure() 
after the paths has created in vecs .

Thanks,
Tang





发件人: Bart Van Assche 
收件人: Christophe Varoqui , 
抄送:   device-mapper development 
日期:   2016/10/05 01:45
主题:   [dm-devel] [PATCH 07/15] multipathd: Fix a data race
发件人: dm-devel-boun...@redhat.com



Avoid that the path check interval initialization loop races with
other code that accesses the path vectors by executing that code
on the context of the main thread instead of the checker loop.
This patch avoids that DRD reports the following:

Conflicting store by thread 1 at 0x07f3f1f8 size 8
   at 0x40B50C: reconfigure (main.c:2014)
   by 0x40C2EC: child (main.c:2371)
   by 0x40CDA1: main (main.c:2609)
Address 0x7f3f1f8 is at offset 40 from 0x7f3f1d0. Allocation context:
   at 0x4C32995: calloc (in 
/usr/lib64/valgrind/vgpreload_drd-amd64-linux.so)
   by 0x5DDAC36: zalloc (memory.c:34)
   by 0x40B61A: init_vecs (main.c:2043)
   by 0x40BF01: child (main.c:2295)
   by 0x40CDA1: main (main.c:2609)

Signed-off-by: Bart Van Assche 
---
 multipathd/main.c | 24 +++-
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/multipathd/main.c b/multipathd/main.c
index 3030e85..cdfafe8 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1742,6 +1742,19 @@ check_path (struct vectors * vecs, struct path * 
pp, int ticks)
 return 1;
 }
 
+static void init_path_check_interval(struct vectors *vecs)
+{
+struct config *conf;
+struct path *pp;
+unsigned int i;
+
+vector_foreach_slot (vecs->pathvec, pp, i) {
+conf = get_multipath_config();
+pp->checkint = conf->checkint;
+put_multipath_config(conf);
+}
+}
+
 static void *
 checkerloop (void *ap)
 {
@@ -1759,15 +1772,6 @@ checkerloop (void *ap)
 vecs = (struct vectors *)ap;
 condlog(2, "path checkers start up");
 
-/*
- * init the path check interval
- */
-vector_foreach_slot (vecs->pathvec, pp, i) {
-conf = get_multipath_config();
-pp->checkint = conf->checkint;
-put_multipath_config(conf);
-}
-
 /* Tweak start time for initial path check */
 if (clock_gettime(CLOCK_MONOTONIC, _time) != 0)
 last_time.tv_sec = 0;
@@ -2327,6 +2331,8 @@ child (void * param)
  */
 post_config_state(DAEMON_CONFIGURE);
 
+init_path_check_interval(vecs);
+
 /*
  * Start uevent listener early to catch events
  */
-- 
2.10.0

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


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

[dm-devel] [PATCH] multipathd: uxsock_timeout should be assigned by conf->uxsock_timeout

[tang . junhui]

From: "tang.junhui" 

uxsock_timeout should be assigned by conf->uxsock_timeout
before using it in uxclnt() as a CLI client timeout value,
otherwise its default value is 0, and the CLI client timeout
value is 0 + 100(ms), so the CLI client will be timeout very
quickly.

Signed-off-by: tang.junhui 
---
 multipathd/main.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/multipathd/main.c b/multipathd/main.c
index 96ef01f..a08f1a5 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -2549,6 +2549,7 @@ main (int argc, char *argv[])
exit(1);
if (verbosity)
conf->verbosity = verbosity;
+   uxsock_timeout = conf->uxsock_timeout;
uxclnt(optarg, uxsock_timeout + 100);
exit(0);
case 'B':
@@ -2573,6 +2574,7 @@ main (int argc, char *argv[])
exit(1);
if (verbosity)
conf->verbosity = verbosity;
+   uxsock_timeout = conf->uxsock_timeout;
memset(cmd, 0x0, CMDSIZE);
while (optind < argc) {
if (strchr(argv[optind], ' '))
-- 
2.8.1.windows.1

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

[dm-devel] [PATCH] kpartx: partitions of mapped devices cannot be created in redhat OS

[tang . junhui]

From: "tang.junhui" 

Environment variables such as DM_TABLE_STATE are used in kpartx.rules which 
exported
by "dmsetup export" in previous udev rules in SuSE OS, however, there is no 
such command
"dmsetup export" in redhat OS, so these environment variables are not 
initialized and
 partitions cannot be created. This patch replace "dmsetup export" with 
"dmsetup info"
 to get the status of mapped device, which can work well both in SuSE and 
redhat OS.

Signed-off-by: tang.junhui 
---
 kpartx/kpartx.rules | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/kpartx/kpartx.rules b/kpartx/kpartx.rules
index 1713f3c..8c8b836 100644
--- a/kpartx/kpartx.rules
+++ b/kpartx/kpartx.rules
@@ -7,8 +7,12 @@
 KERNEL!="dm-*", GOTO="kpartx_end"
 ACTION=="remove", GOTO="kpartx_end"
 
-ENV{DM_TABLE_STATE}!="LIVE", GOTO="kpartx_end"
-ENV{DM_DEPS}=="0", GOTO="kpartx_end"
+ENV{DMSETUP_SBIN_PATH}="/sbin"
+TEST!="$env{DMSETUP_SBIN_PATH}/dmsetup", ENV{DMSETUP_SBIN_PATH}="/usr/sbin"
+IMPORT{program}="$env{DMSETUP_SBIN_PATH}/dmsetup info -j %M -m %m -c 
--nameprefixes --noheadings --rows -o tables_loaded,device_count"
+
+ENV{DM_TABLES_LOADED}!="Live", GOTO="kpartx_end"
+ENV{DM_DEVICE_COUNT}=="0", GOTO="kpartx_end"
 
 ENV{DM_UUID}=="?*", IMPORT{program}=="kpartx_id %M %m $env{DM_UUID}"
 
@@ -37,7 +41,7 @@ ENV{ID_FS_USAGE}=="filesystem|other", 
ENV{ID_FS_LABEL_ENC}=="?*", \
 # Create dm tables for partitions
 ENV{DM_ACTION}=="PATH_FAILED|PATH_REINSTATED", GOTO="kpartx_end"
 ENV{DM_NR_VALID_PATHS}=="0", GOTO="kpartx_end"
-ENV{DM_STATE}!="SUSPENDED", ENV{DM_UUID}=="mpath-*", \
+ENV{DM_SUSPENDED}!="Suspended", ENV{DM_UUID}=="mpath-*", \
RUN+="/sbin/kpartx -u -p -part /dev/$name"
 
 LABEL="kpartx_end"
-- 
2.8.1.windows.1

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

[dm-devel] [PATCH] libmultipath: fix memory API logic error

[tang . junhui]

From: "tang.junhui" 

Memroy API use mem_allocated to record the total size of used memory,
however, it's wrong to use size(p) as the length of freed memory in xfree(),
and memory may also be allocated by STRDUP() or REALLOC(), which is
not calculated into mem_allocated, so the total size of used memory is
not correctly. For these reasons, we removed these incorrectly code to keep
the code clean.

Signed-off-by: tang.junhui 
---
 libmpathpersist/mpath_updatepr.c |  1 -
 libmultipath/memory.c| 31 ---
 libmultipath/memory.h| 13 ++---
 3 files changed, 2 insertions(+), 43 deletions(-)

diff --git a/libmpathpersist/mpath_updatepr.c b/libmpathpersist/mpath_updatepr.c
index 9ff4b30..5af2e03 100644
--- a/libmpathpersist/mpath_updatepr.c
+++ b/libmpathpersist/mpath_updatepr.c
@@ -16,7 +16,6 @@
 #include "uxsock.h"
 #include "memory.h"
 
-unsigned long mem_allocated;/* Total memory used in Bytes */
 
 int update_prflag(char * arg1, char * arg2, int noisy)
 {
diff --git a/libmultipath/memory.c b/libmultipath/memory.c
index 1366f45..5441e6a 100644
--- a/libmultipath/memory.c
+++ b/libmultipath/memory.c
@@ -22,37 +22,6 @@
 
 #include "memory.h"
 
-/* Global var */
-unsigned long mem_allocated;   /* Total memory used in Bytes */
-
-void *
-xalloc(unsigned long size)
-{
-   void *mem;
-   if ((mem = malloc(size)))
-   mem_allocated += size;
-   return mem;
-}
-
-void *
-zalloc(unsigned long size)
-{
-   void *mem;
-   if ((mem = malloc(size))) {
-   memset(mem, 0, size);
-   mem_allocated += size;
-   }
-   return mem;
-}
-
-void
-xfree(void *p)
-{
-   mem_allocated -= sizeof (p);
-   free(p);
-   p = NULL;
-}
-
 /*
  * Memory management. in debug mode,
  * help finding eventual memory leak.
diff --git a/libmultipath/memory.h b/libmultipath/memory.h
index 8573f6f..99937b2 100644
--- a/libmultipath/memory.h
+++ b/libmultipath/memory.h
@@ -29,15 +29,6 @@
 #include 
 #include 
 
-/* extern types */
-extern unsigned long mem_allocated;
-extern void *xalloc(unsigned long size);
-extern void *zalloc(unsigned long size);
-extern void xfree(void *p);
-
-/* Global alloc macro */
-#define ALLOC(n) (xalloc(n))
-
 /* Local defines */
 #ifdef _DEBUG_
 
@@ -63,8 +54,8 @@ extern void dbg_free_final(char *);
 
 #else
 
-#define MALLOC(n)(zalloc(n))
-#define FREE(p)  (xfree(p))
+#define MALLOC(n)(calloc(1,(n)))
+#define FREE(p)  (free(p))
 #define REALLOC(p,n) (realloc((p),(n)))
 #define STRDUP(n)(strdup(n))
 
-- 
2.8.1.windows.1

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

[dm-devel] [PATCH] libmultipath: retry to get path serial number by get_serial() after failure by get_vpd_sysfs()

[tang . junhui]

From: "tang.junhui" 

Some operating systems have no such file vpd_pg80 in sys directory
such as /sys/devices/platform/host23/session21/target23:0:0/23:0:0:33/,
so path serial number would be failed to get by get_vpd_sysfs(),
we need retry to get it by get_serial to avoid empty serial of the path.

Signed-off-by: tang.junhui 
---
 libmultipath/discovery.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index 0974c92..bb3116d 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -1435,10 +1435,14 @@ scsi_ioctl_pathinfo (struct path * pp, int mask)
if (!attr_path || pp->sg_id.host_no == -1)
return 0;
 
-   if (get_vpd_sysfs(parent, 0x80, pp->serial, SERIAL_SIZE) > 0)
-   condlog(3, "%s: serial = %s",
-   pp->dev, pp->serial);
+   if (get_vpd_sysfs(parent, 0x80, pp->serial, SERIAL_SIZE) <= 0) {
+   if (get_serial(pp->serial, SERIAL_SIZE, pp->fd)) {
+   condlog(2, "%s: fail to get serial", pp->dev);
+   return 0;
+   }
+   }
 
+   condlog(3, "%s: serial = %s", pp->dev, pp->serial);
return 0;
 }
 
-- 
2.8.1.windows.1

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

[dm-devel] [PATCH] multipathd: fail path when path check timeout

[tang . junhui]

From: "tang.junhui" 

path should be failed when path status is PATH_TIMEOUT after check,
otherwise, the valid number of paths in the map would be increased when
the path status is PATH_UP after the next turn check, which would cause
the valid number of paths exceeding the total number of paths in the map.

Signed-off-by: tang.junhui 
---
 multipathd/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/multipathd/main.c b/multipathd/main.c
index f5e9a01..01f1e58 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1585,7 +1585,7 @@ check_path (struct vectors * vecs, struct path * pp, int 
ticks)
pp->checkint = conf->checkint;
put_multipath_config(conf);
 
-   if (newstate == PATH_DOWN || newstate == PATH_SHAKY) {
+   if (newstate == PATH_DOWN || newstate == PATH_SHAKY || newstate 
== PATH_TIMEOUT) {
/*
 * proactively fail path in the DM
 */
-- 
2.8.1.windows.1

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

[dm-devel] [PATCH] multipathd: fix memory leak in reconfigure()

[tang . junhui]

From: "tang.junhui" 

Problem:
Memory leak exists in reconfigure() when multipathd command reconfigure is 
executed.

Reasons:
 * The following judgment condition is not satisfied when there is no path,
   free_pathvec() is not called to free the vector of vecs->pathvec:
   if (VECTOR_SIZE(vecs->pathvec))
   free_pathvec(vecs->pathvec, FREE_PATHS);
 * Then the vecs->pathvec is set to NULL, so the vector memory which
   vecs->pathvec pointed to is leaked:
   vecs->pathvec = NULL;

Signed-off-by: tang.junhui 
---
 multipathd/main.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/multipathd/main.c b/multipathd/main.c
index c129298..d9731cd 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1997,9 +1997,7 @@ reconfigure (struct vectors * vecs)
if (VECTOR_SIZE(vecs->mpvec))
remove_maps_and_stop_waiters(vecs);
 
-   if (VECTOR_SIZE(vecs->pathvec))
-   free_pathvec(vecs->pathvec, FREE_PATHS);
-
+   free_pathvec(vecs->pathvec, FREE_PATHS);
vecs->pathvec = NULL;
 
/* Re-read any timezone changes */
-- 
2.8.1.windows.1


ZTE Information Security Notice: The information contained in this mail (and 
any attachment transmitted herewith) is privileged and confidential and is 
intended for the exclusive use of the addressee(s).  If you are not an intended 
recipient, any disclosure, reproduction, distribution or other dissemination or 
use of the information contained is strictly prohibited.  If you have received 
this mail in error, please delete it and notify us immediately.

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

[dm-devel] [PATCH] libmultipath: fix memory leak in reconfigure

[tang . junhui]

Problem:

 * Memory leak exists in reconfigure function when multipathd command 
"reconfigure" is executed.

Reasons:

 * The following judgement condition is not satisfied when there is no 
path , free_pathvec is not called to free the vector of vecs->pathvec:
if (VECTOR_SIZE(vecs->pathvec))
free_pathvec(vecs->pathvec, FREE_PATHS);

 * Then the vecs->pathvec is set to NULL, so the vector memory which 
vecs->pathvec pointed to is leaked:
vecs->pathvec = NULL;


Please review the solutions to the problem in the attachment:


 
唐军辉  Tang Junhui
主机侧软件开发  Host-side Software Development
无线研究院 / 虚拟化长沙开发部  NIV Changsha Development Dept/Wireless 
Product R Institute/Wireless Product Operation



湖南长沙岳麓区尖山路中电软件园8楼 410205
8/F,CEC Software Park,NO.39, Jianshan Road, Lugu High-tech Industrial
Center, Yuelu District, Changsha, P.R. China
T: 0731 x
M: +86 15616251617
E: tangjun...@zte.com.cn
www.zte.com.cn


ZTE Information Security Notice: The information contained in this mail (and 
any attachment transmitted herewith) is privileged and confidential and is 
intended for the exclusive use of the addressee(s).  If you are not an intended 
recipient, any disclosure, reproduction, distribution or other dissemination or 
use of the information contained is strictly prohibited.  If you have received 
this mail in error, please delete it and notify us immediately.


[dm-devel] [PATCH] libmultipath fix memory leak	in reconfigure function.patch
Description: Binary data
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel