Re: [f2fs-dev] [PATCH v11 19/63] page cache: Convert page deletion to XArray

[Matthew Wilcox]

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(>i_pages, page->index + i,
> > -   , );
> > -
> > -   VM_BUG_ON_PAGE(!node && nr != 1, page);
> > -
> > -   radix_tree_clear_tags(>i_pages, node, slot);
> > -   __radix_tree_replace(>i_pages, node, slot, shadow,
> > -   workingset_lookup_update(mapping));
> > +   i = nr;
> > +repeat:
> > +   xas_store(, shadow);
> > +   xas_init_tags();
> > +   if (--i) {
> > +   xas_next();
> > +   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(>i_pages, page->index + i,
-   , );
-
-   VM_BUG_ON_PAGE(!node && nr != 1, page);
-
-   radix_tree_clear_tags(>i_pages, node, slot);
-   __radix_tree_replace(>i_pages, node, slot, shadow,
-   workingset_lookup_update(mapping));
+   xas_store(, shadow);
+   xas_init_tags();
+   xas_next();
}

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, >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(, mapping);
+
+   /* hugetlb pages are represented by a single entry in the xarray */
+   if (!PageHuge(page)) {
+   xas_set_order(, 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(>i_pages, page->index + i,
-   , );
-
-   VM_BUG_ON_PAGE(!node && nr != 1, page);
-
-   radix_tree_clear_tags(>i_pages, node, slot);
-   __radix_tree_replace(>i_pages, node, slot, shadow,
-   workingset_lookup_update(mapping));
-   }
+   xas_store(, shadow);
+   xas_init_tags();



--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[f2fs-dev] [PATCH v11 19/63] page cache: Convert page deletion to XArray

[Matthew Wilcox]

From: Matthew Wilcox 

The code is slightly shorter and simpler.

Signed-off-by: Matthew Wilcox 
---
 mm/filemap.c | 30 ++
 1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/mm/filemap.c b/mm/filemap.c
index 070b5e4527ac..4af06a1a9818 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -111,30 +111,28 @@
  *   ->tasklist_lock(memory_failure, collect_procs_ao)
  */
 
-static void page_cache_tree_delete(struct address_space *mapping,
+static void page_cache_delete(struct address_space *mapping,
   struct page *page, void *shadow)
 {
-   int i, nr;
+   XA_STATE(xas, >i_pages, page->index);
+   unsigned int i, nr;
 
-   /* hugetlb pages are represented by one entry in the radix tree */
+   mapping_set_update(, mapping);
+
+   /* hugetlb pages are represented by a single entry in the xarray */
nr = PageHuge(page) ? 1 : hpage_nr_pages(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(>i_pages, page->index + i,
-   , );
-
-   VM_BUG_ON_PAGE(!node && nr != 1, page);
-
-   radix_tree_clear_tags(>i_pages, node, slot);
-   __radix_tree_replace(>i_pages, node, slot, shadow,
-   workingset_lookup_update(mapping));
+   i = nr;
+repeat:
+   xas_store(, shadow);
+   xas_init_tags();
+   if (--i) {
+   xas_next();
+   goto repeat;
}
 
page->mapping = NULL;
@@ -234,7 +232,7 @@ void __delete_from_page_cache(struct page *page, void 
*shadow)
trace_mm_filemap_delete_from_page_cache(page);
 
unaccount_page_cache_page(mapping, page);
-   page_cache_tree_delete(mapping, page, shadow);
+   page_cache_delete(mapping, page, shadow);
 }
 
 static void page_cache_free_page(struct address_space *mapping,
-- 
2.17.0


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel