dynup / kpatch

kpatch - live kernel patching
GNU General Public License v2.0
1.49k stars 305 forks source link

Issue with .rela.text.unlikely.* ? #1160

Closed liu-song-6 closed 3 years ago

liu-song-6 commented 3 years ago

Hi folks,

I am testing kpatch on a very simple patch:

---
 drivers/md/md.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/md/md.h b/drivers/md/md.h
index bcbba1b5ec4a7..eda6f344bc91b 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -551,7 +551,7 @@ extern void mddev_unlock(struct mddev *mddev);

 static inline void md_sync_acct(struct block_device *bdev, unsigned long nr_sectors)
 {
-       atomic_add(nr_sectors, &bdev->bd_disk->sync_io);
+       atomic_add(nr_sectors + 1, &bdev->bd_disk->sync_io);
 }

 static inline void md_sync_acct_bio(struct bio *bio, unsigned long nr_sectors)

which hit errors like:

raid1.o: changed section .rela.text.unlikely.sync_request_write not selected for inclusion
raid10.o: changed section .rela.text.unlikely.raid10d not selected for inclusion

Is this expected or a bug? I am using gcc 9.1.1, and I have CONFIG_MD_RAID1=m

Thanks in advance.

sm00th commented 3 years ago

Probably a bug. I failed to reproduce it however. Can you invoke kpatch-build with -d option and then upload ${HOME}/.kpatch/tmp/orig/drivers/md/raid1.o and ${HOME}/.kpatch/tmp/patched/drivers/md/raid1.o somewhere?

jpoimboe commented 3 years ago

Does this fix it? https://github.com/jpoimboe/kpatch/commit/066c4baeafe048d4db53d62007ee573a1a8089ac

liu-song-6 commented 3 years ago

Does this fix it? jpoimboe@066c4ba

It doesn't seem to work.

liu-song-6 commented 3 years ago

Probably a bug. I failed to reproduce it however. Can you invoke kpatch-build with -d option and then upload ${HOME}/.kpatch/tmp/orig/drivers/md/raid1.o and ${HOME}/.kpatch/tmp/patched/drivers/md/raid1.o somewhere?

I added these files and build.log to https://github.com/liu-song-6/kpatch/tree/debug_1160 .

Thanks, Song

liu-song-6 commented 3 years ago

Probably a bug. I failed to reproduce it however. Can you invoke kpatch-build with -d option and then upload ${HOME}/.kpatch/tmp/orig/drivers/md/raid1.o and ${HOME}/.kpatch/tmp/patched/drivers/md/raid1.o somewhere?

I added these files and build.log to https://github.com/liu-song-6/kpatch/tree/debug_1160 .

@sm00th does this help reproducing the issue? Thanks!

sm00th commented 3 years ago

@sm00th does this help reproducing the issue? Thanks!

Yes, I didn't have much time to look at it today, but I can reprdoruce it with these files.

jpoimboe commented 3 years ago

I wasn't able to recreate, but it may be a bug in the ".cold" subfunction handling, i.e. kpatch_detect_child_functions().

sm00th commented 3 years ago

I wasn't able to recreate, but it may be a bug in the ".cold" subfunction handling, i.e. kpatch_detect_child_functions().

Right, we expect '.cold' functions to have an id at the end and the ones in provided objectfiles don't have that. So it could be as easy as this https://github.com/sm00th/kpatch/commit/c1f4f77a77e54b6159fbf888c2c5ce64cb160be0? Or are there any pitfalls here?

jpoimboe commented 3 years ago

Right, we expect '.cold' functions to have an id at the end and the ones in provided objectfiles don't have that. So it could be as easy as this sm00th@c1f4f77? Or are there any pitfalls here?

Oh, good catch. That should be the fix. I made the same fix to objtool (and forgot to fix it for here, oops).

liu-song-6 commented 3 years ago

I wasn't able to recreate, but it may be a bug in the ".cold" subfunction handling, i.e. kpatch_detect_child_functions().

Right, we expect '.cold' functions to have an id at the end and the ones in provided objectfiles don't have that. So it could be as easy as this sm00th@c1f4f77? Or are there any pitfalls here?

Yes, this fixes kpatch-build for my test. Thanks a lot!