Re: [PATCHv3 14/41] filemap: allocate huge page in page_cache_read(), if allowed

[Kirill A. Shutemov]

On Tue, Oct 11, 2016 at 06:15:45PM +0200, Jan Kara wrote:
> On Thu 15-09-16 14:54:56, Kirill A. Shutemov wrote:
> > This patch adds basic functionality to put huge page into page cache.
> > 
> > At the moment we only put huge pages into radix-tree if the range covered
> > by the huge page is empty.
> > 
> > We ignore shadow entires for now, just remove them from the tree before
> > inserting huge page.
> > 
> > Later we can add logic to accumulate information from shadow entires to
> > return to caller (average eviction time?).
> > 
> > Signed-off-by: Kirill A. Shutemov 
> > ---
> >  include/linux/fs.h  |   5 ++
> >  include/linux/pagemap.h |  21 ++-
> >  mm/filemap.c| 148 
> > +++-
> >  3 files changed, 157 insertions(+), 17 deletions(-)
> > 
> ...
> > @@ -663,16 +663,55 @@ static int __add_to_page_cache_locked(struct page 
> > *page,
> > page->index = offset;
> >  
> > spin_lock_irq(>tree_lock);
> > -   error = page_cache_tree_insert(mapping, page, shadowp);
> > +   if (PageTransHuge(page)) {
> > +   struct radix_tree_iter iter;
> > +   void **slot;
> > +   void *p;
> > +
> > +   error = 0;
> > +
> > +   /* Wipe shadow entires */
> > +   radix_tree_for_each_slot(slot, >page_tree, , 
> > offset) {
> > +   if (iter.index >= offset + HPAGE_PMD_NR)
> > +   break;
> > +
> > +   p = radix_tree_deref_slot_protected(slot,
> > +   >tree_lock);
> > +   if (!p)
> > +   continue;
> > +
> > +   if (!radix_tree_exception(p)) {
> > +   error = -EEXIST;
> > +   break;
> > +   }
> > +
> > +   mapping->nrexceptional--;
> > +   rcu_assign_pointer(*slot, NULL);
> 
> I think you also need something like workingset_node_shadows_dec(node)
> here. It would be even better if you used something like
> clear_exceptional_entry() to have the logic in one place (you obviously
> need to factor out only part of clear_exceptional_entry() first).

Good point. Will do.

> > +   }
> > +
> > +   if (!error)
> > +   error = __radix_tree_insert(>page_tree, offset,
> > +   compound_order(page), page);
> > +
> > +   if (!error) {
> > +   count_vm_event(THP_FILE_ALLOC);
> > +   mapping->nrpages += HPAGE_PMD_NR;
> > +   *shadowp = NULL;
> > +   __inc_node_page_state(page, NR_FILE_THPS);
> > +   }
> > +   } else {
> > +   error = page_cache_tree_insert(mapping, page, shadowp);
> > +   }
> 
> And I'd prefer to have this logic moved to page_cache_tree_insert() because
> logically it IMHO belongs there - it is a simply another case of handling
> of radix tree used for page cache.

Okay.

-- 
 Kirill A. Shutemov
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv3 14/41] filemap: allocate huge page in page_cache_read(), if allowed

[Jan Kara]

On Thu 15-09-16 14:54:56, Kirill A. Shutemov wrote:
> This patch adds basic functionality to put huge page into page cache.
> 
> At the moment we only put huge pages into radix-tree if the range covered
> by the huge page is empty.
> 
> We ignore shadow entires for now, just remove them from the tree before
> inserting huge page.
> 
> Later we can add logic to accumulate information from shadow entires to
> return to caller (average eviction time?).
> 
> Signed-off-by: Kirill A. Shutemov 
> ---
>  include/linux/fs.h  |   5 ++
>  include/linux/pagemap.h |  21 ++-
>  mm/filemap.c| 148 
> +++-
>  3 files changed, 157 insertions(+), 17 deletions(-)
> 
...
> @@ -663,16 +663,55 @@ static int __add_to_page_cache_locked(struct page *page,
>   page->index = offset;
>  
>   spin_lock_irq(>tree_lock);
> - error = page_cache_tree_insert(mapping, page, shadowp);
> + if (PageTransHuge(page)) {
> + struct radix_tree_iter iter;
> + void **slot;
> + void *p;
> +
> + error = 0;
> +
> + /* Wipe shadow entires */
> + radix_tree_for_each_slot(slot, >page_tree, , 
> offset) {
> + if (iter.index >= offset + HPAGE_PMD_NR)
> + break;
> +
> + p = radix_tree_deref_slot_protected(slot,
> + >tree_lock);
> + if (!p)
> + continue;
> +
> + if (!radix_tree_exception(p)) {
> + error = -EEXIST;
> + break;
> + }
> +
> + mapping->nrexceptional--;
> + rcu_assign_pointer(*slot, NULL);

I think you also need something like workingset_node_shadows_dec(node)
here. It would be even better if you used something like
clear_exceptional_entry() to have the logic in one place (you obviously
need to factor out only part of clear_exceptional_entry() first).

> + }
> +
> + if (!error)
> + error = __radix_tree_insert(>page_tree, offset,
> + compound_order(page), page);
> +
> + if (!error) {
> + count_vm_event(THP_FILE_ALLOC);
> + mapping->nrpages += HPAGE_PMD_NR;
> + *shadowp = NULL;
> + __inc_node_page_state(page, NR_FILE_THPS);
> + }
> + } else {
> + error = page_cache_tree_insert(mapping, page, shadowp);
> + }

And I'd prefer to have this logic moved to page_cache_tree_insert() because
logically it IMHO belongs there - it is a simply another case of handling
of radix tree used for page cache.

Honza
-- 
Jan Kara 
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html