Re: [PATCH v6 0/2] zswap: remove the memcpy if acomp is not sleepable

From: Barry Song <21cnbao@gmail.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: herbert@gondor.apana.org.au, chriscli@google.com,
	chrisl@kernel.org,  ddstreet@ieee.org,
	linux-kernel@vger.kernel.org, sjenning@redhat.com,
	 vitaly.wool@konsulko.com, Barry Song <v-songbaohua@oppo.com>,
	davem@davemloft.net,  hannes@cmpxchg.org,
	linux-crypto@vger.kernel.org, linux-mm@kvack.org,
	 zhouchengming@bytedance.com, nphamcs@gmail.com,
	yosryahmed@google.com
Subject: Re: [PATCH v6 0/2] zswap: remove the memcpy if acomp is not sleepable
Date: Sat, 9 Mar 2024 11:58:39 +0800	[thread overview]
Message-ID: <CAGsJ_4xSwNZmHN5GvipS-1kELX2NBwHxoGE=aG4sekm1LcYVPw@mail.gmail.com> (raw)
In-Reply-To: <20240308192334.845b27504a663ec8af4f2aad@linux-foundation.org>

On Sat, Mar 9, 2024 at 11:23 AM Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Fri, 8 Mar 2024 19:57:38 +0800 Barry Song <21cnbao@gmail.com> wrote:
>
> > Hi Andrew,
> >
> > On Thu, Feb 22, 2024 at 4:11 PM Barry Song <21cnbao@gmail.com> wrote:
> > >
> > > From: Barry Song <v-songbaohua@oppo.com>
> > >
> > > In zswap, if we use zsmalloc, we cannot sleep while we map the
> > > compressed memory, so we copy it to a temporary buffer. By
> > > knowing the alg won't sleep can help zswap to avoid the
> > > memcpy.
> > > Thus we introduce an API in crypto to expose if acomp is async,
> > > and zswap can use it to decide if it can remove copying to the
> > > tmp buffer.
> > >
> > > -v6:
> > >  * add acked-by of Herbert, Thanks!
> > >  * remove patch 3/3 from the series, as that one will go
> > >    through crypto
> >
> > Can you please pull this into mm-tree? This used to have 3 patches.
> >
> > 3/3 was separated according to Herbert's requirements and has
> > been in a crypto tree.
> > crypto: scomp - remove memcpy if sg_nents is 1 and pages are lowmem
> > https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git/commit/?id=77292bb8ca
> >
> > Two drivers fixes(patch 1 needs) have also been in crypto tree:
> > crypto: hisilicon/zip - fix the missing CRYPTO_ALG_ASYNC in cra_flags
> > https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git/commit/?id=db8ac88385
> >
> > crypto: iaa - fix the missing CRYPTO_ALG_ASYNC in cra_flags
> > https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git/commit/?id=30dd94dba35
> >
> > So it should be quite safe to pull this series into mm-tree now.
>
> But this zswap chage requires the presence of the other patches, yes?

As far as I understand, we rely on two driver fixes because those drivers didn't
set the correct cra_flags needed by our patch1. Without those fixes implemented,
two platforms might encounter issues: Intel with IAA (Intel Analytics
Accelerator)
and Hisilicon with ZIP. Other platforms should be unaffected.

The two driver fixes have been merged into the crypto tree.

>
> So the mm.git tree alone will be buggy?  And if mm.git merges ahead of
> the other trees, there will be a window where mainline will be buggy?

Before 6.9-rc1, there might be issues if mm enters Linus' tree before Herbert's
crypto tree. However, by 6.9-rc1, everything should be fine.

>
> If so, I think it wuold be better to merge the zswap patch in the next
> merge window.
>

Okay, I understand. Since this patch improves zswap's performance, I wanted
it to be integrated sooner to contribute. However, I'm perfectly willing to
respect your concerns and adhere to the community's best practices.

So please handle it in the best way you think :-)

Thanks
Barry