Skip to content

Conversation

@puranjaymohan
Copy link
Contributor

@puranjaymohan puranjaymohan commented Apr 9, 2025

Old linkers don't support mixing ordered and unordered sections. This was fixed in binutils, see discussion in [1]. But without this fix, kpatch-build fails with an error like:

ld.bfd: __patchable_function_entries has both ordered [`__patchable_function_entries' in /builddir/.kpatch/tmp/patch/output.o] and unordered [`__patchable_function_entries' in /builddir/.kpatch/tmp/patch/patch-hook.o] sections
ld.bfd: final link failed: bad value

Fix this by only setting SHF_LINK_ORDER in the output object if the patched object also has this flag set.

[1] https://sourceware.org/bugzilla/show_bug.cgi?id=26256

Old linkers don't support mixing ordered and unordered sections. This
was fixed in binutils, see discussion in [1]. But without this fix,
kpatch-build fails with an error like:

| ld.bfd: __patchable_function_entries has both ordered
| [`__patchable_function_entries' in /builddir/.kpatch/tmp/patch/output.o]
| and unordered [`__patchable_function_entries' in
| /builddir/.kpatch/tmp/patch/patch-hook.o] sections
| ld.bfd: final link failed: bad value

Fix this by only setting SHF_LINK_ORDER in the output object if the
patched object also has this flag set.

Signed-off-by: Puranjay Mohan <pjy@amazon.com>
@joe-lawrence
Copy link
Contributor

@wardenjohn : IIRC, you hit this problem a while back? If so, do you want to try @puranjaymohan 's patch?

@wardenjohn
Copy link
Contributor

wardenjohn commented Apr 18, 2025

@joe-lawrence

Right, I had faced this problem. It seems that I mentioned it in a comment area instead of raising a new issues.

This problem seem only happen under arm64

@wardenjohn
Copy link
Contributor

@puranjaymohan Would you share the discussion link as mention in your pr as [1]?

@joe-lawrence
Copy link
Contributor

@wardenjohn : do you have a link for that discussion and did you ever resolve it (or apply something similar to the patch here)?

@wardenjohn
Copy link
Contributor

wardenjohn commented Apr 20, 2025

@joe-lawrence
Sorry, Joe. I don't have the link to that discussion. This is a discussion by mail. I judge this problem may happen only under aarch64, because from what I know, __patchable_function_entries is only effective in aarch64 and which can be ignore. and the upstream is not supported, that's why I did not raise an issue here...lol...

And yet, I still not have a good solution to it, and @puranjaymohan seems give me a hope to this problem. I am now starting to test this patch if can fix it :)

@wardenjohn
Copy link
Contributor

@wardenjohn : do you have a link for that discussion and did you ever resolve it (or apply something similar to the patch here)?

@joe-lawrence You can check the email I send you at Feb, 21, 2025.

@joe-lawrence
Copy link
Contributor

@wardenjohn : do you have a link for that discussion and did you ever resolve it (or apply something similar to the patch here)?

@joe-lawrence You can check the email I send you at Feb, 21, 2025.

Ah ok, yes that was the bug report I was thinking of. Since it is over e-mail, there's nothing to link to in this PR, but if you are willing to give it a test drive on your Arm64 port and report back, that would be much appreciated, thanks!

@wardenjohn
Copy link
Contributor

wardenjohn commented Apr 22, 2025

@joe-lawrence Sure, I will give you a test report as soon as possible.

Since the master branch do not support Arm64, I may test on @puranjaymohan 's arm64 branch of PR [https://github.com//pull/1439] with this patch.

It that OK?

@wardenjohn
Copy link
Contributor

@joe-lawrence Do you need me copy the bug report and raise an new issuse here?

@puranjaymohan
Copy link
Contributor Author

@puranjaymohan Would you share the discussion link as mention in your pr as [1]?

Here it is: https://sourceware.org/bugzilla/show_bug.cgi?id=26256

@joe-lawrence
Copy link
Contributor

@joe-lawrence Sure, I will give you a test report as soon as possible.

Since the master branch do not support Arm64, I may test on @puranjaymohan 's arm64 branch of PR [https://github.com/https://github.com//pull/1439] with this patch.

It that OK?

Sure that's fine. It's more of a sanity check for your own use (you'll know that the bug is fixed :)

@joe-lawrence Do you need me copy the bug report and raise an new issuse here?

No need to duplicate it, you can just follow up here. Thanks.

@wardenjohn
Copy link
Contributor

wardenjohn commented Apr 27, 2025

@joe-lawrence FYI,I tried a patch made changes fork.c and mmap.c, this patch successfully built in x86_64 and it seems looks well. However, I changed into this branch and tried to build it under aarch64, with the same patch, found the error:

create-diff-object: ERROR: fork.o: find_local_syms: 217: couldn't find matching fork.c local symbols in vmlinux symbol table
create-diff-object: ERROR: mmap.o: find_local_syms: 217: couldn't find matching mmap.c local symbols in vmlinux symbol table

cc @puranjaymohan

@puranjaymohan
Copy link
Contributor Author

puranjaymohan commented Apr 28, 2025

@wardenjohn Can you share the patch that you tested? and did you use this PR on top of the PR for the arm64 support?

@wardenjohn
Copy link
Contributor

@puranjaymohan quite strange.... I don't know what happened to my env. After I rebuild and reinstall your branch (I used your arm64 branch and patch the commit from pfe_ordering_fix). And I retried a patch just chaning meminfo_proc_show and seems good.

From 12221c683d399db241b5ce029e638cd24ab76788 Mon Sep 17 00:00:00 2001
From: Wardenjohn <zhangwarden@gmail.com>
Date: Tue, 17 Dec 2024 11:55:54 +0800
Subject: [PATCH] 1

---
 fs/proc/meminfo.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/proc/meminfo.c b/fs/proc/meminfo.c
index 8ba9b1472390..2dbf507fa57d 100644
--- a/fs/proc/meminfo.c
+++ b/fs/proc/meminfo.c
@@ -57,7 +57,7 @@ static int meminfo_proc_show(struct seq_file *m, void *v)
        sreclaimable = global_node_page_state_pages(NR_SLAB_RECLAIMABLE_B);
        sunreclaim = global_node_page_state_pages(NR_SLAB_UNRECLAIMABLE_B);

-       show_val_kb(m, "MemTotal:       ", i.totalram);
+       show_val_kb(m, "111adghfjad2MemTotal:       ", i.totalram);
        show_val_kb(m, "MemFree:        ", i.freeram);
        show_val_kb(m, "MemAvailable:   ", available);
        show_val_kb(m, "Buffers:        ", i.bufferram);
-- 
2.43.5

And I am sorry for answering late, for I am quite busy that days. :)

Since this patch is quite easy, I will try a more complex patch later. :)

@joe-lawrence
Copy link
Contributor

I'll merge this now and if @wardenjohn has any further issues, we'll just open a new ticket. Thanks guys.

@joe-lawrence joe-lawrence merged commit e0c09ea into dynup:master May 14, 2025
3 checks passed
@puranjaymohan puranjaymohan deleted the pfe_ordering_fix branch July 29, 2025 12:27
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.

3 participants