In x86-64, I am running into a scenario with certain kernel modules
where the patching of lock prefix instructions in sections other than
.text (e.g. .exit.text) is not occurring even though the .smp_locks
relocations in the .ko file specify instructions in sections other
than .text for patching. For example (down in the bottom), in
net/bluetooth/rfcomm.ko, we have a couple of entries in .exit.text for
patching. However, when I look in the memory page containing the
.exit.text section in a uni processor system, it doesn't appear the
lock prefix instructions in .exit.text were patched with NOPs the same
way other instructions from the .text section in the same page
underwent patching. This creates a subtle inconsistency where a page
contains the end of the .text section along with the .exit.text
section - lock prefixes in instructions from the former section gets
patched per .smp_locks entries while that doesn't happen for the
latter within the same page.
Looking at the code, in module_finalize for x86, only .text seems to
be getting picked for the patching of lock prefixes while other
sections such as .exit.text or .init.text are not. Is there a reason
we skip the other *.text code sections from the lock patches? Would
making the application of the patching of lock prefixes generic across
all code sections (rather than just .text) make sense?
Thanks,
Deep
for (s = sechdrs; s < sechdrs + hdr->e_shnum; s++) {
if (!strcmp(".text", secstrings + s->sh_name))
text = s;
...
if (!strcmp(".smp_locks", secstrings + s->sh_name))
locks = s;
...
}
if (locks && text) {
void *lseg = (void *)locks->sh_addr;
void *tseg = (void *)text->sh_addr;
alternatives_smp_module_add(me, me->name,
lseg, lseg + locks->sh_size,
tseg, tseg + text->sh_size);
}
> objdump -r kernel/net/bluetooth/rfcomm/rfcomm.ko
RELOCATION RECORDS FOR [.smp_locks]:
OFFSET TYPE VALUE
0000000000000000 R_X86_64_64 .text+0x00000000000000ac
0000000000000008 R_X86_64_64 .exit.text+0x0000000000000023
0000000000000010 R_X86_64_64 .exit.text+0x0000000000000034
0000000000000018 R_X86_64_64 .text+0x0000000000000619
0000000000000020 R_X86_64_64 .text+0x000000000000061d
On Tue, Jun 21, 2011 at 1:10 AM, Deep Debroy <[email protected]> wrote:
> In x86-64, I am running into a scenario with certain kernel modules
> where the patching of lock prefix instructions in sections other than
> .text (e.g. .exit.text) is not occurring even though the .smp_locks
> relocations in the .ko file specify instructions in sections other
> than .text for patching. For example (down in the bottom), in
> net/bluetooth/rfcomm.ko, we have a couple of entries in .exit.text for
> patching. However, when I look in the memory page containing the
> .exit.text section in a uni processor system, it doesn't appear the
> lock prefix instructions in .exit.text were patched with NOPs the same
> way other instructions from the .text section in the same page
> underwent patching. This creates a subtle inconsistency where a page
> contains the end of the .text section along with the .exit.text
> section - lock prefixes in instructions from the former section gets
> patched per .smp_locks entries while that doesn't happen for the
> latter within the same page.
>
> Looking at the code, in module_finalize for x86, only .text seems to
> be getting picked for the patching of lock prefixes while other
> sections such as .exit.text or .init.text are not. Is there a reason
> we skip the other *.text code sections from the lock patches? Would
> making the application of the patching of lock prefixes generic across
> all code sections (rather than just .text) make sense?
>
> Thanks,
> Deep
>
> ? ? ? ? ? ?for (s = sechdrs; s < sechdrs + hdr->e_shnum; s++) {
> ? ? ? ? ? ? ? ? ? ?if (!strcmp(".text", secstrings + s->sh_name))
> ? ? ? ? ? ? ? ? ? ? ? ? ? ?text = s;
> ? ? ? ? ? ? ? ? ? ?...
> ? ? ? ? ? ? ? ? ? ?if (!strcmp(".smp_locks", secstrings + s->sh_name))
> ? ? ? ? ? ? ? ? ? ? ? ? ? ?locks = s;
> ? ? ? ? ? ? ? ? ? ?...
> ? ? ? ? ? ?}
>
> ? ? ? ? ? ?if (locks && text) {
> ? ? ? ? ? ? ? ? ? ?void *lseg = (void *)locks->sh_addr;
> ? ? ? ? ? ? ? ? ? ?void *tseg = (void *)text->sh_addr;
> ? ? ? ? ? ? ? ? ? ?alternatives_smp_module_add(me, me->name,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?lseg, lseg + locks->sh_size,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?tseg, tseg + text->sh_size);
> ? ? ? ? ? ?}
>
>
>
>> objdump -r kernel/net/bluetooth/rfcomm/rfcomm.ko
>
> RELOCATION RECORDS FOR [.smp_locks]:
> OFFSET ? ? ? ? ? TYPE ? ? ? ? ? ? ?VALUE
> 0000000000000000 R_X86_64_64 ? ? ? .text+0x00000000000000ac
> 0000000000000008 R_X86_64_64 ? ? ? .exit.text+0x0000000000000023
> 0000000000000010 R_X86_64_64 ? ? ? .exit.text+0x0000000000000034
> 0000000000000018 R_X86_64_64 ? ? ? .text+0x0000000000000619
> 0000000000000020 R_X86_64_64 ? ? ? .text+0x000000000000061d
>
+ Gerd Hoffmann who introduced the SMP patching code below back in Jan
2006 as part of 2.6.15.
Any comments on why patching of smp_lock prefixes should be restricted
to .text and not other *.text code sections?
Thanks,
Deep
> > Looking at the code, in module_finalize for x86, only .text seems to
> > be getting picked for the patching of lock prefixes while other
> > sections such as .exit.text or .init.text are not. Is there a reason
> > we skip the other *.text code sections from the lock patches? Would
> + Gerd Hoffmann who introduced the SMP patching code below back in Jan
> 2006 as part of 2.6.15.
Whoa, long time ago.
>
> Any comments on why patching of smp_lock prefixes should be restricted
> to .text and not other *.text code sections?
It could be that at that time the .exit.text or .init.text did not exist.
As in, the patching code just hasn't kept up. One way of checking that
is just finding the ancient 2.6.15 code and seeing if there is any
mention of those extra segments.
Do you have a patch to fix this?
On Wed, Jun 22, 2011 at 6:21 AM, Konrad Rzeszutek Wilk
<[email protected]> wrote:
>> > Looking at the code, in module_finalize for x86, only .text seems to
>> > be getting picked for the patching of lock prefixes while other
>> > sections such as .exit.text or .init.text are not. Is there a reason
>> > we skip the other *.text code sections from the lock patches? Would
>> + Gerd Hoffmann who introduced the SMP patching code below back in Jan
>> 2006 as part of 2.6.15.
>
> Whoa, long time ago.
>
>>
>> Any comments on why patching of smp_lock prefixes should be restricted
>> to .text and not other *.text code sections?
>
> It could be that at that time the .exit.text or .init.text did not exist.
>
> As in, the patching code just hasn't kept up. One way of checking that
> is just finding the ancient 2.6.15 code and seeing if there is any
> mention of those extra segments.
>
Thanks Konrad. One slight correction: after rechecking the kernel
sources, it appears the smp lock prefix code first made it's
appearance in the official trees during 2.6.18. In any case, going
back even to 2.6.16 sources, layout_sections in module.c specially
handled .init prefixed sections from the rest i.e. core sections.
Further, the module struct in include/module/linux.h seems to have had
members such as init_text_size which suggests atleast .init.text did
exit back then as well. While I didn't find any crumbs in the code
that point to the existence of a .exit.text (besides a function
pointer called exit which most likely ended up in the .exit.text), the
ELF headers for Centos 5.6 kernel objects (which uses the 2.6.18
kernel) typically have a .exit.text.
> Do you have a patch to fix this?
>
I can work on that. Just wanted to first make sure that there wasn't
any specific reason to avoid patching non .text sections.
Thanks,
Deep
On Wed, Jun 22, 2011 at 10:27 AM, Deep Debroy <[email protected]> wrote:
> On Wed, Jun 22, 2011 at 6:21 AM, Konrad Rzeszutek Wilk
> <[email protected]> wrote:
>>> > Looking at the code, in module_finalize for x86, only .text seems to
>>> > be getting picked for the patching of lock prefixes while other
>>> > sections such as .exit.text or .init.text are not. Is there a reason
>>> > we skip the other *.text code sections from the lock patches? Would
>>> + Gerd Hoffmann who introduced the SMP patching code below back in Jan
>>> 2006 as part of 2.6.15.
>>
>> Whoa, long time ago.
>>
>>>
>>> Any comments on why patching of smp_lock prefixes should be restricted
>>> to .text and not other *.text code sections?
>>
>> It could be that at that time the .exit.text or .init.text did not exist.
>>
>> As in, the patching code just hasn't kept up. One way of checking that
>> is just finding the ancient 2.6.15 code and seeing if there is any
>> mention of those extra segments.
>>
>
> Thanks Konrad. One slight correction: after rechecking the kernel
> sources, it appears the smp lock prefix code first made it's
> appearance in the official trees during 2.6.18. In any case, going
> back even to 2.6.16 sources, layout_sections in module.c specially
> handled .init prefixed sections from the rest i.e. core sections.
> Further, the module struct in include/module/linux.h seems to have had
> members such as init_text_size which suggests atleast .init.text did
> exit back then as well. While I didn't find any crumbs in the code
> that point to the existence of a .exit.text (besides a function
> pointer called exit which most likely ended up in the .exit.text), the
> ELF headers for Centos 5.6 kernel objects (which uses the 2.6.18
> kernel) typically have a .exit.text.
>
>> Do you have a patch to fix this?
>>
>
> I can work on that. Just wanted to first make sure that there wasn't
> any specific reason to avoid patching non .text sections.
>
> Thanks,
> Deep
>
Some further digging through messages revealed a patch from Randy
Dunlap in June 2006: "[PATCH] ignore smp_locks section warnings from
init/exit code." Given this patch came in after the smp locking
hotpatching mechanism was introduced, there may have been an
assumption that instructions that results in entries in smp_locks
relocations in the object file should not exist in the init/exit.text
sections.
Thanks,
Deep
On Wed, 22 Jun 2011 14:58:12 -0700 Deep Debroy wrote:
> On Wed, Jun 22, 2011 at 10:27 AM, Deep Debroy <[email protected]> wrote:
> > On Wed, Jun 22, 2011 at 6:21 AM, Konrad Rzeszutek Wilk
> > <[email protected]> wrote:
> >>> > Looking at the code, in module_finalize for x86, only .text seems to
> >>> > be getting picked for the patching of lock prefixes while other
> >>> > sections such as .exit.text or .init.text are not. Is there a reason
> >>> > we skip the other *.text code sections from the lock patches? Would
> >>> + Gerd Hoffmann who introduced the SMP patching code below back in Jan
> >>> 2006 as part of 2.6.15.
> >>
> >> Whoa, long time ago.
> >>
> >>>
> >>> Any comments on why patching of smp_lock prefixes should be restricted
> >>> to .text and not other *.text code sections?
> >>
> >> It could be that at that time the .exit.text or .init.text did not exist.
> >>
> >> As in, the patching code just hasn't kept up. One way of checking that
> >> is just finding the ancient 2.6.15 code and seeing if there is any
> >> mention of those extra segments.
> >>
> >
> > Thanks Konrad. One slight correction: after rechecking the kernel
> > sources, it appears the smp lock prefix code first made it's
> > appearance in the official trees during 2.6.18. In any case, going
> > back even to 2.6.16 sources, layout_sections in module.c specially
> > handled .init prefixed sections from the rest i.e. core sections.
> > Further, the module struct in include/module/linux.h seems to have had
> > members such as init_text_size which suggests atleast .init.text did
> > exit back then as well. While I didn't find any crumbs in the code
> > that point to the existence of a .exit.text (besides a function
> > pointer called exit which most likely ended up in the .exit.text), the
> > ELF headers for Centos 5.6 kernel objects (which uses the 2.6.18
> > kernel) typically have a .exit.text.
> >
> >> Do you have a patch to fix this?
> >>
> >
> > I can work on that. Just wanted to first make sure that there wasn't
> > any specific reason to avoid patching non .text sections.
> >
> > Thanks,
> > Deep
> >
>
> Some further digging through messages revealed a patch from Randy
> Dunlap in June 2006: "[PATCH] ignore smp_locks section warnings from
> init/exit code." Given this patch came in after the smp locking
> hotpatching mechanism was introduced, there may have been an
> assumption that instructions that results in entries in smp_locks
> relocations in the object file should not exist in the init/exit.text
> sections.
I don't quite see how this patch (below) would affect your problem
description...
commit 35899c57516be6eaa42cc27151767c52d75b2979
Author: Randy Dunlap <[email protected]>
Date: Wed Jun 7 16:23:26 2006 -0700
kbuild: ignore smp_locks section warnings from init/exit code
Add ".smp_locks" section to whitelist as being safe from
init and exit sections.
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index d0f86ed..94047bc 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -821,6 +821,7 @@ static int init_section_ref_ok(const char *name)
".pci_fixup_final",
".pdr",
"__param",
+ ".smp_locks",
NULL
};
/* Start of section names */
@@ -892,6 +893,7 @@ static int exit_section_ref_ok(const char *name)
".exitcall.exit",
".eh_frame",
".stab",
+ ".smp_locks",
NULL
};
/* Start of section names */
---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
On Wed, Jun 22, 2011 at 4:45 PM, Randy Dunlap <[email protected]> wrote:
> On Wed, 22 Jun 2011 14:58:12 -0700 Deep Debroy wrote:
>
>> On Wed, Jun 22, 2011 at 10:27 AM, Deep Debroy <[email protected]> wrote:
>> > On Wed, Jun 22, 2011 at 6:21 AM, Konrad Rzeszutek Wilk
>> > <[email protected]> wrote:
>> >>> > Looking at the code, in module_finalize for x86, only .text seems to
>> >>> > be getting picked for the patching of lock prefixes while other
>> >>> > sections such as .exit.text or .init.text are not. Is there a reason
>> >>> > we skip the other *.text code sections from the lock patches? Would
>> >>> + Gerd Hoffmann who introduced the SMP patching code below back in Jan
>> >>> 2006 as part of 2.6.15.
>> >>
>> >> Whoa, long time ago.
>> >>
>> >>>
>> >>> Any comments on why patching of smp_lock prefixes should be restricted
>> >>> to .text and not other *.text code sections?
>> >>
>> >> It could be that at that time the .exit.text or .init.text did not exist.
>> >>
>> >> As in, the patching code just hasn't kept up. One way of checking that
>> >> is just finding the ancient 2.6.15 code and seeing if there is any
>> >> mention of those extra segments.
>> >>
>> >
>> > Thanks Konrad. One slight correction: after rechecking the kernel
>> > sources, it appears the smp lock prefix code first made it's
>> > appearance in the official trees during 2.6.18. In any case, going
>> > back even to 2.6.16 sources, layout_sections in module.c specially
>> > handled .init prefixed sections from the rest i.e. core sections.
>> > Further, the module struct in include/module/linux.h seems to have had
>> > members such as init_text_size which suggests atleast .init.text did
>> > exit back then as well. While I didn't find any crumbs in the code
>> > that point to the existence of a .exit.text (besides a function
>> > pointer called exit which most likely ended up in the .exit.text), the
>> > ELF headers for Centos 5.6 kernel objects (which uses the 2.6.18
>> > kernel) typically have a .exit.text.
>> >
>> >> Do you have a patch to fix this?
>> >>
>> >
>> > I can work on that. Just wanted to first make sure that there wasn't
>> > any specific reason to avoid patching non .text sections.
>> >
>> > Thanks,
>> > Deep
>> >
>>
>> Some further digging through messages revealed a patch from Randy
>> Dunlap in June 2006: "[PATCH] ignore smp_locks section warnings from
>> init/exit code." Given this patch came in after the smp locking
>> hotpatching mechanism was introduced, there may have been an
>> assumption that instructions that results in entries in smp_locks
>> relocations in the object file should not exist in the init/exit.text
>> sections.
>
> I don't quite see how this patch (below) would affect your problem
> description...
>
I didn't fully understand what the patch addressed. Initially I
thought if a ko had lock prefixed instructions in .init/.exit, it
would lead to entries in .smp_locks section but modpost would report
warnings in that situation. Since there would be these warnings, I
thought a module author would avoid lock prefixed instructions in
.init/.exit prior to the below patch. Sounds like that is not the
case?
>
> commit 35899c57516be6eaa42cc27151767c52d75b2979
> Author: Randy Dunlap <[email protected]>
> Date: ? Wed Jun 7 16:23:26 2006 -0700
>
> ? ?kbuild: ignore smp_locks section warnings from init/exit code
>
> ? ?Add ".smp_locks" section to whitelist as being safe from
> ? ?init and exit sections.
>
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index d0f86ed..94047bc 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -821,6 +821,7 @@ static int init_section_ref_ok(const char *name)
> ? ? ? ? ? ? ? ?".pci_fixup_final",
> ? ? ? ? ? ? ? ?".pdr",
> ? ? ? ? ? ? ? ?"__param",
> + ? ? ? ? ? ? ? ".smp_locks",
> ? ? ? ? ? ? ? ?NULL
> ? ? ? ?};
> ? ? ? ?/* Start of section names */
> @@ -892,6 +893,7 @@ static int exit_section_ref_ok(const char *name)
> ? ? ? ? ? ? ? ?".exitcall.exit",
> ? ? ? ? ? ? ? ?".eh_frame",
> ? ? ? ? ? ? ? ?".stab",
> + ? ? ? ? ? ? ? ".smp_locks",
> ? ? ? ? ? ? ? ?NULL
> ? ? ? ?};
> ? ? ? ?/* Start of section names */
>
> ---
> ~Randy
> *** Remember to use Documentation/SubmitChecklist when testing your code ***
>