Messages in this thread | | | Date | Mon, 21 Nov 2011 11:19:31 +0000 | From | Mel Gorman <> | Subject | Re: [PATCH 4/5] mm: compaction: Determine if dirty pages can be migreated without blocking within ->migratepage |
| |
On Sat, Nov 19, 2011 at 04:59:10PM +0800, Nai Xia wrote: > > <SNIP> > > @@ -453,13 +494,18 @@ int buffer_migrate_page(struct address_space *mapping, > > if (rc) > > return rc; > > > > - bh = head; > > - do { > > - get_bh(bh); > > - lock_buffer(bh); > > - bh = bh->b_this_page; > > - > > - } while (bh != head); > > + if (!buffer_migrate_lock_buffers(head, sync)) { > > + /* > > + * We have to revert the radix tree update. If this returns > > + * non-zero, it either means that the page count changed > > + * which "can't happen" or the slot changed from underneath > > + * us in which case someone operated on a page that did not > > + * have buffers fully migrated which is alarming so warn > > + * that it happened. > > + */ > > + WARN_ON(migrate_page_move_mapping(mapping, page, newpage)); > > + return -EBUSY; > > If this migrate_page_move_mapping() really fails, seems disk IO will be needed > to bring the previously already cached page back,
Aside from that, I couldn't see a way of handling the case where the page had an elevated count due to a speculative lookup.
> I wonder if we should make the > double check for the two conditions of "page refs is ok " and "all bh > trylocked" > before doing radix_tree_replace_slot() ? which I think does not > involve IO on the > error path. >
I reached the same conclusion when figuring out how to backout of the the elevated page count case. In an updated patch, migrate_page_move_mapping() returns with buffers locked in the async case.
-- Mel Gorman SUSE Labs -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |