[dm-devel] curious why waitevent sets event_nr field of event _thread struct every time

[goggin, edward]

On Wed, 30 Mar 2005 00:31:11 +0200
christophe varoqui <christophe.varoqui at free.fr> wrote
> Have you tried your suggested design ?
> Is there a regression at first glance ?

I've tried it.  The code only blindly sets the event_nr from
the kernel the first time through.  Otherwise, if the kernel's copy
and the pthread's copy are different the code (1) logs a msg,
(2) sets the pthread copy from the kernel copy, and (3) goes to a
new label "out2" to fail the paths and invoke multipath.

I ran three to four times and saw log messages indicating that
several events had the potential of being lost.  Overall, while
this seems to be the right thing to do -- proving that events
would actually be lost is a different thing all together.  I did
not even attempt to do that.

I've included below a patch for multipathd/main.c based on version
0.4.4-pre7 of multipath-tools.
-----------------------------------------------------------------
*** multipath-tools-0.4.4-pre7/multipathd/main.c.orig	2005-04-05
04:46:30.000000000 -0400
--- multipath-tools-0.4.4-pre7/multipathd/main.c	2005-04-05
04:51:03.000000000 -0400
***************
*** 88,93 ****
--- 88,94 ----
  	int event_nr;
  	char mapname[WWID_SIZE];
  	struct paths *allpaths;
+ 	int firstTime;
  };
  
  /*
***************
*** 245,250 ****
--- 246,252 ----
  	char buff[1];
  	char cmd[CMDSIZE];
  	struct dm_task *dmt;
+ 	int event_nr;
  
  	mlockall(MCL_CURRENT | MCL_FUTURE);
  	waiter = (struct event_thread *)et;
***************
*** 256,262 ****
  	}
  	pthread_mutex_lock (waiter->waiter_lock);
  
! 	waiter->event_nr = dm_geteventnr (waiter->mapname);
  	log_safe(LOG_DEBUG, "waiter->event_nr = %i", waiter->event_nr);
  
  	if (!(dmt = dm_task_create(DM_DEVICE_WAITEVENT)))
--- 258,274 ----
  	}
  	pthread_mutex_lock (waiter->waiter_lock);
  
! 	event_nr = dm_geteventnr (waiter->mapname);
! 	if (waiter->firstTime) {
! 		waiter->event_nr = event_nr;
! 		waiter->firstTime = 0;
! 	}
! 	else if (waiter->event_nr != event_nr) {
! 		log_safe(LOG_NOTICE, "event_nr = %i, waiter->event_nr = %i:
avoiding 0x%x lost events on map %s", event_nr, waiter->event_nr, event_nr -
waiter->event_nr, waiter->mapname);
! 		waiter->event_nr = event_nr;
! 		goto out2;
! 	}
! 	//waiter->event_nr = dm_geteventnr (waiter->mapname);
  	log_safe(LOG_DEBUG, "waiter->event_nr = %i", waiter->event_nr);
  
  	if (!(dmt = dm_task_create(DM_DEVICE_WAITEVENT)))
***************
*** 274,280 ****
  
  out:
  	dm_task_destroy(dmt);
! 
  	log_safe(LOG_DEBUG, "%s", cmd);
  	log_safe(LOG_NOTICE, "devmap event on %s", waiter->mapname);
  	mark_failed_path(waiter->allpaths, waiter->mapname);
--- 286,292 ----
  
  out:
  	dm_task_destroy(dmt);
! out2:
  	log_safe(LOG_DEBUG, "%s", cmd);
  	log_safe(LOG_NOTICE, "devmap event on %s", waiter->mapname);
  	mark_failed_path(waiter->allpaths, waiter->mapname);
***************
*** 379,384 ****
--- 391,397 ----
  					MALLOC (sizeof (pthread_mutex_t));
  				pthread_mutex_init (wp->waiter_lock, NULL);
  				wp->allpaths = allpaths;
+ 				wp->firstTime = 1;
  				vector_alloc_slot (waiters);
  				vector_set_slot (waiters, wp);
  			}