lkml.org 
[lkml]   [2002]   [Oct]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH]updated ipc lock patch
    On Thu, 24 Oct 2002 16:30:32 -0700
    Andrew Morton <akpm@digeo.com> wrote:

    > Hugh Dickins wrote:
    > >
    > > ...
    > > Manfred and I have both reviewed the patch (or the 2.5.44 version)
    > > and we both recommend it highly (well, let Manfred speak for himself).
    > >
    >
    > OK, thanks.
    >
    > So I took a look. Wish I hadn't :( The locking rules in there
    > are outrageously uncommented. You must be brave people.

    Agreed. Here's my brief audit:

    >+ int max_id = ids->max_id;
    >
    >- for (id = 0; id <= ids->max_id; id++) {
    >+ read_barrier_depends();
    >+ for (id = 0; id <= max_id; id++) {

    That needs to be a rmb(), not a read_barrier_depends(). And like all
    barriers, it *requires* a comment:
    /* We must read max_id before reading any entries */

    I can't see the following in the patch posted, but:
    > void ipc_rcu_free(void* ptr, int size)
    > {
    > struct rcu_ipc_free* arg;
    >
    > arg = (struct rcu_ipc_free *) kmalloc(sizeof(*arg), GFP_KERNEL);
    > if (arg == NULL)
    > return;
    > arg->ptr = ptr;
    > arg->size = size;
    > call_rcu(&arg->rcu_head, ipc_free_callback, arg);
    > }

    This is unacceptable crap, sorry. You *must* allocate the resources
    required to free the object *at the time you allocate the object*,
    since freeing must not fail.

    > Even better: is it possible to embed the rcu_ipc_free inside the
    > object-to-be-freed? Perhaps not?

    Yes, this must be done.

    Rusty.
    --
    there are those who do and those who hang on and you don't see too
    many doers quoting their contemporaries. -- Larry McVoy
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2005-03-22 13:30    [W:0.030 / U:4.772 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site