> -----Original Message-----
> From: linux-crypto-ow...@vger.kernel.org
> [mailto:linux-crypto-ow...@vger.kernel.org] On Behalf Of Sebastian Andrzej
> Siewior
> Sent: Thursday, July 9, 2020 7:39 PM
> To: Song Bao Hua (Barry Song) <song.bao....@hisilicon.com>
> Cc: a...@linux-foundation.org; herb...@gondor.apana.org.au;
> da...@davemloft.net; linux-crypto@vger.kernel.org; linux...@kvack.org;
> linux-ker...@vger.kernel.org; Linuxarm <linux...@huawei.com>; Luis Claudio
> R . Goncalves <lgonc...@redhat.com>; Mahipal Challa
> <mahipalreddy2...@gmail.com>; Seth Jennings <sjenn...@redhat.com>;
> Dan Streetman <ddstr...@ieee.org>; Vitaly Wool
> <vitaly.w...@konsulko.com>; Wangzhou (B) <wangzh...@hisilicon.com>;
> Colin Ian King <colin.k...@canonical.com>
> Subject: Re: [PATCH v4] mm/zswap: move to use crypto_acomp API for
> hardware acceleration
>
> On 2020-07-09 01:32:38 [+0000], Song Bao Hua (Barry Song) wrote:
> > > This looks using the same synchronous mechanism around an
> asynchronous
> > > interface. It works as a PoC.
> > >
> > > As far as I remember the crypto async interface, the incoming skbs were
> > > fed
> to
> > > the async interface and returned to the caller so the NIC could continue
> > > allocate new RX skbs and move on. Only if the queue of requests was
> getting
> > > to long the code started to throttle. Eventually the async crypto code
> > > completed the decryption operation in a different context and fed the
> > > decrypted packet(s) into the stack.
> > >
> > > From a quick view, you would have to return -EINPROGRESS here and have
> at
> > > the caller side something like that:
> > >
> > > iff --git a/mm/page_io.c b/mm/page_io.c
> > > index e8726f3e3820b..9d1baa46ec3ed 100644
> > > --- a/mm/page_io.c
> > > +++ b/mm/page_io.c
> > > @@ -252,12 +252,15 @@ int swap_writepage(struct page *page, struct
> > > writeback_control *wbc)
> > > unlock_page(page);
> > > goto out;
> > > }
> > > - if (frontswap_store(page) == 0) {
> > > + ret = frontswap_store(page);
> > > + if (ret == 0) {
> > > set_page_writeback(page);
> > > unlock_page(page);
> > > end_page_writeback(page);
> > > goto out;
> > > }
> > > + if (ret = -EINPROGRESS)
> > > + goto out;
> > > ret = __swap_writepage(page, wbc, end_swap_bio_write);
> > > out:
> > > return ret;
> > >
> > Unfortunately, this is not true and things are not that simple.
> >
> > We can't simply depend on -EINPROGRESS and go out.
> > We have to wait for the result of compression to decide if we should
> > do __swap_writepage(). As one page might be compressed into two
> > pages, in this case, we will still need to do _swap_writepage().
> > As I replied in the latest email, all of the async improvement to frontswap
> > needs very careful thinking and benchmark. It can only happen after
> > we build the base in this patch, fixing the broken connection between
> > zswap and those new zip drivers.
>
> At the time the compression finishes you see what happens and based on
> the design you can either complete it immediately (the 0/error case from
> above) or forward the result to the caller and let him decide.
Hello Sebastian, thanks for your reply and careful review.
Right now, frontswap is pretty much one thing which happens before
__swap_writepage().
The whole design is full of the assumption that frontswap is sync. So if
frontswap
consumes a page without any error, this page won't go to __swap_writepage()
which is async. On the other hand, if frontswap's store has any error, that
means
this page needs to swap to disk.
int swap_writepage(struct page *page, struct writeback_control *wbc)
{
int ret = 0;
if (try_to_free_swap(page)) {
unlock_page(page);
goto out;
}
if (frontswap_store(page) == 0) {
set_page_writeback(page);
unlock_page(page);
end_page_writeback(page);
goto out;
}
ret = __swap_writepage(page, wbc, end_swap_bio_write);
out:
return ret;
}
I don't think we can simply "forward the result to the caller and let him
decide".
Would you like to present some pseudo code?
Thanks
Barry
