Re: [patch 1/6] Guest page hinting: core + volatile page cache.

From: Rusty Russell
Date: Wed Mar 12 2008 - 19:13:20 EST


On Thursday 13 March 2008 00:21:33 Martin Schwidefsky wrote:
> @@ -957,6 +975,19 @@ struct page *follow_page(struct vm_area_
>
> if (flags & FOLL_GET)
> get_page(page);
> +
> + if (flags & FOLL_GET) {
> + /*
> + * The page is made stable if a reference is acquired.
> + * If the caller does not get a reference it implies that
> + * the caller can deal with page faults in case the page
> + * is swapped out. In this case the caller can deal with
> + * discard faults as well.
> + */
> + if (unlikely(!page_make_stable(page)))
> + goto out_discard;
> + }

Dumb comment: seems like this if could be folded into the one above.

> + * Attempts to change the state of a page to volatile.
> + * If there is something preventing the state change the page stays
> + * int its current state.

Typo "int its current state".


> return NULL;
>
> pte = pte_offset_map(pmd, address);
> + ptl = pte_lockptr(mm, pmd);
> /* Make a quick check before getting the lock */
> +#ifndef CONFIG_PAGE_STATES
> + /*
> + * If the page table lock for this pte is taken we have to
> + * assume that someone might be mapping the page. To solve
> + * the race of a page discard vs. mapping the page we have
> + * to serialize the two operations by taking the lock,
> + * otherwise we end up with a pte for a page that has been
> + * removed from page cache by the discard fault handler.
> + */
> + if (!spin_is_locked(ptl))
> +#endif
> if (!pte_present(*pte)) {
> pte_unmap(pte);
> return NULL;
> }
>
> - ptl = pte_lockptr(mm, pmd);
> spin_lock(ptl);
> if (pte_present(*pte) && page_to_pfn(page) == pte_pfn(*pte)) {
> *ptlp = ptl;

Did you really mean ifndef here?

(BTW: I'm just reading through the code, not really understanding it, so
this is not a real review).

Cheers,
Rusty.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/