[linux-next RFC v2] mm/gup.c: Convert to use get_user_{page|pages}_fast_only()
API __get_user_pages_fast() renamed to get_user_pages_fast_only()
to align with pin_user_pages_fast_only().
As part of this we will get rid of write parameter. Instead caller
will pass FOLL_WRITE to get_user_pages_fast_only(). This will not
change any existing functionality of the API.
All the callers are changed to pass FOLL_WRITE.
There are few places where 1 is passed to 2nd parameter of
__get_user_pages_fast() and return value is checked for 1
like [1]. Those are replaced with new inline
get_user_page_fast_only().
[1] if (__get_user_pages_fast(hva, 1, 1, ) == 1)
Updated the documentation of the API.
Signed-off-by: Souptick Joarder
Cc: John Hubbard
Cc: Matthew Wilcox
---
v2:
Updated the subject line and change log.
Address Matthew's comment to fix a bug and added
new inline get_user_page_fast_only().
arch/powerpc/kvm/book3s_64_mmu_hv.c| 2 +-
arch/powerpc/kvm/book3s_64_mmu_radix.c | 2 +-
arch/powerpc/perf/callchain_64.c | 4 +---
include/linux/mm.h | 10 --
kernel/events/core.c | 4 ++--
mm/gup.c | 29 -
virt/kvm/kvm_main.c| 8 +++-
7 files changed, 32 insertions(+), 27 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c
b/arch/powerpc/kvm/book3s_64_mmu_hv.c
index 18aed97..ddfc4c9 100644
--- a/arch/powerpc/kvm/book3s_64_mmu_hv.c
+++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c
@@ -581,7 +581,7 @@ int kvmppc_book3s_hv_page_fault(struct kvm_run *run, struct
kvm_vcpu *vcpu,
* We always ask for write permission since the common case
* is that the page is writable.
*/
- if (__get_user_pages_fast(hva, 1, 1, ) == 1) {
+ if (get_user_page_fast_only(hva, FOLL_WRITE, )) {
write_ok = true;
} else {
/* Call KVM generic code to do the slow-path check */
diff --git a/arch/powerpc/kvm/book3s_64_mmu_radix.c
b/arch/powerpc/kvm/book3s_64_mmu_radix.c
index 3248f78..5d4c087 100644
--- a/arch/powerpc/kvm/book3s_64_mmu_radix.c
+++ b/arch/powerpc/kvm/book3s_64_mmu_radix.c
@@ -795,7 +795,7 @@ int kvmppc_book3s_instantiate_page(struct kvm_vcpu *vcpu,
* is that the page is writable.
*/
hva = gfn_to_hva_memslot(memslot, gfn);
- if (!kvm_ro && __get_user_pages_fast(hva, 1, 1, ) == 1) {
+ if (!kvm_ro && get_user_page_fast_only(hva, FOLL_WRITE, )) {
upgrade_write = true;
} else {
unsigned long pfn;
diff --git a/arch/powerpc/perf/callchain_64.c b/arch/powerpc/perf/callchain_64.c
index 1bff896d..814d1c2 100644
--- a/arch/powerpc/perf/callchain_64.c
+++ b/arch/powerpc/perf/callchain_64.c
@@ -29,11 +29,9 @@ int read_user_stack_slow(void __user *ptr, void *buf, int nb)
unsigned long addr = (unsigned long) ptr;
unsigned long offset;
struct page *page;
- int nrpages;
void *kaddr;
- nrpages = __get_user_pages_fast(addr, 1, 1, );
- if (nrpages == 1) {
+ if (get_user_page_fast_only(addr, FOLL_WRITE, )) {
kaddr = page_address(page);
/* align address to page boundary */
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 93d93bd..8d4597f 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1817,10 +1817,16 @@ extern int mprotect_fixup(struct vm_area_struct *vma,
/*
* doesn't attempt to fault and will return short.
*/
-int __get_user_pages_fast(unsigned long start, int nr_pages, int write,
- struct page **pages);
+int get_user_pages_fast_only(unsigned long start, int nr_pages,
+ unsigned int gup_flags, struct page **pages);
int pin_user_pages_fast_only(unsigned long start, int nr_pages,
unsigned int gup_flags, struct page **pages);
+
+static inline bool get_user_page_fast_only(unsigned long addr,
+ unsigned int gup_flags, struct page **pagep)
+{
+ return get_user_pages_fast_only(addr, 1, gup_flags, pagep) == 1;
+}
/*
* per-process(per-mm_struct) statistics.
*/
diff --git a/kernel/events/core.c b/kernel/events/core.c
index c94eb27..856d98c 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -6934,12 +6934,12 @@ static u64 perf_virt_to_phys(u64 virt)
* Walking the pages tables for user address.
* Interrupts are disabled, so it prevents any tear down
* of the page tables.
-* Try IRQ-safe __get_user_pages_fast first.
+* Try IRQ-safe get_user_page_fast_only first.
* If failed, leave phys_addr as 0.
*/
if (current->mm != NULL) {
pagefault_disable();
- if (__get_user_pages_fast(virt, 1, 0, ) == 1)
+ if (get_user_page_fast_only(virt, 0, ))
phys_addr =
Re: [PATCH v2] powerpc: Add ppc_inst_next()
Excerpts from Nicholas Piggin's message of May 24, 2020 9:56 am:
> Excerpts from Michael Ellerman's message of May 22, 2020 11:33 pm:
>> In a few places we want to calculate the address of the next
>> instruction. Previously that was simple, we just added 4 bytes, or if
>> using a u32 * we incremented that pointer by 1.
>>
>> But prefixed instructions make it more complicated, we need to advance
>> by either 4 or 8 bytes depending on the actual instruction. We also
>> can't do pointer arithmetic using struct ppc_inst, because it is
>> always 8 bytes in size on 64-bit, even though we might only need to
>> advance by 4 bytes.
>>
>> So add a ppc_inst_next() helper which calculates the location of the
>> next instruction, if the given instruction was located at the given
>> address. Note the instruction doesn't need to actually be at the
>> address in memory.
>>
>> Although it would seem natural for the value to be passed by value,
>> that makes it too easy to write a loop that will read off the end of a
>> page, eg:
>>
>> for (; src < end; src = ppc_inst_next(src, *src),
>>dest = ppc_inst_next(dest, *dest))
>>
>> As noticed by Christophe and Jordan, if end is the exact end of a
>> page, and the next page is not mapped, this will fault, because *dest
>> will read 8 bytes, 4 bytes into the next page.
>>
>> So value is passed by reference, so the helper can be careful to use
>> ppc_inst_read() on it.
>>
>> Signed-off-by: Michael Ellerman
>> ---
>> arch/powerpc/include/asm/inst.h | 13 +
>> arch/powerpc/kernel/uprobes.c | 2 +-
>> arch/powerpc/lib/feature-fixups.c | 15 ---
>> arch/powerpc/xmon/xmon.c | 2 +-
>> 4 files changed, 23 insertions(+), 9 deletions(-)
>>
>> v2: Pass the value as a pointer and use ppc_inst_read() on it.
>>
>> diff --git a/arch/powerpc/include/asm/inst.h
>> b/arch/powerpc/include/asm/inst.h
>> index d82e0c99cfa1..5b756ba77ed2 100644
>> --- a/arch/powerpc/include/asm/inst.h
>> +++ b/arch/powerpc/include/asm/inst.h
>> @@ -100,6 +100,19 @@ static inline int ppc_inst_len(struct ppc_inst x)
>> return ppc_inst_prefixed(x) ? 8 : 4;
>> }
>>
>> +/*
>> + * Return the address of the next instruction, if the instruction @value was
>> + * located at @location.
>> + */
>> +static inline struct ppc_inst *ppc_inst_next(void *location, struct
>> ppc_inst *value)
>> +{
>> +struct ppc_inst tmp;
>> +
>> +tmp = ppc_inst_read(value);
>> +
>> +return location + ppc_inst_len(tmp);
>> +}
>> +
>> int probe_user_read_inst(struct ppc_inst *inst,
>> struct ppc_inst __user *nip);
>>
>> diff --git a/arch/powerpc/kernel/uprobes.c b/arch/powerpc/kernel/uprobes.c
>> index 83e883e1a42d..d200e7df7167 100644
>> --- a/arch/powerpc/kernel/uprobes.c
>> +++ b/arch/powerpc/kernel/uprobes.c
>> @@ -112,7 +112,7 @@ int arch_uprobe_post_xol(struct arch_uprobe *auprobe,
>> struct pt_regs *regs)
>> * support doesn't exist and have to fix-up the next instruction
>> * to be executed.
>> */
>> -regs->nip = utask->vaddr + ppc_inst_len(ppc_inst_read(>insn));
>> +regs->nip = (unsigned long)ppc_inst_next((void *)utask->vaddr,
>> >insn);
>>
>> user_disable_single_step(current);
>> return 0;
>
> AFAIKS except for here, there is no need for the void *location arg.
>
> I would prefer to drop that and keep this unchanged (it's a slightly
> special case anyway).
Ooops, I didn't read the last thread. I don't think insert_bpts needs it
though, only this case. Anyway it's a minor nitpick so if it's already
been considered, fine.
Thanks,
Nick
Re: [PATCH v2] powerpc: Add ppc_inst_next()
Excerpts from Michael Ellerman's message of May 22, 2020 11:33 pm:
> In a few places we want to calculate the address of the next
> instruction. Previously that was simple, we just added 4 bytes, or if
> using a u32 * we incremented that pointer by 1.
>
> But prefixed instructions make it more complicated, we need to advance
> by either 4 or 8 bytes depending on the actual instruction. We also
> can't do pointer arithmetic using struct ppc_inst, because it is
> always 8 bytes in size on 64-bit, even though we might only need to
> advance by 4 bytes.
>
> So add a ppc_inst_next() helper which calculates the location of the
> next instruction, if the given instruction was located at the given
> address. Note the instruction doesn't need to actually be at the
> address in memory.
>
> Although it would seem natural for the value to be passed by value,
> that makes it too easy to write a loop that will read off the end of a
> page, eg:
>
> for (; src < end; src = ppc_inst_next(src, *src),
> dest = ppc_inst_next(dest, *dest))
>
> As noticed by Christophe and Jordan, if end is the exact end of a
> page, and the next page is not mapped, this will fault, because *dest
> will read 8 bytes, 4 bytes into the next page.
>
> So value is passed by reference, so the helper can be careful to use
> ppc_inst_read() on it.
>
> Signed-off-by: Michael Ellerman
> ---
> arch/powerpc/include/asm/inst.h | 13 +
> arch/powerpc/kernel/uprobes.c | 2 +-
> arch/powerpc/lib/feature-fixups.c | 15 ---
> arch/powerpc/xmon/xmon.c | 2 +-
> 4 files changed, 23 insertions(+), 9 deletions(-)
>
> v2: Pass the value as a pointer and use ppc_inst_read() on it.
>
> diff --git a/arch/powerpc/include/asm/inst.h b/arch/powerpc/include/asm/inst.h
> index d82e0c99cfa1..5b756ba77ed2 100644
> --- a/arch/powerpc/include/asm/inst.h
> +++ b/arch/powerpc/include/asm/inst.h
> @@ -100,6 +100,19 @@ static inline int ppc_inst_len(struct ppc_inst x)
> return ppc_inst_prefixed(x) ? 8 : 4;
> }
>
> +/*
> + * Return the address of the next instruction, if the instruction @value was
> + * located at @location.
> + */
> +static inline struct ppc_inst *ppc_inst_next(void *location, struct ppc_inst
> *value)
> +{
> + struct ppc_inst tmp;
> +
> + tmp = ppc_inst_read(value);
> +
> + return location + ppc_inst_len(tmp);
> +}
> +
> int probe_user_read_inst(struct ppc_inst *inst,
>struct ppc_inst __user *nip);
>
> diff --git a/arch/powerpc/kernel/uprobes.c b/arch/powerpc/kernel/uprobes.c
> index 83e883e1a42d..d200e7df7167 100644
> --- a/arch/powerpc/kernel/uprobes.c
> +++ b/arch/powerpc/kernel/uprobes.c
> @@ -112,7 +112,7 @@ int arch_uprobe_post_xol(struct arch_uprobe *auprobe,
> struct pt_regs *regs)
>* support doesn't exist and have to fix-up the next instruction
>* to be executed.
>*/
> - regs->nip = utask->vaddr + ppc_inst_len(ppc_inst_read(>insn));
> + regs->nip = (unsigned long)ppc_inst_next((void *)utask->vaddr,
> >insn);
>
> user_disable_single_step(current);
> return 0;
AFAIKS except for here, there is no need for the void *location arg.
I would prefer to drop that and keep this unchanged (it's a slightly
special case anyway).
Thanks,
Nick
> diff --git a/arch/powerpc/lib/feature-fixups.c
> b/arch/powerpc/lib/feature-fixups.c
> index 80f320c2e189..4c0a7ee9fa00 100644
> --- a/arch/powerpc/lib/feature-fixups.c
> +++ b/arch/powerpc/lib/feature-fixups.c
> @@ -68,7 +68,7 @@ static int patch_alt_instruction(struct ppc_inst *src,
> struct ppc_inst *dest,
>
> static int patch_feature_section(unsigned long value, struct fixup_entry
> *fcur)
> {
> - struct ppc_inst *start, *end, *alt_start, *alt_end, *src, *dest;
> + struct ppc_inst *start, *end, *alt_start, *alt_end, *src, *dest, nop;
>
> start = calc_addr(fcur, fcur->start_off);
> end = calc_addr(fcur, fcur->end_off);
> @@ -84,14 +84,15 @@ static int patch_feature_section(unsigned long value,
> struct fixup_entry *fcur)
> src = alt_start;
> dest = start;
>
> - for (; src < alt_end; src = (void *)src +
> ppc_inst_len(ppc_inst_read(src)),
> - (dest = (void *)dest + ppc_inst_len(ppc_inst_read(dest {
> + for (; src < alt_end; src = ppc_inst_next(src, src),
> + dest = ppc_inst_next(dest, dest)) {
> if (patch_alt_instruction(src, dest, alt_start, alt_end))
> return 1;
> }
>
> - for (; dest < end; dest = (void *)dest +
> ppc_inst_len(ppc_inst(PPC_INST_NOP)))
> - raw_patch_instruction(dest, ppc_inst(PPC_INST_NOP));
> + nop = ppc_inst(PPC_INST_NOP);
> + for (; dest < end; dest = ppc_inst_next(dest, ))
> + raw_patch_instruction(dest, nop);
>
> return 0;
> }
> @@ -405,8 +406,8 @@ static void do_final_fixups(void)
> while (src < end) {
> inst =
[Bug 207873] BUG at swapops + rcu stall + soft lockup at running btrfs test suite (TEST=013\* ./misc-tests.sh)
https://bugzilla.kernel.org/show_bug.cgi?id=207873 --- Comment #4 from Erhard F. (erhar...@mailbox.org) --- Created attachment 289261 --> https://bugzilla.kernel.org/attachment.cgi?id=289261=edit transcript of both screenshots -- You are receiving this mail because: You are watching the assignee of the bug.
Re: [linux-next RFC] mm/gup.c: Convert to use get_user_pages_fast_only()
On 2020-05-23 12:35, Souptick Joarder wrote:
...
Everything you have done here is an improvement, and I'd be happy to
see it go in (after fixing the bug I note below).
But in reading through it, I noticed almost every user ...
- if (__get_user_pages_fast(hva, 1, 1, ) == 1) {
+ if (get_user_pages_fast_only(hva, 1, FOLL_WRITE, ) == 1) {
passes '1' as the second parameter. So do we want to add:
static inline bool get_user_page_fast_only(unsigned long addr,
unsigned int gup_flags, struct page **pagep)
{
return get_user_pages_fast_only(addr, 1, gup_flags, pagep) == 1;
}
Yes, this can be added. Does get_user_page_fast_only() naming is fine ?
It seems like a good name to me. And I think that the new wrapper call is
a good move, too.
I did pause and reflect for a moment about the number gup/pup API calls we
are building up, but that's merely an indication of the wide usage of this
functionality. So it all feels about right.
thanks,
--
John Hubbard
NVIDIA
[Bug 207873] BUG at swapops + rcu stall + soft lockup at running btrfs test suite (TEST=013\* ./misc-tests.sh)
https://bugzilla.kernel.org/show_bug.cgi?id=207873 --- Comment #3 from Erhard F. (erhar...@mailbox.org) --- Created attachment 289259 --> https://bugzilla.kernel.org/attachment.cgi?id=289259=edit screenshot 02 -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 207873] BUG at swapops + rcu stall + soft lockup at running btrfs test suite (TEST=013\* ./misc-tests.sh)
https://bugzilla.kernel.org/show_bug.cgi?id=207873 --- Comment #2 from Erhard F. (erhar...@mailbox.org) --- Created attachment 289257 --> https://bugzilla.kernel.org/attachment.cgi?id=289257=edit screenshot 01 -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 207873] BUG at swapops + rcu stall + soft lockup at running btrfs test suite (TEST=013\* ./misc-tests.sh)
https://bugzilla.kernel.org/show_bug.cgi?id=207873 --- Comment #1 from Erhard F. (erhar...@mailbox.org) --- Created attachment 289255 --> https://bugzilla.kernel.org/attachment.cgi?id=289255=edit kernel dmesg (5.7-rc6, PowerMac G4 DP) -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 207873] New: BUG at swapops + rcu stall + soft lockup at running btrfs test suite (TEST=013\* ./misc-tests.sh)
https://bugzilla.kernel.org/show_bug.cgi?id=207873 Bug ID: 207873 Summary: BUG at swapops + rcu stall + soft lockup at running btrfs test suite (TEST=013\* ./misc-tests.sh) Product: Platform Specific/Hardware Version: 2.5 Kernel Version: 5.7-rc6 Hardware: PPC-32 OS: Linux Tree: Mainline Status: NEW Severity: normal Priority: P1 Component: PPC-32 Assignee: platform_ppc...@kernel-bugs.osdl.org Reporter: erhar...@mailbox.org CC: fs_bt...@kernel-bugs.osdl.org Regression: No Created attachment 289253 --> https://bugzilla.kernel.org/attachment.cgi?id=289253=edit kernel .config (5.7-rc6, PowerMac G4 DP) The bug is triggered by running "TEST=013\* ./misc-tests.sh" of btrfs-progs test suite, built from git master: # git clone https://github.com/kdave/btrfs-progs && cd btrfs-progs/ # ./autogen.sh && ./configure --disable-documentation # make && make fssum # cd tests/ # TEST=013\* ./misc-tests.sh The G4 crashes and the reboot timer kicks in. Before it shows a series of stack traces, starting with the "kernel BUG at include/linux/swapops.h:197!"-part from bug #207221. After that I get an rcu stall and a soft lockup. For the full stacktrace have a look at the transcript of both screenshots. [...] rcu: INFO: rcu_sched self-detected stall on CPU rcu: o1-: (7799 ticks this GP) idle=a06/1/0x4002 soft irq=11075/11075 fqs=2599 o(t=7804 jiffies g=21629 q=59) Task dump for CPU 1: dd R running task0 2200394 0x000c Call Trace: [f49fb458] [c00fcddc] sched_show_task+0x3bc/Ox3fe (unreliable) [f49fb498] [c01c650c] rcu_dump_cpu_stacks+0x228/0x23c [f49fb4e8] [c01c2e18] rcu_sched_clock_irq+0x81c/0x1360 [f49fb568] [c01d8940] update_process_times+0x2c/0x98 [f49fb588] [c02027d4] tick_sched_timer+0x128/0x1d8 [f49fb5a8] [c01dc49c] __hrtimer_run_queues+0x490/Oxae8 [f49fb698] [c01dd788] hrtimer_interrupt+0x278/0x520 [f49fb6f8] [c001710c] timer_interrupt+0x374/0xb4c [f49fb738] [c002c5e4] ret_from_except+0x0/0x14 --- interrupt: 901 at do_raw_spin_lock+0x1c8/0x2cc LR = do_raw_spin_lock+0x1a4/0x2cc [f49fb800] [c0180e0c] do_raw_spin_lock+0x188/0x2cc (unrelable) [f49fb830] [c0428890] unmap_page_range+0x244/0xb08 [f49fb910] [c0429610] unmap_vmas+0x94/0xdc [f49fb930] [c043c25c] exit_mmap+0x340/0x46c [f49fba20] [c0078260] __mmput+0x78/0x360 [f49fba50] [c0090514] do_exit+0x9c4/0x21fc [f49fbb20] [c0019d38] user_single_step_report+0x0/0x74 [f49fbb70] [c002c5e0] ret_from_except+0x0/0x4 --- interrupt: 700 at __migration_entry_wait+0x13c/0x198 LR = __migration_entry_wait+0xf0/0x198 [f49fbc58] [c042c0f0] do_swap_page+0x1f0/0x198 [f49fbd28] [c042e7e4] handle_mm_fault+0x794/0x16f4 [f49fbe48] [c0039868] do_page_fault+0xf50/0x12f8 [f49fbf38] [c002c468] handle_page_fault+0x10/0x3c --- interrupt: 301 at 0x87e378 LR = 0x87e33c [...] I don't know wether this is a btrfs bug, or a bug only triggered by this specific test. So I am filing this as platform specific as I have not seen it on x86 yet. Unlike bug #207221 KASAN is enabled here, so the stack trace looks slightly different. -- You are receiving this mail because: You are watching the assignee of the bug.
Re: Kernel bug in 5.7 for PPC32 on PowerBook G4 - bisected to commit 697ece7
On 5/23/20 12:30 PM, Christophe Leroy wrote:
Hi Larry,
Le 23/05/2020 à 19:24, Larry Finger a écrit :
Hi Christophe,
Although kernel 5.7.0-rc2 appeared to boot cleanly, it failed on my G4 when I
tried to generate a new kernel. The following BUG message is logged:
[...]
This problem was bisected to commit 697ece7 ("powerpc/32s: reorder Linux PTE
bits to better match Hash PTE bits").
Being reversed in new -rc , see
https://patchwork.ozlabs.org/project/linuxppc-dev/patch/87sgfsf4hs@mpe.ellerman.id.au/
Christophe,
Thanks for the update.
Larry
Re: [linux-next RFC] mm/gup.c: Convert to use get_user_pages_fast_only()
On Sat, May 23, 2020 at 10:55 PM Matthew Wilcox wrote:
>
> On Sat, May 23, 2020 at 10:11:12PM +0530, Souptick Joarder wrote:
> > Renaming the API __get_user_pages_fast() to get_user_pages_
> > fast_only() to align with pin_user_pages_fast_only().
>
> Please don't split a function name across lines. That messes
> up people who are grepping for the function name in the changelog.
Ok.
>
> > As part of this we will get rid of write parameter.
> > Instead caller will pass FOLL_WRITE to get_user_pages_fast_only().
> > This will not change any existing functionality of the API.
> >
> > All the callers are changed to pass FOLL_WRITE.
> >
> > Updated the documentation of the API.
>
> Everything you have done here is an improvement, and I'd be happy to
> see it go in (after fixing the bug I note below).
>
> But in reading through it, I noticed almost every user ...
>
> > - if (__get_user_pages_fast(hva, 1, 1, ) == 1) {
> > + if (get_user_pages_fast_only(hva, 1, FOLL_WRITE, ) == 1) {
>
> passes '1' as the second parameter. So do we want to add:
>
> static inline bool get_user_page_fast_only(unsigned long addr,
> unsigned int gup_flags, struct page **pagep)
> {
> return get_user_pages_fast_only(addr, 1, gup_flags, pagep) == 1;
> }
>
Yes, this can be added. Does get_user_page_fast_only() naming is fine ?
> > @@ -2797,10 +2803,7 @@ int __get_user_pages_fast(unsigned long start, int
> > nr_pages, int write,
> >* FOLL_FAST_ONLY is required in order to match the API description of
> >* this routine: no fall back to regular ("slow") GUP.
> >*/
> > - unsigned int gup_flags = FOLL_GET | FOLL_FAST_ONLY;
> > -
> > - if (write)
> > - gup_flags |= FOLL_WRITE;
> > + gup_flags = FOLL_GET | FOLL_FAST_ONLY;
>
> Er ... gup_flags |=, surely?
Poor mistake.
@@ -1998,7 +1998,7 @@ int gfn_to_page_many_atomic(struct
kvm_memory_slot *slot, gfn_t gfn,
if (entry < nr_pages)
return 0;
- return __get_user_pages_fast(addr, nr_pages, 1, pages);
+ return get_user_pages_fast(addr, nr_pages, FOLL_WRITE, pages);
Also this needs to be corrected.
Re: Kernel bug in 5.7 for PPC32 on PowerBook G4 - bisected to commit 697ece7
Hi Larry,
Le 23/05/2020 à 19:24, Larry Finger a écrit :
Hi Christophe,
Although kernel 5.7.0-rc2 appeared to boot cleanly, it failed on my G4
when I tried to generate a new kernel. The following BUG message is logged:
[...]
This problem was bisected to commit 697ece7 ("powerpc/32s: reorder Linux
PTE bits to better match Hash PTE bits").
Being reversed in new -rc , see
https://patchwork.ozlabs.org/project/linuxppc-dev/patch/87sgfsf4hs@mpe.ellerman.id.au/
If I had done more rigorous tests earlier, I would have found the bug
with more time to fix it before release of 5.7, but every other problem
I have found happened at boot, not when the machine had to swap.
Thanks,
Larry
Christophe
Re: [linux-next RFC] mm/gup.c: Convert to use get_user_pages_fast_only()
On Sat, May 23, 2020 at 10:11:12PM +0530, Souptick Joarder wrote:
> Renaming the API __get_user_pages_fast() to get_user_pages_
> fast_only() to align with pin_user_pages_fast_only().
Please don't split a function name across lines. That messes
up people who are grepping for the function name in the changelog.
> As part of this we will get rid of write parameter.
> Instead caller will pass FOLL_WRITE to get_user_pages_fast_only().
> This will not change any existing functionality of the API.
>
> All the callers are changed to pass FOLL_WRITE.
>
> Updated the documentation of the API.
Everything you have done here is an improvement, and I'd be happy to
see it go in (after fixing the bug I note below).
But in reading through it, I noticed almost every user ...
> - if (__get_user_pages_fast(hva, 1, 1, ) == 1) {
> + if (get_user_pages_fast_only(hva, 1, FOLL_WRITE, ) == 1) {
passes '1' as the second parameter. So do we want to add:
static inline bool get_user_page_fast_only(unsigned long addr,
unsigned int gup_flags, struct page **pagep)
{
return get_user_pages_fast_only(addr, 1, gup_flags, pagep) == 1;
}
> @@ -2797,10 +2803,7 @@ int __get_user_pages_fast(unsigned long start, int
> nr_pages, int write,
>* FOLL_FAST_ONLY is required in order to match the API description of
>* this routine: no fall back to regular ("slow") GUP.
>*/
> - unsigned int gup_flags = FOLL_GET | FOLL_FAST_ONLY;
> -
> - if (write)
> - gup_flags |= FOLL_WRITE;
> + gup_flags = FOLL_GET | FOLL_FAST_ONLY;
Er ... gup_flags |=, surely?
Kernel bug in 5.7 for PPC32 on PowerBook G4 - bisected to commit 697ece7
Hi Christophe,
Although kernel 5.7.0-rc2 appeared to boot cleanly, it failed on my G4 when I
tried to generate a new kernel. The following BUG message is logged:
[ 336.148935] [ cut here ]
[ 336.148950] kernel BUG at ./include/linux/swapops.h:195!
[ 336.148971] Oops: Exception in kernel mode, sig: 5 [#1]
[ 336.148975] BE PAGE_SIZE=4K MMU=Hash PowerMac
[ 336.148978] Modules linked in: cpufreq_ondemand fuse ctr ccm b43 mac80211
sha256_generic libsha256 cfg80211 hid_apple hid_generic joydev rfkill libarc4
rng_core cordic uinput radeon evdev
ttm drm_kms_helper usbhid hid appletouch drm rack_meter ehci_pci ehci_hcd
drm_panel_orientation_quirks ssb fb_sys_fops yenta_socket sysimgblt sysfillrect
pcmcia_rsrc syscopyarea pcmcia pcmcia
_core i2c_powermac therm_adt746x loop ohci_pci ohci_hcd usbcore sungem
sungem_phy usb_common
[ 336.149052] CPU: 0 PID: 8346 Comm: ld Not tainted 5.6.0-rc2-00086-g36b7840
#249
[ 336.149056] NIP: c0166624 LR: c016661c CTR:
[ 336.149062] REGS: f42ddb08 TRAP: 0700 Not tainted
(5.6.0-rc2-00086-g36b7840)
[ 336.149064] MSR: 00029032 CR: 24000424 XER:
[ 336.149072]
[ 336.149072] GPR00: f42ddbc0 c24fcb80 0001 ef438f00 c1957b1c
f42ddc4c 0004
[ 336.149072] GPR08: 0050 0100 edb9d000 100cba68
10051b10 10051b08
[ 336.149072] GPR16: 01be ee68c078 105a c1957b1c
[ 336.149072] GPR24: ec5d2540 7c002bf8 c1957ae0 ec5d2540
ef438f00 ef438f00
[ 336.149107] NIP [c0166624] _einittext+0x3f9d38a8/0x3fe4a284
[ 336.149111] LR [c016661c] _einittext+0x3f9d38a0/0x3fe4a284
[ 336.149114] Call Trace:
[ 336.149118] [f42ddbc0] [c07b9b60] 0xc07b9b60 (unreliable)
[ 336.149123] [f42ddbd0] [c013ff64] _einittext+0x3f9ad1e8/0x3fe4a284
[ 336.149128] [f42ddc10] [c0140d4c] _einittext+0x3f9adfd0/0x3fe4a284
[ 336.149133] [f42ddc90] [c002aadc] _einittext+0x3f897d60/0x3fe4a284
[ 336.149137] [f42ddce0] [c00153a4] _einittext+0x3f882628/0x3fe4a284
[ 336.149144] --- interrupt: 301 at _einittext+0x3fb52a50/0x3fe4a284
[ 336.149144] LR = _einittext+0x3fb52a4c/0x3fe4a284
[ 336.149148] [f42ddda8] [c02e56c0] _einittext+0x3fb52944/0x3fe4a284
(unreliable)
[ 336.149153] [f42ddde8] [c011644c] _einittext+0x3f9836d0/0x3fe4a284
[ 336.149158] [f42dde38] [c01f5950] _einittext+0x3fa62bd4/0x3fe4a284
[ 336.149163] [f42dde58] [c016b98c] _einittext+0x3f9d8c10/0x3fe4a284
[ 336.149167] [f42ddec8] [c016ba60] _einittext+0x3f9d8ce4/0x3fe4a284
[ 336.149172] [f42ddef8] [c016bd00] _einittext+0x3f9d8f84/0x3fe4a284
[ 336.149177] [f42ddf38] [c0015174] _einittext+0x3f8823f8/0x3fe4a284
[ 336.149182] --- interrupt: c01 at 0xfdf99fc
[ 336.149182] LR = 0xfd9cce0
[ 336.149184] Instruction dump:
[ 336.149189] 40be0018 4bffe359 3c80c06a 3884e48f 4bfd4c9d 0fe0 4bffe345
7c641b78
[ 336.149196] 3860 4bffe045 7c630034 5463d97e <0f03> 3940 393f001c
3961
[ 336.149208] ---[ end trace d08833cae9c66ce3 ]---
[ 336.149210]
[ 336.193729] [ cut here ]
This problem was bisected to commit 697ece7 ("powerpc/32s: reorder Linux PTE
bits to better match Hash PTE bits").
If I had done more rigorous tests earlier, I would have found the bug with more
time to fix it before release of 5.7, but every other problem I have found
happened at boot, not when the machine had to swap.
Thanks,
Larry
Re: [PATCH] kbuild: reuse vmlinux.o in vmlinux_link
Hi Masahiro. On Sun, May 24, 2020 at 12:12:35AM +0900, Masahiro Yamada wrote: > Hi Nicholas, > (+CC: Sam Ravnborg) > > > On Sat, May 23, 2020 at 7:06 PM Nicholas Piggin wrote: > > > > Excerpts from Masahiro Yamada's message of May 23, 2020 3:44 am: > > > + Michael, and PPC ML. > > > > > > They may know something about the reason of failure. > > > > Because the linker can't put branch stubs within object code sections, > > so when you incrementally link them too large, the linker can't resolve > > branches into other object files. > > > Ah, you are right. > > So, this is a problem not only for PPC > but also for ARM (both 32 and 64 bit), etc. > > ARM needs to insert a veneer to jump far. > > Prior to thin archive, we could not compile > ARCH=arm allyesconfig because > drivers/built-in.o was too large. > > This patch gets us back to the too large > incremental object situation. > > With my quick compile-testing, > ARCH=arm allyesconfig > and ARCH=arm64 allyesconfig are broken. > > > > This is why we added incremental linking in the first place. I suppose > > it could be made conditional for platforms that can use this > > optimization. > > > > What'd be really nice is if we could somehow build and link kallsyms > > without relinking everything twice, and if we could do section mismatch > > analysis without making that vmlinux.o as well. I had a few ideas but > > not enough time to do much work on it. > > > Right, kallsyms links 3 times. (not twice) > > > Hmm, I think Sami's main motivation is Clang LTO. > > LTO is very time-consuming. > So, the android common kernel implements Clang LTO > in the pre modpost stage: > > > 1) LTO against vmlinux.o > > 2) modpost against vmlinux.o > > 3) Link vmlinux.o + kallsyms into vmlinux >(this requires linking 3 times) We have kallsyms we had to link three times because the linking increased the object a little in size so symbols did not match. The last time was added more or less only to check that we did have stable symbol addresses. All this predates LTO stuff which we only introduced later. The reason for doing modpost on vmlinux.o was that we had cases where everything in drivers/ was fine but there was section mismatch references from arch/* to drivers/* This is back when there were much more drivers in arch/ than what we have today. And back then we also had much more to check ad we had cPU hotplug that could really cause section mismatches - this is no longer the case which is a good thing. ... > > The following two commits. > I did not fully understand the background, though. > > I CC'ed Sam in case he may add some comments. > > commit 85bd2fddd68e757da8e1af98f857f61a3c9ce647 > Author: Sam Ravnborg > Date: Mon Feb 26 15:33:52 2007 +0100 > > kbuild: fix section mismatch check for vmlinux > > vmlinux does not contain relocation entries which is > used by the section mismatch checks. > Reported by: Atsushi Nemoto > > Use the individual objects as inputs to overcome > this limitation. > In modpost check the .o files and skip non-ELF files. > > Signed-off-by: Sam Ravnborg So we checked vmlinx - but vmlinx did have too much stripped away. so in reality nothing was checked. To allow the warnings to be as precise as possible move the checks out to the indovidual .o files. Sometimes the names was mangled a little so if warnigns was only reported on vmlinx level in could be difficult to track down the offender. This would then also do the check on .o files that had all the relocation symbols rtequired. > > commit 741f98fe298a73c9d47ed53703c1279a29718581 > Author: Sam Ravnborg > Date: Tue Jul 17 10:54:06 2007 +0200 > > kbuild: do section mismatch check on full vmlinux > > Previously we did do the check on the .o files used to link > vmlinux but that failed to find questionable references across > the .o files. > Create a dedicated vmlinux.o file used only for section mismatch checks > that uses the defualt linker script so section does not get renamed. > > The vmlinux.o may later be used as part of the the final link of vmlinux > but for now it is used fo section mismatch only. > For a defconfig build this is instant but for an allyesconfig this > add two minutes to a full build (that anyways takes ~2 hours). > > Signed-off-by: Sam Ravnborg But when we introduced check of the individual .o fiules we missed when the references spanned outside the .o files as explained previously. So included a link of vmlinx.o that did NOT drop the relocations so we could use it to check for the remaining section mismatch warnings. Remember - back when we started this we had many hundred warnings and it was a fight to keep that number low. But we also wanted to report as much as possible. There was back then several discussions if this was really worth the effort. How much was gained from discarding the memory where the section mismatch warnigns was
[linux-next RFC] mm/gup.c: Convert to use get_user_pages_fast_only()
Renaming the API __get_user_pages_fast() to get_user_pages_
fast_only() to align with pin_user_pages_fast_only().
As part of this we will get rid of write parameter.
Instead caller will pass FOLL_WRITE to get_user_pages_fast_only().
This will not change any existing functionality of the API.
All the callers are changed to pass FOLL_WRITE.
Updated the documentation of the API.
Signed-off-by: Souptick Joarder
Cc: John Hubbard
Cc: Matthew Wilcox
---
arch/powerpc/kvm/book3s_64_mmu_hv.c| 2 +-
arch/powerpc/kvm/book3s_64_mmu_radix.c | 2 +-
arch/powerpc/perf/callchain_64.c | 2 +-
include/linux/mm.h | 4 ++--
kernel/events/core.c | 4 ++--
mm/gup.c | 29 -
virt/kvm/kvm_main.c| 6 +++---
7 files changed, 26 insertions(+), 23 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c
b/arch/powerpc/kvm/book3s_64_mmu_hv.c
index 18aed97..34fc5c8 100644
--- a/arch/powerpc/kvm/book3s_64_mmu_hv.c
+++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c
@@ -581,7 +581,7 @@ int kvmppc_book3s_hv_page_fault(struct kvm_run *run, struct
kvm_vcpu *vcpu,
* We always ask for write permission since the common case
* is that the page is writable.
*/
- if (__get_user_pages_fast(hva, 1, 1, ) == 1) {
+ if (get_user_pages_fast_only(hva, 1, FOLL_WRITE, ) == 1) {
write_ok = true;
} else {
/* Call KVM generic code to do the slow-path check */
diff --git a/arch/powerpc/kvm/book3s_64_mmu_radix.c
b/arch/powerpc/kvm/book3s_64_mmu_radix.c
index 3248f78..3b6e342 100644
--- a/arch/powerpc/kvm/book3s_64_mmu_radix.c
+++ b/arch/powerpc/kvm/book3s_64_mmu_radix.c
@@ -795,7 +795,7 @@ int kvmppc_book3s_instantiate_page(struct kvm_vcpu *vcpu,
* is that the page is writable.
*/
hva = gfn_to_hva_memslot(memslot, gfn);
- if (!kvm_ro && __get_user_pages_fast(hva, 1, 1, ) == 1) {
+ if (!kvm_ro && get_user_pages_fast_only(hva, 1, FOLL_WRITE, ) ==
1) {
upgrade_write = true;
} else {
unsigned long pfn;
diff --git a/arch/powerpc/perf/callchain_64.c b/arch/powerpc/perf/callchain_64.c
index 1bff896d..f719a74 100644
--- a/arch/powerpc/perf/callchain_64.c
+++ b/arch/powerpc/perf/callchain_64.c
@@ -32,7 +32,7 @@ int read_user_stack_slow(void __user *ptr, void *buf, int nb)
int nrpages;
void *kaddr;
- nrpages = __get_user_pages_fast(addr, 1, 1, );
+ nrpages = get_user_pages_fast_only(addr, 1, FOLL_WRITE, );
if (nrpages == 1) {
kaddr = page_address(page);
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 93d93bd..10a6758 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1817,8 +1817,8 @@ extern int mprotect_fixup(struct vm_area_struct *vma,
/*
* doesn't attempt to fault and will return short.
*/
-int __get_user_pages_fast(unsigned long start, int nr_pages, int write,
- struct page **pages);
+int get_user_pages_fast_only(unsigned long start, int nr_pages,
+ unsigned int gup_flags, struct page **pages);
int pin_user_pages_fast_only(unsigned long start, int nr_pages,
unsigned int gup_flags, struct page **pages);
/*
diff --git a/kernel/events/core.c b/kernel/events/core.c
index c94eb27..81d6e73 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -6934,12 +6934,12 @@ static u64 perf_virt_to_phys(u64 virt)
* Walking the pages tables for user address.
* Interrupts are disabled, so it prevents any tear down
* of the page tables.
-* Try IRQ-safe __get_user_pages_fast first.
+* Try IRQ-safe get_user_pages_fast_only first.
* If failed, leave phys_addr as 0.
*/
if (current->mm != NULL) {
pagefault_disable();
- if (__get_user_pages_fast(virt, 1, 0, ) == 1)
+ if (get_user_pages_fast_only(virt, 1, 0, ) == 1)
phys_addr = page_to_phys(p) + virt % PAGE_SIZE;
pagefault_enable();
}
diff --git a/mm/gup.c b/mm/gup.c
index 80f51a36..d8aabc0 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -2278,7 +2278,7 @@ static int gup_pte_range(pmd_t pmd, unsigned long addr,
unsigned long end,
* to be special.
*
* For a futex to be placed on a THP tail page, get_futex_key requires a
- * __get_user_pages_fast implementation that can pin pages. Thus it's still
+ * get_user_pages_fast_only implementation that can pin pages. Thus it's still
* useful to have gup_huge_pmd even if we can't operate on ptes.
*/
static int gup_pte_range(pmd_t pmd, unsigned long addr, unsigned long end,
@@ -2683,7 +2683,7 @@ static inline void gup_pgd_range(unsigned long addr,
unsigned long end,
#ifndef
Re: [PATCH] kbuild: reuse vmlinux.o in vmlinux_link
Hi Nicholas,
(+CC: Sam Ravnborg)
On Sat, May 23, 2020 at 7:06 PM Nicholas Piggin wrote:
>
> Excerpts from Masahiro Yamada's message of May 23, 2020 3:44 am:
> > + Michael, and PPC ML.
> >
> > They may know something about the reason of failure.
>
> Because the linker can't put branch stubs within object code sections,
> so when you incrementally link them too large, the linker can't resolve
> branches into other object files.
Ah, you are right.
So, this is a problem not only for PPC
but also for ARM (both 32 and 64 bit), etc.
ARM needs to insert a veneer to jump far.
Prior to thin archive, we could not compile
ARCH=arm allyesconfig because
drivers/built-in.o was too large.
This patch gets us back to the too large
incremental object situation.
With my quick compile-testing,
ARCH=arm allyesconfig
and ARCH=arm64 allyesconfig are broken.
> This is why we added incremental linking in the first place. I suppose
> it could be made conditional for platforms that can use this
> optimization.
>
> What'd be really nice is if we could somehow build and link kallsyms
> without relinking everything twice, and if we could do section mismatch
> analysis without making that vmlinux.o as well. I had a few ideas but
> not enough time to do much work on it.
Right, kallsyms links 3 times. (not twice)
Hmm, I think Sami's main motivation is Clang LTO.
LTO is very time-consuming.
So, the android common kernel implements Clang LTO
in the pre modpost stage:
1) LTO against vmlinux.o
2) modpost against vmlinux.o
3) Link vmlinux.o + kallsyms into vmlinux
(this requires linking 3 times)
If we move LTO to 3), we need to do LTO 3 times.
And, this was how GCC LTO was implemented in 2014,
(then rejected by Linus).
How to do modpost without making vmlinux.o ?
In old days, the section mismatch analysis was done
against the final vmlinux.
85bd2fddd68e757da8e1af98f857f61a3c9ce647 changed
it to run modpost for individual .o files.
Then, 741f98fe298a73c9d47ed53703c1279a29718581
introduced vmlinux.o to use it for modpost.
The following two commits.
I did not fully understand the background, though.
I CC'ed Sam in case he may add some comments.
commit 85bd2fddd68e757da8e1af98f857f61a3c9ce647
Author: Sam Ravnborg
Date: Mon Feb 26 15:33:52 2007 +0100
kbuild: fix section mismatch check for vmlinux
vmlinux does not contain relocation entries which is
used by the section mismatch checks.
Reported by: Atsushi Nemoto
Use the individual objects as inputs to overcome
this limitation.
In modpost check the .o files and skip non-ELF files.
Signed-off-by: Sam Ravnborg
commit 741f98fe298a73c9d47ed53703c1279a29718581
Author: Sam Ravnborg
Date: Tue Jul 17 10:54:06 2007 +0200
kbuild: do section mismatch check on full vmlinux
Previously we did do the check on the .o files used to link
vmlinux but that failed to find questionable references across
the .o files.
Create a dedicated vmlinux.o file used only for section mismatch checks
that uses the defualt linker script so section does not get renamed.
The vmlinux.o may later be used as part of the the final link of vmlinux
but for now it is used fo section mismatch only.
For a defconfig build this is instant but for an allyesconfig this
add two minutes to a full build (that anyways takes ~2 hours).
Signed-off-by: Sam Ravnborg
>
> Thanks,
> Nick
>
> >
> >
> > On Sat, May 23, 2020 at 2:41 AM Masahiro Yamada
> > wrote:
> >>
> >> On Fri, May 22, 2020 at 5:27 AM Sami Tolvanen
> >> wrote:
> >> >
> >> > Instead of linking all compilation units again each time vmlinux_link is
> >> > called, reuse vmlinux.o from modpost_link.
> >> >
> >> > With x86_64 allyesconfig, vmlinux_link is called three times and reusing
> >> > vmlinux.o reduces the build time ~38 seconds on my system (59% reduction
> >> > in the time spent in vmlinux_link).
> >> >
> >> > Signed-off-by: Sami Tolvanen
> >> > ---
> >> > scripts/link-vmlinux.sh | 5 +
> >> > 1 file changed, 1 insertion(+), 4 deletions(-)
> >> >
> >> > diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> >> > index d09ab4afbda4..c6cc4305950c 100755
> >> > --- a/scripts/link-vmlinux.sh
> >> > +++ b/scripts/link-vmlinux.sh
> >> > @@ -77,11 +77,8 @@ vmlinux_link()
> >> >
> >> > if [ "${SRCARCH}" != "um" ]; then
> >> > objects="--whole-archive\
> >> > - ${KBUILD_VMLINUX_OBJS} \
> >> > + vmlinux.o \
> >> > --no-whole-archive \
> >> > - --start-group \
> >> > - ${KBUILD_VMLINUX_LIBS} \
> >> > - --end-group \
> >> > ${@}"
> >> >
> >> > ${LD} ${KBUILD_LDFLAGS}
Re: [PATCH v4 1/6] printk: Collapse shutdown types into a single dump reason
Kees Cook writes:
> To turn the KMSG_DUMP_* reasons into a more ordered list, collapse
> the redundant KMSG_DUMP_(RESTART|HALT|POWEROFF) reasons into
> KMSG_DUMP_SHUTDOWN. The current users already don't meaningfully
> distinguish between them, so there's no need to, as discussed here:
> https://lore.kernel.org/lkml/ca+ck2bapv5u1ih5y9t5funtyximtfctdyxjcpuyjoyhnokr...@mail.gmail.com/
>
> Signed-off-by: Kees Cook
> ---
> arch/powerpc/kernel/nvram_64.c | 4 +---
> fs/pstore/platform.c | 8 ++--
> include/linux/kmsg_dump.h | 4 +---
> kernel/reboot.c| 6 +++---
> 4 files changed, 7 insertions(+), 15 deletions(-)
>
> diff --git a/arch/powerpc/kernel/nvram_64.c b/arch/powerpc/kernel/nvram_64.c
> index fb4f61096613..0cd1c88bfc8b 100644
> --- a/arch/powerpc/kernel/nvram_64.c
> +++ b/arch/powerpc/kernel/nvram_64.c
> @@ -655,9 +655,7 @@ static void oops_to_nvram(struct kmsg_dumper *dumper,
> int rc = -1;
>
> switch (reason) {
> - case KMSG_DUMP_RESTART:
> - case KMSG_DUMP_HALT:
> - case KMSG_DUMP_POWEROFF:
> + case KMSG_DUMP_SHUTDOWN:
> /* These are almost always orderly shutdowns. */
> return;
> case KMSG_DUMP_OOPS:
Acked-by: Michael Ellerman (powerpc)
cheers
Re: [PATCH] kbuild: reuse vmlinux.o in vmlinux_link
Excerpts from Masahiro Yamada's message of May 23, 2020 3:44 am:
> + Michael, and PPC ML.
>
> They may know something about the reason of failure.
Because the linker can't put branch stubs within object code sections,
so when you incrementally link them too large, the linker can't resolve
branches into other object files.
This is why we added incremental linking in the first place. I suppose
it could be made conditional for platforms that can use this
optimization.
What'd be really nice is if we could somehow build and link kallsyms
without relinking everything twice, and if we could do section mismatch
analysis without making that vmlinux.o as well. I had a few ideas but
not enough time to do much work on it.
Thanks,
Nick
>
>
> On Sat, May 23, 2020 at 2:41 AM Masahiro Yamada wrote:
>>
>> On Fri, May 22, 2020 at 5:27 AM Sami Tolvanen
>> wrote:
>> >
>> > Instead of linking all compilation units again each time vmlinux_link is
>> > called, reuse vmlinux.o from modpost_link.
>> >
>> > With x86_64 allyesconfig, vmlinux_link is called three times and reusing
>> > vmlinux.o reduces the build time ~38 seconds on my system (59% reduction
>> > in the time spent in vmlinux_link).
>> >
>> > Signed-off-by: Sami Tolvanen
>> > ---
>> > scripts/link-vmlinux.sh | 5 +
>> > 1 file changed, 1 insertion(+), 4 deletions(-)
>> >
>> > diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
>> > index d09ab4afbda4..c6cc4305950c 100755
>> > --- a/scripts/link-vmlinux.sh
>> > +++ b/scripts/link-vmlinux.sh
>> > @@ -77,11 +77,8 @@ vmlinux_link()
>> >
>> > if [ "${SRCARCH}" != "um" ]; then
>> > objects="--whole-archive\
>> > - ${KBUILD_VMLINUX_OBJS} \
>> > + vmlinux.o \
>> > --no-whole-archive \
>> > - --start-group \
>> > - ${KBUILD_VMLINUX_LIBS} \
>> > - --end-group \
>> > ${@}"
>> >
>> > ${LD} ${KBUILD_LDFLAGS} ${LDFLAGS_vmlinux} \
>> >
>> > base-commit: b85051e755b0e9d6dd8f17ef1da083851b83287d
>> > --
>> > 2.27.0.rc0.183.gde8f92d652-goog
>> >
>>
>>
>> I like this patch irrespective of CLANG_LTO, but
>> unfortunately, my build test failed.
>>
>>
>> ARCH=powerpc failed to build as follows:
>>
>>
>>
>> MODPOST vmlinux.o
>> MODINFO modules.builtin.modinfo
>> GEN modules.builtin
>> LD .tmp_vmlinux.kallsyms1
>> vmlinux.o:(__ftr_alt_97+0x20): relocation truncated to fit:
>> R_PPC64_REL14 against `.text'+4b1c
>> vmlinux.o:(__ftr_alt_97+0x164): relocation truncated to fit:
>> R_PPC64_REL14 against `.text'+1cf78
>> vmlinux.o:(__ftr_alt_97+0x288): relocation truncated to fit:
>> R_PPC64_REL14 against `.text'+1dac4
>> vmlinux.o:(__ftr_alt_97+0x2f0): relocation truncated to fit:
>> R_PPC64_REL14 against `.text'+1e254
>> make: *** [Makefile:1125: vmlinux] Error 1
>>
>>
>>
>> I used powerpc-linux-gcc
>> available at
>> https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/9.2.0/
>>
>>
>> Build command:
>>
>> make -j24 ARCH=powerpc CROSS_COMPILE=powerpc-linux- defconfig all
>>
>>
>> Could you check it please?
>>
>>
>>
>> I will apply it to my test branch.
>> Perhaps, 0-day bot may find more failure cases.
>>
>>
>> --
>> Best Regards
>> Masahiro Yamada
>
>
>
> --
> Best Regards
> Masahiro Yamada
>
