Date: Thu, 4 Aug 2005 13:40:57 +0200 From: Hans Petter Selasky <hselasky@c2i.net> To: John Baldwin <jhb@freebsd.org> Cc: freebsd-hackers@freebsd.org Subject: Re: How to do proper locking Message-ID: <200508041340.58851.hselasky@c2i.net> In-Reply-To: <200508031321.32276.jhb@FreeBSD.org> References: <200508030023.04748.hselasky@c2i.net> <200508031321.32276.jhb@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wednesday 03 August 2005 19:21, John Baldwin wrote:
> On Tuesday 02 August 2005 06:23 pm, Hans Petter Selasky wrote:
> > Hi,
> >
> > I am looking for a safe way to access structures that can be freed. The
> > solution I am looking for must not:
> >
> > - hinder scaleability
> > - lead to use of a single lock
> > - lead to lock order reversal
There is one more thing. When the structure is freed it should not block
waiting for any reference counts.
>
> These aren't a very clear set of requirements.
If you think about the situation of accessing a structure in parallell with
freeing it, and want to do it 100 percent safe, you will see that you have to
do some kind of test before accessing this structure to see if it is still
present. So how should this test look like if you want to keep the points
I've listed above in mind?
> Note that you can't hold a
> lock across malloc(), so your allocation code isn't safe.
I was thinking that the M_NOWAIT flag was passed malloc(). Else in this case,
I think it is safe to unlock/lock "lock_A" when malloc is called.
> You can try taking at look at some existing refcounted structures such
> as ucreds, p_args, etc. or looking at structures held in a global list
> like proc.
It looks to me like some parent lock is held, or that these structures are
only accessed from a single thread, to prevent synchronization problems.
This is a copy and paste from the kernel sources:
struct ucred *
crhold(struct ucred *cr)
{
The problem is, what happens if the kernel switches thread right here, and
then the other thread calls "crfree()" on this structure, that will "free()"
memory pointed to by "cr". Then the first line of code below will access
freed memory, and then we are going for a segment fault or worse.
mtx_lock(cr->cr_mtxp);
cr->cr_ref++;
mtx_unlock(cr->cr_mtxp);
return (cr);
}
So this is what I want this routine and alike to look like:
struct cr_callback_args {
struct ucred *cr;
struct mtx *p_mtx;
u_int32_t refcount_copy,
u_int32_t *p_refcount;
};
struct ucred *
crhold(struct cr_ballback_args *args)
{
mtx_lock(args->p_mtx);
if(args->refcount_copy == ref_atomic_read(args->p_refcount))
{
/* the structure pointed to by "args->cr" is still valid */
/* maybe we don't need the refcount below, but instead
* can return with this structure locked, hence the
* reference count will be incremented before
* "args->cr" is freed, and after that the test
* above will fail.
*/
args->cr->cr_ref++;
}
mtx_lock(args->p_mtx);
return (args->cr);
}
> What actual problem are you trying to solve?
Generic callbacks:
- setting up timers. Make sure that callback is never called after that timer
has been stopped.
- setting up devices. Make sure that read/write/ioctl/poll callbacks are never
called after that the device has been freed.
- setting up interrupts. Make sure that interrupt routine is never called
after that it is been teared down.
And more. There are lots of situations in the kernel that run into the same
scheme, without a proper solution.
In my initial suggestion, I suggested that there be a "ref_atomic_increment()"
routine, but I think it is better without. Then every time one needs to
increment the refcount, one has to go through a ref_free/ref_alloc loop. The
idea is that "ref_free()" will detect when the refcount is equal to
(u_int32_t)(-1) and put the refount on a trash list. Then ideally one never
recycles "used up" refcounts.
> How does code obtain references to my_struct objects?
For example when you set up a timer, you pass the pointer to the structure
that the callback is going to access.
struct my_struct *sc = ...;
callout_reset(&callback, 1*hz, &my_callback, sc);
> Are they hung off another object that has
> a lock?
Usually, but the "parent" lock cannot help much.
> Are they in a global list?
Yes and no. Sometimes these structures have roots in structures that are freed
too, in the same manner, but at the bottom there is a static and global list.
Was this what you meant?
-- HPS
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200508041340.58851.hselasky>