lkml.org 
[lkml]   [2011]   [Nov]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH RFC] mm: abort inode pruning if it has active pages
Andrew Morton wrote:
> On Wed, 16 Nov 2011 17:47:47 +0300
> Konstantin Khlebnikov<khlebnikov@openvz.org> wrote:
>
>> Inode cache pruning can throw out some usefull data from page cache.
>> This patch aborts inode invalidation and keep inode alive if it still has
>> active pages.
>>
>
> hm, I suppose so.
>
> I also suppose there are various risks related to failing to reclaim
> inodes due to ongoing userspace activity and then running out of lowmem
> pages.

Ok, I think we can bypass active-page protection if CONFIG_HIGHMEM=y and
there is no __GFP_HIGHMEM in gfp_mask.

>
>> It improves interaction between inode cache and page cache.
>
> Well, this is the key part of the patch and it is the thing which we
> are most interested in. But you didn't tell us anything about it!
>
> So please, provide us with much more detailed information on the
> observed benefits.

Currently this is based only on thought experiment.
I think rising pginodesteal and kswapd_inodesteal in /proc/vstat is sign of
inefficient memory reclaiming, because page-cache lru has a much more detailed
information about memory activity.

I plan to run some containers related tests/benchmarks,
something like multiple heavy-loaded web-servers.

>
>>
>> diff --git a/fs/inode.c b/fs/inode.c
>> index 1f6c48d..8d55a63 100644
>> --- a/fs/inode.c
>> +++ b/fs/inode.c
>> @@ -663,8 +663,8 @@ void prune_icache_sb(struct super_block *sb, int nr_to_scan)
>> spin_unlock(&inode->i_lock);
>> spin_unlock(&sb->s_inode_lru_lock);
>> if (remove_inode_buffers(inode))
>> - reap += invalidate_mapping_pages(&inode->i_data,
>> - 0, -1);
>> + reap += invalidate_inode_inactive_pages(
>> + &inode->i_data, 0, -1);
>> iput(inode);
>> spin_lock(&sb->s_inode_lru_lock);
>>
>> diff --git a/include/linux/fs.h b/include/linux/fs.h
>> index 0c4df26..05875d7 100644
>> --- a/include/linux/fs.h
>> +++ b/include/linux/fs.h
>> @@ -2211,6 +2211,8 @@ extern int invalidate_partition(struct gendisk *, int);
>> #endif
>> unsigned long invalidate_mapping_pages(struct address_space *mapping,
>> pgoff_t start, pgoff_t end);
>> +unsigned long invalidate_inode_inactive_pages(struct address_space *mapping,
>> + pgoff_t start, pgoff_t end);
>>
>> static inline void invalidate_remote_inode(struct inode *inode)
>> {
>> diff --git a/mm/truncate.c b/mm/truncate.c
>> index 632b15e..ac739bc 100644
>> --- a/mm/truncate.c
>> +++ b/mm/truncate.c
>> @@ -379,6 +379,52 @@ unsigned long invalidate_mapping_pages(struct address_space *mapping,
>> EXPORT_SYMBOL(invalidate_mapping_pages);
>>
>> /*
>> + * This is like invalidate_mapping_pages(),
>> + * except it aborts invalidation at the first active page.
>> + */
>> +unsigned long invalidate_inode_inactive_pages(struct address_space *mapping,
>> + pgoff_t start, pgoff_t end)
>> +{
>> + struct pagevec pvec;
>> + pgoff_t index = start;
>> + unsigned long ret;
>> + unsigned long count = 0;
>> + int i;
>> +
>> + pagevec_init(&pvec, 0);
>> + while (index<= end&& pagevec_lookup(&pvec, mapping, index,
>> + min(end - index, (pgoff_t)PAGEVEC_SIZE - 1) + 1)) {
>> +
>> + mem_cgroup_uncharge_start();
>> + for (i = 0; i< pagevec_count(&pvec); i++) {
>> + struct page *page = pvec.pages[i];
>> +
>> + if (PageActive(page)) {
>> + index = end;
>> + break;
>> + }
>> +
>> + /* We rely upon deletion not changing page->index */
>> + index = page->index;
>> + if (index> end)
>> + break;
>> +
>> + if (!trylock_page(page))
>> + continue;
>> + WARN_ON(page->index != index);
>> + ret = invalidate_inode_page(page);
>> + unlock_page(page);
>> + count += ret;
>> + }
>> + pagevec_release(&pvec);
>> + mem_cgroup_uncharge_end();
>> + cond_resched();
>> + index++;
>> + }
>> + return count;
>> +}
>
> We shouldn't just copy-n-paste invalidate_mapping_pages() like this.
> Can't we share the function by passing in a pointer to a callback
> function (invalidate_inode_page or a new
> invalidate_inode_page_unless_it_is_active).
>

Ok, I'll think how to implement this more accurate.


\
 
 \ /
  Last update: 2011-11-18 09:17    [W:0.843 / U:0.128 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site