Re: Patch for inode SLIST conversion

[Michael Neumann <mneumann@xxxxxxxx>]

Matthew Dillon wrote:
>     I think the concept is a perfectly good programming abstraction, though
>     I have to again caution that it can result in fairly fragile code.  It's
>     definitely not needed for UFS's inode hash because the chain lengths
>     are so short.
>
>     Going back to your token patch... lets simplify it.  Get rid of
>     lwkt_staleref() and lwkt_initstale() entirely.  Build the semantics
>     SOLELY out of lwkt_is_stale().  The definition of lwkt_is_stale() would
>     be:
>
>     * Returns 0 if the token in question had not been temporarily acquired
>       while we were blocked since our last lwkt_gettoken(), lwkt_gettokref(),
>       or lwkt_is_stale() call.
>
>     * Returns non-zero if it was.
>
>     How do you detect the situation?  Really simple I think.  Augment
>     the lwkt_token structure with a pointer to the last lwkt_tokref that
>     acquired it.  Get rid of the 64 bit generation counter, it isn't needed.
>
>     lwkt_token {
> 	...
> 	struct lwkt_tokref *lastref;
>     }
>
>     Whenever a token is acquired (_lwkt_gettokref()) is called lastref is
>     set to the tokref.  Whenever a token is released lastref is set to NULL.
>
>     The special case of the thread scheduler reacquiring the token with
>     lwkt_getalltokens() would NULL the field out if lastref != tokref,
>     and otherwise leave the field intact.
>
>     What do you think?

That's the perfect solution!

Patch appended. _lwkt_trytokref needs the same change, as it might 
acquire the token as well. I think setting lastref = NULL in 
lwkt_reltoken is also not absolutely neccessary, but it's not wrong
either, so I did it.

Regards,

  Michael
Index: kern/lwkt_token.c
===================================================================
RCS file: /home/dcvs/src/sys/kern/lwkt_token.c,v
retrieving revision 1.29
diff -u -r1.29 lwkt_token.c
--- kern/lwkt_token.c	27 Dec 2006 06:51:47 -0000	1.29
+++ kern/lwkt_token.c	23 Jan 2008 12:52:51 -0000
@@ -169,6 +169,12 @@
 	}
 	++tok->t_count;
 	refs->tr_state = 1;
+	/*
+	 * Detect the situation where the token was acquired by 
+	 * a different thread while the token was released from
+	 * the current thread due to a blocking condition.  
+ 	 */
+	if (tok->t_lastref != refs) tok->t_lastref = NULL; 
     }
     return (TRUE);
 }
@@ -258,6 +264,7 @@
     /* NOTE: 'td' invalid after loop */
     ++tok->t_globalcount;
 #endif
+    tok->t_lastref = ref;
     ref->tr_state = 1;
 }
 
@@ -313,7 +320,9 @@
     /* NOTE: 'td' invalid after loop */
     ++tok->t_globalcount;
 #endif
+    tok->t_lastref = ref;
     ref->tr_state = 1;
+
     return (TRUE);
 }
 
@@ -351,7 +360,7 @@
  * Release a serializing token
  */
 void
-lwkt_reltoken(lwkt_tokref *ref)
+lwkt_reltoken(lwkt_tokref_t ref)
 {
     struct lwkt_tokref **scanp;
     lwkt_token_t tok;
@@ -371,6 +380,7 @@
     *scanp = ref->tr_next;
     ref->tr_state = 0;
 
+    tok->t_lastref = NULL;
 #ifdef SMP
     if (--tok->t_count == 0) {
 	tok->t_owner = NULL;
@@ -421,6 +431,7 @@
 #else
     tok->t_globalcount = 0;
 #endif
+    tok->t_lastref = NULL;
 }
 
 void
@@ -428,3 +439,19 @@
 {
     /* empty */
 }
+
+int
+lwkt_is_stale(lwkt_tokref_t ref)
+{
+    /* Token is not stale */
+    if (ref->tr_tok->t_lastref == ref) 
+	return 0;
+
+    /*
+     * The token is stale. Reset to not stale so that the next call to 
+     * lwkt_is_stale will return "not stale" unless the token was acquired 
+     * again by a different thread in-between.
+     */
+    ref->tr_tok->t_lastref = ref;
+    return 1;
+}
Index: sys/thread.h
===================================================================
RCS file: /home/dcvs/src/sys/sys/thread.h,v
retrieving revision 1.90
diff -u -r1.90 thread.h
--- sys/thread.h	12 Dec 2007 23:49:24 -0000	1.90
+++ sys/thread.h	23 Jan 2008 12:20:05 -0000
@@ -106,6 +106,7 @@
     struct spinlock	t_spinlock;	/* Controls access */
     struct thread	*t_owner;	/* The current owner of the token */
     int			t_count;	/* Per-thread count */
+    lwkt_tokref_t	*t_lastref;	/* Last tokref that acquired token */ 
 } lwkt_token;
 
 #else
@@ -114,6 +115,7 @@
     struct spinlock	t_unused01;
     struct thread	*t_unused02;
     int			t_globalcount;	/* Global reference count */
+    lwkt_tokref_t	*t_lastref;	/* Last tokref that acquired token */ 
 } lwkt_token;
 
 #endif
@@ -361,6 +363,7 @@
 extern void lwkt_drain_token_requests(void);
 extern void lwkt_token_init(lwkt_token_t);
 extern void lwkt_token_uninit(lwkt_token_t);
+extern int  lwkt_is_stale(lwkt_tokref_t);
 
 extern void lwkt_token_pool_init(void);
 extern lwkt_token_t lwkt_token_pool_get(void *);