From: Minchan Kim <minchan@kernel.org>
To: Nitin Gupta <ngupta@vflare.org>,
Dan Magenheimer <dan.magenheimer@oracle.com>,
Seth Jennings <sjenning@linux.vnet.ibm.com>,
Hugh Dickins <hughd@google.com>,
Konrad Rzeszutek Wilk <konrad@darnok.org>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
Andrew Morton <akpm@linux-foundation.org>
Subject: Questin about swap_slot free and invalidate page
Date: Thu, 31 Jan 2013 14:11:40 +0900 [thread overview]
Message-ID: <20130131051140.GB23548@blaptop> (raw)
When I reviewed zswap, I was curious about frontswap_store.
It said following as.
* If frontswap already contains a page with matching swaptype and
* offset, the frontswap implementation may either overwrite the data and
* return success or invalidate the page from frontswap and return failure.
It didn't say why it happens. we already have __frontswap_invalidate_page
and call it whenever swap_slot frees. If we don't free swap slot,
scan_swap_map can't find the slot for swap out so I thought overwriting of
data shouldn't happen in frontswap.
As I looked the code, the curplit is reuse_swap_page. It couldn't free swap
slot if the page founded is PG_writeback but miss calling frontswap_invalidate_page
so data overwriting on frontswap can happen. I'm not sure frontswap guys
already discussed it long time ago.
If we can fix it, we can remove duplication entry handling logic
in all of backend of frontswap. All of backend should handle it although
it's pretty rare. Of course, zram could be fixed. It might be trivial now
but more there are many backend of frontswap, more it would be a headache.
If we are trying to fix it in swap layer, we might fix it following as
int reuse_swap_page(struct page *page)
{
..
..
if (count == 1) {
if (!PageWriteback(page)) {
delete_from_swap_cache(page);
SetPageDirty(page);
} else {
frontswap_invalidate_page();
swap_slot_free_notify();
}
}
}
But not sure, it is worth at the moment and there might be other places
to be fixed.(I hope Hugh can point out if we are missing something if he
has a time)
If we are reluctant to it, at least, we should write out comment above
frontswap_store about that to notice curious guys who spend many
time to know WHY and smart guys who are going to fix it with nice way.
Mr. Frontswap, What do you think about it?
--
Kind regards,
Minchan Kim
--
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 reply other threads:[~2013-01-31 5:11 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-01-31 5:11 Minchan Kim [this message]
2013-02-04 1:51 ` Questin about swap_slot free and invalidate page Hugh Dickins
2013-02-04 2:49 ` Minchan Kim
2013-02-04 21:28 ` Dan Magenheimer
2013-02-05 1:24 ` Minchan Kim
2013-02-19 12:12 ` Ric Mason
2013-02-19 15:27 ` Dan Magenheimer
2013-02-20 2:03 ` Ric Mason
2013-02-21 21:42 ` Dan Magenheimer
2013-02-22 3:13 ` Ric Mason
2013-02-25 17:20 ` Dan Magenheimer
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=20130131051140.GB23548@blaptop \
--to=minchan@kernel.org \
--cc=akpm@linux-foundation.org \
--cc=dan.magenheimer@oracle.com \
--cc=hughd@google.com \
--cc=konrad@darnok.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=ngupta@vflare.org \
--cc=sjenning@linux.vnet.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 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).