Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
From:   "NeilBrown" <neilb@suse.de>
To:     "Miaohe Lin" <linmiaohe@huawei.com>
Cc:     "Christoph Hellwig" <hch@infradead.org>,
        "David Howells" <dhowells@redhat.com>, linux-nfs@vger.kernel.org,
        linux-mm@kvack.org, linux-kernel@vger.kernel.org,
        "Andrew Morton" <akpm@linux-foundation.org>
Subject: Re: [PATCH 09/10] MM: submit multipage write for SWP_FS_OPS swap-space
In-reply-to: <033ccf1a-c6b5-fd77-0ad0-4915ff07bc15@huawei.com>
References: <164859751830.29473.5309689752169286816.stgit@noble.brown>,
 <164859778128.29473.5191868522654408537.stgit@noble.brown>,
 <033ccf1a-c6b5-fd77-0ad0-4915ff07bc15@huawei.com>
Date:   Tue, 26 Apr 2022 11:58:37 +1000
Message-id: <165093831774.1648.3187486020864614234@noble.neil.brown.name>
Precedence: bulk

On Mon, 18 Apr 2022, Miaohe Lin wrote:
> On 2022/3/30 7:49, NeilBrown wrote:
> > swap_writepage() is given one page at a time, but may be called repeatedly
> > in succession.
> > For block-device swapspace, the blk_plug functionality allows the
> > multiple pages to be combined together at lower layers.
> > That cannot be used for SWP_FS_OPS as blk_plug may not exist - it is
> > only active when CONFIG_BLOCK=3Dy.  Consequently all swap reads over NFS
> > are single page reads.
> >=20
> > With this patch we pass a pointer-to-pointer via the wbc.
> > swap_writepage can store state between calls - much like the pointer
> > passed explicitly to swap_readpage.  After calling swap_writepage() some
> > number of times, the state will be passed to swap_write_unplug() which
> > can submit the combined request.
> >=20
> > Reviewed-by: Christoph Hellwig <hch@lst.de>
> > Signed-off-by: NeilBrown <neilb@suse.de>
> ...
> > =20
> >  static int swap_writepage_fs(struct page *page, struct writeback_control=
 *wbc)
> >  {
> > -	struct swap_iocb *sio;
> > +	struct swap_iocb *sio =3D NULL;
> >  	struct swap_info_struct *sis =3D page_swap_info(page);
> >  	struct file *swap_file =3D sis->swap_file;
> > -	struct address_space *mapping =3D swap_file->f_mapping;
> > -	struct iov_iter from;
> > -	int ret;
> > +	loff_t pos =3D page_file_offset(page);
> > =20
> >  	set_page_writeback(page);
> >  	unlock_page(page);
> > -	sio =3D mempool_alloc(sio_pool, GFP_NOIO);
> > -	init_sync_kiocb(&sio->iocb, swap_file);
> > -	sio->iocb.ki_complete =3D sio_write_complete;
> > -	sio->iocb.ki_pos =3D page_file_offset(page);
> > -	sio->bvec[0].bv_page =3D page;
> > -	sio->bvec[0].bv_len =3D PAGE_SIZE;
> > -	sio->bvec[0].bv_offset =3D 0;
> > -	iov_iter_bvec(&from, WRITE, &sio->bvec[0], 1, PAGE_SIZE);
> > -	ret =3D mapping->a_ops->swap_rw(&sio->iocb, &from);
> > -	if (ret !=3D -EIOCBQUEUED)
> > -		sio_write_complete(&sio->iocb, ret);
> > -	return ret;
> > +	if (wbc->swap_plug)
> > +		sio =3D *wbc->swap_plug;
> > +	if (sio) {
> > +		if (sio->iocb.ki_filp !=3D swap_file ||
> > +		    sio->iocb.ki_pos + sio->pages * PAGE_SIZE !=3D pos) {
> > +			swap_write_unplug(sio);
> > +			sio =3D NULL;
> > +		}
> > +	}
> > +	if (!sio) {
> > +		sio =3D mempool_alloc(sio_pool, GFP_NOIO);
> > +		init_sync_kiocb(&sio->iocb, swap_file);
> > +		sio->iocb.ki_complete =3D sio_write_complete;
> > +		sio->iocb.ki_pos =3D pos;
> > +		sio->pages =3D 0;
> > +	}
> > +	sio->bvec[sio->pages].bv_page =3D page;
> > +	sio->bvec[sio->pages].bv_len =3D PAGE_SIZE;
>=20
> Many thanks for your patch. And sorry for late responding and newbie questi=
on. Does swap_writepage_fs
> support transhuge page now? We could come across transhuge page here. But b=
v_len =3D=3D PAGE_SIZE and pages
> =3D=3D 1 is assumed here. Do we need something like below:
>=20
> sio->bvec[sio->pages].bv_len =3D thp_size(page);
> sio->pages +=3D thp_nr_pages(page);

Yes, that probably makes sense.  I'll have a closer look and maybe
resend later this week.

Thanks,
NeilBrown


>=20
> Thanks! :)
>=20
> > +	sio->bvec[sio->pages].bv_offset =3D 0;
> ...
> > .
> >=20
>=20
>=20