linux-kernel - Re: [PATCH v2] erofs: use kmap_local_page() only for erofs

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <Y1DFhKvOWvJacTIk@B-P7TQMD6M-0146.local>
Date:   Thu, 20 Oct 2022 11:50:28 +0800
From:   Gao Xiang <hsiangkao@...ux.alibaba.com>
To:     Ira Weiny <ira.weiny@...el.com>
Cc:     "Fabio M. De Francesco" <fmdefrancesco@...il.com>,
        linux-erofs@...ts.ozlabs.org, Chao Yu <chao@...nel.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] erofs: use kmap_local_page() only for erofs_bread()

On Wed, Oct 19, 2022 at 08:17:31PM -0700, Ira Weiny wrote:
> On Thu, Oct 20, 2022 at 10:18:36AM +0800, Gao Xiang wrote:
> > On Wed, Oct 19, 2022 at 08:17:05PM +0200, Fabio M. De Francesco wrote:
> > > On Wednesday, October 19, 2022 1:36:55 AM CEST Gao Xiang wrote:
> > > > On Wed, Oct 19, 2022 at 01:21:27AM +0200, Fabio M. De Francesco wrote:
> > > > > On Tuesday, October 18, 2022 11:29:21 PM CEST Gao Xiang wrote:
> 
> [snip]
> 
> > 
> > That is not the simple nested unmapped case as you said above, I could take
> > a very brief example:
> 
> Building on this.  The uncompressed pages always outnumber the compressed
> pages, right?

Yes, it's always true for EROFS case.

I think if the locking order is reversed I could unmap and remap the
pages in the correct order.  But as you said below, it just could work
but complex the code (I think if you have extra time you could check
the code first.)

So as I said before, I don't really care HIGHMEM performance so here
kmap_local() cannot benefit such case.  Anyway, it'd be much better
if kmap() is kept on my side anyway.

Thanks,
Gao Xiang

> 
> > 
> > 1. map a decompresed page
> > 2. map a compressed page
> 
> First reverse these because you are going to need to map a new decompressed
> page before another compressed page.  So:
> 
> 1. map compressed
> 2. map decompressed
> 
> Then 4/5 and 7/8 become unmap/map new without issue.
> 
> > 3. working
> > 4. decompressed page is all consumed, unmap the current decompressed page
> > 5. map the next decompressed page
> > 6. working
> > 7. decompressed page is all consumed, unmap the current decompressed page
> > 8. map the next decompressed page
> > 9. working
> 
> This is more complicated but not overly so.
> 
> Simply
> 
> 9.1 unmap decompressed
> 
> > 10. compressed page is all consumed, unmap the current compressed page
> > 11. map the next compressed page
> 
> 11.1 remap decompressed
> 
> > 12. working
> > 13. ... (anyway, unmap and remap a compressed page or a decompressed page
> >          in any order.)
> > 
> > until all process is finished.  by using kmap(), it's much simple to
> > implement this, but kmap_local(), it only complexes the code.
> 
> Agreed kmap() is easier but I think this could work.
> 
> Basically you keep the compressed mapped first.
> 
> I also assume there is also a reverse of this so reverse the pages in that
> case.
> 
> Thoughts?
> Ira