| Message ID | 1445966960-31724-11-git-send-email-james.morse@arm.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
Hi Pavel, Rafael, Do you have any feedback on this patch ? It is fundamental to this series and affects Hibernate core code so if you have any feedback that would be much appreciated. Thanks a lot, Lorenzo On Tue, Oct 27, 2015 at 05:29:19PM +0000, James Morse wrote: > Some architectures require code written to memory as if it were data to be > 'cleaned' from any data caches so that the processor can fetch them as new > instructions. > > During resume from hibernate, the snapshot code copies some pages directly, > meaning these architectures do not get a chance to perform their cache > maintenance. Add a call to flush_icache_range(), which is provided by > architectures that require it, to perform the maintenance. > > This mirrors the kernel's behaviour when loading kernel modules and when > mapping executable pages to user space. > > Signed-off-by: James Morse <james.morse@arm.com> > --- > kernel/power/snapshot.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c > index 5235dd4e1e2f..139fc449ad75 100644 > --- a/kernel/power/snapshot.c > +++ b/kernel/power/snapshot.c > @@ -31,6 +31,7 @@ > #include <linux/ktime.h> > > #include <asm/uaccess.h> > +#include <asm/cacheflush.h> > #include <asm/mmu_context.h> > #include <asm/pgtable.h> > #include <asm/tlbflush.h> > @@ -1196,9 +1197,12 @@ static unsigned int count_data_pages(void) > static inline void do_copy_page(long *dst, long *src) > { > int n; > + unsigned long __maybe_unused start = (unsigned long)dst; > > for (n = PAGE_SIZE / sizeof(long); n; n--) > *dst++ = *src++; > + > + flush_icache_range(start, start+PAGE_SIZE); > } > > > -- > 2.1.4 > -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wednesday, November 11, 2015 11:40:39 AM Lorenzo Pieralisi wrote: > Hi Pavel, Rafael, > > Do you have any feedback on this patch ? > > It is fundamental to this series and affects Hibernate core code so if you > have any feedback that would be much appreciated. I'm really not familiar with the flush_icache_range() interface. What exactly does it do? Rafael > On Tue, Oct 27, 2015 at 05:29:19PM +0000, James Morse wrote: > > Some architectures require code written to memory as if it were data to be > > 'cleaned' from any data caches so that the processor can fetch them as new > > instructions. > > > > During resume from hibernate, the snapshot code copies some pages directly, > > meaning these architectures do not get a chance to perform their cache > > maintenance. Add a call to flush_icache_range(), which is provided by > > architectures that require it, to perform the maintenance. > > > > This mirrors the kernel's behaviour when loading kernel modules and when > > mapping executable pages to user space. > > > > Signed-off-by: James Morse <james.morse@arm.com> > > --- > > kernel/power/snapshot.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c > > index 5235dd4e1e2f..139fc449ad75 100644 > > --- a/kernel/power/snapshot.c > > +++ b/kernel/power/snapshot.c > > @@ -31,6 +31,7 @@ > > #include <linux/ktime.h> > > > > #include <asm/uaccess.h> > > +#include <asm/cacheflush.h> > > #include <asm/mmu_context.h> > > #include <asm/pgtable.h> > > #include <asm/tlbflush.h> > > @@ -1196,9 +1197,12 @@ static unsigned int count_data_pages(void) > > static inline void do_copy_page(long *dst, long *src) > > { > > int n; > > + unsigned long __maybe_unused start = (unsigned long)dst; > > > > for (n = PAGE_SIZE / sizeof(long); n; n--) > > *dst++ = *src++; > > + > > + flush_icache_range(start, start+PAGE_SIZE); > > } > > > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-pm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, > -----Original Message----- > From: linux-pm-owner@vger.kernel.org [mailto:linux-pm- > owner@vger.kernel.org] On Behalf Of Lorenzo Pieralisi > Sent: Wednesday, November 11, 2015 7:41 PM > To: James Morse; Rafael J. Wysocki; Pavel Machek > Cc: linux-arm-kernel@lists.infradead.org; linux-pm@vger.kernel.org; Will > Deacon; Sudeep Holla; Kevin Kang; Geoff Levand; Catalin Marinas; Mark > Rutland; AKASHI Takahiro; wangfei; Marc Zyngier > Subject: Re: [PATCH v2 10/11] PM / Hibernate: clean cached pages on > architectures that require it > > Hi Pavel, Rafael, > > Do you have any feedback on this patch ? > > It is fundamental to this series and affects Hibernate core code so if you have > any feedback that would be much appreciated. > > Thanks a lot, > Lorenzo > > On Tue, Oct 27, 2015 at 05:29:19PM +0000, James Morse wrote: > > Some architectures require code written to memory as if it were data > > to be 'cleaned' from any data caches so that the processor can fetch > > them as new instructions. > > > > During resume from hibernate, the snapshot code copies some pages > > directly, meaning these architectures do not get a chance to perform > > their cache maintenance. Add a call to flush_icache_range(), which is > > provided by architectures that require it, to perform the maintenance. > > > > This mirrors the kernel's behaviour when loading kernel modules and > > when mapping executable pages to user space. > > > > Signed-off-by: James Morse <james.morse@arm.com> > > --- > > kernel/power/snapshot.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c index > > 5235dd4e1e2f..139fc449ad75 100644 > > --- a/kernel/power/snapshot.c > > +++ b/kernel/power/snapshot.c > > @@ -31,6 +31,7 @@ > > #include <linux/ktime.h> > > > > #include <asm/uaccess.h> > > +#include <asm/cacheflush.h> > > #include <asm/mmu_context.h> > > #include <asm/pgtable.h> > > #include <asm/tlbflush.h> > > @@ -1196,9 +1197,12 @@ static unsigned int count_data_pages(void) > > static inline void do_copy_page(long *dst, long *src) { > > int n; > > + unsigned long __maybe_unused start = (unsigned long)dst; > > > > for (n = PAGE_SIZE / sizeof(long); n; n--) > > *dst++ = *src++; > > + > > + flush_icache_range(start, start+PAGE_SIZE); > > } How about invalid all icache lines before doing do_copy_page, since do_copy_page might deal with both data pages and execute pages, that might be redundant to do it for every page? Yu -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Nov 12, 2015 at 01:48:32AM +0100, Rafael J. Wysocki wrote: > On Wednesday, November 11, 2015 11:40:39 AM Lorenzo Pieralisi wrote: > > Hi Pavel, Rafael, > > > > Do you have any feedback on this patch ? > > > > It is fundamental to this series and affects Hibernate core code so if you > > have any feedback that would be much appreciated. > > I'm really not familiar with the flush_icache_range() interface. > > What exactly does it do? It is used to sync a memory range that is written into (eg loading modules, copying from snapshot is basically the same thing, reads from storage and restore pages that might well be executable code), in particular to sync the I-cache and the D-cache, eg on arm64 the page that the snapshot code is copying might be executable code that has to be cleaned from the D-cache so that it is made visible to the I-cache. On x86 it is a NOP AFAIK. Thanks, Lorenzo > > Rafael > > > > On Tue, Oct 27, 2015 at 05:29:19PM +0000, James Morse wrote: > > > Some architectures require code written to memory as if it were data to be > > > 'cleaned' from any data caches so that the processor can fetch them as new > > > instructions. > > > > > > During resume from hibernate, the snapshot code copies some pages directly, > > > meaning these architectures do not get a chance to perform their cache > > > maintenance. Add a call to flush_icache_range(), which is provided by > > > architectures that require it, to perform the maintenance. > > > > > > This mirrors the kernel's behaviour when loading kernel modules and when > > > mapping executable pages to user space. > > > > > > Signed-off-by: James Morse <james.morse@arm.com> > > > --- > > > kernel/power/snapshot.c | 4 ++++ > > > 1 file changed, 4 insertions(+) > > > > > > diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c > > > index 5235dd4e1e2f..139fc449ad75 100644 > > > --- a/kernel/power/snapshot.c > > > +++ b/kernel/power/snapshot.c > > > @@ -31,6 +31,7 @@ > > > #include <linux/ktime.h> > > > > > > #include <asm/uaccess.h> > > > +#include <asm/cacheflush.h> > > > #include <asm/mmu_context.h> > > > #include <asm/pgtable.h> > > > #include <asm/tlbflush.h> > > > @@ -1196,9 +1197,12 @@ static unsigned int count_data_pages(void) > > > static inline void do_copy_page(long *dst, long *src) > > > { > > > int n; > > > + unsigned long __maybe_unused start = (unsigned long)dst; > > > > > > for (n = PAGE_SIZE / sizeof(long); n; n--) > > > *dst++ = *src++; > > > + > > > + flush_icache_range(start, start+PAGE_SIZE); > > > } > > > > > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-pm" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > -- > I speak only for myself. > Rafael J. Wysocki, Intel Open Source Technology Center. > -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Nov 12, 2015 at 02:53:35AM +0000, Chen, Yu C wrote: [...] > > > static inline void do_copy_page(long *dst, long *src) { > > > int n; > > > + unsigned long __maybe_unused start = (unsigned long)dst; > > > > > > for (n = PAGE_SIZE / sizeof(long); n; n--) > > > *dst++ = *src++; > > > + > > > + flush_icache_range(start, start+PAGE_SIZE); > > > } > How about invalid all icache lines before doing do_copy_page, since > do_copy_page might deal with both data pages and execute pages, that might be > redundant to do it for every page? The point here is to make sure I-cache and D-cache are in sync, because the page we are copying can be executable code, so flush_icache_range is there to make sure that the code is cleaned from the D-cache to a cache level that is visible to the I-cache (yes, the I-cache range is invalidate too in the process). I agree it is redundant for data pages, the point is, you do not really know what you are copying at this stage, we can extend the snapshot code to flag the pages accordingly, but it might well be overkill, so we posted this patch to get consensus before proceeding. Thanks, Lorenzo -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thursday, November 12, 2015 11:47:05 AM Lorenzo Pieralisi wrote: > On Thu, Nov 12, 2015 at 01:48:32AM +0100, Rafael J. Wysocki wrote: > > On Wednesday, November 11, 2015 11:40:39 AM Lorenzo Pieralisi wrote: > > > Hi Pavel, Rafael, > > > > > > Do you have any feedback on this patch ? > > > > > > It is fundamental to this series and affects Hibernate core code so if you > > > have any feedback that would be much appreciated. > > > > I'm really not familiar with the flush_icache_range() interface. > > > > What exactly does it do? > > It is used to sync a memory range that is written into (eg loading > modules, copying from snapshot is basically the same thing, reads from > storage and restore pages that might well be executable code), in particular > to sync the I-cache and the D-cache, eg on arm64 the page that the snapshot > code is copying might be executable code that has to be cleaned from the > D-cache so that it is made visible to the I-cache. > > On x86 it is a NOP AFAIK. If that's the case, I have no problems with this change as long as the code works on architectures with non-trivial flush_icache_range(). Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue 2015-10-27 17:29:19, James Morse wrote: > Some architectures require code written to memory as if it were data to be > 'cleaned' from any data caches so that the processor can fetch them as new > instructions. > > During resume from hibernate, the snapshot code copies some pages directly, > meaning these architectures do not get a chance to perform their cache > maintenance. Add a call to flush_icache_range(), which is provided by > architectures that require it, to perform the maintenance. > > This mirrors the kernel's behaviour when loading kernel modules and when > mapping executable pages to user space. > > Signed-off-by: James Morse <james.morse@arm.com> Looks reasonable. Acked-by: Pavel Machek <pavel@ucw.cz> > --- > kernel/power/snapshot.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c > index 5235dd4e1e2f..139fc449ad75 100644 > --- a/kernel/power/snapshot.c > +++ b/kernel/power/snapshot.c > @@ -1196,9 +1197,12 @@ static unsigned int count_data_pages(void) > static inline void do_copy_page(long *dst, long *src) > { > int n; > + unsigned long __maybe_unused start = (unsigned long)dst; Why the "maybe unused"? > for (n = PAGE_SIZE / sizeof(long); n; n--) > *dst++ = *src++; > + > + flush_icache_range(start, start+PAGE_SIZE); > } Pavel
On 14/11/15 20:26, Pavel Machek wrote: > On Tue 2015-10-27 17:29:19, James Morse wrote: >> Some architectures require code written to memory as if it were data to be >> 'cleaned' from any data caches so that the processor can fetch them as new >> instructions. >> >> During resume from hibernate, the snapshot code copies some pages directly, >> meaning these architectures do not get a chance to perform their cache >> maintenance. Add a call to flush_icache_range(), which is provided by >> architectures that require it, to perform the maintenance. >> >> This mirrors the kernel's behaviour when loading kernel modules and when >> mapping executable pages to user space. >> >> Signed-off-by: James Morse <james.morse@arm.com> > > Looks reasonable. > > Acked-by: Pavel Machek <pavel@ucw.cz> Thanks! > >> --- >> kernel/power/snapshot.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c >> index 5235dd4e1e2f..139fc449ad75 100644 >> --- a/kernel/power/snapshot.c >> +++ b/kernel/power/snapshot.c >> @@ -1196,9 +1197,12 @@ static unsigned int count_data_pages(void) >> static inline void do_copy_page(long *dst, long *src) >> { >> int n; >> + unsigned long __maybe_unused start = (unsigned long)dst; > > Why the "maybe unused"? To avoid a build warning on x86_64, and any other architectures that don't use the arguments in their flush_icache_range() implementation. Without __maybe_unused, gcc 4.9.2, building for x86_64: > ../kernel/power/snapshot.c: In function ‘do_copy_page’: > ../kernel/power/snapshot.c:1200:16: warning: unused variable ‘start’ > [-Wunused-variable] > unsigned long start = (unsigned long)dst; >> for (n = PAGE_SIZE / sizeof(long); n; n--) >> *dst++ = *src++; >> + >> + flush_icache_range(start, start+PAGE_SIZE); >> } Thanks, James -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon 2015-11-16 12:27:07, James Morse wrote: > On 14/11/15 20:26, Pavel Machek wrote: > > On Tue 2015-10-27 17:29:19, James Morse wrote: > >> Some architectures require code written to memory as if it were data to be > >> 'cleaned' from any data caches so that the processor can fetch them as new > >> instructions. > >> > >> During resume from hibernate, the snapshot code copies some pages directly, > >> meaning these architectures do not get a chance to perform their cache > >> maintenance. Add a call to flush_icache_range(), which is provided by > >> architectures that require it, to perform the maintenance. > >> > >> This mirrors the kernel's behaviour when loading kernel modules and when > >> mapping executable pages to user space. > >> > >> Signed-off-by: James Morse <james.morse@arm.com> > > > > Looks reasonable. > > > > Acked-by: Pavel Machek <pavel@ucw.cz> > > Thanks! > > > > >> --- > >> kernel/power/snapshot.c | 4 ++++ > >> 1 file changed, 4 insertions(+) > >> > >> diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c > >> index 5235dd4e1e2f..139fc449ad75 100644 > >> --- a/kernel/power/snapshot.c > >> +++ b/kernel/power/snapshot.c > >> @@ -1196,9 +1197,12 @@ static unsigned int count_data_pages(void) > >> static inline void do_copy_page(long *dst, long *src) > >> { > >> int n; > >> + unsigned long __maybe_unused start = (unsigned long)dst; > > > > Why the "maybe unused"? > > To avoid a build warning on x86_64, and any other architectures that don't > use the arguments in their flush_icache_range() implementation. That's wrong fix, I believe. flush_icache_range() should use their arguments. We should not have all the callers caring about this. Pavel
[Cc'ed maintainers of affected arches] On Sat, Nov 14, 2015 at 12:38:50AM +0100, Rafael J. Wysocki wrote: > On Thursday, November 12, 2015 11:47:05 AM Lorenzo Pieralisi wrote: > > On Thu, Nov 12, 2015 at 01:48:32AM +0100, Rafael J. Wysocki wrote: > > > On Wednesday, November 11, 2015 11:40:39 AM Lorenzo Pieralisi wrote: > > > > Hi Pavel, Rafael, > > > > > > > > Do you have any feedback on this patch ? > > > > > > > > It is fundamental to this series and affects Hibernate core code so if you > > > > have any feedback that would be much appreciated. > > > > > > I'm really not familiar with the flush_icache_range() interface. > > > > > > What exactly does it do? > > > > It is used to sync a memory range that is written into (eg loading > > modules, copying from snapshot is basically the same thing, reads from > > storage and restore pages that might well be executable code), in particular > > to sync the I-cache and the D-cache, eg on arm64 the page that the snapshot > > code is copying might be executable code that has to be cleaned from the > > D-cache so that it is made visible to the I-cache. > > > > On x86 it is a NOP AFAIK. > > If that's the case, I have no problems with this change as long as the code > works on architectures with non-trivial flush_icache_range(). I Cc'ed the respective arches maintainers, it should work (it may make resuming a bit slower, owing to the cache syncing), problem is that we have no way of testing it on platforms other than arm/arm64. How do you want us to go on about this ? Should we add a config option to prevent calling flush_icache_range() on all platforms (where it is not a nop) ? Thanks a lot ! Lorenzo -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue 2015-11-17 12:38:07, Lorenzo Pieralisi wrote: > [Cc'ed maintainers of affected arches] > > On Sat, Nov 14, 2015 at 12:38:50AM +0100, Rafael J. Wysocki wrote: > > On Thursday, November 12, 2015 11:47:05 AM Lorenzo Pieralisi wrote: > > > On Thu, Nov 12, 2015 at 01:48:32AM +0100, Rafael J. Wysocki wrote: > > > > On Wednesday, November 11, 2015 11:40:39 AM Lorenzo Pieralisi wrote: > > > > > Hi Pavel, Rafael, > > > > > > > > > > Do you have any feedback on this patch ? > > > > > > > > > > It is fundamental to this series and affects Hibernate core code so if you > > > > > have any feedback that would be much appreciated. > > > > > > > > I'm really not familiar with the flush_icache_range() interface. > > > > > > > > What exactly does it do? > > > > > > It is used to sync a memory range that is written into (eg loading > > > modules, copying from snapshot is basically the same thing, reads from > > > storage and restore pages that might well be executable code), in particular > > > to sync the I-cache and the D-cache, eg on arm64 the page that the snapshot > > > code is copying might be executable code that has to be cleaned from the > > > D-cache so that it is made visible to the I-cache. > > > > > > On x86 it is a NOP AFAIK. > > > > If that's the case, I have no problems with this change as long as the code > > works on architectures with non-trivial flush_icache_range(). > > I Cc'ed the respective arches maintainers, it should work (it may > make resuming a bit slower, owing to the cache syncing), problem is > that we have no way of testing it on platforms other than arm/arm64. Sure you can find x86 and x86-64 machine near you? And as hibernation is only supported on x86 and arm, that should be ok. Or just merge it to the next and let the world do testing for you... > How do you want us to go on about this ? Should we add a config option > to prevent calling flush_icache_range() on all platforms (where it > is not a nop) ? Config option is definitely not an option ;-). Pavel
On Tue, Nov 17, 2015 at 02:13:45PM +0100, Pavel Machek wrote: > On Tue 2015-11-17 12:38:07, Lorenzo Pieralisi wrote: > > [Cc'ed maintainers of affected arches] > > > > On Sat, Nov 14, 2015 at 12:38:50AM +0100, Rafael J. Wysocki wrote: > > > On Thursday, November 12, 2015 11:47:05 AM Lorenzo Pieralisi wrote: > > > > On Thu, Nov 12, 2015 at 01:48:32AM +0100, Rafael J. Wysocki wrote: > > > > > On Wednesday, November 11, 2015 11:40:39 AM Lorenzo Pieralisi wrote: > > > > > > Hi Pavel, Rafael, > > > > > > > > > > > > Do you have any feedback on this patch ? > > > > > > > > > > > > It is fundamental to this series and affects Hibernate core code so if you > > > > > > have any feedback that would be much appreciated. > > > > > > > > > > I'm really not familiar with the flush_icache_range() interface. > > > > > > > > > > What exactly does it do? > > > > > > > > It is used to sync a memory range that is written into (eg loading > > > > modules, copying from snapshot is basically the same thing, reads from > > > > storage and restore pages that might well be executable code), in particular > > > > to sync the I-cache and the D-cache, eg on arm64 the page that the snapshot > > > > code is copying might be executable code that has to be cleaned from the > > > > D-cache so that it is made visible to the I-cache. > > > > > > > > On x86 it is a NOP AFAIK. > > > > > > If that's the case, I have no problems with this change as long as the code > > > works on architectures with non-trivial flush_icache_range(). > > > > I Cc'ed the respective arches maintainers, it should work (it may > > make resuming a bit slower, owing to the cache syncing), problem is > > that we have no way of testing it on platforms other than arm/arm64. > > Sure you can find x86 and x86-64 machine near you? Yes but on x86 this patch is a NOP so testing on it does not really add much. > And as hibernation is only supported on x86 and arm, that should be > ok. And what are the other arches swsusp_arch_{suspend/resume} there for then ? I just had a look at arch code implementing swsusp_arch_{suspend/resume}, if it has to work only on x86 and arm we will give it a spin on arm (32-bit) platforms (I really doubt it is going to be even noticeable) and we are done. > Or just merge it to the next and let the world do testing for you... I wanted to ask before because I am not sure that's something people regularly test and I do not want to end up with regressions that might have been prevented by just enquiring, I am not sure that's something -next will help detect either. > > How do you want us to go on about this ? Should we add a config option > > to prevent calling flush_icache_range() on all platforms (where it > > is not a nop) ? > > Config option is definitely not an option ;-). Ok, thanks. Lorenzo -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 27/10/15 17:29, James Morse wrote: > Some architectures require code written to memory as if it were data to be > 'cleaned' from any data caches so that the processor can fetch them as new > instructions. > > During resume from hibernate, the snapshot code copies some pages directly, > meaning these architectures do not get a chance to perform their cache > maintenance. Add a call to flush_icache_range(), which is provided by > architectures that require it, to perform the maintenance. > > This mirrors the kernel's behaviour when loading kernel modules and when > mapping executable pages to user space. While trying to benchmark the impact of this patch on 32bit ARM, I've discovered the fix is in the wrong place! do_copy_page() isn't used on the resume path for the pages restored 'in place'. I will produce another version of the series - hopefully later today. James -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c index 5235dd4e1e2f..139fc449ad75 100644 --- a/kernel/power/snapshot.c +++ b/kernel/power/snapshot.c @@ -31,6 +31,7 @@ #include <linux/ktime.h> #include <asm/uaccess.h> +#include <asm/cacheflush.h> #include <asm/mmu_context.h> #include <asm/pgtable.h> #include <asm/tlbflush.h> @@ -1196,9 +1197,12 @@ static unsigned int count_data_pages(void) static inline void do_copy_page(long *dst, long *src) { int n; + unsigned long __maybe_unused start = (unsigned long)dst; for (n = PAGE_SIZE / sizeof(long); n; n--) *dst++ = *src++; + + flush_icache_range(start, start+PAGE_SIZE); }
Some architectures require code written to memory as if it were data to be 'cleaned' from any data caches so that the processor can fetch them as new instructions. During resume from hibernate, the snapshot code copies some pages directly, meaning these architectures do not get a chance to perform their cache maintenance. Add a call to flush_icache_range(), which is provided by architectures that require it, to perform the maintenance. This mirrors the kernel's behaviour when loading kernel modules and when mapping executable pages to user space. Signed-off-by: James Morse <james.morse@arm.com> --- kernel/power/snapshot.c | 4 ++++ 1 file changed, 4 insertions(+)