lkml.org 
[lkml]   [2020]   [Jul]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [PATCH v4] mm/zswap: move to use crypto_acomp API for hardware acceleration
Date


> -----Original Message-----
> From: linux-crypto-owner@vger.kernel.org
> [mailto:linux-crypto-owner@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.hua@hisilicon.com>
> Cc: akpm@linux-foundation.org; herbert@gondor.apana.org.au;
> davem@davemloft.net; linux-crypto@vger.kernel.org; linux-mm@kvack.org;
> linux-kernel@vger.kernel.org; Linuxarm <linuxarm@huawei.com>; Luis Claudio
> R . Goncalves <lgoncalv@redhat.com>; Mahipal Challa
> <mahipalreddy2006@gmail.com>; Seth Jennings <sjenning@redhat.com>;
> Dan Streetman <ddstreet@ieee.org>; Vitaly Wool
> <vitaly.wool@konsulko.com>; Wangzhou (B) <wangzhou1@hisilicon.com>;
> Colin Ian King <colin.king@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

\
 
 \ /
  Last update: 2020-07-09 09:55    [W:0.066 / U:0.088 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site