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

Linux 5.0 compat: ASM_BUG macro #8725

Merged
merged 1 commit into from
May 8, 2019

Conversation

behlendorf
Copy link
Contributor

Motivation and Context

Issue #8545

In file included from /home/fedora/zfs/module/zfs/vdev_raidz_math_aarch64_neon.c:30:
/home/fedora/zfs/module/zfs/vdev_raidz_math_aarch64_neon_common.h:126: error: "ASM_BUG" redefined [-Werror]
 #define ASM_BUG() ASSERT(0)
 
In file included from ./arch/arm64/include/asm/bug.h:23,
                 from ./include/linux/bug.h:5,
                 from ./arch/arm64/include/asm/ptrace.h:93,
                 from ./arch/arm64/include/asm/fpsimd.h:20,
                 from ./arch/arm64/include/asm/neon.h:15,
                 from /home/fedora/zfs/include/linux/simd_aarch64.h:43,
                 from /home/fedora/zfs/module/zfs/vdev_raidz_math_aarch64_neon_common.h:26,
                 from /home/fedora/zfs/module/zfs/vdev_raidz_math_aarch64_neon.c:30:
./arch/arm64/include/asm/asm-bug.h:52: note: this is the location of the previous definition
 #define ASM_BUG() ASM_BUG_FLAGS(0)
 
cc1: all warnings being treated as errors

Description

The 5.0 kernel defines the macro ASM_BUG. In order to prevent a
conflict and build failure rename ASM_BUG to ZFS_ASM_BUG. This
is currently only an issue on aarch64 but all instances of
ASM_BUG we're renamed to avoid any future conflict on x86_64.

How Has This Been Tested?

Compiled on Fedora 29 using the 5.0.10-200.fc29.aarch64 kernel. A subset of the ZTS was then run successfully.

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)
  • Documentation (a change to man pages or other documentation)

Checklist:

The 5.0 kernel defines the macro ASM_BUG.  In order to prevent a
conflict and build failure rename ASM_BUG to ZFS_ASM_BUG.  This
is currently only an issue on aarch64 but all instances of
ASM_BUG we're renamed to avoid any future conflict on x86_64.

Signed-off-by: Brian Behlendorf <[email protected]>
Issue openzfs#8545
@chrisrd
Copy link
Contributor

chrisrd commented May 8, 2019

LGTM

@codecov
Copy link

codecov bot commented May 8, 2019

Codecov Report

Merging #8725 into master will decrease coverage by 0.05%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #8725      +/-   ##
==========================================
- Coverage   78.82%   78.77%   -0.06%     
==========================================
  Files         381      381              
  Lines      117630   117630              
==========================================
- Hits        92724    92660      -64     
- Misses      24906    24970      +64
Flag Coverage Δ
#kernel 79.34% <ø> (ø) ⬆️
#user 67.37% <ø> (-0.26%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9c53e51...7f7f62e. Read the comment docs.

Copy link
Member

@rlaager rlaager left a comment

Choose a reason for hiding this comment

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

This change LGTM, and I see the ASM_BUG definition does exist in upstream Linux git.

I do have one question: I'm not a kernel developer, so take this with a big grain of salt, but is this macro doing anything useful (for readability/understandability) over just ASSERT(0)?

@behlendorf
Copy link
Contributor Author

but is this macro doing anything useful (for readability/understandability) over just ASSERT(0)?

In fact, in the original version of this patch I did just replace ASM_BUG with ASSERT(0). But after further consideration I decided there was value in keeping the macro, since it makes it easy to map ZFS_ASM_BUG to the kernels ASM_BUG which may be useful for different architectures.

@behlendorf behlendorf merged commit a20f43b into openzfs:master May 8, 2019
@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels May 8, 2019
allanjude pushed a commit to allanjude/zfs that referenced this pull request Jun 7, 2019
The 5.0 kernel defines the macro ASM_BUG.  In order to prevent a
conflict and build failure rename ASM_BUG to ZFS_ASM_BUG.  This
is currently only an issue on aarch64 but all instances of
ASM_BUG we're renamed to avoid any future conflict on x86_64.

Reviewed-by: Tomohiro Kusumi <[email protected]>
Reviewed-by: Richard Laager <[email protected]>
Reviewed-by: Chris Dunlop <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes openzfs#8725 
Issue openzfs#8545
allanjude pushed a commit to allanjude/zfs that referenced this pull request Jun 15, 2019
The 5.0 kernel defines the macro ASM_BUG.  In order to prevent a
conflict and build failure rename ASM_BUG to ZFS_ASM_BUG.  This
is currently only an issue on aarch64 but all instances of
ASM_BUG we're renamed to avoid any future conflict on x86_64.

Reviewed-by: Tomohiro Kusumi <[email protected]>
Reviewed-by: Richard Laager <[email protected]>
Reviewed-by: Chris Dunlop <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes openzfs#8725 
Issue openzfs#8545
@behlendorf behlendorf deleted the linux-5.0-aarch64 branch April 19, 2021 19:26
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) Type: Building Indicates an issue related to building binaries
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants