| Message ID | 20181218103420.38148-1-ldir@darbyshire-bryant.me.uk |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [OpenWrt-Devel,RFC] kernel: drop MIPS: fix cache flushing for highmem pages | expand |
On Tue, Dec 18, 2018 at 2:35 AM Kevin 'ldir' Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk> wrote: > > Signed-off-by: Kevin Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk> > --- > > This patch, in a variety of forms, has been around since beginning 2016 > as e756c2bb07, ending up in present form 0aa6c7df60 (kernel 4.4.13 bump) > and carried forward ever since. > > There have been a number of MIPS kernel memory handling changes since, > including VDSO fixes that meant openwrt patches have been dropped with > no apparent fallout. > > I'm basically wondering if this patch needs to still exist in the kernel > 4.14.88 world? I have been running without this patch for 3+ months on > Archer C7 v2 with no obvious ill effects (I'd expect to see "nasty > segfaults and kernel crashes") > > If it does still need to exist, should it go upstream? > > Thoughts, comments, more testers? I've tested removing the patch on a 512MB mt7621 device where HIGHMEM is used for the second 256MB. No issues. > > > ...fix-cache-flushing-for-highmem-pages.patch | 30 ------------------- > 1 file changed, 30 deletions(-) > delete mode 100644 target/linux/generic/pending-4.14/100-MIPS-fix-cache-flushing-for-highmem-pages.patch > > diff --git a/target/linux/generic/pending-4.14/100-MIPS-fix-cache-flushing-for-highmem-pages.patch b/target/linux/generic/pending-4.14/100-MIPS-fix-cache-flushing-for-highmem-pages.patch > deleted file mode 100644 > index b1c65f7cd8..0000000000 > --- a/target/linux/generic/pending-4.14/100-MIPS-fix-cache-flushing-for-highmem-pages.patch > +++ /dev/null > @@ -1,30 +0,0 @@ > -From: Felix Fietkau <nbd@nbd.name> > -Subject: MIPS: fix cache flushing for highmem pages > - > -Most cache flush ops were no-op for highmem pages. This led to nasty > -segfaults and (in the case of page_address(page) == NULL) kernel > -crashes. > - > -Fix this by always flushing highmem pages using kmap/kunmap_atomic > -around the actual cache flush. This might be a bit inefficient, but at > -least it's stable. > - > -Signed-off-by: Felix Fietkau <nbd@nbd.name> > ---- > - > ---- a/arch/mips/mm/cache.c > -+++ b/arch/mips/mm/cache.c > -@@ -116,6 +116,13 @@ void __flush_anon_page(struct page *page > - { > - unsigned long addr = (unsigned long) page_address(page); > - > -+ if (PageHighMem(page)) { > -+ addr = (unsigned long)kmap_atomic(page); > -+ flush_data_cache_page(addr); > -+ __kunmap_atomic((void *)addr); > -+ return; > -+ } > -+ > - if (pages_do_alias(addr, vmaddr)) { > - if (page_mapcount(page) && !Page_dcache_dirty(page)) { > - void *kaddr; > -- > 2.17.2 (Apple Git-113) > > > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/mailman/listinfo/openwrt-devel
On 2018-12-18 17:43, Rosen Penev wrote: > On Tue, Dec 18, 2018 at 2:35 AM Kevin 'ldir' Darbyshire-Bryant > <ldir@darbyshire-bryant.me.uk> wrote: >> >> Signed-off-by: Kevin Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk> >> --- >> >> This patch, in a variety of forms, has been around since beginning 2016 >> as e756c2bb07, ending up in present form 0aa6c7df60 (kernel 4.4.13 bump) >> and carried forward ever since. >> >> There have been a number of MIPS kernel memory handling changes since, >> including VDSO fixes that meant openwrt patches have been dropped with >> no apparent fallout. >> >> I'm basically wondering if this patch needs to still exist in the kernel >> 4.14.88 world? I have been running without this patch for 3+ months on >> Archer C7 v2 with no obvious ill effects (I'd expect to see "nasty >> segfaults and kernel crashes") >> >> If it does still need to exist, should it go upstream? >> >> Thoughts, comments, more testers? > I've tested removing the patch on a 512MB mt7621 device where HIGHMEM > is used for the second 256MB. No issues. I think to be on the safe side we should test running fuse (ntfs-3g or sshfs or something similar) on such a device. If that doesn't turn up any weird hangs or data corruption within a few minutes of use, I'm all for dropping this patch. Thanks, - Felix
> On 19 Dec 2018, at 09:57, Felix Fietkau <nbd@nbd.name> wrote: > > On 2018-12-18 17:43, Rosen Penev wrote: >>> >> I've tested removing the patch on a 512MB mt7621 device where HIGHMEM >> is used for the second 256MB. No issues. > I think to be on the safe side we should test running fuse (ntfs-3g or > sshfs or something similar) on such a device. If that doesn't turn up > any weird hangs or data corruption within a few minutes of use, I'm all > for dropping this patch. > Rosen, is that something you could take a closer look at with your 512MB device? Kevin
On Wed, Dec 19, 2018 at 9:54 AM Kevin 'ldir' Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk> wrote: > > > > > On 19 Dec 2018, at 09:57, Felix Fietkau <nbd@nbd.name> wrote: > > > > On 2018-12-18 17:43, Rosen Penev wrote: > >>> > >> I've tested removing the patch on a 512MB mt7621 device where HIGHMEM > >> is used for the second 256MB. No issues. > > I think to be on the safe side we should test running fuse (ntfs-3g or > > sshfs or something similar) on such a device. If that doesn't turn up > > any weird hangs or data corruption within a few minutes of use, I'm all > > for dropping this patch. > > > > Rosen, is that something you could take a closer look at with your 512MB device? Just ran a sha256sum on a video file followed by a 10 minute dd read of the whole drive(interrupted) followed by another sha256sum. They check out. This was on an ntfs formatted drive mounted by ntfs-3g. > > Kevin
diff --git a/target/linux/generic/pending-4.14/100-MIPS-fix-cache-flushing-for-highmem-pages.patch b/target/linux/generic/pending-4.14/100-MIPS-fix-cache-flushing-for-highmem-pages.patch deleted file mode 100644 index b1c65f7cd8..0000000000 --- a/target/linux/generic/pending-4.14/100-MIPS-fix-cache-flushing-for-highmem-pages.patch +++ /dev/null @@ -1,30 +0,0 @@ -From: Felix Fietkau <nbd@nbd.name> -Subject: MIPS: fix cache flushing for highmem pages - -Most cache flush ops were no-op for highmem pages. This led to nasty -segfaults and (in the case of page_address(page) == NULL) kernel -crashes. - -Fix this by always flushing highmem pages using kmap/kunmap_atomic -around the actual cache flush. This might be a bit inefficient, but at -least it's stable. - -Signed-off-by: Felix Fietkau <nbd@nbd.name> ---- - ---- a/arch/mips/mm/cache.c -+++ b/arch/mips/mm/cache.c -@@ -116,6 +116,13 @@ void __flush_anon_page(struct page *page - { - unsigned long addr = (unsigned long) page_address(page); - -+ if (PageHighMem(page)) { -+ addr = (unsigned long)kmap_atomic(page); -+ flush_data_cache_page(addr); -+ __kunmap_atomic((void *)addr); -+ return; -+ } -+ - if (pages_do_alias(addr, vmaddr)) { - if (page_mapcount(page) && !Page_dcache_dirty(page)) { - void *kaddr;
Signed-off-by: Kevin Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk> --- This patch, in a variety of forms, has been around since beginning 2016 as e756c2bb07, ending up in present form 0aa6c7df60 (kernel 4.4.13 bump) and carried forward ever since. There have been a number of MIPS kernel memory handling changes since, including VDSO fixes that meant openwrt patches have been dropped with no apparent fallout. I'm basically wondering if this patch needs to still exist in the kernel 4.14.88 world? I have been running without this patch for 3+ months on Archer C7 v2 with no obvious ill effects (I'd expect to see "nasty segfaults and kernel crashes") If it does still need to exist, should it go upstream? Thoughts, comments, more testers? ...fix-cache-flushing-for-highmem-pages.patch | 30 ------------------- 1 file changed, 30 deletions(-) delete mode 100644 target/linux/generic/pending-4.14/100-MIPS-fix-cache-flushing-for-highmem-pages.patch