From: Yang Shi <shy828301@gmail.com>
To: NeilBrown <neilb@suse.de>, Huang Ying <ying.huang@intel.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
Geert Uytterhoeven <geert+renesas@glider.be>,
Christoph Hellwig <hch@lst.de>, Miaohe Lin <linmiaohe@huawei.com>,
linux-nfs@vger.kernel.org, Linux MM <linux-mm@kvack.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 1/2] MM: handle THP in swap_*page_fs()
Date: Mon, 2 May 2022 10:48:49 -0700 [thread overview]
Message-ID: <CAHbLzkpF4zedBmipjX8Zy5F=Fffez+xgxTAvveaz1nRHb9Wg_Q@mail.gmail.com> (raw)
In-Reply-To: <165146539609.24404.4051313590023463843@noble.neil.brown.name>
On Sun, May 1, 2022 at 9:23 PM NeilBrown <neilb@suse.de> wrote:
>
> On Sat, 30 Apr 2022, Yang Shi wrote:
> > On Thu, Apr 28, 2022 at 5:44 PM NeilBrown <neilb@suse.de> wrote:
> > >
> > > Pages passed to swap_readpage()/swap_writepage() are not necessarily all
> > > the same size - there may be transparent-huge-pages involves.
> > >
> > > The BIO paths of swap_*page() handle this correctly, but the SWP_FS_OPS
> > > path does not.
> > >
> > > So we need to use thp_size() to find the size, not just assume
> > > PAGE_SIZE, and we need to track the total length of the request, not
> > > just assume it is "page * PAGE_SIZE".
> >
> > Swap-over-nfs doesn't support THP swap IIUC. So SWP_FS_OPS should not
> > see THP at all. But I agree to remove the assumption about page size
> > in this path.
>
> Can you help me understand this please. How would the swap code know
> that swap-over-NFS doesn't support THP swap? There is no reason that
> NFS wouldn't be able to handle 2MB writes. Even 1GB should work though
> NFS would have to split into several smaller WRITE requests.
AFAICT, THP swap is only supported on non-rotate block devices, for
example, SSD, PMEM, etc. IIRC, the swap device has to support the
cluster in order to swap THP. The cluster is only supported by
non-rotate block devices.
Looped Ying in, who is the author of THP swap.
>
> Thanks,
> NeilBrown
>
>
> >
> > >
> > > Reported-by: Miaohe Lin <linmiaohe@huawei.com>
> > > Signed-off-by: NeilBrown <neilb@suse.de>
> > > ---
> > > mm/page_io.c | 23 +++++++++++++----------
> > > 1 file changed, 13 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/mm/page_io.c b/mm/page_io.c
> > > index c132511f521c..d636a3531cad 100644
> > > --- a/mm/page_io.c
> > > +++ b/mm/page_io.c
> > > @@ -239,6 +239,7 @@ struct swap_iocb {
> > > struct kiocb iocb;
> > > struct bio_vec bvec[SWAP_CLUSTER_MAX];
> > > int pages;
> > > + int len;
> > > };
> > > static mempool_t *sio_pool;
> > >
> > > @@ -261,7 +262,7 @@ static void sio_write_complete(struct kiocb *iocb, long ret)
> > > struct page *page = sio->bvec[0].bv_page;
> > > int p;
> > >
> > > - if (ret != PAGE_SIZE * sio->pages) {
> > > + if (ret != sio->len) {
> > > /*
> > > * In the case of swap-over-nfs, this can be a
> > > * temporary failure if the system has limited
> > > @@ -301,7 +302,7 @@ static int swap_writepage_fs(struct page *page, struct writeback_control *wbc)
> > > sio = *wbc->swap_plug;
> > > if (sio) {
> > > if (sio->iocb.ki_filp != swap_file ||
> > > - sio->iocb.ki_pos + sio->pages * PAGE_SIZE != pos) {
> > > + sio->iocb.ki_pos + sio->len != pos) {
> > > swap_write_unplug(sio);
> > > sio = NULL;
> > > }
> > > @@ -312,10 +313,12 @@ static int swap_writepage_fs(struct page *page, struct writeback_control *wbc)
> > > sio->iocb.ki_complete = sio_write_complete;
> > > sio->iocb.ki_pos = pos;
> > > sio->pages = 0;
> > > + sio->len = 0;
> > > }
> > > sio->bvec[sio->pages].bv_page = page;
> > > - sio->bvec[sio->pages].bv_len = PAGE_SIZE;
> > > + sio->bvec[sio->pages].bv_len = thp_size(page);
> > > sio->bvec[sio->pages].bv_offset = 0;
> > > + sio->len += thp_size(page);
> > > sio->pages += 1;
> > > if (sio->pages == ARRAY_SIZE(sio->bvec) || !wbc->swap_plug) {
> > > swap_write_unplug(sio);
> > > @@ -371,8 +374,7 @@ void swap_write_unplug(struct swap_iocb *sio)
> > > struct address_space *mapping = sio->iocb.ki_filp->f_mapping;
> > > int ret;
> > >
> > > - iov_iter_bvec(&from, WRITE, sio->bvec, sio->pages,
> > > - PAGE_SIZE * sio->pages);
> > > + iov_iter_bvec(&from, WRITE, sio->bvec, sio->pages, sio->len);
> > > ret = mapping->a_ops->swap_rw(&sio->iocb, &from);
> > > if (ret != -EIOCBQUEUED)
> > > sio_write_complete(&sio->iocb, ret);
> > > @@ -383,7 +385,7 @@ static void sio_read_complete(struct kiocb *iocb, long ret)
> > > struct swap_iocb *sio = container_of(iocb, struct swap_iocb, iocb);
> > > int p;
> > >
> > > - if (ret == PAGE_SIZE * sio->pages) {
> > > + if (ret == sio->len) {
> > > for (p = 0; p < sio->pages; p++) {
> > > struct page *page = sio->bvec[p].bv_page;
> > >
> > > @@ -415,7 +417,7 @@ static void swap_readpage_fs(struct page *page,
> > > sio = *plug;
> > > if (sio) {
> > > if (sio->iocb.ki_filp != sis->swap_file ||
> > > - sio->iocb.ki_pos + sio->pages * PAGE_SIZE != pos) {
> > > + sio->iocb.ki_pos + sio->len != pos) {
> > > swap_read_unplug(sio);
> > > sio = NULL;
> > > }
> > > @@ -426,10 +428,12 @@ static void swap_readpage_fs(struct page *page,
> > > sio->iocb.ki_pos = pos;
> > > sio->iocb.ki_complete = sio_read_complete;
> > > sio->pages = 0;
> > > + sio->len = 0;
> > > }
> > > sio->bvec[sio->pages].bv_page = page;
> > > - sio->bvec[sio->pages].bv_len = PAGE_SIZE;
> > > + sio->bvec[sio->pages].bv_len = thp_size(page);
> > > sio->bvec[sio->pages].bv_offset = 0;
> > > + sio->len += thp_size(page);
> > > sio->pages += 1;
> > > if (sio->pages == ARRAY_SIZE(sio->bvec) || !plug) {
> > > swap_read_unplug(sio);
> > > @@ -521,8 +525,7 @@ void __swap_read_unplug(struct swap_iocb *sio)
> > > struct address_space *mapping = sio->iocb.ki_filp->f_mapping;
> > > int ret;
> > >
> > > - iov_iter_bvec(&from, READ, sio->bvec, sio->pages,
> > > - PAGE_SIZE * sio->pages);
> > > + iov_iter_bvec(&from, READ, sio->bvec, sio->pages, sio->len);
> > > ret = mapping->a_ops->swap_rw(&sio->iocb, &from);
> > > if (ret != -EIOCBQUEUED)
> > > sio_read_complete(&sio->iocb, ret);
> > >
> > >
> > >
> >
next prev parent reply other threads:[~2022-05-02 17:49 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-29 0:43 [PATCH 0/2] Finalising swap-over-NFS patches NeilBrown
2022-04-29 0:43 ` [PATCH 1/2] MM: handle THP in swap_*page_fs() NeilBrown
2022-04-29 1:21 ` Andrew Morton
2022-04-29 1:57 ` NeilBrown
2022-04-29 8:13 ` Miaohe Lin
2022-04-29 19:04 ` Yang Shi
2022-05-02 4:23 ` NeilBrown
2022-05-02 17:48 ` Yang Shi [this message]
2022-05-04 23:41 ` NeilBrown
2022-05-06 2:56 ` ying.huang
2022-04-29 0:43 ` [PATCH 2/2] NFS: rename nfs_direct_IO and use as ->swap_rw NeilBrown
2022-04-29 1:23 ` Andrew Morton
2022-04-29 2:05 ` NeilBrown
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='CAHbLzkpF4zedBmipjX8Zy5F=Fffez+xgxTAvveaz1nRHb9Wg_Q@mail.gmail.com' \
--to=shy828301@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=geert+renesas@glider.be \
--cc=hch@lst.de \
--cc=linmiaohe@huawei.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linux-nfs@vger.kernel.org \
--cc=neilb@suse.de \
--cc=ying.huang@intel.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.