lkml.org 
[lkml]   [2008]   [Aug]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH 12/30] mm: memory reserve management
    On Thursday July 24, a.p.zijlstra@chello.nl wrote:
    > Generic reserve management code.
    >
    > It provides methods to reserve and charge. Upon this, generic alloc/free style
    > reserve pools could be build, which could fully replace mempool_t
    > functionality.

    This looks quite different to last time I looked at the code (I
    think).

    You now have a more structured "kmalloc_reserve" interface which
    returns a flag to say if the allocation was from an emergency pool. I
    think this will be a distinct improvement at the call sites, though I
    haven't looked at them yet. :-)

    > +
    > +struct mem_reserve {
    > + struct mem_reserve *parent;
    > + struct list_head children;
    > + struct list_head siblings;
    > +
    > + const char *name;
    > +
    > + long pages;
    > + long limit;
    > + long usage;
    > + spinlock_t lock; /* protects limit and usage */
    ^^^^^
    > +
    > + wait_queue_head_t waitqueue;
    > +};

    ....
    > +static void __calc_reserve(struct mem_reserve *res, long pages, long limit)
    > +{
    > + unsigned long flags;
    > +
    > + for ( ; res; res = res->parent) {
    > + res->pages += pages;
    > +
    > + if (limit) {
    > + spin_lock_irqsave(&res->lock, flags);
    > + res->limit += limit;
    > + spin_unlock_irqrestore(&res->lock, flags);
    > + }
    > + }
    > +}

    I cannot figure out why the spinlock is being used to protect updates
    to 'limit'.
    As far as I can see, mem_reserve_mutex already protects all those
    updates.
    Certainly we need the spinlock for usage, but why for limit??

    > +
    > +void *___kmalloc_reserve(size_t size, gfp_t flags, int node, void *ip,
    > + struct mem_reserve *res, int *emerg)
    > +{
    ....
    > + if (emerg)
    > + *emerg |= 1;

    Why not just

    if (emerg)
    *emerg = 1.

    I can't we where '*emerg' can have any value but 0 or 1, so the '|' is
    pointless ???

    Thanks,
    NeilBrown


    \
     
     \ /
      Last update: 2008-08-12 08:25    [W:0.028 / U:30.048 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site