linux-kernel - Re: [PATCH 07/12] mm, thp, tmpfs: handle huge page in shmem_undo

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:48:40 -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 07/12] mm, thp, tmpfs: handle huge page in
 shmem_undo_range for truncate

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


On Tue, Oct 15, 2013 at 4:01 AM, Kirill A. Shutemov
<kirill.shutemov@...ux.intel.com> wrote:
> Ning Qu wrote:
>> When comes to truncate file, add support to handle huge page in the
>> truncate range.
>>
>> Signed-off-by: Ning Qu <quning@...il.com>
>> ---
>>  mm/shmem.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-------
>>  1 file changed, 86 insertions(+), 11 deletions(-)
>>
>> diff --git a/mm/shmem.c b/mm/shmem.c
>> index 0a423a9..90f2e0e 100644
>> --- a/mm/shmem.c
>> +++ b/mm/shmem.c
>> @@ -559,6 +559,7 @@ static void shmem_undo_range(struct inode *inode, loff_t lstart, loff_t lend,
>>       struct shmem_inode_info *info = SHMEM_I(inode);
>>       pgoff_t start = (lstart + PAGE_CACHE_SIZE - 1) >> PAGE_CACHE_SHIFT;
>>       pgoff_t end = (lend + 1) >> PAGE_CACHE_SHIFT;
>> +     /* Whether we have to do partial truncate */
>>       unsigned int partial_start = lstart & (PAGE_CACHE_SIZE - 1);
>>       unsigned int partial_end = (lend + 1) & (PAGE_CACHE_SIZE - 1);
>>       struct pagevec pvec;
>> @@ -570,12 +571,16 @@ static void shmem_undo_range(struct inode *inode, loff_t lstart, loff_t lend,
>>       if (lend == -1)
>>               end = -1;       /* unsigned, so actually very big */
>>
>> +     i_split_down_read(inode);
>>       pagevec_init(&pvec, 0);
>>       index = start;
>>       while (index < end) {
>> +             bool thp = false;
>> +
>>               pvec.nr = shmem_find_get_pages_and_swap(mapping, index,
>>                               min(end - index, (pgoff_t)PAGEVEC_SIZE),
>>                                                       pvec.pages, indices);
>> +
>>               if (!pvec.nr)
>>                       break;
>>               mem_cgroup_uncharge_start();
>> @@ -586,6 +591,25 @@ static void shmem_undo_range(struct inode *inode, loff_t lstart, loff_t lend,
>>                       if (index >= end)
>>                               break;
>>
>> +                     thp = PageTransHugeCache(page);
>> +#ifdef CONFIG_TRANSPARENT_HUGEPAGE_PAGECACHE
>
> Again. Here and below ifdef is redundant: PageTransHugeCache() is zero
> compile-time and  thp case will be optimize out.

The problem is actually from HPAGE_CACHE_INDEX_MASK, it is marked as
build bug when CONFIG_TRANSPARENT_HUGEPAGE_PAGECACHE is false. So we
either wrap some logic inside a inline function, or we have to be like
this .. Or we don't treat the HPAGE_CACHE_INDEX_MASK as a build bug?

>
> And do we really need a copy of truncate logic here? Is there a way to
> share code?
>
The truncate between tmpfs and general one is similar but not exactly
the same (no readahead), so share the whole function might not be a
good choice from the perspective of tmpfs? Anyway, there are other
similar functions in tmpfs, e.g. the one you mentioned for
shmem_add_to_page_cache. It is possible to share the code, I am just
worried it will make the logic more complicated?

Maybe Hugh is in better position to judge on this? Thanks!

> --
>  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/