Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 17 Jun 2008 21:48:37 -0600
From:      James Gritton <jamie@gritton.org>
To:        freebsd-virtualization@freebsd.org
Subject:   V_* meta-symbols and locking
Message-ID:  <48588595.7020709@gritton.org>

next in thread | raw e-mail | index | archive | help
Like everything I have to say about the V_* issue, perhaps this doesn't 
apply to the vnet stuff.  But to the two symbols I currently care about, 
hostname and rootvnode, locking is a problem.

Current kernel code plays fast and loose with both these symbols.  Check 
out getcredhostname for example:

void
getcredhostname(struct ucred *cred, char *buf, size_t size)
{
    struct prison *pr;

    pr = cred->cr_prison;
    if (pr != &prison0) {
        mtx_lock(&pr->pr_mtx);
        strlcpy(buf, (pr->pr_flags & PR_NOHOST)
            ? hostname : pr->pr_host, size);
        mtx_unlock(&pr->pr_mtx);
    } else
        strlcpy(buf, hostname, size);
}

In the prison case, it nicely locks the prison record.  But for the 
global hostname, it just copies it.  The hostname sysctl is no better 
about setting it.  And rootvnode is referred to all over the place 
without any sort of lock - pretty safe since it's not expected to change 
(though it theoretically can).

This same no-locking assumption seems to be going on with V_hostname.  
But now this macro applies not only to the "real" hostname but to the 
"virtual" one as well - no locking the vimage record.  As I try to add a 
similar macro to my new jail framework, I find I can't.  Instead of a 
mere variable redirection, I need to lock-copy-unlock much like 
getcredhostname does.  Luckily, much hostname access is already 
jail-aware.  But anything using the "real" hostname should have the same 
locking on prison0.  Perhaps not wholly necessary since it's just a 
string that we know will always have a null byte at the end of the 
buffer, but still good form and unknown prevention.  And in the case of 
actually virtual hostnames, it's essential since they'll be changing 
from fixed arrays in struct prison into pointers that may be freed.

Rootvnode is a stickier problem.  There's much more code that refers to 
it, and it's a more essential part of the system.  I don't relish 
digging in everywhere and changing the whole rootvnode paradigm with 
locking.  So instead my solution is to make the jail "path" parameter 
(and thus root vnode) set-once.  So as long as the V_rootvnode is taken 
from a context that will remain for the duration of its use (curthread 
is a good bet), it will be safe to access it without locks.  In 
particular, the real rootvnode that lives at prison0 isn't going anywhere.

So in summary:

I won't use V_hostname (or G_hostname), opting for explicit locking.

I will V_rootvnode (and perhaps G_rootvnode).

All the other network-related V_stuff may deserve a look, but it out of 
my purview.

- Jamie



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