[Xen-devel] Re: An issue in xen_limit_pages_to_max_mfn() in Xenlinux Ver. 2.6.18

[Daniel Kiper <dkiper@xxxxxxxxxxxx>]

On Thu, Nov 10, 2011 at 10:27:57AM +0000, Jan Beulich wrote:
> >>> On 10.11.11 at 09:58, Daniel Kiper <dkiper@xxxxxxxxxxxx> wrote:
> > Hi Jan,
> >
> > During work on kexec/kdump for Xen domU I found that
> > xen_limit_pages_to_max_mfn() registers undo_limit_pages()
> > destructor which breaks __free_pages(). When __free_pages()
> > is called then at beginning of this function put_page_testzero()
> > is called which decrements page count for given page. Later
> > undo_limit_pages() destructor is called which once again
> > calls __free_pages() and in consequence put_page_testzero()
> > fails (BUG_ON() is called) because page count is 0.
>
> Seems like (on newer kernels, where this is a VM_BUG_ON()) this was
> never hit on a configuration with CONFIG_DEBUG_VM, and on the older
> kernels the function was never used for memory that would get freed
> later (only kexec uses it there).
>
> > It could
> > be easily fixed, however, after reviewing xen_limit_pages_to_max_mfn()
> > I could not find any good reason for which undo_limit_pages()
> > destructor is registered. Maybe it could be removed at all because
> > all pages are freed when __free_pages() is called and in this
> > case we do not care where they live. However, maybe I missed
> > something important.
>
> It does matter - otherwise, over time, we could exhaust memory
> below a certain boundary in Xen.

Ahhhh... I thought about that once, however, I could not find it
in the code. It is not clear because there is no any comment. As
I understand HYPERVISOR_memory_op(XENMEM_exchange, &exchange) prefers
to allocate pages from Xen which live on higher addresses (at the
end of physical memory) if exchange.out.address_bits is 0. This
behavior leads to situation where pages with higher addresses
are more prefered than with lower addresses. In consequence pages
are moved from lower MFNs to higher MFNs.

Please correct me if I am wrong.

> So I think we need to add an init_page_count() call to undo_limit_pages().

Hmmm... I think it is not good solution in that point. I suppose that you
found this usage in balloon driver, however, here it is not the case.
Balloon driver initializes page structure for pages allocated from Xen
by calling init_page_count() (inter alia). It means that it is used
more or less accordingly to a comment in include/linux/mm.h (Setup the
page count before being freed into the page allocator for the first time
(boot or memory hotplug)). However, I think we should not simply reset
page count in undo_limit_pages(). It could lead to unexpected results.

Now I think about two solutions for that problem:
  1) We could simply remove undo_limit_pages() destructor and move 
responsibility
     of pages relocation before freeing them to the caller. I prefer that 
solution,
     because it gives more flexibility to the developer and similar solution
     must/will be used in latest kernels (PageForeign mechanism is not available
     there or I missed something).

  2) We could prepare new function, e.g.

     fastcall void __free_pages_core(struct page *page, unsigned int order) {
             if (order == 0)
                     free_hot_page(page);
             else
                     __free_pages_ok(page, order);
     }

     and call it from undo_limit_pages() destructor instead of __free_pages().

What do you think about that ???

Daniel

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel