linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Konstantin Khlebnikov <khlebnikov@openvz.org>
To: Hugh Dickins <hughd@google.com>
Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Johannes Weiner <jweiner@redhat.com>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 3/7 v2] mm: rework __isolate_lru_page() file/anon filter
Date: Fri, 09 Mar 2012 11:16:12 +0400	[thread overview]
Message-ID: <4F59AE3C.5040200@openvz.org> (raw)
In-Reply-To: <alpine.LSU.2.00.1203081758490.18195@eggly.anvils>

Hugh Dickins wrote:
> On Thu, 8 Mar 2012, KAMEZAWA Hiroyuki wrote:
>> On Tue, 6 Mar 2012 19:22:21 -0800 (PST)
>> Hugh Dickins<hughd@google.com>  wrote:
>>>
>>> What does the compiler say (4.5.1 here, OPTIMIZE_FOR_SIZE off)?
>>>     text	   data	    bss	    dec	    hex	filename
>>>    17723	    113	     17	  17853	   45bd	vmscan.o.0
>>>    17671	    113	     17	  17801	   4589	vmscan.o.1
>>>    17803	    113	     17	  17933	   460d	vmscan.o.2
>>>
>>> That suggests that your v2 is the worst and your v1 the best.
>>> Kame, can I persuade you to let the compiler decide on this?
>>>
>>
>> Hmm. How about Costa' proposal ? as
>>
>> int tmp_var = PageActive(page) ? ISOLATE_ACTIVE : ISOLATE_INACTIVE
>> if (!(mode&  tmp_var))
>>      ret;
>
> Yes, that would have been a good compromise (given a better name
> than "tmp_var"!), I didn't realize that one was acceptable to you.
>
> But I see that Konstantin has been inspired by our disagreement to a
> more creative solution.
>
> I like very much the look of what he's come up with, but I'm still
> puzzling over why it barely makes any improvement to __isolate_lru_page():
> seems significantly inferior (in code size terms) to his original (which
> I imagine Glauber's compromise would be equivalent to).
>
> At some point I ought to give up on niggling about this,
> but I haven't quite got there yet.

(with if())
$ ./scripts/bloat-o-meter built-in.o built-in.o-v1
add/remove: 0/0 grow/shrink: 2/1 up/down: 32/-20 (12)
function                                     old     new   delta
static.shrink_active_list                    837     853     +16
shrink_inactive_list                        1259    1275     +16
static.isolate_lru_pages                    1055    1035     -20

(with switch())
$ ./scripts/bloat-o-meter built-in.o built-in.o-v2
add/remove: 0/0 grow/shrink: 4/2 up/down: 111/-23 (88)
function                                     old     new   delta
__isolate_lru_page                           301     377     +76
static.shrink_active_list                    837     853     +16
shrink_inactive_list                        1259    1275     +16
page_evictable                               170     173      +3
__remove_mapping                             322     319      -3
static.isolate_lru_pages                    1055    1035     -20

(without __always_inline on page_lru())
$ ./scripts/bloat-o-meter built-in.o built-in.o-v5-noinline
add/remove: 0/0 grow/shrink: 5/2 up/down: 93/-23 (70)
function                                     old     new   delta
__isolate_lru_page                           301     333     +32
isolate_lru_page                             359     385     +26
static.shrink_active_list                    837     853     +16
putback_inactive_pages                       635     651     +16
page_evictable                               170     173      +3
__remove_mapping                             322     319      -3
static.isolate_lru_pages                    1055    1035     -20

$ ./scripts/bloat-o-meter built-in.o built-in.o-v5
add/remove: 0/0 grow/shrink: 3/4 up/down: 35/-67 (-32)
function                                     old     new   delta
static.shrink_active_list                    837     853     +16
__isolate_lru_page                           301     317     +16
page_evictable                               170     173      +3
__remove_mapping                             322     319      -3
mem_cgroup_lru_del                            73      65      -8
static.isolate_lru_pages                    1055    1035     -20
__mem_cgroup_commit_charge                   676     640     -36

Actually __isolate_lru_page() even little bit bigger

>
> Hugh

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2012-03-09  7:16 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-29  9:15 [PATCH v4 ch1 0/7] mm: some cleanup/rework before lru_lock splitting Konstantin Khlebnikov
2012-02-29  9:15 ` [PATCH 1/7] mm/memcg: scanning_global_lru means mem_cgroup_disabled Konstantin Khlebnikov
2012-03-02  5:12   ` KAMEZAWA Hiroyuki
2012-03-06 11:46     ` Glauber Costa
2012-02-29  9:15 ` [PATCH 2/7] mm/memcg: move reclaim_stat into lruvec Konstantin Khlebnikov
2012-03-02  5:14   ` KAMEZAWA Hiroyuki
2012-02-29  9:15 ` [PATCH 3/7] mm: rework __isolate_lru_page() file/anon filter Konstantin Khlebnikov
2012-03-02  5:17   ` KAMEZAWA Hiroyuki
2012-03-02  5:51     ` Konstantin Khlebnikov
2012-03-02  8:17       ` KAMEZAWA Hiroyuki
2012-03-02  8:53         ` Konstantin Khlebnikov
2012-03-06 11:57         ` Glauber Costa
2012-03-06 12:53           ` Konstantin Khlebnikov
2012-03-03  0:22   ` Hugh Dickins
2012-03-03  8:27     ` Konstantin Khlebnikov
2012-03-03  9:20       ` Konstantin Khlebnikov
2012-03-03  9:16   ` [PATCH 3/7 v2] " Konstantin Khlebnikov
2012-03-05  0:27     ` KAMEZAWA Hiroyuki
2012-03-07  3:22     ` Hugh Dickins
2012-03-08  5:30       ` KAMEZAWA Hiroyuki
2012-03-09  2:06         ` Hugh Dickins
2012-03-09  7:16           ` Konstantin Khlebnikov [this message]
2012-03-10  0:04             ` Hugh Dickins
2012-03-10  6:55               ` Konstantin Khlebnikov
2012-03-10  9:46                 ` Konstantin Khlebnikov
2012-03-15  1:47                   ` Hugh Dickins
2012-03-15  6:03                     ` Konstantin Khlebnikov
2012-03-15 23:58                       ` Hugh Dickins
2012-02-29  9:15 ` [PATCH 4/7] mm: push lru index into shrink_[in]active_list() Konstantin Khlebnikov
2012-03-02  5:21   ` KAMEZAWA Hiroyuki
2012-03-03  0:24   ` Hugh Dickins
2012-02-29  9:15 ` [PATCH 5/7] mm: rework reclaim_stat counters Konstantin Khlebnikov
2012-03-02  5:28   ` KAMEZAWA Hiroyuki
2012-03-02  6:11     ` Konstantin Khlebnikov
2012-03-02  8:03       ` KAMEZAWA Hiroyuki
2012-02-29  9:16 ` [PATCH 6/7] mm/memcg: rework inactive_ratio calculation Konstantin Khlebnikov
2012-03-02  5:31   ` KAMEZAWA Hiroyuki
2012-03-02  6:24     ` Konstantin Khlebnikov
2012-03-08  5:36       ` KAMEZAWA Hiroyuki
2012-02-29  9:16 ` [PATCH 7/7] mm/memcg: use vm_swappiness from target memory cgroup Konstantin Khlebnikov
2012-03-02  5:32   ` KAMEZAWA Hiroyuki

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=4F59AE3C.5040200@openvz.org \
    --to=khlebnikov@openvz.org \
    --cc=akpm@linux-foundation.org \
    --cc=hughd@google.com \
    --cc=jweiner@redhat.com \
    --cc=kamezawa.hiroyu@jp.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).