Re: [PATCH 03/12] mm, thp, tmpfs: handle huge page cases in shmem_getpage_gfp

[Ning Qu]

Best wishes,
-- 
Ning Qu (æå) | Software Engineer | quning@xxxxxxxxxx | +1-408-418-6066


On Tue, Oct 15, 2013 at 3:29 AM, Kirill A. Shutemov
<kirill.shutemov@xxxxxxxxxxxxxxx> 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@xxxxxxxxx>
>> ---
>>  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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/