All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Hansen <haveblue@us.ibm.com>
To: Chandra Seetharaman <sekharan@us.ibm.com>
Cc: ckrm-tech@lists.sourceforge.net, linux-mm@kvack.org
Subject: Re: [PATCH 1/6] CKRM: Basic changes to the core kernel
Date: Mon, 04 Apr 2005 06:45:13 -0700	[thread overview]
Message-ID: <1112622313.7189.50.camel@localhost> (raw)
In-Reply-To: <20050402031206.GB23284@chandralinux.beaverton.ibm.com>

>  static inline void
>  add_page_to_active_list(struct zone *zone, struct page *page)
>  {
>         list_add(&page->lru, &zone->active_list);
>         zone->nr_active++;
> +       ckrm_mem_inc_active(page);
>  }

Are any of the current zone statistics used any more when this is
compiled in?

Also, why does everything have to say ckrm_* on it?  What if somebody
else comes along and wants to use the same functions to do some other
kind of accounting? 

I think names like this are plenty long and descriptive enough:

        mem_inc_active(page);
        clear_page_class(page);
        set_page_class(...);
        
I'd drop the "ckrm_".
        
> +#define PG_ckrm_account                21      /* CKRM accounting */

Are you sure you really need this bit *and* a whole new pointer in
'struct page'?  We already do some tricks with ->mapping so that we can
tell what is stored in it.  You could easily do something with the low
bit of your new structure member.

> @@ -355,6 +356,7 @@ free_pages_bulk(struct zone *zone, int c
>                 /* have to delete it as __free_pages_bulk list manipulates */
>                 list_del(&page->lru);
>                 __free_pages_bulk(page, zone, order);
> +               ckrm_clear_page_class(page);
>                 ret++;
>         }
>         spin_unlock_irqrestore(&zone->lock, flags);

When your option is on, how costly is the addition of code, here?  How
much does it hurt the microbenchmarks?  How much larger does it
make .text?

> +       if (!in_interrupt() && !ckrm_class_limit_ok(ckrm_get_mem_class(p)))
> +               return NULL;

ckrm_class_limit_ok() is called later on in the same hot path, and
there's a for loop in there over each zone.  How expensive is this on
SGI's machines?  What about an 8-node x44[05]?  Why can't you call it
from interrupts?

-- Dave

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"aart@kvack.org"> aart@kvack.org </a>

  reply	other threads:[~2005-04-04 13:45 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2005-04-02  3:12 [PATCH 1/6] CKRM: Basic changes to the core kernel Chandra Seetharaman
2005-04-04 13:45 ` Dave Hansen [this message]
2005-04-05 17:25   ` Chandra Seetharaman
2005-04-05 17:54     ` Dave Hansen
2005-04-05 18:22       ` Chandra Seetharaman
2005-04-05 18:57         ` Dave Hansen
2005-04-05 19:38           ` Chandra Seetharaman
  -- strict thread matches above, loose matches on Subject: below --
2005-05-19  0:31 Chandra Seetharaman
2005-06-24 22:21 Chandra Seetharaman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1112622313.7189.50.camel@localhost \
    --to=haveblue@us.ibm.com \
    --cc=ckrm-tech@lists.sourceforge.net \
    --cc=linux-mm@kvack.org \
    --cc=sekharan@us.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.