Date: Thu, 28 Sep 2000 11:39:09 -0700 From: Alfred Perlstein <bright@wintelcom.net> To: Mike Smith <msmith@freebsd.org> Cc: arch@freebsd.org Subject: Re: we need atomic_t Message-ID: <20000928113907.V7553@fw.wintelcom.net> In-Reply-To: <200009281816.e8SIGLA01632@mass.osd.bsdi.com>; from msmith@freebsd.org on Thu, Sep 28, 2000 at 11:16:21AM -0700 References: <20000928110637.U7553@fw.wintelcom.net> <200009281816.e8SIGLA01632@mass.osd.bsdi.com>
next in thread | previous in thread | raw e-mail | index | archive | help
* Mike Smith <msmith@freebsd.org> [000928 11:14] wrote:
> > Linux has a datatype called "atomic_t", very useful for refcounts
> > and struct counters like tcpstat. My impression is that it's the
> > largest type an arch can support atomic ops on without weird
> > gyrations and/or extremely expensive operations.
>
> sig_atomic_t.
I'll look at that.
>
> > This would replace our atomic_op_type with just atomic_op and make
> > code easier to read and get right.
>
> I strongly disagree. The atomic_<op>_<type> interface is useful and
> necessary and should remain. I don't agree that this would make anything
> easier. In particular, the explicit use of the atomic_* operations makes
> the atomicity constraints very clear.
I really hate it, it slows down my programming, most of these counters
need to be the largest the platform will allow just avoid overflows,
you remeber the struct file refcount problem we had with apache right?
What's the point of having counters and refcounts that easily overflow?
> > I'm already seeing a pretty good examples of where this can be
> > applied:
> >
> > 1) struct ucred->cr_ref
> > 2) struct uidinfo->ui_ref
> > 3) tcpstats
> > 4) other stats :)
> > 5) mbuf external ref counts
> >
> > I don't have the gcc-assembler-foo to do this optimally without
> > directly copying from Linux which isn't acceptable.
>
> In most cases, you're manipulating the reference count under a mutex
> (since there's no other way to avoid the race where someone else frees
> your structure while you're in the process of dereferencing it), so this
> is largely unnecessary.
It's not possible for this to happen, this is why struct ucred,
mbuf and uidinfo lend themselves to mpsafeness pretty easily with
atomic refcounts.
You do need to own the parent structure lock (struct proc/socket/etc)
so that two codepaths can not deref the same pointer at the same
time, but you need to do that anyway.
Basically you need to own a lock on whatever allows access to the
pointer to the ucred/uidinfo/mbuf.
The atomic ops are particularly useful when you have multiple
different structures that point to a single object (ucred and
mbuf particularly)
When you instantiate a ucred, it has a refcount of 1 and you can
be garanteed that no one else if referencing it, right before it's
shallow copied (a point to it is in more than one place) the count
is at 2 which prevents free() from other codepaths, you're expected
to "own" the parent structure of be it a refcount of 1 or a lock
on the parent.
I'd much rather have:
void
crfree(cr)
struct ucred *cr;
{
if (atomic_dec_test(&cr->cr_ref, 1) == 0) {
/*
* Some callers of crget(), such as nfs_statfs(),
* allocate a temporary credential, but don't
* allocate a uidinfo structure.
*/
if (cr->cr_uidinfo != NULL)
uifree(cr->cr_uidinfo);
FREE((caddr_t)cr, M_CRED);
}
}
than:
void
crfree(cr)
struct ucred *cr;
{
mtx_enter(&cr->cr_mtx, MTX_DEF);
if (--cr->cr_ref == 0) {
mtx_exit(&cr->cr_mtx, MTX_DEF);
/*
* Some callers of crget(), such as nfs_statfs(),
* allocate a temporary credential, but don't
* allocate a uidinfo structure.
*/
if (cr->cr_uidinfo != NULL)
uifree(cr->cr_uidinfo);
FREE((caddr_t)cr, M_CRED);
} else {
mtx_exit(&cr->cr_mtx, MTX_DEF);
}
}
Note that there's a interesting cascade assertion going on:
if (atomic_dec_test(&cr->cr_ref, 1) == 0) {
/*
* then i have exclusive ownership of this ucred
* so I can free the uidinfo it references without a lock
* because my parent is locked.
*/
Is there a problem with this scheme?
> > Can anyone snap this up? I'd really appreciate it.
>
> Hold it right there, sunshine. 8)
pfft! :)
--
-Alfred Perlstein - [bright@wintelcom.net|alfred@freebsd.org]
"I have the heart of a child; I keep it in a jar on my desk."
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-arch" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20000928113907.V7553>