mbox series

[GIT,PULL] Kmap conversions for 5.12, take 2

Message ID cover.1614616683.git.dsterba@suse.com (mailing list archive)
State New, archived
Headers show
Series [GIT,PULL] Kmap conversions for 5.12, take 2 | expand

Pull-request

git://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git kmap-conversion-for-5.12

Message

David Sterba March 1, 2021, 4:50 p.m. UTC
Hi,

this pull request contains changes regarding kmap API use and eg.
conversion from kmap_atomic to kmap_local_page.

Changes against v1 [1]:

- dropped patches using zero_user
- retested with my regular fstests-based workloads over the weekend

I'm keeping the original merge changelog as it seems to apply to v2 as
well.

Please pull, thanks.

[1] https://lore.kernel.org/lkml/cover.1614090658.git.dsterba@suse.com/

The API belongs to memory management but to save cross-tree dependency
headaches we've agreed to take it through the btrfs tree because there
are some trivial conversions possible, while the rest will need some
time and getting the easy cases out of the way would be convenient.

The final patchset arrived shortly before merge window, which is not
perfect, but given that it's straightforward I don't think it's too
risky.

I've added it to my for-next branch and it's been in linux-next for more
than a week.  Meanwhile I've been testing it among my regular branches
with additional MM related debugging options.

The changes can be grouped:

- function exports, new helpers

- new VM_BUG_ON for additional verification; it's been discussed if it
  should be VM_BUG_ON or BUG_ON, the former was chosen due to
  performance reasons

- code replaced by relevant helpers

----------------------------------------------------------------
The following changes since commit 92bf22614b21a2706f4993b278017e437f7785b3:

  Linux 5.11-rc7 (2021-02-07 13:57:38 -0800)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git kmap-conversion-for-5.12

for you to fetch changes up to 80cc83842394e5ad3e93487359106aab3420bcb7:

  btrfs: use copy_highpage() instead of 2 kmaps() (2021-02-26 12:45:15 +0100)

----------------------------------------------------------------
Ira Weiny (6):
      mm/highmem: Lift memcpy_[to|from]_page to core
      mm/highmem: Convert memcpy_[to|from]_page() to kmap_local_page()
      mm/highmem: Introduce memcpy_page(), memmove_page(), and memset_page()
      mm/highmem: Add VM_BUG_ON() to mem*_page() calls
      btrfs: use memcpy_[to|from]_page() and kmap_local_page()
      btrfs: use copy_highpage() instead of 2 kmaps()

 fs/btrfs/compression.c  |  6 ++----
 fs/btrfs/lzo.c          |  4 ++--
 fs/btrfs/raid56.c       | 10 +--------
 fs/btrfs/reflink.c      |  6 +-----
 fs/btrfs/send.c         |  7 ++-----
 fs/btrfs/zlib.c         |  5 ++---
 fs/btrfs/zstd.c         |  6 ++----
 include/linux/highmem.h | 56 +++++++++++++++++++++++++++++++++++++++++++++++++
 lib/iov_iter.c          | 14 -------------
 9 files changed, 68 insertions(+), 46 deletions(-)

Comments

Linus Torvalds March 1, 2021, 7:42 p.m. UTC | #1
On Mon, Mar 1, 2021 at 8:52 AM David Sterba <dsterba@suse.com> wrote:
>
> Ira Weiny (6):
>       mm/highmem: Lift memcpy_[to|from]_page to core
>       mm/highmem: Convert memcpy_[to|from]_page() to kmap_local_page()
>       mm/highmem: Introduce memcpy_page(), memmove_page(), and memset_page()
>       mm/highmem: Add VM_BUG_ON() to mem*_page() calls
>       btrfs: use memcpy_[to|from]_page() and kmap_local_page()
>       btrfs: use copy_highpage() instead of 2 kmaps()

So I've pulled this now, although I still end up wondering about one case there:

-       char *map;
-
-       map = kmap(page);
-       memcpy(map, data_start, datal);
+       memcpy_to_page(page, 0, data_start, datal);
        flush_dcache_page(page);
-       kunmap(page);

where that flush_dcache_page() is now done outside the kmap of the page.

If you have an architecture that does both (a) highmem and (b) virtual
caches, it means that the "memcpy_to_page()" gets done using one
virtual address, and the flush_dcache_page() could in theory be done
using another virtual address.

I do not believe this is a problem in practice (flush_dcache_page()
might have to kmap it again, but presumably get the same virtual
address, although who the heck knows). And I personally don't know
that we should even care any more - I've been arguing that we should
start deprecating highmem entirely, and while there are 32-bit arm
chips that still use them, I hope to $DEITY that those ARM chips
aren't the garbage virtual cached ones.

Furthermore, I think that kunmap() always guaranteed that the cache
was flushed anyway before unmapping, because anything else would have
been too broken for words anyway. So I think _all_ of those
flush_dcache_page() cases were just largely bogus.

I can't be bothered to really look into it, because at some point,
crap hardware is just too crap to even care about. Pure virtual caches
are where I personally say "I don't care". But I'm mentioning it
because there might be some masochistic person out there that finds
this issue interesting, and wants to do some self-flagellation to dive
into this all and make sure it's ok.

           Linus
pr-tracker-bot@kernel.org March 1, 2021, 7:44 p.m. UTC | #2
The pull request you sent on Mon,  1 Mar 2021 17:50:13 +0100:

> git://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git kmap-conversion-for-5.12

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/7a7fd0de4a9804299793e564a555a49c1fc924cb

Thank you!