linux-kernel - Re: [PATCH 03/12] mm, thp, tmpfs: handle huge page cases in shmem_getpage

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 15 Oct 2013 11:58:09 -0700
From:	Ning Qu <quning@...gle.com>
To:	"Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>
Cc:	Andrea Arcangeli <aarcange@...hat.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Hugh Dickins <hughd@...gle.com>,
	Al Viro <viro@...iv.linux.org.uk>,
	Wu Fengguang <fengguang.wu@...el.com>, Jan Kara <jack@...e.cz>,
	Mel Gorman <mgorman@...e.de>, linux-mm@...ck.org,
	Andi Kleen <ak@...ux.intel.com>,
	Matthew Wilcox <willy@...ux.intel.com>,
	Hillf Danton <dhillf@...il.com>, Dave Hansen <dave@...1.net>,
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 03/12] mm, thp, tmpfs: handle huge page cases in shmem_getpage_gfp

Best wishes,
-- 
Ning Qu (曲宁) | Software Engineer | quning@...gle.com | +1-408-418-6066


On Tue, Oct 15, 2013 at 3:29 AM, Kirill A. Shutemov
<kirill.shutemov@...ux.intel.com> wrote:
> Ning Qu wrote:
>> We don't support huge page when page is moved from page cache to swap.
>> So in this function, we enable huge page handling in two case:
>>
>> 1) when a huge page is found in the page cache,
>> 2) or we need to alloc a huge page for page cache
>>
>> We have to refactor all the calls to shmem_getpages to simplify the job
>> of caller. Right now shmem_getpage does:
>>
>> 1) simply request a page, default as a small page
>> 2) or caller specify a flag to request either a huge page or a small page,
>> then leave the caller to decide how to use it
>>
>> Signed-off-by: Ning Qu <quning@...il.com>
>> ---
>>  mm/shmem.c | 139 +++++++++++++++++++++++++++++++++++++++++++++++--------------
>>  1 file changed, 108 insertions(+), 31 deletions(-)
>>
>> diff --git a/mm/shmem.c b/mm/shmem.c
>> index 447bd14..8fe17dd 100644
>> --- a/mm/shmem.c
>> +++ b/mm/shmem.c
>> @@ -115,15 +115,43 @@ static unsigned long shmem_default_max_inodes(void)
>>  static bool shmem_should_replace_page(struct page *page, gfp_t gfp);
>>  static int shmem_replace_page(struct page **pagep, gfp_t gfp,
>>                               struct shmem_inode_info *info, pgoff_t index);
>> +
>>  static int shmem_getpage_gfp(struct inode *inode, pgoff_t index,
>> -     struct page **pagep, enum sgp_type sgp, gfp_t gfp, int *fault_type);
>> +     struct page **pagep, enum sgp_type sgp, gfp_t gfp, int flags,
>> +     int *fault_type);
>> +
>> +#ifdef CONFIG_TRANSPARENT_HUGEPAGE_PAGECACHE
>> +static inline int shmem_getpage(struct inode *inode, pgoff_t index,
>> +     struct page **pagep, enum sgp_type sgp, gfp_t gfp, int flags,
>> +     int *fault_type)
>> +{
>> +     int ret = 0;
>> +     struct page *page = NULL;
>>
>> +     if ((flags & AOP_FLAG_TRANSHUGE) &&
>> +         mapping_can_have_hugepages(inode->i_mapping)) {
>
> I don't think we need ifdef here. mapping_can_have_hugepages() will be 0
> compile-time, if CONFIG_TRANSPARENT_HUGEPAGE_PAGECACHE is not defined and
> compiler should optimize out thp case.

The same problem since HPAGE_CACHE_INDEX_MASK is build bug?

>
>> @@ -1298,27 +1348,37 @@ repeat:
>>                               error = -ENOSPC;
>>                               goto unacct;
>>                       }
>> -                     percpu_counter_inc(&sbinfo->used_blocks);
>>               }
>>
>> -             page = shmem_alloc_page(gfp, info, index);
>> +             if (must_use_thp) {
>> +                     page = shmem_alloc_hugepage(gfp, info, index);
>> +                     if (page) {
>> +                             count_vm_event(THP_WRITE_ALLOC);
>> +                             nr = hpagecache_nr_pages(page);
>
> nr = hpagecache_nr_pages(page) can be moved below if (must_use_thp).
> hpagecache_nr_pages(page) evaluates to 0 for small pages.
>
you mean something like this? If so, then fixed.

               if (must_use_thp) {
                        page = shmem_alloc_hugepage(gfp, info, index);
                        if (page) {
                                count_vm_event(THP_WRITE_ALLOC);
                        } else
                                count_vm_event(THP_WRITE_ALLOC_FAILED);
                } else {
                        page = shmem_alloc_page(gfp, info, index);
                }

                if (!page) {
                        error = -ENOMEM;
                        goto unacct;
                }
                nr = hpagecache_nr_pages(page);


>
> --
>  Kirill A. Shutemov
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/