From: Hugh Dickins <hughd@google.com>
To: Yu Zhao <yuzhao@google.com>
Cc: Hugh Dickins <hughd@google.com>,
Andrew Morton <akpm@linux-foundation.org>,
linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] mm: fix swap offset when replacing shmem page
Date: Mon, 19 Nov 2018 21:07:27 -0800 (PST) [thread overview]
Message-ID: <alpine.LSU.2.11.1811192057490.2185@eggly.anvils> (raw)
In-Reply-To: <20181120012950.GA94981@google.com>
On Mon, 19 Nov 2018, Yu Zhao wrote:
> On Mon, Nov 19, 2018 at 02:11:27PM -0800, Hugh Dickins wrote:
> > On Sun, 18 Nov 2018, Yu Zhao wrote:
> >
> > > We used to have a single swap address space with swp_entry_t.val
> > > as its radix tree index. This is not the case anymore. Now Each
> > > swp_type() has its own address space and should use swp_offset()
> > > as radix tree index.
> > >
> > > Signed-off-by: Yu Zhao <yuzhao@google.com>
> >
> > This fix is a great find, thank you! But completely mis-described!
>
> Yes, now I remember making swap offset as key was done long after per
> swap device radix tree.
>
> > And could you do a smaller patch, keeping swap_index, that can go to
> > stable without getting into trouble with the recent xarrifications?
> >
> > Fixes: bde05d1ccd51 ("shmem: replace page if mapping excludes its zone")
> > Cc: stable@vger.kernel.org # 3.5+
> >
> > Seems shmem_replace_page() has been wrong since the day I wrote it:
> > good enough to work on swap "type" 0, which is all most people ever use
> > (especially those few who need shmem_replace_page() at all), but broken
> > once there are any non-0 swp_type bits set in the higher order bits.
>
> But you did get it right when you wrote the function, which was before
> the per swap device radix tree. so
> Fixes: f6ab1f7f6b2d ("mm, swap: use offset of swap entry as key of swap cache")
> looks good?
Oh, you're right, thank you. Yes, the fix is to that one, in 4.9 onwards.
I don't much like my original use of the name "swap_index", when it was
not the index in a swapfile (though it was the index in the radix tree);
but it will become a correct name with your patch.
Though Matthew Wilcox seems to want us to avoid saying "radix tree"...
Hugh
next prev parent reply other threads:[~2018-11-20 5:07 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-19 0:47 [PATCH] mm: fix swap offset when replacing shmem page Yu Zhao
2018-11-19 1:09 ` [PATCH v2] " Yu Zhao
2018-11-19 22:11 ` Hugh Dickins
2018-11-20 1:29 ` Yu Zhao
2018-11-20 5:07 ` Hugh Dickins [this message]
2018-11-20 13:12 ` Matthew Wilcox
2018-11-21 21:54 ` [PATCH v3] mm: use swp_offset as key in shmem_replace_page() Yu Zhao
2018-11-21 22:11 ` Matthew Wilcox
2018-11-22 0:36 ` Hugh Dickins
2018-11-22 5:01 ` Andrew Morton
2018-11-23 20:11 ` Hugh Dickins
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=alpine.LSU.2.11.1811192057490.2185@eggly.anvils \
--to=hughd@google.com \
--cc=akpm@linux-foundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=yuzhao@google.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).