On Fri, Apr 20, 2018 at 07:00:57AM -0500, Goldwyn Rodrigues wrote:
> On 04/14/2018 09:12 AM, Matthew Wilcox wrote:
> >
> > - for (i = 0; i < nr; i++) {
> > - struct radix_tree_node *node;
> > - void **slot;
> > -
> > - __radix_tree_lookup(&mapping->i_pages, page->index + i,
> > - &node, &slot);
> > -
> > - VM_BUG_ON_PAGE(!node && nr != 1, page);
> > -
> > - radix_tree_clear_tags(&mapping->i_pages, node, slot);
> > - __radix_tree_replace(&mapping->i_pages, node, slot, shadow,
> > - workingset_lookup_update(mapping));
> > + i = nr;
> > +repeat:
> > + xas_store(&xas, shadow);
> > + xas_init_tags(&xas);
> > + if (--i) {
> > + xas_next(&xas);
> > + goto repeat;
> > }
>
> Can this be converted into a do {} while (or even for) loop instead?
> Loops are easier to read and understand in such a situation.
I wish I'd fixed this up earlier because our peers made fun of me at
LSFMM for this loop ;-)
The obvious way to write this loop is:
for (i = 0; i < nr; i++) {
- struct radix_tree_node *node;
- void **slot;
-
- __radix_tree_lookup(&mapping->i_pages, page->index + i,
- &node, &slot);
-
- VM_BUG_ON_PAGE(!node && nr != 1, page);
-
- radix_tree_clear_tags(&mapping->i_pages, node, slot);
- __radix_tree_replace(&mapping->i_pages, node, slot, shadow,
- workingset_lookup_update(mapping));
+ xas_store(&xas, shadow);
+ xas_init_tags(&xas);
+ xas_next(&xas);
}
But since we're storing the same value to every entry and the range is
a power of two, there's a better way to rewrite this:
{
- int i, nr;
+ XA_STATE(xas, &mapping->i_pages, page->index);
+ unsigned int nr = 1;
- /* hugetlb pages are represented by one entry in the radix tree */
- nr = PageHuge(page) ? 1 : hpage_nr_pages(page);
+ mapping_set_update(&xas, mapping);
+
+ /* hugetlb pages are represented by a single entry in the xarray */
+ if (!PageHuge(page)) {
+ xas_set_order(&xas, page->index, compound_order(page));
+ nr = 1U << compound_order(page);
+ }
VM_BUG_ON_PAGE(!PageLocked(page), page);
VM_BUG_ON_PAGE(PageTail(page), page);
VM_BUG_ON_PAGE(nr != 1 && shadow, page);
- for (i = 0; i < nr; i++) {
- struct radix_tree_node *node;
- void **slot;
-
- __radix_tree_lookup(&mapping->i_pages, page->index + i,
- &node, &slot);
-
- VM_BUG_ON_PAGE(!node && nr != 1, page);
-
- radix_tree_clear_tags(&mapping->i_pages, node, slot);
- __radix_tree_replace(&mapping->i_pages, node, slot, shadow,
- workingset_lookup_update(mapping));
- }
+ xas_store(&xas, shadow);
+ xas_init_tags(&xas);
--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
