Rafael J. Wysocki wrote:
> On Tuesday, 13 March 2007 05:47, Nick Piggin wrote:
>
>>Rafael J. Wysocki wrote:
>>
>>
>>> }
>>>
>>> /**
>>>+ * This structure represents a range of page frames the contents of which
>>>+ * should not be saved during the suspend.
>>>+ */
>>>+
>>>+struct nosave_region {
>>>+ struct list_head list;
>>>+ unsigned long start_pfn;
>>>+ unsigned long end_pfn;
>>>+};
>>>+
>>>+static LIST_HEAD(nosave_regions);
>>>+
>>>+/**
>>>+ * register_nosave_region - register a range of page frames the contents
>>>+ * of which should not be saved during the suspend (to be used in the early
>>>+ * initializatoion code)
>>>+ */
>>>+
>>>+void __init
>>>+register_nosave_region(unsigned long start_pfn, unsigned long end_pfn)
>>>+{
>>>+ struct nosave_region *region;
>>>+
>>>+ if (start_pfn >= end_pfn)
>>>+ return;
>>>+
>>>+ if (!list_empty(&nosave_regions)) {
>>>+ /* Try to extend the previous region (they should be sorted) */
>>>+ region = list_entry(nosave_regions.prev,
>>>+ struct nosave_region, list);
>>>+ if (region->end_pfn == start_pfn) {
>>>+ region->end_pfn = end_pfn;
>>>+ goto Report;
>>>+ }
>>>+ }
>>>+ /* This allocation cannot fail */
>>>+ region = alloc_bootmem_low(sizeof(struct nosave_region));
>>>+ region->start_pfn = start_pfn;
>>>+ region->end_pfn = end_pfn;
>>>+ list_add_tail(®ion->list, &nosave_regions);
>>>+ Report:
>>>+ printk("swsusp: Registered nosave memory region: %016lx - %016lx\n",
>>>+ start_pfn << PAGE_SHIFT, end_pfn << PAGE_SHIFT);
>>>+}
>>
>>
>>I wonder why you reimplemented this and put it in snapshot.c, rather than
>>use my version which was nicely in its own file, had appropriate locking,
>>etc.?
>
>
> Well, the locking is not necessary and we only need a list for that. Also,
I wouldn't say that. You're creating an interface here that is going to be
used outside swsusp. Users of that interface may not need locking now, but
that could cause problems down the line.
Sure you don't _need_ an rbtree, but our implementation makes it so simple
that there isn't much downside.
> mark_nosave_pages() refers to a function that's invisible outside snapshot.c
> and I didn't think it was a good idea to separate mark_nosave_pages()
> from register_nosave_region().
But that's because you even use mark_nosave_pages in your implementation.
Mine uses the nosave regions directly.
--
SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com
_______________________________________________
linux-pm mailing list
linux-pm@xxxxxxxxxxxxxx
https://lists.osdl.org/mailman/listinfo/linux-pm
