![]() | |||||||||||||
Messages in this thread
|
On Thu, May 25, 2006 at 03:26:00PM +1000, Nick Piggin wrote:
> Wu Fengguang wrote:> >+ cond_resched();> >+ read_lock_irq(&mapping->tree_lock);> >+ index = radix_tree_scan_hole_backward(&mapping->page_tree,
> >+ offset, ra_max);> >+ read_unlock_irq(&mapping->tree_lock);> >> > Why do you drop this lock just to pick it up again a few instructions
> down the line? (is ra_cache_hit_ok or cound_cache_hit very big or
> unable to be called without the lock?)
Nice catch, will fix it.
> >+> >+ *remain = offset - index;> >+> >+ if (offset == ra->readahead_index && ra_cache_hit_ok(ra))
> >+ count = *remain;
> >+ else if (count_cache_hit(mapping, index + 1, offset) *
> >+ readahead_hit_rate >=
> >*remain)
> >+ count = *remain;
> >+ else
> >+ count = ra_min;
> >+
> >+ /*
> >+ * Unnecessary to count more?
> >+ */
> >+ if (count < ra_max)
> >+ goto out;
> >+
> >+ if (unlikely(ra->flags & RA_FLAG_NO_LOOKAHEAD))
> >+ goto out;
> >+
> >+ /*
> >+ * Check the far pages coarsely.
> >+ * The enlarged count here helps increase la_size.
> >+ */
> >+ nr_lookback = ra_max * (LOOKAHEAD_RATIO + 1) *
> >+ 100 / (readahead_ratio | 1);
> >+
> >+ cond_resched();
> >+ radix_tree_cache_init(&cache);
> >+ read_lock_irq(&mapping->tree_lock);
> >+ for (count += ra_max; count < nr_lookback; count += ra_max) {
> >+ struct radix_tree_node *node;
> >+ node = radix_tree_cache_lookup_parent(&mapping->page_tree,
> >+ &cache, offset - count, 1);
> >+ if (!node)
> >+ break;
> >+ }
> >+ read_unlock_irq(&mapping->tree_lock);
> >> > Yuck. Apart from not being commented, this depends on internal
> implementation of radix-tree. This should just be packaged up in some
> radix-tree function to do exactly what you want (eg. is there a hole of
> N contiguous pages).
Yes, it is ugly.
Maybe we can make it a function named radix_tree_scan_hole_coarse().
> And then again you can be rid of the radix-tree cache.
> > Yes, it increasingly appears that you're using the cache because you're
> using the wrong abstractions. Eg. this is basically half implementing
> some data-structure internal detail.
Sorry for not being aware of this problem :)
Wu
-
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/
| ||||||||||||
| Last update: 2006-05-25 08:05 [from the cache] ©2003-2008 | |||||||||||||