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

[tang.junhui@xxxxxxxxxx]

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 <bart.vanassche@xxxxxxxxxxx>
收件人:         Christophe Varoqui <christophe.varoqui@xxxxxxxxxxx>,
抄送:        device-mapper development <dm-devel@xxxxxxxxxx>
日期:         2016/10/05 01:45
主题:        [dm-devel] [PATCH 07/15] multipathd: Fix a data race
发件人:        dm-devel-bounces@xxxxxxxxxx

---

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 <bart.vanassche@xxxxxxxxxxx>
---
 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, &last_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@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel

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