[PATCH] shmem : remove redundant code

[PATCH] shmem : remove redundant code

[Huang Shijie]

The  prep_new_page() will call set_page_private(page, 0) to initiate
the page.

So the code is redundant.

Signed-off-by: Huang Shijie <shijie8@gmail.com>
---
 mm/shmem.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/mm/shmem.c b/mm/shmem.c
index eef4ebe..dde4363 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -433,8 +433,6 @@ static swp_entry_t *shmem_swp_alloc(struct shmem_inode_info *info, unsigned long
 
 		spin_unlock(&info->lock);
 		page = shmem_dir_alloc(mapping_gfp_mask(inode->i_mapping));
-		if (page)
-			set_page_private(page, 0);
 		spin_lock(&info->lock);
 
 		if (!page) {
-- 
1.6.6

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] shmem : remove redundant code

[Minchan Kim]

On Mon, 2010-03-08 at 17:33 +0800, Huang Shijie wrote:
> The  prep_new_page() will call set_page_private(page, 0) to initiate
> the page.
> 
> So the code is redundant.
> 
> Signed-off-by: Huang Shijie <shijie8@gmail.com>
Reviewed-by: Minchan Kim <minchan.kim@gmail.com>

Long time ago, nr_swapped named is meaningful as a comment at least.
But as split page table lock is introduced in 4c21e2f2441, it was 
changed by just set_page_private. 
So even it's not meaningful any more as a comment, I think. 
So let's remove redundant code. 

-- 
Kind regards,
Minchan Kim


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] shmem : remove redundant code

[Huang Shijie]

> On Mon, 2010-03-08 at 17:33 +0800, Huang Shijie wrote:
>    
>> The  prep_new_page() will call set_page_private(page, 0) to initiate
>> the page.
>>
>> So the code is redundant.
>>
>> Signed-off-by: Huang Shijie<shijie8@gmail.com>
>>      
> Reviewed-by: Minchan Kim<minchan.kim@gmail.com>
>    
Thanks Minchan. :)
> Long time ago, nr_swapped named is meaningful as a comment at least.
> But as split page table lock is introduced in 4c21e2f2441, it was
> changed by just set_page_private.
> So even it's not meaningful any more as a comment, I think.
> So let's remove redundant code.
>
>    

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH] kvm : remove redundant initialization of page->private

[Minchan Kim]

On Mon, 2010-03-08 at 17:33 +0800, Huang Shijie wrote:
> The  prep_new_page() will call set_page_private(page, 0) to initiate
> the page.
> 
> So the code is redundant.
> 
> Signed-off-by: Huang Shijie <shijie8@gmail.com>
> ---
>  mm/shmem.c |    2 --
>  1 files changed, 0 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/shmem.c b/mm/shmem.c
> index eef4ebe..dde4363 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -433,8 +433,6 @@ static swp_entry_t *shmem_swp_alloc(struct shmem_inode_info *info, unsigned long
>  
>  		spin_unlock(&info->lock);
>  		page = shmem_dir_alloc(mapping_gfp_mask(inode->i_mapping));
> -		if (page)
> -			set_page_private(page, 0);
>  		spin_lock(&info->lock);
>  
>  		if (!page) {

And I found another place while I review the code.

>From e64322cde914e43d080d8f3be6f72459d809a934 Mon Sep 17 00:00:00 2001
From: Minchan Kim <barrios@barrios-desktop.(none)>
Date: Tue, 9 Mar 2010 01:09:56 +0900
Subject: [PATCH] kvm : remove redundant initialization of page->private.

The prep_new_page() in page allocator calls set_page_private(page, 0).
So we don't need to reinitialize private of page.

Signed-off-by: Minchan Kim <minchan.kim@gmail.com>
Cc: Avi Kivity <avi@redhat.com>
---
 arch/x86/kvm/mmu.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 741373e..9851d0e 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -326,7 +326,6 @@ static int mmu_topup_memory_cache_page(struct
kvm_mmu_memory_cache *cache,
 		page = alloc_page(GFP_KERNEL);
 		if (!page)
 			return -ENOMEM;
-		set_page_private(page, 0);
 		cache->objects[cache->nobjs++] = page_address(page);
 	}
 	return 0;
-- 
1.6.5



-- 
Kind regards,
Minchan Kim


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] kvm : remove redundant initialization of page->private

[Avi Kivity]

On 03/08/2010 06:20 PM, Minchan Kim wrote:
> On Mon, 2010-03-08 at 17:33 +0800, Huang Shijie wrote:
>    
>> The  prep_new_page() will call set_page_private(page, 0) to initiate
>> the page.
>>
>> So the code is redundant.
>>
>> Signed-off-by: Huang Shijie<shijie8@gmail.com>
>> ---
>>   mm/shmem.c |    2 --
>>   1 files changed, 0 insertions(+), 2 deletions(-)
>>
>> diff --git a/mm/shmem.c b/mm/shmem.c
>> index eef4ebe..dde4363 100644
>> --- a/mm/shmem.c
>> +++ b/mm/shmem.c
>> @@ -433,8 +433,6 @@ static swp_entry_t *shmem_swp_alloc(struct shmem_inode_info *info, unsigned long
>>
>>   		spin_unlock(&info->lock);
>>   		page = shmem_dir_alloc(mapping_gfp_mask(inode->i_mapping));
>> -		if (page)
>> -			set_page_private(page, 0);
>>   		spin_lock(&info->lock);
>>
>>   		if (!page) {
>>      
> And I found another place while I review the code.
>
> > From e64322cde914e43d080d8f3be6f72459d809a934 Mon Sep 17 00:00:00 2001
> From: Minchan Kim<barrios@barrios-desktop.(none)>
> Date: Tue, 9 Mar 2010 01:09:56 +0900
> Subject: [PATCH] kvm : remove redundant initialization of page->private.
>
> The prep_new_page() in page allocator calls set_page_private(page, 0).
> So we don't need to reinitialize private of page.
>
> Signed-off-by: Minchan Kim<minchan.kim@gmail.com>
> Cc: Avi Kivity<avi@redhat.com>
> ---
>   arch/x86/kvm/mmu.c |    1 -
>   1 files changed, 0 insertions(+), 1 deletions(-)
>
> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> index 741373e..9851d0e 100644
> --- a/arch/x86/kvm/mmu.c
> +++ b/arch/x86/kvm/mmu.c
> @@ -326,7 +326,6 @@ static int mmu_topup_memory_cache_page(struct
> kvm_mmu_memory_cache *cache,
>   		page = alloc_page(GFP_KERNEL);
>   		if (!page)
>   			return -ENOMEM;
> -		set_page_private(page, 0);
>   		cache->objects[cache->nobjs++] = page_address(page);
>   	}
>   	return 0;
>    

Whitespace damage, please resend.

-- 
error compiling committee.c: too many arguments to function

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] kvm : remove redundant initialization of page->private

[Minchan Kim]

On Wed, 2010-03-10 at 12:19 +0200, Avi Kivity wrote:

> Whitespace damage, please resend.
> 

Sorry for forgetting preformatted option of mail client.
Here is resend version.

== CUT_HERE ==

>From e64322cde914e43d080d8f3be6f72459d809a934 Mon Sep 17 00:00:00 2001
From: Minchan Kim<barrios@barrios-desktop.(none)>
Date: Tue, 9 Mar 2010 01:09:56 +0900
Subject: [PATCH] kvm : remove redundant initialization of page->private.

The prep_new_page() in page allocator calls set_page_private(page, 0). 
So we don't need to reinitialize private of page.

Signed-off-by: Minchan Kim <minchan.kim@gmail.com>
Cc: Avi Kivity<avi@redhat.com>
---
 arch/x86/kvm/mmu.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 741373e..9851d0e 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -326,7 +326,6 @@ static int mmu_topup_memory_cache_page(struct kvm_mmu_memory_cache *cache,
 		page = alloc_page(GFP_KERNEL);
 		if (!page)
 			return -ENOMEM;
-		set_page_private(page, 0);
 		cache->objects[cache->nobjs++] = page_address(page);
 	}
 	return 0;
-- 
1.6.5



-- 
Kind regards,
Minchan Kim



--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] kvm : remove redundant initialization of page->private

[Avi Kivity]

On 03/10/2010 04:31 PM, Minchan Kim wrote:
>
> The prep_new_page() in page allocator calls set_page_private(page, 0).
> So we don't need to reinitialize private of page.
>
>    

Applied, thanks.  Please copy the kvm mailing list in the future on kvm 
patches.

> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> index 741373e..9851d0e 100644
> --- a/arch/x86/kvm/mmu.c
> +++ b/arch/x86/kvm/mmu.c
> @@ -326,7 +326,6 @@ static int mmu_topup_memory_cache_page(struct kvm_mmu_memory_cache *cache,
>   		page = alloc_page(GFP_KERNEL);
>   		if (!page)
>   			return -ENOMEM;
> -		set_page_private(page, 0);
>   		cache->objects[cache->nobjs++] = page_address(page);
>   	}
>   	return 0;
>    

Jan, this is kvm-kmod unfriendly.  kvm_alloc_page()?

-- 
error compiling committee.c: too many arguments to function

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] kvm : remove redundant initialization of page->private

[Jan Kiszka]

Avi Kivity wrote:
> On 03/10/2010 04:31 PM, Minchan Kim wrote:
>> The prep_new_page() in page allocator calls set_page_private(page, 0).
>> So we don't need to reinitialize private of page.
>>
>>    
> 
> Applied, thanks.  Please copy the kvm mailing list in the future on kvm 
> patches.
> 
>> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
>> index 741373e..9851d0e 100644
>> --- a/arch/x86/kvm/mmu.c
>> +++ b/arch/x86/kvm/mmu.c
>> @@ -326,7 +326,6 @@ static int mmu_topup_memory_cache_page(struct kvm_mmu_memory_cache *cache,
>>   		page = alloc_page(GFP_KERNEL);
>>   		if (!page)
>>   			return -ENOMEM;
>> -		set_page_private(page, 0);
>>   		cache->objects[cache->nobjs++] = page_address(page);
>>   	}
>>   	return 0;
>>    
> 
> Jan, this is kvm-kmod unfriendly.  kvm_alloc_page()?
> 

Thanks for pointing out! Since which kernel can we rely on the implicit
set_page_private?

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] kvm : remove redundant initialization of page->private

[Avi Kivity]

On 03/11/2010 10:51 AM, Jan Kiszka wrote:
> Avi Kivity wrote:
>    
>> On 03/10/2010 04:31 PM, Minchan Kim wrote:
>>      
>>> The prep_new_page() in page allocator calls set_page_private(page, 0).
>>> So we don't need to reinitialize private of page.
>>>
>>>
>>>        
>> Applied, thanks.  Please copy the kvm mailing list in the future on kvm
>> patches.
>>
>>      
>>> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
>>> index 741373e..9851d0e 100644
>>> --- a/arch/x86/kvm/mmu.c
>>> +++ b/arch/x86/kvm/mmu.c
>>> @@ -326,7 +326,6 @@ static int mmu_topup_memory_cache_page(struct kvm_mmu_memory_cache *cache,
>>>    		page = alloc_page(GFP_KERNEL);
>>>    		if (!page)
>>>    			return -ENOMEM;
>>> -		set_page_private(page, 0);
>>>    		cache->objects[cache->nobjs++] = page_address(page);
>>>    	}
>>>    	return 0;
>>>
>>>        
>> Jan, this is kvm-kmod unfriendly.  kvm_alloc_page()?
>>
>>      
> Thanks for pointing out! Since which kernel can we rely on the implicit
> set_page_private?
>
>    

Um, git blame shows it goes all the way back to 2.6.12.  So it was 
redundant all along.

-- 
error compiling committee.c: too many arguments to function

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] kvm : remove redundant initialization of page->private

[Hugh Dickins]

On Thu, 11 Mar 2010, Avi Kivity wrote:
> On 03/11/2010 10:51 AM, Jan Kiszka wrote:
> > >      
> > Thanks for pointing out! Since which kernel can we rely on the implicit
> > set_page_private?
> 
> Um, git blame shows it goes all the way back to 2.6.12.  So it was redundant
> all along.

Not that it matters at all, but no, that was just the dawning of the git age.
prep_new_page() started initializing page->private in 2.6.0-test3:

<akpm@osdl.org>
	[PATCH] initialise page->private
	
	From: Nathan Scott <nathans@sgi.com>
	
	XFS wants to use page->private as a bitmap of uptodate indicators for
	sub-page-sized blocks (which is one of the things ->provate was intended
	for).
	
	But someone needs to initialise ->private somewhere.  best to do it in the
	page allocator, so the zeroness of a new page's ->private becomes a
	system-wide thing.

Hugh

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] shmem : remove redundant code

[Hugh Dickins]

On Mon, 8 Mar 2010, Huang Shijie wrote:

> The  prep_new_page() will call set_page_private(page, 0) to initiate
> the page.
> 
> So the code is redundant.
> 
> Signed-off-by: Huang Shijie <shijie8@gmail.com>

Acked-by: Hugh Dickins <hugh.dickins@tiscali.co.uk>

I didn't feel too enthusiastic at first, since private used not to be initialized
by page allocation, and I don't know of a strong reason why it should be: we do
strongly demand that page->mapping be NULL on allocation, but we leave page->index
with whatever it already contains, and I had thought page->_private the same.

But it seems we have been initializing private to 0 for nearly seven years now,
and it was done intentionally for something (XFS) to depend upon, so yes,
let's rely on that here too - thanks.

Hugh

> ---
>  mm/shmem.c |    2 --
>  1 files changed, 0 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/shmem.c b/mm/shmem.c
> index eef4ebe..dde4363 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -433,8 +433,6 @@ static swp_entry_t *shmem_swp_alloc(struct shmem_inode_info *info, unsigned long
>  
>  		spin_unlock(&info->lock);
>  		page = shmem_dir_alloc(mapping_gfp_mask(inode->i_mapping));
> -		if (page)
> -			set_page_private(page, 0);
>  		spin_lock(&info->lock);
>  
>  		if (!page) {
> -- 
> 1.6.6

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>