Hi!
In order to faciliate Will's READ_ONCE() patches:
https://lkml.kernel.org/r/[email protected]
we need to fix m68k/motorola to not have a giant pmd_t. These patches do so and
are tested using ARAnyM/68040.
It would be very good if someone can either test or tell us what emulator to
use for 020/030.
Please consider.
Hi Peter!
On 1/29/20 11:39 AM, Peter Zijlstra wrote:
> It would be very good if someone can either test or tell us what emulator to
> use for 020/030.
If possible, please also test on qemu-system, see [1] for a how-to.
I can test the patches on a real machine, Michael Schmitz could maybe
include them in one of his next test builds which we are running on one
of the Amigas I have.
Adrian
> [1] https://wiki.debian.org/M68k/QemuSystemM68k
--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer - [email protected]
`. `' Freie Universitaet Berlin - [email protected]
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
On Wed, Jan 29, 2020 at 11:49:13AM +0100, John Paul Adrian Glaubitz wrote:
> > [1] https://wiki.debian.org/M68k/QemuSystemM68k
Now, if only debian would actually ship that :/
AFAICT that emulates a q800 which is another 68040 and should thus not
differ from ARAnyM.
I'm fairly confident in the 040 bits, it's the 020/030 things that need
coverage.
On 1/29/20 12:54 PM, Peter Zijlstra wrote:
> On Wed, Jan 29, 2020 at 11:49:13AM +0100, John Paul Adrian Glaubitz wrote:
>
>>> [1] https://wiki.debian.org/M68k/QemuSystemM68k
>
> Now, if only debian would actually ship that :/
Debian should receive the QEMU version that supports full m68k emulation
soonish.
> AFAICT that emulates a q800 which is another 68040 and should thus not
> differ from ARAnyM.
Right. You could switch to a different CPU emulation though, Laurent
Vivier should be able to say more on that.
> I'm fairly confident in the 040 bits, it's the 020/030 things that need
> coverage.
I'm currently setting up an Amiga 500 with an ACA-1233n/40 accelerator
which has an 68030 CPU clocked at 40 MHz and 128 MB RAM which will be
used for developing a driver for a new network card card for the Amiga
500 called X-Surf 500.
I can definitely test the patches on that setup, but I certainly won't
have the time to set everything up until after FOSDEM.
Adrian
--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer - [email protected]
`. `' Freie Universitaet Berlin - [email protected]
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
On Wed, Jan 29, 2020 at 01:05:10PM +0100, John Paul Adrian Glaubitz wrote:
> On 1/29/20 12:54 PM, Peter Zijlstra wrote:
> > On Wed, Jan 29, 2020 at 11:49:13AM +0100, John Paul Adrian Glaubitz wrote:
> >
> >>> [1] https://wiki.debian.org/M68k/QemuSystemM68k
> >
> > Now, if only debian would actually ship that :/
>
> Debian should receive the QEMU version that supports full m68k emulation
> soonish.
Excellent!
> > AFAICT that emulates a q800 which is another 68040 and should thus not
> > differ from ARAnyM.
>
> Right. You could switch to a different CPU emulation though, Laurent
> Vivier should be able to say more on that.
The link you provided only mentioned that Q-88 thing, let me go rummage
through the actual qemu-patch to see if it supports more.
> > I'm fairly confident in the 040 bits, it's the 020/030 things that need
> > coverage.
>
> I'm currently setting up an Amiga 500 with an ACA-1233n/40 accelerator
> which has an 68030 CPU clocked at 40 MHz and 128 MB RAM which will be
> used for developing a driver for a new network card card for the Amiga
> 500 called X-Surf 500.
I remember playing 'Another World' on the Amiga-500, I'm thinking this
accelerator is a wee bit overkill for that though ;-)
> I can definitely test the patches on that setup, but I certainly won't
> have the time to set everything up until after FOSDEM.
That would be great, thanks!
Peter,
On Thu, Jan 30, 2020 at 12:54 AM Peter Zijlstra <[email protected]> wrote:
>
> On Wed, Jan 29, 2020 at 11:49:13AM +0100, John Paul Adrian Glaubitz wrote:
>
> > > [1] https://wiki.debian.org/M68k/QemuSystemM68k
>
> Now, if only debian would actually ship that :/
>
> AFAICT that emulates a q800 which is another 68040 and should thus not
> differ from ARAnyM.
>
> I'm fairly confident in the 040 bits, it's the 020/030 things that need
> coverage.
I'll take a look - unless this eats up way more kernel memory for page
tables, it should still boot on my Falcon.
Cheers,
Michael
On Thu, Jan 30, 2020 at 07:52:11AM +1300, Michael Schmitz wrote:
> Peter,
>
> On Thu, Jan 30, 2020 at 12:54 AM Peter Zijlstra <[email protected]> wrote:
> >
> > On Wed, Jan 29, 2020 at 11:49:13AM +0100, John Paul Adrian Glaubitz wrote:
> >
> > > > [1] https://wiki.debian.org/M68k/QemuSystemM68k
> >
> > Now, if only debian would actually ship that :/
> >
> > AFAICT that emulates a q800 which is another 68040 and should thus not
> > differ from ARAnyM.
> >
> > I'm fairly confident in the 040 bits, it's the 020/030 things that need
> > coverage.
>
> I'll take a look - unless this eats up way more kernel memory for page
> tables, it should still boot on my Falcon.
It should actually be better in most cases I think, since we no longer
require all 16 pte-tables to map consecutive (virtual) memory.
Peter,
Am 30.01.2020 um 08:31 schrieb Peter Zijlstra:
> On Thu, Jan 30, 2020 at 07:52:11AM +1300, Michael Schmitz wrote:
>> Peter,
>>
>> On Thu, Jan 30, 2020 at 12:54 AM Peter Zijlstra <[email protected]> wrote:
>>>
>>> On Wed, Jan 29, 2020 at 11:49:13AM +0100, John Paul Adrian Glaubitz wrote:
>>>
>>>>> [1] https://wiki.debian.org/M68k/QemuSystemM68k
>>>
>>> Now, if only debian would actually ship that :/
>>>
>>> AFAICT that emulates a q800 which is another 68040 and should thus not
>>> differ from ARAnyM.
>>>
>>> I'm fairly confident in the 040 bits, it's the 020/030 things that need
>>> coverage.
>>
>> I'll take a look - unless this eats up way more kernel memory for page
>> tables, it should still boot on my Falcon.
>
> It should actually be better in most cases I think, since we no longer
> require all 16 pte-tables to map consecutive (virtual) memory.
Not much difference:
total used free shared buffers cached
Mem: 10712 10120 592 0 1860 2276
-/+ buffers/cache: 5984 4728
Swap: 2097144 1552 2095592
vs. vanilla 5.5rc5:
total used free shared buffers cached
Mem: 10716 10104 612 0 1588 2544
-/+ buffers/cache: 5972 4744
Swap: 2097144 1296 2095848
By sheer coincidence, the boot with your patch series happened to run a
full filesystem check on the root filesystem, so I'd say it got a good
workout re: paging and swapping (even though it's just a paltry 4 GB).
Haven't tried any VM stress testing yet (not sure what to do for that;
it's been years since I tried that sort of stuff).
Cheers,
Michael
Hi Michael,
On Thu, Jan 30, 2020 at 08:31:13PM +1300, Michael Schmitz wrote:
> Not much difference:
>
> total used free shared buffers cached
> Mem: 10712 10120 592 0 1860 2276
> -/+ buffers/cache: 5984 4728
> Swap: 2097144 1552 2095592
>
>
> vs. vanilla 5.5rc5:
> total used free shared buffers cached
> Mem: 10716 10104 612 0 1588 2544
> -/+ buffers/cache: 5972 4744
> Swap: 2097144 1296 2095848
>
> By sheer coincidence, the boot with your patch series happened to run a full
> filesystem check on the root filesystem, so I'd say it got a good workout
> re: paging and swapping (even though it's just a paltry 4 GB).
Sweet!, can I translate this into a Tested-by: from you?
> Haven't tried any VM stress testing yet (not sure what to do for that; it's
> been years since I tried that sort of stuff).
I think, this not being SMP, doing what you just did tickled just about
everything there is.
There is one more potential issue with MMU-gather / TLB invalidate on
m68k (and a whole bunch of other archs) and I have patches for that
(although I now need to redo the m68k one.
Meanwhile the build robot gifted me with a build issue, and Will had
some nitpicks, so I'll go respin and repost these patches.
Peter,
On 30/01/20 9:16 PM, Peter Zijlstra wrote:
> Hi Michael,
>
> On Thu, Jan 30, 2020 at 08:31:13PM +1300, Michael Schmitz wrote:
>
>> Not much difference:
>>
>> total used free shared buffers cached
>> Mem: 10712 10120 592 0 1860 2276
>> -/+ buffers/cache: 5984 4728
>> Swap: 2097144 1552 2095592
>>
>>
>> vs. vanilla 5.5rc5:
>> total used free shared buffers cached
>> Mem: 10716 10104 612 0 1588 2544
>> -/+ buffers/cache: 5972 4744
>> Swap: 2097144 1296 2095848
>>
>> By sheer coincidence, the boot with your patch series happened to run a full
>> filesystem check on the root filesystem, so I'd say it got a good workout
>> re: paging and swapping (even though it's just a paltry 4 GB).
> Sweet!, can I translate this into a Tested-by: from you?
If the test coverage is sufficient, you may certainly do that.
Cheers,
Michael
>
>> Haven't tried any VM stress testing yet (not sure what to do for that; it's
>> been years since I tried that sort of stuff).
> I think, this not being SMP, doing what you just did tickled just about
> everything there is.
>
> There is one more potential issue with MMU-gather / TLB invalidate on
> m68k (and a whole bunch of other archs) and I have patches for that
> (although I now need to redo the m68k one.
>
> Meanwhile the build robot gifted me with a build issue, and Will had
> some nitpicks, so I'll go respin and repost these patches.
Hi Peter,
On 29/1/20 8:39 pm, Peter Zijlstra wrote:
> Hi!
>
> In order to faciliate Will's READ_ONCE() patches:
>
> https://lkml.kernel.org/r/[email protected]
>
> we need to fix m68k/motorola to not have a giant pmd_t. These patches do so and
> are tested using ARAnyM/68040.
>
> It would be very good if someone can either test or tell us what emulator to
> use for 020/030.
This series breaks compilation for the ColdFire (with MMU) variant of
the m68k family:
CC arch/m68k/kernel/sys_m68k.o
In file included from ./arch/m68k/include/asm/pgalloc.h:12,
from ./include/asm-generic/tlb.h:16,
from ./arch/m68k/include/asm/tlb.h:5,
from arch/m68k/kernel/sys_m68k.c:35:
./arch/m68k/include/asm/mcf_pgalloc.h: In function ‘__pte_free_tlb’:
./arch/m68k/include/asm/mcf_pgalloc.h:41:24: error: passing argument 1 of ‘pgtable_pte_page_dtor’ from incompatible pointer type [-Werror=incompatible-pointer-types]
pgtable_pte_page_dtor(page);
^~~~
In file included from arch/m68k/kernel/sys_m68k.c:13:
./include/linux/mm.h:1949:55: note: expected ‘struct page *’ but argument is of type ‘pgtable_t’ {aka ‘struct <anonymous> *’}
static inline void pgtable_pte_page_dtor(struct page *page)
~~~~~~~~~~~~~^~~~
In file included from ./include/linux/mm.h:10,
from arch/m68k/kernel/sys_m68k.c:13:
./include/linux/gfp.h:577:40: error: passing argument 1 of ‘__free_pages’ from incompatible pointer type [-Werror=incompatible-pointer-types]
#define __free_page(page) __free_pages((page), 0)
^~~~~~
./arch/m68k/include/asm/mcf_pgalloc.h:42:2: note: in expansion of macro ‘__free_page’
__free_page(page);
^~~~~~~~~~~
./include/linux/gfp.h:566:39: note: expected ‘struct page *’ but argument is of type ‘pgtable_t’ {aka ‘struct <anonymous> *’}
extern void __free_pages(struct page *page, unsigned int order);
~~~~~~~~~~~~~^~~~
cc1: some warnings being treated as errors
scripts/Makefile.build:265: recipe for target 'arch/m68k/kernel/sys_m68k.o' failed
Easy to reproduce. Build for the m5475evb_defconfig.
Regards
Greg
Hi Greg,
On Fri, Jan 31, 2020 at 04:31:48PM +1000, Greg Ungerer wrote:
> On 29/1/20 8:39 pm, Peter Zijlstra wrote:
> > In order to faciliate Will's READ_ONCE() patches:
> >
> > https://lkml.kernel.org/r/[email protected]
> >
> > we need to fix m68k/motorola to not have a giant pmd_t. These patches do so and
> > are tested using ARAnyM/68040.
> >
> > It would be very good if someone can either test or tell us what emulator to
> > use for 020/030.
>
> This series breaks compilation for the ColdFire (with MMU) variant of
> the m68k family:
[...]
> Easy to reproduce. Build for the m5475evb_defconfig.
I've hacked up a fix below, but I don't know how to test whether it actually
works (it does fix the build). However, I also notice that building for
m5475evb_defconfig with vanilla v5.5 triggers this scary looking warning
due to a mismatch between the pgd size and the (8k!) page size:
| In function 'pgd_alloc.isra.111',
| inlined from 'mm_alloc_pgd' at kernel/fork.c:634:12,
| inlined from 'mm_init.isra.112' at kernel/fork.c:1043:6:
| ./arch/m68k/include/asm/string.h:72:25: warning: '__builtin_memcpy' forming offset [4097, 8192] is out of the bounds [0, 4096] of object 'kernel_pg_dir' with type 'pgd_t[1024]' {aka 'struct <anonymous>[1024]'} [-Warray-bounds]
| #define memcpy(d, s, n) __builtin_memcpy(d, s, n)
| ^~~~~~~~~~~~~~~~~~~~~~~~~
| ./arch/m68k/include/asm/mcf_pgalloc.h:93:2: note: in expansion of macro 'memcpy'
| memcpy(new_pgd, swapper_pg_dir, PAGE_SIZE);
| ^~~~~~
I think the correct fix is to add this:
diff --git a/arch/m68k/include/asm/mcf_pgalloc.h b/arch/m68k/include/asm/mcf_pgalloc.h
index 82ec54c2eaa4..c335e6a381a1 100644
--- a/arch/m68k/include/asm/mcf_pgalloc.h
+++ b/arch/m68k/include/asm/mcf_pgalloc.h
@@ -90,7 +90,7 @@ static inline pgd_t *pgd_alloc(struct mm_struct *mm)
new_pgd = (pgd_t *)__get_free_page(GFP_DMA | __GFP_NOWARN);
if (!new_pgd)
return NULL;
- memcpy(new_pgd, swapper_pg_dir, PAGE_SIZE);
+ memcpy(new_pgd, swapper_pg_dir, PTRS_PER_PGD * sizeof(pgd_t));
memset(new_pgd, 0, PAGE_OFFSET >> PGDIR_SHIFT);
return new_pgd;
}
but maybe it should be done as a separate patch give that it's not caused
by the rework we've been doing.
Will
--->8
diff --git a/arch/m68k/include/asm/mcf_pgalloc.h b/arch/m68k/include/asm/mcf_pgalloc.h
index 82ec54c2eaa4..955d54a6e973 100644
--- a/arch/m68k/include/asm/mcf_pgalloc.h
+++ b/arch/m68k/include/asm/mcf_pgalloc.h
@@ -28,21 +28,22 @@ extern inline pmd_t *pmd_alloc_kernel(pgd_t *pgd, unsigned long address)
return (pmd_t *) pgd;
}
-#define pmd_populate(mm, pmd, page) (pmd_val(*pmd) = \
- (unsigned long)(page_address(page)))
+#define pmd_populate(mm, pmd, pte) (pmd_val(*pmd) = (unsigned long)(pte))
-#define pmd_populate_kernel(mm, pmd, pte) (pmd_val(*pmd) = (unsigned long)(pte))
+#define pmd_populate_kernel pmd_populate
-#define pmd_pgtable(pmd) pmd_page(pmd)
+#define pmd_pgtable(pmd) pfn_to_virt(pmd_val(pmd) >> PAGE_SHIFT)
-static inline void __pte_free_tlb(struct mmu_gather *tlb, pgtable_t page,
+static inline void __pte_free_tlb(struct mmu_gather *tlb, pgtable_t pgtable,
unsigned long address)
{
+ struct page *page = virt_to_page(pgtable);
+
pgtable_pte_page_dtor(page);
__free_page(page);
}
-static inline struct page *pte_alloc_one(struct mm_struct *mm)
+static inline pgtable_t pte_alloc_one(struct mm_struct *mm)
{
struct page *page = alloc_pages(GFP_DMA, 0);
pte_t *pte;
@@ -54,20 +55,19 @@ static inline struct page *pte_alloc_one(struct mm_struct *mm)
return NULL;
}
- pte = kmap(page);
- if (pte) {
- clear_page(pte);
- __flush_page_to_ram(pte);
- flush_tlb_kernel_page(pte);
- nocache_page(pte);
- }
- kunmap(page);
+ pte = page_address(page);
+ clear_page(pte);
+ __flush_page_to_ram(pte);
+ flush_tlb_kernel_page(pte);
+ nocache_page(pte);
- return page;
+ return pte;
}
-static inline void pte_free(struct mm_struct *mm, struct page *page)
+static inline void pte_free(struct mm_struct *mm, pgtable_t pgtable)
{
+ struct page *page = virt_to_page(pgtable);
+
pgtable_pte_page_dtor(page);
__free_page(page);
}
On Fri, Jan 31, 2020 at 09:38:13AM +0000, Will Deacon wrote:
> > This series breaks compilation for the ColdFire (with MMU) variant of
> > the m68k family:
That's like the same I had reported by the build robots for sun3, which
I fixed by frobbing pgtable_t. That said, this is probably a more
consistent change.
One note below:
> -static inline struct page *pte_alloc_one(struct mm_struct *mm)
> +static inline pgtable_t pte_alloc_one(struct mm_struct *mm)
> {
> struct page *page = alloc_pages(GFP_DMA, 0);
> pte_t *pte;
> @@ -54,20 +55,19 @@ static inline struct page *pte_alloc_one(struct mm_struct *mm)
> return NULL;
> }
>
> - pte = kmap(page);
> - if (pte) {
> - clear_page(pte);
> - __flush_page_to_ram(pte);
> - flush_tlb_kernel_page(pte);
> - nocache_page(pte);
> - }
> - kunmap(page);
> + pte = page_address(page);
> + clear_page(pte);
> + __flush_page_to_ram(pte);
> + flush_tlb_kernel_page(pte);
> + nocache_page(pte);
See how it does the nocache dance ^
>
> - return page;
> + return pte;
> }
>
> -static inline void pte_free(struct mm_struct *mm, struct page *page)
> +static inline void pte_free(struct mm_struct *mm, pgtable_t pgtable)
> {
> + struct page *page = virt_to_page(pgtable);
> +
but never sets it cached again!
> pgtable_pte_page_dtor(page);
> __free_page(page);
> }
Also, the alloc_one_kernel() also suspicioudly doesn't do the nocache
thing.
So either, alloc_one() shouldn't either, or it's all buggered.
On Fri, Jan 31, 2020 at 11:22:39AM +0100, Peter Zijlstra wrote:
> On Fri, Jan 31, 2020 at 09:38:13AM +0000, Will Deacon wrote:
>
> > > This series breaks compilation for the ColdFire (with MMU) variant of
> > > the m68k family:
>
> That's like the same I had reported by the build robots for sun3, which
> I fixed by frobbing pgtable_t. That said, this is probably a more
> consistent change.
>
> One note below:
>
>
> > -static inline struct page *pte_alloc_one(struct mm_struct *mm)
> > +static inline pgtable_t pte_alloc_one(struct mm_struct *mm)
> > {
> > struct page *page = alloc_pages(GFP_DMA, 0);
> > pte_t *pte;
> > @@ -54,20 +55,19 @@ static inline struct page *pte_alloc_one(struct mm_struct *mm)
> > return NULL;
> > }
> >
> > - pte = kmap(page);
> > - if (pte) {
> > - clear_page(pte);
> > - __flush_page_to_ram(pte);
> > - flush_tlb_kernel_page(pte);
> > - nocache_page(pte);
> > - }
> > - kunmap(page);
> > + pte = page_address(page);
> > + clear_page(pte);
> > + __flush_page_to_ram(pte);
> > + flush_tlb_kernel_page(pte);
> > + nocache_page(pte);
>
> See how it does the nocache dance ^
> So either, alloc_one() shouldn't either, or it's all buggered.
Damn, we weren't going to touch coldfire! :-))
So now I found the coldfire docs, and it looks like this thing is a
software tlb-miss arch, so there is no reason what so ever for this to
be nocache. I'll 'fix' that.
On Fri, Jan 31, 2020 at 12:14:59PM +0100, Peter Zijlstra wrote:
> On Fri, Jan 31, 2020 at 11:22:39AM +0100, Peter Zijlstra wrote:
> > On Fri, Jan 31, 2020 at 09:38:13AM +0000, Will Deacon wrote:
> >
> > > > This series breaks compilation for the ColdFire (with MMU) variant of
> > > > the m68k family:
> >
> > That's like the same I had reported by the build robots for sun3, which
> > I fixed by frobbing pgtable_t. That said, this is probably a more
> > consistent change.
> >
> > One note below:
> >
> >
> > > -static inline struct page *pte_alloc_one(struct mm_struct *mm)
> > > +static inline pgtable_t pte_alloc_one(struct mm_struct *mm)
> > > {
> > > struct page *page = alloc_pages(GFP_DMA, 0);
> > > pte_t *pte;
> > > @@ -54,20 +55,19 @@ static inline struct page *pte_alloc_one(struct mm_struct *mm)
> > > return NULL;
> > > }
> > >
> > > - pte = kmap(page);
> > > - if (pte) {
> > > - clear_page(pte);
> > > - __flush_page_to_ram(pte);
> > > - flush_tlb_kernel_page(pte);
> > > - nocache_page(pte);
> > > - }
> > > - kunmap(page);
> > > + pte = page_address(page);
> > > + clear_page(pte);
> > > + __flush_page_to_ram(pte);
> > > + flush_tlb_kernel_page(pte);
> > > + nocache_page(pte);
> >
> > See how it does the nocache dance ^
>
> > So either, alloc_one() shouldn't either, or it's all buggered.
>
> Damn, we weren't going to touch coldfire! :-))
>
> So now I found the coldfire docs, and it looks like this thing is a
> software tlb-miss arch, so there is no reason what so ever for this to
> be nocache. I'll 'fix' that.
Does that mean we can drop the GFP_DMA too? If so, this all ends up
looking very similar to the sun3 code wrt alloc/free and they could
probably use the same implementation (since the generic code doesn't
like out pgtable_t definition).
Will
On Fri, Jan 31, 2020 at 11:18:24AM +0000, Will Deacon wrote:
> > > > +static inline pgtable_t pte_alloc_one(struct mm_struct *mm)
> > > > {
> > > > struct page *page = alloc_pages(GFP_DMA, 0);
> > > > pte_t *pte;
> Does that mean we can drop the GFP_DMA too? If so, this all ends up
> looking very similar to the sun3 code wrt alloc/free and they could
> probably use the same implementation (since the generic code doesn't
> like out pgtable_t definition).
Many software TLB archs have limits on what memory the TLB miss handler
itself can access (chicken-egg issues), it might be this is where the
GFP_DMA comes from.
I can't quickly find this in the CFV4e docs, but I'm not really reading
it carefully either.
On Fri, Jan 31, 2020 at 12:31:39PM +0100, Peter Zijlstra wrote:
> On Fri, Jan 31, 2020 at 11:18:24AM +0000, Will Deacon wrote:
> > > > > +static inline pgtable_t pte_alloc_one(struct mm_struct *mm)
> > > > > {
> > > > > struct page *page = alloc_pages(GFP_DMA, 0);
> > > > > pte_t *pte;
>
> > Does that mean we can drop the GFP_DMA too? If so, this all ends up
> > looking very similar to the sun3 code wrt alloc/free and they could
> > probably use the same implementation (since the generic code doesn't
> > like out pgtable_t definition).
>
> Many software TLB archs have limits on what memory the TLB miss handler
> itself can access (chicken-egg issues), it might be this is where the
> GFP_DMA comes from.
Fair enough, that sounds plausible.
> I can't quickly find this in the CFV4e docs, but I'm not really reading
> it carefully either.
I can't find any code under arch/m68k/ which suggests it, but for now
I guess we should stick with the old pgtable_t definition for sun3 with
a comment (and keep the GFP_DMA in for coldfire).
Will
Hi Will,
On 31/1/20 7:38 pm, Will Deacon wrote:
> On Fri, Jan 31, 2020 at 04:31:48PM +1000, Greg Ungerer wrote:
>> On 29/1/20 8:39 pm, Peter Zijlstra wrote:
>>> In order to faciliate Will's READ_ONCE() patches:
>>>
>>> https://lkml.kernel.org/r/[email protected]
>>>
>>> we need to fix m68k/motorola to not have a giant pmd_t. These patches do so and
>>> are tested using ARAnyM/68040.
>>>
>>> It would be very good if someone can either test or tell us what emulator to
>>> use for 020/030.
>>
>> This series breaks compilation for the ColdFire (with MMU) variant of
>> the m68k family:
>
> [...]
>
>> Easy to reproduce. Build for the m5475evb_defconfig.
>
> I've hacked up a fix below, but I don't know how to test whether it actually
> works (it does fix the build).
Yep, I can confirm that too.
There is no emulators for the MMU based ColdFires (qemu only supports
a non-MMU variant).
I can test on real hardware - but not until Monday when I am back
in my lab. I'll report back then.
> However, I also notice that building for
> m5475evb_defconfig with vanilla v5.5 triggers this scary looking warning
> due to a mismatch between the pgd size and the (8k!) page size:
>
>
> | In function 'pgd_alloc.isra.111',
> | inlined from 'mm_alloc_pgd' at kernel/fork.c:634:12,
> | inlined from 'mm_init.isra.112' at kernel/fork.c:1043:6:
> | ./arch/m68k/include/asm/string.h:72:25: warning: '__builtin_memcpy' forming offset [4097, 8192] is out of the bounds [0, 4096] of object 'kernel_pg_dir' with type 'pgd_t[1024]' {aka 'struct <anonymous>[1024]'} [-Warray-bounds]
> | #define memcpy(d, s, n) __builtin_memcpy(d, s, n)
> | ^~~~~~~~~~~~~~~~~~~~~~~~~
> | ./arch/m68k/include/asm/mcf_pgalloc.h:93:2: note: in expansion of macro 'memcpy'
> | memcpy(new_pgd, swapper_pg_dir, PAGE_SIZE);
> | ^~~~~~
>
>
> I think the correct fix is to add this:
>
>
> diff --git a/arch/m68k/include/asm/mcf_pgalloc.h b/arch/m68k/include/asm/mcf_pgalloc.h
> index 82ec54c2eaa4..c335e6a381a1 100644
> --- a/arch/m68k/include/asm/mcf_pgalloc.h
> +++ b/arch/m68k/include/asm/mcf_pgalloc.h
> @@ -90,7 +90,7 @@ static inline pgd_t *pgd_alloc(struct mm_struct *mm)
> new_pgd = (pgd_t *)__get_free_page(GFP_DMA | __GFP_NOWARN);
> if (!new_pgd)
> return NULL;
> - memcpy(new_pgd, swapper_pg_dir, PAGE_SIZE);
> + memcpy(new_pgd, swapper_pg_dir, PTRS_PER_PGD * sizeof(pgd_t));
> memset(new_pgd, 0, PAGE_OFFSET >> PGDIR_SHIFT);
> return new_pgd;
> }
>
>
> but maybe it should be done as a separate patch give that it's not caused
> by the rework we've been doing.
Indeed I hadn't noticed that before. But good idea, a separate patch
would make sense.
Regards
Greg
> Will
>
> --->8
>
> diff --git a/arch/m68k/include/asm/mcf_pgalloc.h b/arch/m68k/include/asm/mcf_pgalloc.h
> index 82ec54c2eaa4..955d54a6e973 100644
> --- a/arch/m68k/include/asm/mcf_pgalloc.h
> +++ b/arch/m68k/include/asm/mcf_pgalloc.h
> @@ -28,21 +28,22 @@ extern inline pmd_t *pmd_alloc_kernel(pgd_t *pgd, unsigned long address)
> return (pmd_t *) pgd;
> }
>
> -#define pmd_populate(mm, pmd, page) (pmd_val(*pmd) = \
> - (unsigned long)(page_address(page)))
> +#define pmd_populate(mm, pmd, pte) (pmd_val(*pmd) = (unsigned long)(pte))
>
> -#define pmd_populate_kernel(mm, pmd, pte) (pmd_val(*pmd) = (unsigned long)(pte))
> +#define pmd_populate_kernel pmd_populate
>
> -#define pmd_pgtable(pmd) pmd_page(pmd)
> +#define pmd_pgtable(pmd) pfn_to_virt(pmd_val(pmd) >> PAGE_SHIFT)
>
> -static inline void __pte_free_tlb(struct mmu_gather *tlb, pgtable_t page,
> +static inline void __pte_free_tlb(struct mmu_gather *tlb, pgtable_t pgtable,
> unsigned long address)
> {
> + struct page *page = virt_to_page(pgtable);
> +
> pgtable_pte_page_dtor(page);
> __free_page(page);
> }
>
> -static inline struct page *pte_alloc_one(struct mm_struct *mm)
> +static inline pgtable_t pte_alloc_one(struct mm_struct *mm)
> {
> struct page *page = alloc_pages(GFP_DMA, 0);
> pte_t *pte;
> @@ -54,20 +55,19 @@ static inline struct page *pte_alloc_one(struct mm_struct *mm)
> return NULL;
> }
>
> - pte = kmap(page);
> - if (pte) {
> - clear_page(pte);
> - __flush_page_to_ram(pte);
> - flush_tlb_kernel_page(pte);
> - nocache_page(pte);
> - }
> - kunmap(page);
> + pte = page_address(page);
> + clear_page(pte);
> + __flush_page_to_ram(pte);
> + flush_tlb_kernel_page(pte);
> + nocache_page(pte);
>
> - return page;
> + return pte;
> }
>
> -static inline void pte_free(struct mm_struct *mm, struct page *page)
> +static inline void pte_free(struct mm_struct *mm, pgtable_t pgtable)
> {
> + struct page *page = virt_to_page(pgtable);
> +
> pgtable_pte_page_dtor(page);
> __free_page(page);
> }
>