* Christoph Hellwig <hch(a)lst.de> wrote:
On Wed, Apr 01, 2015 at 05:25:22PM +0300, Boaz Harrosh wrote:
> pfn = PFN_DOWN(ei->addr + ei->size);
>
> - switch (ei->type) {
> - case E820_RAM:
> - case E820_PRAM:
> - case E820_RESERVED_KERN:
> - break;
> - default:
> + if (ei->type != E820_RAM && ei->type != E820_RESERVED_KERN)
> register_nosave_region(PFN_UP(ei->addr), pfn);
> - }
I guess this makes sense - if the content is persistent already we don't need
to save it.
> - if (e820.map[i].type != E820_RESERVED || res->start < (1ULL<<20)) {
> - if (e820.map[i].type != E820_PRAM)
> - res->flags |= IORESOURCE_BUSY;
> + if (((e820.map[i].type != E820_RESERVED) &&
> + (e820.map[i].type != E820_PRAM)) ||
> + res->start < (1ULL<<20)) {
So now we also trigger for PRAM regions under 1ULL<<20, was that the
intentional change? Honestly I don't really understand this 1ULL<<20
magic here even for the existing case. Guess this is magic from the
old ISA PC days?
> + res->flags |= IORESOURCE_BUSY;
Guess this is the real change, and I'd love to understand why this
makes a difference for you. IORESOURCE_BUSY is checked almost
never, and is intented to mean it's a driver mapping.
So assuming this works on your test setup I'm inclined to squash
Boaz's fixes into the original patch, assuming you see no outright bug
in them. Anything else can be done as delta improvements.
Agreed?
Thanks,
Ingo