Hi Pavel,

On 08/12/15 08:19, Pavel Machek wrote:

quoted

I didn't do it here as it would clean every page copied, which was the
worrying part of the previous approach. If there is an architecture
where this cache-clean operation is expensive, it would slow down
restore. I was trying to benchmark the impact of this on 32bit arm when
I spotted it was broken.

You have just loaded the page from slow storage (hard drive,
MMC). Cleaning a page should be pretty fast compared to that.

(One day I hope to own a laptop that hibernates to almost-memory speed
nvram!)

Speed is one issue - another is I don't think its correct to assume that
any architecture with a flush_icache_range() function can/should have
that called on any page of data.

There is also the possibility that an architecture needs to do something
other than flush_icache_range() on the pages that were copied. (I can
see lots of s390 hooks for 'page keys', Intel's memory protection keys
may want something similar...)

This patch is the general-purpose fix, matching the existing list of
'these pages need copying' with a 'these pages were already copied'.

quoted

This allocated-same-page code path doesn't happen very often, so we
don't want this to have an impact on the 'normal' code path. On 32bit
arm I saw ~20 of these allocations out of ~60,000 pages.

This new way allocates a few extra pages during restore, and doesn't
assume that flush_cache_range() needs calling. It should have no impact
on architectures that aren't using the new list.

It is also complex.

Its symmetric with the existing restore_pblist code, I think that this
is the simplest way of doing it.

quoted

quoted

Alternatively, can you just clean the whole cache before jumping to
the new kernel?

On arm64, cleaning the whole cache means cleaning all of memory by
virtual address, which would be a high price to pay when we only need to
clean the pages we copied. The current implementation does clean all

How high price to pay? I mean, hibernation/restore takes
_seconds_. Paying miliseconds to have cleaner code is acceptable
price.

I agree, but the code to clean all 8GB of memory on Juno takes ~3
seconds, and this will probably scale linearly. We only need to clean
the ~250MB that was copied by hibernate, (and of that, only the
executable pages). The sticking point is the few pages it copies, but
doesn't tell us about.

I will put together the flush_icache_range()-during-decompression
version of this patch... it looks like powerpc will suffer the most from
this, from the comments, its flush_icache_range() code pushes data all
the way out to memory...


Thanks,

James