Messages in this thread | | | Date | Thu, 19 Aug 2010 09:57:52 +0800 | From | Wu Fengguang <> | Subject | Re: [PATCH 9/9] hugetlb: add corrupted hugepage counter |
| |
> +void increment_corrupted_huge_page(struct page *page); > +void decrement_corrupted_huge_page(struct page *page);
nitpick: increment/decrement are not verbs.
> +void increment_corrupted_huge_page(struct page *hpage) > +{ > + struct hstate *h = page_hstate(hpage); > + spin_lock(&hugetlb_lock); > + h->corrupted_huge_pages++; > + spin_unlock(&hugetlb_lock); > +} > + > +void decrement_corrupted_huge_page(struct page *hpage) > +{ > + struct hstate *h = page_hstate(hpage); > + spin_lock(&hugetlb_lock); > + BUG_ON(!h->corrupted_huge_pages);
There is no point to have BUG_ON() here:
/* * Don't use BUG() or BUG_ON() unless there's really no way out; one * example might be detecting data structure corruption in the middle * of an operation that can't be backed out of. If the (sub)system * can somehow continue operating, perhaps with reduced functionality, * it's probably not BUG-worthy. * * If you're tempted to BUG(), think again: is completely giving up * really the *only* solution? There are usually better options, where * users don't need to reboot ASAP and can mostly shut down cleanly. */
And there is a race case that (corrupted_huge_pages==0)! Suppose the user space calls unpoison_memory() on a good pfn, and the page happen to be hwpoisoned between lock_page() and TestClearPageHWPoison(), corrupted_huge_pages will go negative.
Thanks, Fengguang
> + h->corrupted_huge_pages--; > + spin_unlock(&hugetlb_lock); > +}
| |