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

Make spl_kmem_cache size check consistent #15757

Merged
merged 2 commits into from
Jan 16, 2024

Conversation

youzhongyang
Copy link
Contributor

Define spl_kmem_cache_slab_limit to be SPL_MAX_KMEM_ORDER_NR_PAGES * PAGE_SIZE (16K on x86_64).

Motivation and Context

On Linux x86_64, kmem cache can have size up to 4M, however increasing spl_kmem_cache_slab_limit can lead to crash due to the size check inconsitency.

Description

How Has This Been Tested?

Built and tested on a VM in-house.

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:

On Linux x86_64, kmem cache can have size up to 4M,
however increasing spl_kmem_cache_slab_limit can lead
to crash due to the size check inconsitency.

Signed-off-by: Youzhong Yang <yyang@mathworks.com>
module/os/linux/spl/spl-kmem-cache.c Outdated Show resolved Hide resolved
@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Jan 12, 2024
Signed-off-by: Youzhong Yang <yyang@mathworks.com>
@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Jan 16, 2024
@behlendorf behlendorf merged commit 29ea6fa into openzfs:master Jan 16, 2024
24 of 25 checks passed
@youzhongyang
Copy link
Contributor Author

Just FYI - I've done stress testing with the limit raised to 2M, no issue has been observed, but I didn't get a chance to do performance testing.

One caveat - in crash kernel mode, zfs crashes as it fails to create ddt_cache of size 24840. This is most likely a bug in Linux slub allocator which only happens in that mode (single cpu core?) and I will dig deeper when time permits.

Anyway, this change does no harm.

behlendorf pushed a commit to behlendorf/zfs that referenced this pull request Jan 29, 2024
On Linux x86_64, kmem cache can have size up to 4M,
however increasing spl_kmem_cache_slab_limit can lead
to crash due to the size check inconsistency.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Youzhong Yang <yyang@mathworks.com>
Closes openzfs#15757
behlendorf pushed a commit that referenced this pull request Jan 29, 2024
On Linux x86_64, kmem cache can have size up to 4M,
however increasing spl_kmem_cache_slab_limit can lead
to crash due to the size check inconsistency.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Youzhong Yang <yyang@mathworks.com>
Closes #15757
lundman pushed a commit to openzfsonwindows/openzfs that referenced this pull request Mar 13, 2024
On Linux x86_64, kmem cache can have size up to 4M,
however increasing spl_kmem_cache_slab_limit can lead
to crash due to the size check inconsistency.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Youzhong Yang <yyang@mathworks.com>
Closes openzfs#15757
lundman pushed a commit to openzfsonwindows/openzfs that referenced this pull request Mar 13, 2024
On Linux x86_64, kmem cache can have size up to 4M,
however increasing spl_kmem_cache_slab_limit can lead
to crash due to the size check inconsistency.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Youzhong Yang <yyang@mathworks.com>
Closes openzfs#15757
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants