Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 3 May 2012 10:40:12 GMT
From:      dfilter@FreeBSD.ORG (dfilter service)
To:        freebsd-bugs@FreeBSD.org
Subject:   Re: kern/166340: commit references a PR
Message-ID:  <201205031040.q43AeCLJ065238@freefall.freebsd.org>

next in thread | raw e-mail | index | archive | help
The following reply was made to PR kern/166340; it has been noted by GNATS.

From: dfilter@FreeBSD.ORG (dfilter service)
To: bug-followup@FreeBSD.org
Cc:  
Subject: Re: kern/166340: commit references a PR
Date: Thu,  3 May 2012 10:38:14 +0000 (UTC)

 Author: kib
 Date: Thu May  3 10:38:02 2012
 New Revision: 234952
 URL: http://svn.freebsd.org/changeset/base/234952
 
 Log:
   When callout_reset_on() cannot immediately migrate a callout since it
   is running on other cpu, the CALLOUT_PENDING flag is temporarily
   cleared. Then, callout_stop() on this, in fact active, callout fails
   because CALLOUT_PENDING is not set, and callout_stop() returns 0.
   
   Now, in sleepq_check_timeout(), the failed callout_stop() causes the
   sleepq code to execute mi_switch() without even setting the wmesg,
   since the switch-out is supposed to be transient. In fact, the thread
   is put off the CPU for full timeout interval, instead of being put on
   runq immediately.  Until timeout fires, the process is unkillable for
   obvious reasons.
   
   Fix this by marking the migrating callouts with CALLOUT_DFRMIGRATION
   flag. The flag is cleared by callout_stop_safe() when the function
   detects a migration, besides returning the success. The softclock()
   rechecks the flag for migrating callout and cancels its execution if
   the flag was cleared meantime.
   
   PR:	 misc/166340
   Reported, debugging traces provided and tested by:
   	Christian Esken <christian.esken trivago com>
   Reviewed by:	 avg, jhb
   MFC after:	 1 week
 
 Modified:
   head/sys/kern/kern_timeout.c
   head/sys/sys/callout.h
 
 Modified: head/sys/kern/kern_timeout.c
 ==============================================================================
 --- head/sys/kern/kern_timeout.c	Thu May  3 10:26:33 2012	(r234951)
 +++ head/sys/kern/kern_timeout.c	Thu May  3 10:38:02 2012	(r234952)
 @@ -645,6 +645,32 @@ softclock(void *arg)
  					cc_cme_cleanup(cc);
  
  					/*
 +					 * Handle deferred callout stops
 +					 */
 +					if ((c->c_flags & CALLOUT_DFRMIGRATION)
 +					    == 0) {
 +						CTR3(KTR_CALLOUT,
 +					"deferred cancelled %p func %p arg %p",
 +						    c, new_func, new_arg);
 +						if (cc->cc_next == c) {
 +							cc->cc_next =
 +							    TAILQ_NEXT(c,
 +							    c_links.tqe);
 +						}
 +						if (c->c_flags &
 +						    CALLOUT_LOCAL_ALLOC) {
 +							c->c_func = NULL;
 +							SLIST_INSERT_HEAD(
 +							    &cc->cc_callfree, c,
 +							    c_links.sle);
 +						}
 +						goto nextc;
 +					} else {
 +						c->c_flags &= ~
 +						    CALLOUT_DFRMIGRATION;
 +					}
 +
 +					/*
  					 * It should be assert here that the
  					 * callout is not destroyed but that
  					 * is not easy.
 @@ -659,6 +685,9 @@ softclock(void *arg)
  					panic("migration should not happen");
  #endif
  				}
 +#ifdef SMP
 +nextc:
 +#endif
  				steps = 0;
  				c = cc->cc_next;
  			}
 @@ -814,6 +843,7 @@ callout_reset_on(struct callout *c, int 
  			cc->cc_migration_ticks = to_ticks;
  			cc->cc_migration_func = ftn;
  			cc->cc_migration_arg = arg;
 +			c->c_flags |= CALLOUT_DFRMIGRATION;
  			CTR5(KTR_CALLOUT,
  		    "migration of %p func %p arg %p in %d to %u deferred",
  			    c, c->c_func, c->c_arg, to_ticks, cpu);
 @@ -984,6 +1014,12 @@ again:
  			CC_UNLOCK(cc);
  			KASSERT(!sq_locked, ("sleepqueue chain locked"));
  			return (1);
 +		} else if ((c->c_flags & CALLOUT_DFRMIGRATION) != 0) {
 +			c->c_flags &= ~CALLOUT_DFRMIGRATION;
 +			CTR3(KTR_CALLOUT, "postponing stop %p func %p arg %p",
 +			    c, c->c_func, c->c_arg);
 +			CC_UNLOCK(cc);
 +			return (1);
  		}
  		CTR3(KTR_CALLOUT, "failed to stop %p func %p arg %p",
  		    c, c->c_func, c->c_arg);
 
 Modified: head/sys/sys/callout.h
 ==============================================================================
 --- head/sys/sys/callout.h	Thu May  3 10:26:33 2012	(r234951)
 +++ head/sys/sys/callout.h	Thu May  3 10:38:02 2012	(r234952)
 @@ -46,6 +46,7 @@
  #define	CALLOUT_MPSAFE		0x0008 /* callout handler is mp safe */
  #define	CALLOUT_RETURNUNLOCKED	0x0010 /* handler returns with mtx unlocked */
  #define	CALLOUT_SHAREDLOCK	0x0020 /* callout lock held in shared mode */
 +#define	CALLOUT_DFRMIGRATION	0x0040 /* callout in deferred migration mode */
  
  struct callout_handle {
  	struct callout *callout;
 _______________________________________________
 svn-src-all@freebsd.org mailing list
 http://lists.freebsd.org/mailman/listinfo/svn-src-all
 To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
 



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201205031040.q43AeCLJ065238>