On Tue, 6 August 2013 15:27:53 +0200, Vlastimil Babka wrote:
> On 08/05/2013 07:21 PM, Jörn Engel wrote:
> > On Mon, 5 August 2013 16:32:02 +0200, Vlastimil Babka wrote:
> >>
> >> /*
> >> + * Munlock a batch of pages from the same zone
> >> + *
> >> + * The work is split to two main phases. First phase clears the Mlocked flag
> >> + * and attempts to isolate the pages, all under a single zone lru lock.
> >> + * The second phase finishes the munlock only for pages where isolation
> >> + * succeeded.
> >> + */
> >> +static void __munlock_pagevec(struct pagevec *pvec, struct zone *zone)
> >> +{
> >> + int i;
> >> + int nr = pagevec_count(pvec);
> >> +
> >> + /* Phase 1: page isolation */
> >> + spin_lock_irq(&zone->lru_lock);
> >> + for (i = 0; i < nr; i++) {
> >> + struct page *page = pvec->pages[i];
> >> +
> >> + if (TestClearPageMlocked(page)) {
> >> + struct lruvec *lruvec;
> >> + int lru;
> >> +
> >> + /* we have disabled interrupts */
> >> + __mod_zone_page_state(zone, NR_MLOCK, -1);
> >> +
> >> + switch (__isolate_lru_page(page,
> >> + ISOLATE_UNEVICTABLE)) {
> >> + case 0:
> >> + lruvec = mem_cgroup_page_lruvec(page, zone);
> >> + lru = page_lru(page);
> >> + del_page_from_lru_list(page, lruvec, lru);
> >> + break;
> >> +
> >> + case -EINVAL:
> >> + __munlock_isolation_failed(page);
> >> + goto skip_munlock;
> >> +
> >> + default:
> >> + BUG();
> >> + }
> > more serious is that you don't handle -EBUSY gracefully. I guess you
> > would have to mlock() the empty zero page to excercise this code path.
>
> From what I see in the implementation, -EBUSY can only happen with flags
> that I don't use, or when get_page_unless_zero() fails. But it cannot
> fail since I already have get_page() from follow_page_mask(). (the
> function is about zero get_page() pins, not about being zero page).
You are right. Not sure if this should be explained in a comment in
the code as well.
> > + }
> > + if (PageTransHuge(page)) {
> > + /*
> > + * THP pages are not handled by pagevec due
> > + * to their possible split (see below).
> > + */
> > + if (pagevec_count(&pvec))
> > + __munlock_pagevec(&pvec, zone);
> > Should you re-initialize the pvec after this call?
> __munlock_pagevec() does it as the last thing
Right you are.
Jörn
--
The key to performance is elegance, not battalions of special cases.
-- Jon Bentley and Doug McIlroy
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>