From: Dan Magenheimer <dan.magenheimer@oracle.com>
To: Konrad Wilk <konrad.wilk@oracle.com>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
Hugh Dickins <hughd@google.com>
Subject: RE: [PATCH] frontswap: support exclusive gets if tmem backend is capable
Date: Fri, 31 Aug 2012 10:23:21 -0700 (PDT) [thread overview]
Message-ID: <89702248-0c3f-465c-bc1f-2115a21c8c89@default> (raw)
In-Reply-To: <20120831170814.GF18929@localhost.localdomain>
> From: Konrad Rzeszutek Wilk
Hi Konrad --
Thanks for the fast feedback!
> > +#define FRONTSWAP_HAS_EXCLUSIVE_GETS
> > +extern void frontswap_tmem_exclusive_gets(bool);
>
> I don't think you need the #define here..
The #define is used by an ifdef in the backend to ensure
that it is using a version of frontswap that has this feature,
so avoids the need for the frontend (frontswap) and
the backend (e.g. zcache2) to merge in lockstep.
> > +EXPORT_SYMBOL(frontswap_tmem_exclusive_gets);
>
> We got two of these now - the writethrough and this one. Merging
> them in one function and one flag might be better. So something like:
> static int frontswap_mode = 0;
>
> void frontswap_set_mode(int set_mode)
> {
> if (mode & (FRONTSWAP_WRITETH | FRONTSWAP_EXCLUS..)
> mode |= set_mode;
> }
IMHO, it's too soon to try to optimize this. One or
both of these may go away. Or the mode may become
more fine-grained in the future (e.g. to allow individual
gets to be exclusive).
So unless you object strongly, let's just leave this
as is for now and revisit in the future if more "modes"
are needed.
> ... and
> > +
> > +/*
> > * Called when a swap device is swapon'd.
> > */
> > void __frontswap_init(unsigned type)
> > @@ -174,8 +190,13 @@ int __frontswap_load(struct page *page)
> > BUG_ON(sis == NULL);
> > if (frontswap_test(sis, offset))
> > ret = (*frontswap_ops.load)(type, offset, page);
> > - if (ret == 0)
> > + if (ret == 0) {
> > inc_frontswap_loads();
> > + if (frontswap_tmem_exclusive_gets_enabled) {
>
> For these perhaps use asm goto for optimization? Is this showing up in
> perf as a hotspot? The asm goto might be a bit too much.
This is definitely not a performance hotspot. Frontswap code
only is ever executed in situations where a swap-to-disk would
otherwise have occurred. And in this case, this code only
gets executed after the frontswap_test has confirmed that
tmem does already contain the page of data, in which case
there is thousands of cycles spent copying and/or decompressing.
Dan
--
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:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2012-08-31 17:24 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-08-31 15:48 Dan Magenheimer
2012-08-31 17:08 ` Konrad Rzeszutek Wilk
2012-08-31 17:23 ` Dan Magenheimer [this message]
2012-08-31 21:10 ` Konrad Rzeszutek Wilk
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=89702248-0c3f-465c-bc1f-2115a21c8c89@default \
--to=dan.magenheimer@oracle.com \
--cc=hughd@google.com \
--cc=konrad.wilk@oracle.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