tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: proposal: some pointer and arithmetic utilities



On Thu 21 Mar 2013 at 20:29:46 +0000, Taylor R Campbell wrote:
> This transformation doesn't make sense when the caller of mumble works
> with bars, not foos, and it's the caller of the caller of mumble that
> needs to pass in a foo to mumble by way of passing a bar to the caller
> of mumble.
> 
> For example, consider workqueues and struct work.  If you want to put
> an object on a workqueue (i.e., pass an argument in a request for
> deferred work), you embed a struct work in the object and then the
> workqueue's worker takes the struct work and finds the containing
> object from of it.

> kern_physio.c relies on the fact that the struct work is at the
> beginning of struct buf when it does
> 
> static void
> physio_done(struct work *wk, void *dummy)
> {
>       struct buf *bp = (void *)wk;
>       ...
> }
> 
> to recover bp after workqueue_enqueue(physio_workqueue, &bp->b_work,
> NULL).  But instead it could use
> 
>       struct buf *bp = container_of(wk, struct work, b_work);
> 
> In either case, the workqueue abstraction doesn't know or care that
> the struct work we're putting into the workqueue is from struct buf.
> We have various other abstractions like this in tree too.

But the reality of the code is that physio_done() takes a pointer to
struct buf. The fact that you're not writing this down doesn't make it
less true, but it does make the code riskier.

Even if you use your container_of macro, there is no proof that the
pointer you receive is actually of the type you expect, only that you're
using the correct offsets if by accident it is.

What you should IMHO really do is convert the function pointer to
physio_done() to the correct type as expected by workqueue_enqueue(),
and at *that* point prove somehow that it is correct. Not inside
physio_done().

I have something like this in mind (but with some better actual proof):

typedef void (*work_done)(struct work *wk, void *dummy);

static void
physio_done(struct work *wk, struct buf *bp)
{
        ...
}

...
    error = workqueue_create(&physio_workqueue, "physiod",
            (work_done)physio_done, NULL, PRI_BIO, IPL_BIO, WQ_MPSAFE);
    /*
     * Note: physio_done() will always be called with struct buf *bp,
     * because workqueue_enqueue() is called with such a pointer.
     * */

    struct buf *bp;
    workqueue_enqueue(physio_workqueue, bp, NULL);

In theory, the compiler will check at this point that *bp begins with a
struct work, since a pointer to a structure can be converted to a
pointer to its initial member (ISO/IEC 9899:TC3 section 6.7.2.1
paragraph 13, page 103).

Unfortunately C doesn't have a type "points into structure X at offset
Y", so anything using that will never be safe. Even the
pointer-to-member type of C++ cannot be simply reversed.

-Olaf.
-- 
___ Olaf 'Rhialto' Seibert  -- The Doctor: No, 'eureka' is Greek for
\X/ rhialto/at/xs4all.nl    -- 'this bath is too hot.'



Home | Main Index | Thread Index | Old Index