linux-kernel - Re: [PATCH v4] mm/zswap: move to use crypto

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200709071714.32m7hatmkr4pk2f4@linutronix.de>
Date:   Thu, 9 Jul 2020 09:17:14 +0200
From:   Sebastian Andrzej Siewior <bigeasy@...utronix.de>
To:     "Song Bao Hua (Barry Song)" <song.bao.hua@...ilicon.com>
Cc:     "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
        "herbert@...dor.apana.org.au" <herbert@...dor.apana.org.au>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "linux-crypto@...r.kernel.org" <linux-crypto@...r.kernel.org>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Linuxarm <linuxarm@...wei.com>,
        "Luis Claudio R . Goncalves" <lgoncalv@...hat.com>,
        Mahipal Challa <mahipalreddy2006@...il.com>,
        Seth Jennings <sjenning@...hat.com>,
        Dan Streetman <ddstreet@...e.org>,
        Vitaly Wool <vitaly.wool@...sulko.com>,
        "Wangzhou (B)" <wangzhou1@...ilicon.com>,
        Colin Ian King <colin.king@...onical.com>
Subject: Re: [PATCH v4] mm/zswap: move to use crypto_acomp API for hardware
 acceleration

On 2020-07-08 21:45:47 [+0000], Song Bao Hua (Barry Song) wrote:
> > On 2020-07-08 00:52:10 [+1200], Barry Song wrote:
> > > @@ -127,9 +129,17 @@
> > > +struct crypto_acomp_ctx {
> > > +	struct crypto_acomp *acomp;
> > > +	struct acomp_req *req;
> > > +	struct crypto_wait wait;
> > > +	u8 *dstmem;
> > > +	struct mutex mutex;
> > > +};
> > …
> > > @@ -1074,12 +1138,32 @@ static int zswap_frontswap_store(unsigned
> > type, pgoff_t offset,
> > >  	}
> > >
> > >  	/* compress */
> > > -	dst = get_cpu_var(zswap_dstmem);
> > > -	tfm = *get_cpu_ptr(entry->pool->tfm);
> > > -	src = kmap_atomic(page);
> > > -	ret = crypto_comp_compress(tfm, src, PAGE_SIZE, dst, &dlen);
> > > -	kunmap_atomic(src);
> > > -	put_cpu_ptr(entry->pool->tfm);
> > > +	acomp_ctx = *this_cpu_ptr(entry->pool->acomp_ctx);
> > > +
> > > +	mutex_lock(&acomp_ctx->mutex);
> > > +
> > > +	src = kmap(page);
> > > +	dst = acomp_ctx->dstmem;
> > 
> > that mutex is per-CPU, per-context. The dstmem pointer is per-CPU. So if
> > I read this right, you can get preempted after crypto_wait_req() and
> > another context in this CPU writes its data to the same dstmem and then…
> > 
> 
> This isn't true. Another thread in this cpu will be blocked by the mutex.
> It is impossible for two threads to write the same dstmem.
> If thread1 ran on cpu1, it held cpu1's mutex; if another thread wants to run on cpu1, it is blocked.
> If thread1 ran on cpu1 first, it held cpu1's mutex, then it migrated to cpu2 (with very rare chance)
> 	a. if another thread wants to run on cpu1, it is blocked;

How it is blocked? That "struct crypto_acomp_ctx" is
"this_cpu_ptr(entry->pool->acomp_ctx)" - which is per-CPU of a pool
which you can have multiple of. But `dstmem' you have only one per-CPU
no matter have many pools you have.
So pool1 on CPU1 uses the same `dstmem' as pool2 on CPU1. But pool1 and
pool2 on CPU1 use a different mutex for protection of this `dstmem'.

> Thanks
> Barry

Sebastian