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.17 API updates #12989

Closed
wants to merge 3 commits into from
Closed

Conversation

nabijaczleweli
Copy link
Contributor

@nabijaczleweli nabijaczleweli commented Jan 19, 2022

Motivation and Context

The buildd was failing.

Description

See individual commits.

How Has This Been Tested?

Pre-5.16 local builds, 5.17 on CI.

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:

  • My code follows the OpenZFS code style requirements.
  • I have updated the documentation accordingly.
  • I have read the contributing document.
  • I have added tests to cover my changes.
  • I have run the ZFS Test Suite with this change applied. – CI take my hand
  • All commit messages are properly formatted and contain Signed-off-by.

@nabijaczleweli
Copy link
Contributor Author

nabijaczleweli commented Jan 19, 2022

The buildd now also notes this:

fs/zfs/zfs/../os/linux/zfs/zvol_os.c: In function ‘zvol_os_create_minor’:
fs/zfs/zfs/../os/linux/zfs/zvol_os.c:1113:3: warning: ignoring return value of ‘add_disk’, declared with attribute warn_unused_result [-Wunused-result]
   add_disk(zv->zv_zso->zvo_disk);
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

And, indeed, there are no provisions for device_add_disk() failing, but I personally don't know what they'd, well, be, so.

@gamanakis
Copy link
Contributor

gamanakis commented Jan 19, 2022

Have you seen this #12975 ?
Edit: I think you address different issues here. The add_disk() issue is handled in the above PR.

@nabijaczleweli
Copy link
Contributor Author

nabijaczleweli commented Jan 19, 2022

I haven't, but neither of these is touched by that PR (add_disk() is, though).

Both of these commits landed in this merge, and the PR is from the 14th:

commit 35ce8ae9ae2e471f92759f9d6880eab42cc1c3b6
Merge: 6661224e66f0 a403df29789b
Author: Linus Torvalds <torvalds@linux-foundation.org>
Date:   Mon Jan 17 05:49:30 2022 +0200

    Merge branch 'signal-for-v5.17' of git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/user-namespace

@rincebrain
Copy link
Contributor

I would suspect you mean s/5.16/5.17/, since 5.16 builds without these for me, and the merge commit you cite is labeled "for 5.17"?

@nabijaczleweli
Copy link
Contributor Author

Hm, the describes say v5.16-rc1-2-g5768d8906bc2 and v5.16-rc1-15-gcead18552660.

@rincebrain
Copy link
Contributor

rincebrain commented Jan 20, 2022 via email

@nabijaczleweli
Copy link
Contributor Author

nabijaczleweli commented Jan 20, 2022

And the merge is v5.16-10543-g35ce8ae9ae2e, so fair cop. Relabeled.

@nabijaczleweli nabijaczleweli changed the title Linux 5.16 API updates Linux 5.17 API updates Jan 20, 2022
Copy link
Contributor

@behlendorf behlendorf left a comment

Choose a reason for hiding this comment

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

Looks good. Nice and straight forward, thanks.

@behlendorf behlendorf added the Status: Accepted Ready to integrate (reviewed, tested) label Jan 21, 2022
dnl # 5.17 API,
dnl # cead18552660702a4a46f58e65188fe5f36e9dfe ("exit: Rename complete_and_exit to kthread_complete_and_exit")
dnl #
dnl # Also moves the definition from i/l/kernel.h to i/l/kthread.h
Copy link
Contributor

Choose a reason for hiding this comment

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

- dnl # Also moves the definition from i/l/kernel.h to i/l/kthread.h 
+ dnl # Also moves the definition from include/linux/kernel.h to include/linux/kthread.h 

@tonyhutter
Copy link
Contributor

tonyhutter commented Jan 22, 2022

We typically use "Linux x.y compat:" commit labeling for tracking kernel changes. Would you mind changing your commit messages to read:

Linux 5.17 compat: dequeue_signal() added a 3rd argument

dequeue_signal() now takes a 3rd argument for pid_type.

Upstream commit cead18552660702a4a46f58e65188fe5f36e9dfe
("exit: Rename complete_and_exit to kthread_complete_and_exit")

Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
Linux 5.17 compat: detect complete_and_exit() rename

The 5.17 kernel renames complete_and_exit() to
kthread_complete_and_exit().

Upstream commit cead18552660702a4a46f58e65188fe5f36e9dfe
("exit: Rename complete_and_exit to kthread_complete_and_exit")

Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>

@nabijaczleweli
Copy link
Contributor Author

Rebased, both applied, messages reworded

@behlendorf
Copy link
Contributor

@nabijaczleweli this needs a rebase to resolve the minor conflict after merging the 5.16 changes. Then it should be good to go.

@nabijaczleweli
Copy link
Contributor Author

Rebased

@nabijaczleweli
Copy link
Contributor Author

Done to both

#include <linux/sched/signal.h>
], [
struct task_struct *task = NULL;
signet_t *mask = NULL;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
signet_t *mask = NULL;
sigset_t *mask = NULL;

I made a typo here. I should have mentioned that I hadn't actually compiled this to verify it works. After fixing the typo it correctly detects the kernel interface for me. I also checked the kthread_complete_and_exit() test.

What I'd suggest is adding the other 5.17 fix to this PR when you update it. This should at least confirm via the CI that it compiles for the latest kernel.

@nabijaczleweli
Copy link
Contributor Author

Both done

@nabijaczleweli
Copy link
Contributor Author

Aaand the built-in buildd passes this day.

@behlendorf
Copy link
Contributor

Skimming the kernel build-in build log I see we have a couple more warnings which I haven't seen before with the normal builds.

fs/zfs/zfs/txg.o: warning: objtool: txg_thread_exit() falls through to next function txg_do_callbacks()

Seems like the __noreturn added to __thread_exit in this PR may have caused this, and

ld: warning: orphan section `.eh_frame' from `fs/zfs/icp/asm-x86_64/modes/aesni-gcm-x86_64.o' being placed in section `.eh_frame'.
ld: warning: orphan section `.eh_frame' from `fs/zfs/icp/asm-x86_64/modes/ghash-x86_64.o' being placed in section `.eh_frame'.
ld: warning: orphan section `.eh_frame' from `fs/zfs/icp/asm-x86_64/sha2/sha256_impl.o' being placed in section `.eh_frame'.
ld: warning: orphan section `.eh_frame' from `fs/zfs/icp/asm-x86_64/sha2/sha512_impl.o' being placed in section `.eh_frame'.

which are likely a consequence of the recent icp cleanup.

@ckane
Copy link
Contributor

ckane commented Jan 25, 2022

I have this code running on my laptop now, seems to be working

@AttilaFueloep
Copy link
Contributor

@behlendorf I'm to blame for the ld warnings and I've seen them way before @nabijaczleweli changes. They are due to the cfi instruction added to the assembler files and me failing to figure out how to add linker scripts to the build (or locate existing ones). I'd say they are harmless since the .eh_frame is the right place for them.

Linux 5.17 sees a rename from complete_and_exit()
to kthread complete_and_exit()

Upstream commit cead18552660702a4a46f58e65188fe5f36e9dfe
("exit: Rename complete_and_exit to kthread_complete_and_exit")

Ref: http://build.zfsonlinux.org/builders/Kernel.org%20Built-in%20x86_64%20%28BUILD%29/builds/43905/steps/shell_1/logs/make
Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
Linux 5.17's dequeue_signal() takes an additional enum pid_type *
output argument

Upstream commit 5768d8906bc23d512b1a736c1e198aa833a6daa4
("signal: Requeue signals in the appropriate queue")

Ref: http://build.zfsonlinux.org/builders/Kernel.org%20Built-in%20x86_64%20%28BUILD%29/builds/43905/steps/shell_1/logs/make
Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
Upstream commit 359745d78351c6f5442435f81549f0207ece28aa
("proc: remove PDE_DATA() completely")

Link: https://lore.kernel.org/all/20211124081956.87711-2-songmuchun@bytedance.com/T/#u
Closes: openzfs#13004
Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
@nabijaczleweli
Copy link
Contributor Author

Yeah, dunno why, especially seeing as both KTHREAD_COMPLETE_AND_EXITs are declared __noreturn. Removed the attributes.

@behlendorf
Copy link
Contributor

@AttilaFueloep got it, thanks. I just hadn't noticed them until now.

behlendorf pushed a commit that referenced this pull request Jan 25, 2022
Linux 5.17's dequeue_signal() takes an additional enum pid_type *
output argument

Upstream commit 5768d8906bc23d512b1a736c1e198aa833a6daa4
("signal: Requeue signals in the appropriate queue")

Reviewed-by: Tony Hutter <hutter2@llnl.gov>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
Closes #12989
behlendorf pushed a commit that referenced this pull request Jan 25, 2022
Upstream commit 359745d78351c6f5442435f81549f0207ece28aa
("proc: remove PDE_DATA() completely")

Link: https://lore.kernel.org/all/20211124081956.87711-2-songmuchun@bytedance.com/T/#u

Reviewed-by: Tony Hutter <hutter2@llnl.gov>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
Closes #13004
Closes #12989
tonyhutter pushed a commit that referenced this pull request Feb 4, 2022
Linux 5.17 sees a rename from complete_and_exit()
to kthread complete_and_exit()

Upstream commit cead18552660702a4a46f58e65188fe5f36e9dfe
("exit: Rename complete_and_exit to kthread_complete_and_exit")

Reviewed-by: Tony Hutter <hutter2@llnl.gov>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
Closes #12989
tonyhutter pushed a commit that referenced this pull request Feb 4, 2022
Linux 5.17's dequeue_signal() takes an additional enum pid_type *
output argument

Upstream commit 5768d8906bc23d512b1a736c1e198aa833a6daa4
("signal: Requeue signals in the appropriate queue")

Reviewed-by: Tony Hutter <hutter2@llnl.gov>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
Closes #12989
tonyhutter pushed a commit that referenced this pull request Feb 4, 2022
Upstream commit 359745d78351c6f5442435f81549f0207ece28aa
("proc: remove PDE_DATA() completely")

Link: https://lore.kernel.org/all/20211124081956.87711-2-songmuchun@bytedance.com/T/#u

Reviewed-by: Tony Hutter <hutter2@llnl.gov>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
Closes #13004
Closes #12989
nicman23 pushed a commit to nicman23/zfs that referenced this pull request Aug 22, 2022
Linux 5.17 sees a rename from complete_and_exit()
to kthread complete_and_exit()

Upstream commit cead18552660702a4a46f58e65188fe5f36e9dfe
("exit: Rename complete_and_exit to kthread_complete_and_exit")

Reviewed-by: Tony Hutter <hutter2@llnl.gov>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
Closes openzfs#12989
nicman23 pushed a commit to nicman23/zfs that referenced this pull request Aug 22, 2022
Linux 5.17's dequeue_signal() takes an additional enum pid_type *
output argument

Upstream commit 5768d8906bc23d512b1a736c1e198aa833a6daa4
("signal: Requeue signals in the appropriate queue")

Reviewed-by: Tony Hutter <hutter2@llnl.gov>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
Closes openzfs#12989
nicman23 pushed a commit to nicman23/zfs that referenced this pull request Aug 22, 2022
Upstream commit 359745d78351c6f5442435f81549f0207ece28aa
("proc: remove PDE_DATA() completely")

Link: https://lore.kernel.org/all/20211124081956.87711-2-songmuchun@bytedance.com/T/#u

Reviewed-by: Tony Hutter <hutter2@llnl.gov>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
Closes openzfs#13004
Closes openzfs#12989
nicman23 pushed a commit to nicman23/zfs that referenced this pull request Aug 22, 2022
Linux 5.17 sees a rename from complete_and_exit()
to kthread complete_and_exit()

Upstream commit cead18552660702a4a46f58e65188fe5f36e9dfe
("exit: Rename complete_and_exit to kthread_complete_and_exit")

Reviewed-by: Tony Hutter <hutter2@llnl.gov>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
Closes openzfs#12989
nicman23 pushed a commit to nicman23/zfs that referenced this pull request Aug 22, 2022
Linux 5.17's dequeue_signal() takes an additional enum pid_type *
output argument

Upstream commit 5768d8906bc23d512b1a736c1e198aa833a6daa4
("signal: Requeue signals in the appropriate queue")

Reviewed-by: Tony Hutter <hutter2@llnl.gov>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
Closes openzfs#12989
nicman23 pushed a commit to nicman23/zfs that referenced this pull request Aug 22, 2022
Upstream commit 359745d78351c6f5442435f81549f0207ece28aa
("proc: remove PDE_DATA() completely")

Link: https://lore.kernel.org/all/20211124081956.87711-2-songmuchun@bytedance.com/T/#u

Reviewed-by: Tony Hutter <hutter2@llnl.gov>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
Closes openzfs#13004
Closes openzfs#12989
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.

7 participants