linux-ext4 - Re: [PATCH RFC PKS/PMEM 22/58] fs/f2fs: Utilize new kmap

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]

Date:   Fri, 9 Oct 2020 18:30:36 -0700
From:   Eric Biggers <ebiggers@...nel.org>
To:     Matthew Wilcox <willy@...radead.org>
Cc:     ira.weiny@...el.com, Andrew Morton <akpm@...ux-foundation.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        Andy Lutomirski <luto@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>, linux-aio@...ck.org,
        linux-efi@...r.kernel.org, kvm@...r.kernel.org,
        linux-doc@...r.kernel.org, linux-mmc@...r.kernel.org,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        dri-devel@...ts.freedesktop.org, linux-mm@...ck.org,
        target-devel@...r.kernel.org, linux-mtd@...ts.infradead.org,
        linux-kselftest@...r.kernel.org, samba-technical@...ts.samba.org,
        ceph-devel@...r.kernel.org, drbd-dev@...ts.linbit.com,
        devel@...verdev.osuosl.org, linux-cifs@...r.kernel.org,
        linux-nilfs@...r.kernel.org, linux-scsi@...r.kernel.org,
        linux-nvdimm@...ts.01.org, linux-rdma@...r.kernel.org,
        x86@...nel.org, amd-gfx@...ts.freedesktop.org,
        linux-afs@...ts.infradead.org, cluster-devel@...hat.com,
        linux-cachefs@...hat.com, intel-wired-lan@...ts.osuosl.org,
        xen-devel@...ts.xenproject.org, linux-ext4@...r.kernel.org,
        Fenghua Yu <fenghua.yu@...el.com>, ecryptfs@...r.kernel.org,
        linux-um@...ts.infradead.org, intel-gfx@...ts.freedesktop.org,
        linux-erofs@...ts.ozlabs.org, reiserfs-devel@...r.kernel.org,
        linux-block@...r.kernel.org, linux-bcache@...r.kernel.org,
        Jaegeuk Kim <jaegeuk@...nel.org>,
        Dan Williams <dan.j.williams@...el.com>,
        io-uring@...r.kernel.org, linux-nfs@...r.kernel.org,
        linux-ntfs-dev@...ts.sourceforge.net, netdev@...r.kernel.org,
        kexec@...ts.infradead.org, linux-kernel@...r.kernel.org,
        linux-f2fs-devel@...ts.sourceforge.net,
        linux-fsdevel@...r.kernel.org, bpf@...r.kernel.org,
        linuxppc-dev@...ts.ozlabs.org, linux-btrfs@...r.kernel.org
Subject: Re: [PATCH RFC PKS/PMEM 22/58] fs/f2fs: Utilize new kmap_thread()

On Sat, Oct 10, 2020 at 01:39:54AM +0100, Matthew Wilcox wrote:
> On Fri, Oct 09, 2020 at 02:34:34PM -0700, Eric Biggers wrote:
> > On Fri, Oct 09, 2020 at 12:49:57PM -0700, ira.weiny@...el.com wrote:
> > > The kmap() calls in this FS are localized to a single thread.  To avoid
> > > the over head of global PKRS updates use the new kmap_thread() call.
> > >
> > > @@ -2410,12 +2410,12 @@ static inline struct page *f2fs_pagecache_get_page(
> > >  
> > >  static inline void f2fs_copy_page(struct page *src, struct page *dst)
> > >  {
> > > -	char *src_kaddr = kmap(src);
> > > -	char *dst_kaddr = kmap(dst);
> > > +	char *src_kaddr = kmap_thread(src);
> > > +	char *dst_kaddr = kmap_thread(dst);
> > >  
> > >  	memcpy(dst_kaddr, src_kaddr, PAGE_SIZE);
> > > -	kunmap(dst);
> > > -	kunmap(src);
> > > +	kunmap_thread(dst);
> > > +	kunmap_thread(src);
> > >  }
> > 
> > Wouldn't it make more sense to switch cases like this to kmap_atomic()?
> > The pages are only mapped to do a memcpy(), then they're immediately unmapped.
> 
> Maybe you missed the earlier thread from Thomas trying to do something
> similar for rather different reasons ...
> 
> https://lore.kernel.org/lkml/20200919091751.011116649@linutronix.de/

I did miss it.  I'm not subscribed to any of the mailing lists it was sent to.

Anyway, it shouldn't matter.  Patchsets should be standalone, and not require
reading random prior threads on linux-kernel to understand.

And I still don't really understand.  After this patchset, there is still code
nearly identical to the above (doing a temporary mapping just for a memcpy) that
would still be using kmap_atomic().  Is the idea that later, such code will be
converted to use kmap_thread() instead?  If not, why use one over the other?

- Eric