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 from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
