namecache coherency 3rd turn

[Simon 'corecode' Schubert]

On 03.04.2006, at 09:51, Csaba Henk wrote:

+static struct shadowinfo *
+shadowinfo_fetch(void)
+{
+	struct shadowinfo *shinf = STAILQ_FIRST(&shadowinfo_freeq);
+
+	if (! shinf)
+		goto alloc;
I'm strongly against adding a cache mechanism here.  We should use a 
generic object cache whereever possible (and bring that one in shape, 
hint, hint).  Then this would be a matter of:

shinf = objcache_alloc(shadowinfo_cache, M_WAITOK);

the approach you are taking is not MP-friendly at all, as it needs a 
lock, but I think you aware of this anyways.  Just let us not do such 
optimizations prematurely.  using malloc() should be sufficient right 
now.

@@ -295,6 +360,10 @@ cache_alloc(int nlen)
 	ncp->nc_error = ENOTCONN;	/* needs to be resolved */
 	ncp->nc_refs = 1;
 	ncp->nc_fsmid = 1;
+	ncp->nc_shadowinfo = &ncp->nc_shadowinfo_internal;
+	ncp->nc_shadowinfo_internal.sh_refs = 2;
+	ncp->nc_shadow_prev = NULL;
+	ncp->nc_shadow_next = NULL;
why is refs == 2?  This would at least need a comment explaining this 
number.

sncp is locked here:
[1]:
+int
+cache_shadow_attach(struct namecache *ncp, struct namecache *sncp)
+{
+	struct migrate_param mpm;
[..]
+	if (sncp->nc_shadowinfo == &sncp->nc_shadowinfo_internal) {
+		mpm.heightdelta = 0;
+		mpm.shadowinfo = shadowinfo_fetch();
+		mpm.exlocks = sncp->nc_shadowinfo->sh_exlocks;
+		migrate_updater(sncp, &mpm);
+	}
[2]:
[..]
@@ -372,16 +628,15 @@ cache_lock(struct namecache *ncp)
[..]
-		ncp->nc_flag |= NCF_LOCKREQ;
-		if (tsleep(ncp, 0, "clock", nclockwarn) == EWOULDBLOCK) {
+		shinf->sh_lockreq = 1;
+		if (tsleep(shinf, 0, "clock", nclockwarn) == EWOULDBLOCK) {
[..]

[3]:
@@ -436,17 +695,45 @@ cache_unlock(struct namecache *ncp)
 cache_unlock(struct namecache *ncp)
 {
[..]
+	if (--shinf->sh_exlocks == 0) {
+		shinf->sh_locktd = NULL;
+		if (shinf->sh_lockreq) {
+			shinf->sh_lockreq = 0;
+			wakeup(shinf);


I think there is a race condition which Matt already pointed out:

Thread1		Thread2
[2]
			[2] (tsleeps on ncp->nc_shadowinfo_internal)

[1] (migrates shinfo to an external struct)
[3] (wakes up external shinfo)
ncp is unlocked and gets freed
thread2 will sleep on ncp, but will never get woken up.  tsleep will 
return after some time and then access a non-existing ncp.

cheers
  simon
--
Serve - BSD     +++  RENT this banner advert  +++    ASCII Ribbon   /"\
Work - Mac      +++  space for low €€€ NOW!1  +++      Campaign     \ /
Party Enjoy Relax   |   http://dragonflybsd.org      Against  HTML   \
Dude 2c 2 the max   !   http://golden-apple.biz       Mail + News   / \
Attachment:
PGP.sig
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pgp00000.pgp
Type: application/octet-stream
Size: 186 bytes
Desc: "Description: This is a digitally signed message part"
URL: <http://lists.dragonflybsd.org/pipermail/submit/attachments/20060403/4b254b81/attachment-0019.obj>