linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 02/12] mm, thp, tmpfs: support to add huge page into page cache for tmpfs
@ 2013-10-15  0:12 Ning Qu
  2013-10-15 10:02 ` Kirill A. Shutemov
  0 siblings, 1 reply; 6+ messages in thread
From: Ning Qu @ 2013-10-15  0:12 UTC (permalink / raw)
  To: Andrea Arcangeli, Andrew Morton, Kirill A. Shutemov, Hugh Dickins
  Cc: Al Viro, Hugh Dickins, Wu Fengguang, Jan Kara, Mel Gorman,
	linux-mm, Andi Kleen, Matthew Wilcox, Hillf Danton, Dave Hansen,
	Alexander Shishkin, linux-fsdevel, linux-kernel, Ning Qu

For replacing a page inside page cache, we assume the huge page
has been splitted before getting here.

For adding a new page to page cache, huge page support has been added.

Also refactor the shm_add_to_page_cache function.

Signed-off-by: Ning Qu <quning@gmail.com>
---
 mm/shmem.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 88 insertions(+), 9 deletions(-)

diff --git a/mm/shmem.c b/mm/shmem.c
index a857ba8..447bd14 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -277,27 +277,23 @@ static bool shmem_confirm_swap(struct address_space *mapping,
 }
 
 /*
- * Like add_to_page_cache_locked, but error if expected item has gone.
+ * Replace the swap entry with page cache entry
  */
-static int shmem_add_to_page_cache(struct page *page,
+static int shmem_replace_page_page_cache(struct page *page,
 				   struct address_space *mapping,
 				   pgoff_t index, gfp_t gfp, void *expected)
 {
 	int error;
 
-	VM_BUG_ON(!PageLocked(page));
-	VM_BUG_ON(!PageSwapBacked(page));
+	BUG_ON(PageTransHugeCache(page));
 
 	page_cache_get(page);
 	page->mapping = mapping;
 	page->index = index;
 
 	spin_lock_irq(&mapping->tree_lock);
-	if (!expected)
-		error = radix_tree_insert(&mapping->page_tree, index, page);
-	else
-		error = shmem_radix_tree_replace(mapping, index, expected,
-								 page);
+
+	error = shmem_radix_tree_replace(mapping, index, expected, page);
 	if (!error) {
 		mapping->nrpages++;
 		__inc_zone_page_state(page, NR_FILE_PAGES);
@@ -312,6 +308,87 @@ static int shmem_add_to_page_cache(struct page *page,
 }
 
 /*
+ * Insert new page into with page cache
+ */
+static int shmem_insert_page_page_cache(struct page *page,
+				   struct address_space *mapping,
+				   pgoff_t index, gfp_t gfp)
+{
+	int error;
+	int i, nr;
+
+	if (PageTransHugeCache(page))
+		BUILD_BUG_ON(HPAGE_CACHE_NR > RADIX_TREE_PRELOAD_NR);
+
+	nr = hpagecache_nr_pages(page);
+
+	error = radix_tree_maybe_preload_contig(nr, gfp & ~__GFP_HIGHMEM);
+	if (error)
+		return error;
+
+	spin_lock_irq(&mapping->tree_lock);
+	page_cache_get(page);
+	page->index = index;
+	page->mapping = mapping;
+	for (i = 0; i < nr; i++) {
+		error = radix_tree_insert(&mapping->page_tree,
+				index + i, page);
+		/*
+		 * In the midle of THP we can collide with small page which was
+		 * established before THP page cache is enabled or by other VMA
+		 * with bad alignement (most likely MAP_FIXED).
+		 */
+		if (error) {
+			i--; /* failed to insert anything at offset + i */
+			goto err;
+		}
+	}
+	radix_tree_preload_end();
+	mapping->nrpages += nr;
+	__mod_zone_page_state(page_zone(page), NR_FILE_PAGES, nr);
+	__mod_zone_page_state(page_zone(page), NR_SHMEM, nr);
+	if (PageTransHugeCache(page))
+		__inc_zone_page_state(page, NR_FILE_TRANSPARENT_HUGEPAGES);
+	spin_unlock_irq(&mapping->tree_lock);
+	return 0;
+err:
+	radix_tree_preload_end();
+	if (i != 0)
+		error = -ENOSPC; /* no space for a huge page */
+
+	/* Leave page->index set: truncation relies upon it */
+	page->mapping = NULL;
+	for (; i >= 0; i--)
+		radix_tree_delete(&mapping->page_tree, index + i);
+
+	spin_unlock_irq(&mapping->tree_lock);
+	page_cache_release(page);
+	return error;
+}
+
+/*
+ * Like add_to_page_cache_locked, but error if expected item has gone.
+ */
+static int shmem_add_to_page_cache(struct page *page,
+				   struct address_space *mapping,
+				   pgoff_t index, gfp_t gfp, void *expected)
+{
+	int error;
+
+	VM_BUG_ON(!PageLocked(page));
+	VM_BUG_ON(!PageSwapBacked(page));
+
+	if (expected) {
+		BUG_ON(PageTransHugeCache(page));
+		error = shmem_replace_page_page_cache(page, mapping, index, gfp,
+							expected);
+	} else
+		error = shmem_insert_page_page_cache(page, mapping, index, gfp);
+
+	return error;
+}
+
+/*
  * Like delete_from_page_cache, but substitutes swap for page.
  */
 static void shmem_delete_from_page_cache(struct page *page, void *radswap)
@@ -319,6 +396,8 @@ static void shmem_delete_from_page_cache(struct page *page, void *radswap)
 	struct address_space *mapping = page->mapping;
 	int error;
 
+	BUG_ON(PageTransHugeCache(page));
+
 	spin_lock_irq(&mapping->tree_lock);
 	error = shmem_radix_tree_replace(mapping, page->index, page, radswap);
 	page->mapping = NULL;
-- 
1.8.4



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* RE: [PATCH 02/12] mm, thp, tmpfs: support to add huge page into page cache for tmpfs
  2013-10-15  0:12 [PATCH 02/12] mm, thp, tmpfs: support to add huge page into page cache for tmpfs Ning Qu
@ 2013-10-15 10:02 ` Kirill A. Shutemov
  2013-10-15 17:27   ` Ning Qu
  0 siblings, 1 reply; 6+ messages in thread
From: Kirill A. Shutemov @ 2013-10-15 10:02 UTC (permalink / raw)
  To: Ning Qu
  Cc: Andrea Arcangeli, Andrew Morton, Kirill A. Shutemov,
	Hugh Dickins, Al Viro, Hugh Dickins, Wu Fengguang, Jan Kara,
	Mel Gorman, linux-mm, Andi Kleen, Matthew Wilcox, Hillf Danton,
	Dave Hansen, Alexander Shishkin, linux-fsdevel, linux-kernel,
	Ning Qu

Ning Qu wrote:
> For replacing a page inside page cache, we assume the huge page
> has been splitted before getting here.
> 
> For adding a new page to page cache, huge page support has been added.
> 
> Also refactor the shm_add_to_page_cache function.
> 
> Signed-off-by: Ning Qu <quning@gmail.com>
> ---
>  mm/shmem.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 88 insertions(+), 9 deletions(-)
> 
> diff --git a/mm/shmem.c b/mm/shmem.c
> index a857ba8..447bd14 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -277,27 +277,23 @@ static bool shmem_confirm_swap(struct address_space *mapping,
>  }
>  
>  /*
> - * Like add_to_page_cache_locked, but error if expected item has gone.
> + * Replace the swap entry with page cache entry
>   */
> -static int shmem_add_to_page_cache(struct page *page,
> +static int shmem_replace_page_page_cache(struct page *page,
>  				   struct address_space *mapping,
>  				   pgoff_t index, gfp_t gfp, void *expected)
>  {
>  	int error;
>  
> -	VM_BUG_ON(!PageLocked(page));
> -	VM_BUG_ON(!PageSwapBacked(page));
> +	BUG_ON(PageTransHugeCache(page));
>  
>  	page_cache_get(page);
>  	page->mapping = mapping;
>  	page->index = index;
>  
>  	spin_lock_irq(&mapping->tree_lock);
> -	if (!expected)
> -		error = radix_tree_insert(&mapping->page_tree, index, page);
> -	else
> -		error = shmem_radix_tree_replace(mapping, index, expected,
> -								 page);
> +
> +	error = shmem_radix_tree_replace(mapping, index, expected, page);
>  	if (!error) {
>  		mapping->nrpages++;
>  		__inc_zone_page_state(page, NR_FILE_PAGES);
> @@ -312,6 +308,87 @@ static int shmem_add_to_page_cache(struct page *page,
>  }
>  
>  /*
> + * Insert new page into with page cache
> + */
> +static int shmem_insert_page_page_cache(struct page *page,
> +				   struct address_space *mapping,
> +				   pgoff_t index, gfp_t gfp)
> +{

You copy-paste most of add_to_page_cache_locked() code here. Is there a
way to share the code? Move common part into __add_to_page_cache_locked()
or something.

-- 
 Kirill A. Shutemov

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 02/12] mm, thp, tmpfs: support to add huge page into page cache for tmpfs
  2013-10-15 10:02 ` Kirill A. Shutemov
@ 2013-10-15 17:27   ` Ning Qu
  2013-10-16 12:26     ` Kirill A. Shutemov
  0 siblings, 1 reply; 6+ messages in thread
From: Ning Qu @ 2013-10-15 17:27 UTC (permalink / raw)
  To: Kirill A. Shutemov
  Cc: Andrea Arcangeli, Andrew Morton, Hugh Dickins, Al Viro,
	Wu Fengguang, Jan Kara, Mel Gorman, linux-mm, Andi Kleen,
	Matthew Wilcox, Hillf Danton, Dave Hansen, Alexander Shishkin,
	linux-fsdevel, linux-kernel

Yes, I can try. The code is pretty much similar with some minor difference.

One thing I can do is to move the spin lock part (together with the
corresponding err handling into a common function.

The only problem I can see right now is we need the following
additional line for shm:

__mod_zone_page_state(page_zone(page), NR_SHMEM, nr);

Which means we need to tell if it's coming from shm or not, is that OK
to add additional parameter just for that? Or is there any other
better way we can infer that information? Thanks!
Best wishes,
-- 
Ning Qu (曲宁) | Software Engineer | quning@google.com | +1-408-418-6066


On Tue, Oct 15, 2013 at 3:02 AM, Kirill A. Shutemov
<kirill.shutemov@linux.intel.com> wrote:
> Ning Qu wrote:
>> For replacing a page inside page cache, we assume the huge page
>> has been splitted before getting here.
>>
>> For adding a new page to page cache, huge page support has been added.
>>
>> Also refactor the shm_add_to_page_cache function.
>>
>> Signed-off-by: Ning Qu <quning@gmail.com>
>> ---
>>  mm/shmem.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++------
>>  1 file changed, 88 insertions(+), 9 deletions(-)
>>
>> diff --git a/mm/shmem.c b/mm/shmem.c
>> index a857ba8..447bd14 100644
>> --- a/mm/shmem.c
>> +++ b/mm/shmem.c
>> @@ -277,27 +277,23 @@ static bool shmem_confirm_swap(struct address_space *mapping,
>>  }
>>
>>  /*
>> - * Like add_to_page_cache_locked, but error if expected item has gone.
>> + * Replace the swap entry with page cache entry
>>   */
>> -static int shmem_add_to_page_cache(struct page *page,
>> +static int shmem_replace_page_page_cache(struct page *page,
>>                                  struct address_space *mapping,
>>                                  pgoff_t index, gfp_t gfp, void *expected)
>>  {
>>       int error;
>>
>> -     VM_BUG_ON(!PageLocked(page));
>> -     VM_BUG_ON(!PageSwapBacked(page));
>> +     BUG_ON(PageTransHugeCache(page));
>>
>>       page_cache_get(page);
>>       page->mapping = mapping;
>>       page->index = index;
>>
>>       spin_lock_irq(&mapping->tree_lock);
>> -     if (!expected)
>> -             error = radix_tree_insert(&mapping->page_tree, index, page);
>> -     else
>> -             error = shmem_radix_tree_replace(mapping, index, expected,
>> -                                                              page);
>> +
>> +     error = shmem_radix_tree_replace(mapping, index, expected, page);
>>       if (!error) {
>>               mapping->nrpages++;
>>               __inc_zone_page_state(page, NR_FILE_PAGES);
>> @@ -312,6 +308,87 @@ static int shmem_add_to_page_cache(struct page *page,
>>  }
>>
>>  /*
>> + * Insert new page into with page cache
>> + */
>> +static int shmem_insert_page_page_cache(struct page *page,
>> +                                struct address_space *mapping,
>> +                                pgoff_t index, gfp_t gfp)
>> +{
>
> You copy-paste most of add_to_page_cache_locked() code here. Is there a
> way to share the code? Move common part into __add_to_page_cache_locked()
> or something.
>
> --
>  Kirill A. Shutemov

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 02/12] mm, thp, tmpfs: support to add huge page into page cache for tmpfs
  2013-10-15 17:27   ` Ning Qu
@ 2013-10-16 12:26     ` Kirill A. Shutemov
  2013-10-16 17:49       ` Ning Qu
  0 siblings, 1 reply; 6+ messages in thread
From: Kirill A. Shutemov @ 2013-10-16 12:26 UTC (permalink / raw)
  To: Ning Qu
  Cc: Kirill A. Shutemov, Andrea Arcangeli, Andrew Morton,
	Hugh Dickins, Al Viro, Wu Fengguang, Jan Kara, Mel Gorman,
	linux-mm, Andi Kleen, Matthew Wilcox, Hillf Danton, Dave Hansen,
	Alexander Shishkin, linux-fsdevel, linux-kernel

Ning Qu wrote:
> Yes, I can try. The code is pretty much similar with some minor difference.
> 
> One thing I can do is to move the spin lock part (together with the
> corresponding err handling into a common function.
> 
> The only problem I can see right now is we need the following
> additional line for shm:
> 
> __mod_zone_page_state(page_zone(page), NR_SHMEM, nr);
> 
> Which means we need to tell if it's coming from shm or not, is that OK
> to add additional parameter just for that? Or is there any other
> better way we can infer that information? Thanks!

I think you can account NR_SHMEM after common code succeed, don't you?

-- 
 Kirill A. Shutemov

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 02/12] mm, thp, tmpfs: support to add huge page into page cache for tmpfs
  2013-10-16 12:26     ` Kirill A. Shutemov
@ 2013-10-16 17:49       ` Ning Qu
  2013-10-16 23:20         ` Ning Qu
  0 siblings, 1 reply; 6+ messages in thread
From: Ning Qu @ 2013-10-16 17:49 UTC (permalink / raw)
  To: Kirill A. Shutemov
  Cc: Andrea Arcangeli, Andrew Morton, Hugh Dickins, Al Viro,
	Wu Fengguang, Jan Kara, Mel Gorman, linux-mm, Andi Kleen,
	Matthew Wilcox, Hillf Danton, Dave Hansen, Alexander Shishkin,
	linux-fsdevel, linux-kernel

Yes, I guess I can if I just put whatever inside the spin lock into a
common function. Thanks!
Best wishes,
-- 
Ning Qu (曲宁) | Software Engineer | quning@google.com | +1-408-418-6066


On Wed, Oct 16, 2013 at 5:26 AM, Kirill A. Shutemov
<kirill.shutemov@linux.intel.com> wrote:
> Ning Qu wrote:
>> Yes, I can try. The code is pretty much similar with some minor difference.
>>
>> One thing I can do is to move the spin lock part (together with the
>> corresponding err handling into a common function.
>>
>> The only problem I can see right now is we need the following
>> additional line for shm:
>>
>> __mod_zone_page_state(page_zone(page), NR_SHMEM, nr);
>>
>> Which means we need to tell if it's coming from shm or not, is that OK
>> to add additional parameter just for that? Or is there any other
>> better way we can infer that information? Thanks!
>
> I think you can account NR_SHMEM after common code succeed, don't you?
>
> --
>  Kirill A. Shutemov

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 02/12] mm, thp, tmpfs: support to add huge page into page cache for tmpfs
  2013-10-16 17:49       ` Ning Qu
@ 2013-10-16 23:20         ` Ning Qu
  0 siblings, 0 replies; 6+ messages in thread
From: Ning Qu @ 2013-10-16 23:20 UTC (permalink / raw)
  To: Kirill A. Shutemov
  Cc: Andrea Arcangeli, Andrew Morton, Hugh Dickins, Al Viro,
	Wu Fengguang, Jan Kara, Mel Gorman, linux-mm, Andi Kleen,
	Matthew Wilcox, Hillf Danton, Dave Hansen, Alexander Shishkin,
	linux-fsdevel, linux-kernel

Consider this fixed. I have extracted the common function and the new
shmem_insert_page_page_cache function looks like this:

       spin_lock_irq(&mapping->tree_lock);
       error = __add_to_page_cache_locked(page, mapping, index);

       if (!error)
            __mod_zone_page_state(page_zone(page), NR_SHMEM, nr);

        radix_tree_preload_end();
        spin_unlock_irq(&mapping->tree_lock);

        if (error)
                page_cache_release(page);

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


On Wed, Oct 16, 2013 at 10:49 AM, Ning Qu <quning@google.com> wrote:
> Yes, I guess I can if I just put whatever inside the spin lock into a
> common function. Thanks!
> Best wishes,
> --
> Ning Qu (曲宁) | Software Engineer | quning@google.com | +1-408-418-6066
>
>
> On Wed, Oct 16, 2013 at 5:26 AM, Kirill A. Shutemov
> <kirill.shutemov@linux.intel.com> wrote:
>> Ning Qu wrote:
>>> Yes, I can try. The code is pretty much similar with some minor difference.
>>>
>>> One thing I can do is to move the spin lock part (together with the
>>> corresponding err handling into a common function.
>>>
>>> The only problem I can see right now is we need the following
>>> additional line for shm:
>>>
>>> __mod_zone_page_state(page_zone(page), NR_SHMEM, nr);
>>>
>>> Which means we need to tell if it's coming from shm or not, is that OK
>>> to add additional parameter just for that? Or is there any other
>>> better way we can infer that information? Thanks!
>>
>> I think you can account NR_SHMEM after common code succeed, don't you?
>>
>> --
>>  Kirill A. Shutemov

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-10-16 23:20 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-10-15  0:12 [PATCH 02/12] mm, thp, tmpfs: support to add huge page into page cache for tmpfs Ning Qu
2013-10-15 10:02 ` Kirill A. Shutemov
2013-10-15 17:27   ` Ning Qu
2013-10-16 12:26     ` Kirill A. Shutemov
2013-10-16 17:49       ` Ning Qu
2013-10-16 23:20         ` Ning Qu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).