Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use kmap_local_page instead of kmap_atomic #16329

Merged
merged 1 commit into from
Jul 17, 2024
Merged

Conversation

calccrypto
Copy link
Contributor

@calccrypto calccrypto commented Jul 8, 2024

Motivation and Context

In Linux Kernel 5.11, kmap_atomic() was deprecated in favor of kmap_local_page().

Description

Changed

#define zfs_kmap_atomic(page)   kmap_atomic(page)
#define zfs_kunmap_atomic(addr) kunmap_atomic(addr)

to

#define	zfs_kmap_local(page)	kmap_local_page(page)
#define	zfs_kunmap_local(addr)	kunmap_local(addr)

for kernels that have kmap_local_page().

How Has This Been Tested?

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist:

Copy link
Member

@amotin amotin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to conflict with #10018, already having zfs_kmap()/zfs_kunmap(). I'd either kept it as _atomic, or made _local().

@calccrypto
Copy link
Contributor Author

calccrypto commented Jul 8, 2024

I've changed the macros to zfs_kmap_temp and zfs_kunmap_temp to reflect the temporary nature of the mappings as described in High Memory Handling.

@calccrypto calccrypto force-pushed the kmap-update branch 2 times, most recently from dfb9562 to 5be58af Compare July 8, 2024 20:24
@amotin
Copy link
Member

amotin commented Jul 8, 2024

I'll let others decide, but to me _local has pretty specific meaning -- it is local to this thread/CPU. Definition of temporary is much less specific, plus it is different from kernel and so confusing, plus you have to use abbreviation.

Changed zfs_k(un)map_atomic to zfs_k(un)map_local

Signed-off-by: Jason Lee <jasonlee@lanl.gov>
@calccrypto
Copy link
Contributor Author

calccrypto commented Jul 8, 2024

Changed to _local

@tonyhutter tonyhutter merged commit 41902c8 into openzfs:master Jul 17, 2024
21 of 25 checks passed
calccrypto added a commit to hpc/zfs that referenced this pull request Jul 17, 2024
Changed zfs_k(un)map_atomic to zfs_k(un)map_local

Signed-off-by: Jason Lee <jasonlee@lanl.gov>
Reviewed-by: Alexander Motin <mav@FreeBSD.org>
Reviewed-by: Brian Atkinson <batkinson@lanl.gov>
@calccrypto calccrypto deleted the kmap-update branch July 17, 2024 19:28
lundman pushed a commit to openzfsonwindows/openzfs that referenced this pull request Sep 4, 2024
Changed zfs_k(un)map_atomic to zfs_k(un)map_local

Signed-off-by: Jason Lee <jasonlee@lanl.gov>
Reviewed-by: Alexander Motin <mav@FreeBSD.org>
Reviewed-by: Brian Atkinson <batkinson@lanl.gov>
ptr1337 pushed a commit to CachyOS/zfs that referenced this pull request Nov 14, 2024
Changed zfs_k(un)map_atomic to zfs_k(un)map_local

Signed-off-by: Jason Lee <jasonlee@lanl.gov>
Reviewed-by: Alexander Motin <mav@FreeBSD.org>
Reviewed-by: Brian Atkinson <batkinson@lanl.gov>
ptr1337 pushed a commit to CachyOS/zfs that referenced this pull request Nov 21, 2024
Changed zfs_k(un)map_atomic to zfs_k(un)map_local

Signed-off-by: Jason Lee <jasonlee@lanl.gov>
Reviewed-by: Alexander Motin <mav@FreeBSD.org>
Reviewed-by: Brian Atkinson <batkinson@lanl.gov>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants