> -----Original Message-----
> From: [email protected] [mailto:[email protected]] On
> Behalf Of Sebastian Andrzej Siewior
> Sent: Thursday, July 9, 2020 8:41 PM
> To: Song Bao Hua (Barry Song) <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; Linuxarm <[email protected]>; Luis Claudio
> R . Goncalves <[email protected]>; Mahipal Challa
> <[email protected]>; Seth Jennings <[email protected]>;
> Dan Streetman <[email protected]>; Vitaly Wool
> <[email protected]>; Wangzhou (B) <[email protected]>;
> Colin Ian King <[email protected]>
> Subject: Re: [PATCH v4] mm/zswap: move to use crypto_acomp API for
> hardware acceleration
>
> On 2020-07-09 07:55:22 [+0000], Song Bao Hua (Barry Song) wrote:
> > Hello Sebastian, thanks for your reply and careful review.
> Hi,
>
> > 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?
>
> I provided just some pseudo code to illustrate an example how the async
> interface should look like (more or less). The essential part is where
> you allow to feed multiple requests without blocking.
Sebastian, Do you mean the below code?
@@ -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;
I think this won' work. -EINPROGRESS won't be able to decide if we should goto
out. We can only goto out if the compression
has done without any error. The error might be because of HW like EIO or
because the data is not suitable to compress. We can
only know the result after the compression is really done and the completion
callback is called by ZIP drivers.
If the compression is still INPROGRESS, we don't know what will happen.
> I went up the call-chain and found one potential user which seem to have
> a list of pages which are processed. This looked like a nice example. I
> haven't looked at the details.
>
> I have no opinion whether or not it makes sense to switch to the async
> interface in a sync way.
I always appreciate your comment and your opinion.
The real problem here is that all of those new zip drivers are adapted to async
interface. There is no old interface support
for those new drivers mainlined these years. zswap doesn’t work on those new
drivers as they totally don't support
crypto_comp_compress()
crypto_comp_decompress()
...
So the initial goal of this patch is fixing the disconnected bridge between new
zip drivers and zswap.
Making frontswap async can probably happen if we see performance improvement.
But it seems it is a big project, not
that simple. On the other hand, it seems hisi_zip in drivers/crypto is the only
async driver till now. Sorry if I am missing
any one. other drivers are adapted to acomp APIs by scomp APIs. For example:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/crypto/lz4.c?id=8cd9330e0a
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/crypto/lzo.c?id=ac9d2c4b3
So even we make frontswap totally async, most zip drivers are still sync and we
don't get the benefit. From my prospective,
I am glad to try the possibility of making frontswap async to leverage the
power of ZIP hardware. This would probably and
only happen after we have a base to support acomp APIs.
Thanks
Barry
