Just an update for the list:

I have replied privately to Roi that I have some issue with this patch, but 
a bigger issue with fixing the discoveryd problem this way, i.e. 
single-threading it.

If I can get a copy of the finer-grained locking code from him I will test 
it out. He says that it has some 2nd-layer issues.

On Sunday, August 23, 2015 at 6:33:02 AM UTC-7, Roi Dayan wrote:
>
> Since iscsi responses from the kernel are multicast we might get into 
> a situation where multiple discoveryd handle the same response. 
> With this commit we use a shared mutex to lock between the discoveryd 
> forks 
> so each fork will handle its intended portal. 
> So we wait for login and stale logouts to finish between releasing the 
> mutex. 
>
> Signed-off-by: Roi Dayan <[email protected] <javascript:>> 
> --- 
>
>
> Hi, 
>
> The issue was discovered after working with discoveryd and offload driver 
> which needed the following patch that was already submitted to the mailing 
> list: 
>
> [PATCH] Discovery daemon via non-tcp transport needs 'ipc' set 
>
> Thanks, 
> Roi 
>
>
> V2: 
>     didn't actually need iscsi_login_portal_wait 
>
>
>  usr/Makefile     |  2 +- 
>  usr/discoveryd.c | 57 
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++-- 
>  2 files changed, 56 insertions(+), 3 deletions(-) 
>
> diff --git a/usr/Makefile b/usr/Makefile 
> index e23fee1..dca7b6e 100644 
> --- a/usr/Makefile 
> +++ b/usr/Makefile 
> @@ -55,7 +55,7 @@ all: $(PROGRAMS) 
>   
>  iscsid: $(ISCSI_LIB_SRCS) $(INITIATOR_SRCS) $(DISCOVERY_SRCS) \ 
>          iscsid.o session_mgmt.o discoveryd.o 
> -        $(CC) $(CFLAGS) $(LDFLAGS) $^ -o $@  -L../utils/open-isns -lisns 
> -lrt -lmount 
> +        $(CC) $(CFLAGS) $(LDFLAGS) $^ -o $@  -L../utils/open-isns -lisns 
> -lrt -lmount -pthread 
>   
>  iscsiadm: $(ISCSI_LIB_SRCS) $(DISCOVERY_SRCS) iscsiadm.o session_mgmt.o 
>          $(CC) $(CFLAGS) $(LDFLAGS) $^ -o $@ -L../utils/open-isns -lisns 
> diff --git a/usr/discoveryd.c b/usr/discoveryd.c 
> index 2d3ccbc..6943662 100644 
> --- a/usr/discoveryd.c 
> +++ b/usr/discoveryd.c 
> @@ -27,6 +27,10 @@ 
>  #include <time.h> 
>  #include <sys/types.h> 
>  #include <sys/wait.h> 
> +#include <sys/mman.h> 
> +#include <sys/stat.h> 
> +#include <pthread.h> 
> +#include <fcntl.h> 
>   
>  #include "discovery.h" 
>  #include "idbm.h" 
> @@ -47,6 +51,9 @@ 
>  #include "iscsi_err.h" 
>   
>  #define DISC_DEF_POLL_INVL        30 
> +#define MUTEX "/lock" 
> + 
> +pthread_mutex_t *mutex = NULL; 
>   
>  static LIST_HEAD(iscsi_targets); 
>  static int stop_discoveryd; 
> @@ -62,6 +69,43 @@ static void isns_reg_refresh_by_eid_qry(void *data); 
>  typedef void (do_disc_and_login_fn)(const char *def_iname, 
>                                      struct discovery_rec *drec, int 
> poll_inval); 
>   
> +static void discoveryd_create_shared_mutex(void) 
> +{ 
> +        int mode = S_IRWXU | S_IRWXG; 
> +        int des_mutex; 
> +        pthread_mutexattr_t attr; 
> + 
> +        des_mutex = shm_open(MUTEX, O_CREAT | O_RDWR | O_TRUNC, mode); 
> + 
> +        if (des_mutex < 0) { 
> +            log_error("Error creating mutex"); 
> +            return; 
> +        } 
> + 
> +        if (ftruncate(des_mutex, sizeof(pthread_mutex_t)) == -1) { 
> +                log_error("Error on truncate"); 
> +                return; 
> +        } 
> + 
> +        mutex = (pthread_mutex_t*) mmap(NULL, sizeof(pthread_mutex_t), 
> +                    PROT_READ | PROT_WRITE, MAP_SHARED, des_mutex, 0); 
> + 
> +        if (mutex == MAP_FAILED ) { 
> +                log_error("Error on mmap"); 
> +                return; 
> +        } 
> + 
> +        pthread_mutexattr_init(&attr); 
> +        pthread_mutexattr_setpshared(&attr, PTHREAD_PROCESS_SHARED); 
> +        pthread_mutex_init(mutex, &attr); 
> +        pthread_mutexattr_destroy(&attr); 
> +} 
> + 
> +static void discoveryd_destroy_shared_mutex(void) 
> +{ 
> +        pthread_mutex_destroy(mutex); 
> +} 
> + 
>  static int logout_session(void *data, struct list_head *list, 
>                            struct session_info *info) 
>  { 
> @@ -96,6 +140,7 @@ static void discoveryd_stop(void) 
>          } 
>   
>  done: 
> +        discoveryd_destroy_shared_mutex(); 
>          exit(0); 
>  } 
>   
> @@ -174,7 +219,7 @@ static void update_sessions(struct list_head 
> *new_rec_list, 
>   
>          list_for_each_entry_safe(rec, tmp_rec, new_rec_list, list) { 
>                  if (!iscsi_check_for_running_session(rec)) 
> -                        iscsi_login_portal_nowait(rec); 
> +                        iscsi_login_portal(NULL, NULL, rec); 
>   
>                  if (!idbm_find_rec_in_list(&iscsi_targets, rec->name, 
>                                             rec->conn[0].address, 
> @@ -190,7 +235,7 @@ static void update_sessions(struct list_head 
> *new_rec_list, 
>          } 
>   
>          if (!list_empty(&stale_rec_list)) { 
> -                iscsi_logout_portals(&stale_rec_list, &nr_found, 0, 
> +                iscsi_logout_portals(&stale_rec_list, &nr_found, 1, 
>                                       logout_session); 
>                  list_for_each_entry_safe(rec, tmp_rec, &stale_rec_list, 
> list) { 
>                          list_del(&rec->list); 
> @@ -1059,7 +1104,12 @@ static void do_st_disc_and_login(const char 
> *def_iname, 
>                  poll_inval = DISC_DEF_POLL_INVL; 
>   
>          do { 
> +                pthread_mutex_lock(mutex); 
> +                log_debug(1, "discoveryd start scan %s:%d", 
> drec->address, drec->port); 
>                  __do_st_disc_and_login(drec); 
> +                pthread_mutex_unlock(mutex); 
> +                log_debug(1, "discoveryd end scan %s:%d", drec->address, 
> drec->port); 
> + 
>                  if (!poll_inval) 
>                          break; 
>          } while (!stop_discoveryd && !sleep(poll_inval)); 
> @@ -1081,6 +1131,9 @@ static int st_start(void *data, struct discovery_rec 
> *drec) 
>   
>  static void discoveryd_st_start(void) 
>  { 
> +        discoveryd_create_shared_mutex(); 
> +        if (mutex == NULL) 
> +            return; 
>          idbm_for_each_st_drec(NULL, st_start); 
>  } 
>   
> -- 
> 1.8.4.3 
>
>

-- 
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/d/optout.

Reply via email to