On Mon 07-06-10 18:09:03, Jan Kara wrote: > On Sat 05-06-10 11:38:02, Nick Piggin wrote: > > On Fri, Jun 04, 2010 at 08:47:11PM +0200, Jan Kara wrote: > > > done_index = index; > > > while (!done && (index <= end)) { > > > int i; > > > > > > - nr_pages = pagevec_lookup_tag(&pvec, mapping, &index, > > > - PAGECACHE_TAG_DIRTY, > > > + nr_pages = pagevec_lookup_tag(&pvec, mapping, &index, tag, > > > min(end - index, (pgoff_t)PAGEVEC_SIZE-1) + 1); > > > if (nr_pages == 0) > > > break; > > > > Would it be neat to clear the tag even if we didn't set page to > > writeback? It should be uncommon case. > Yeah, why not. Looking at this more, we shouldn't leave any TOWRITE tags dangling in WB_SYNC_ALL mode - as soon as someone writes the page, he does set_page_writeback() which clears the tag. Similarly if the page is removed from the mapping, the tag is cleared. Or am I missing something? > > > @@ -1319,6 +1356,9 @@ int test_set_page_writeback(struct page *page) > > > radix_tree_tag_clear(&mapping->page_tree, > > > page_index(page), > > > PAGECACHE_TAG_DIRTY); > > > + radix_tree_tag_clear(&mapping->page_tree, > > > + page_index(page), > > > + PAGECACHE_TAG_TOWRITE); > > > spin_unlock_irqrestore(&mapping->tree_lock, flags); > > > } else { > > > ret = TestSetPageWriteback(page); > > > > It would be nice to have bitwise tag clearing so we can clear multiple > > at once. Then > > > > clear_tag = PAGECACHE_TAG_TOWRITE; > > if (!PageDirty(page)) > > clear_tag |= PAGECACHE_TAG_DIRTY; > > > > That could reduce overhead a bit more. > Good idea. Will do. On a second thought, will it bring us enough to justify a new interface (which will be inconsistent with all the other radix tree interfaces because they use tag numbers and not bitmaps)? Because looking at the code, all we could save is the transformation of page index into a radix tree path. We would have to do all the other work for each tag separately anyway and it won't probably have any great cache locality either because radix trees for different tags are separate. Honza -- Jan Kara <jack@xxxxxxx> SUSE Labs, CR -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxxx For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx"> email@xxxxxxxxx </a>