All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] mm: remove unnecessary condition in remove_inode_hugepages
@ 2016-09-27 14:16 zhongjiang
  2016-09-27 19:23 ` Mike Kravetz
  2016-09-28  5:48 ` Naoya Horiguchi
  0 siblings, 2 replies; 5+ messages in thread
From: zhongjiang @ 2016-09-27 14:16 UTC (permalink / raw)
  To: mike.kravetz, mhocko, akpm; +Cc: linux-mm, n-horiguchi

From: zhong jiang <zhongjiang@huawei.com>

when the huge page is added to the page cahce (huge_add_to_page_cache),
the page private flag will be cleared. since this code
(remove_inode_hugepages) will only be called for pages in the
page cahce, PagePrivate(page) will always be false.

The patch remove the code without any functional change.

Signed-off-by: zhong jiang <zhongjiang@huawei.com>
---
 fs/hugetlbfs/inode.c    | 11 +++++------
 include/linux/hugetlb.h |  2 +-
 mm/hugetlb.c            |  4 ++--
 3 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
index 4ea71eb..40d0afe 100644
--- a/fs/hugetlbfs/inode.c
+++ b/fs/hugetlbfs/inode.c
@@ -458,18 +458,17 @@ static void remove_inode_hugepages(struct inode *inode, loff_t lstart,
 			 * cache (remove_huge_page) BEFORE removing the
 			 * region/reserve map (hugetlb_unreserve_pages).  In
 			 * rare out of memory conditions, removal of the
-			 * region/reserve map could fail.  Before free'ing
-			 * the page, note PagePrivate which is used in case
-			 * of error.
+			 * region/reserve map could fail. Correspondingly,
+			 * the subpool and global reserve usage count can need
+			 * to be adjusted.
 			 */
-			rsv_on_error = !PagePrivate(page);
+			VM_BUG_ON(PagePrivate(page));
 			remove_huge_page(page);
 			freed++;
 			if (!truncate_op) {
 				if (unlikely(hugetlb_unreserve_pages(inode,
 							next, next + 1, 1)))
-					hugetlb_fix_reserve_counts(inode,
-								rsv_on_error);
+					hugetlb_fix_reserve_counts(inode);
 			}
 
 			unlock_page(page);
diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index c26d463..d2e0fc5 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -90,7 +90,7 @@ int dequeue_hwpoisoned_huge_page(struct page *page);
 bool isolate_huge_page(struct page *page, struct list_head *list);
 void putback_active_hugepage(struct page *page);
 void free_huge_page(struct page *page);
-void hugetlb_fix_reserve_counts(struct inode *inode, bool restore_reserve);
+void hugetlb_fix_reserve_counts(struct inode *inode);
 extern struct mutex *hugetlb_fault_mutex_table;
 u32 hugetlb_fault_mutex_hash(struct hstate *h, struct mm_struct *mm,
 				struct vm_area_struct *vma,
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 87e11d8..28a079a 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -567,13 +567,13 @@ retry:
  * appear as a "reserved" entry instead of simply dangling with incorrect
  * counts.
  */
-void hugetlb_fix_reserve_counts(struct inode *inode, bool restore_reserve)
+void hugetlb_fix_reserve_counts(struct inode *inode)
 {
 	struct hugepage_subpool *spool = subpool_inode(inode);
 	long rsv_adjust;
 
 	rsv_adjust = hugepage_subpool_get_pages(spool, 1);
-	if (restore_reserve && rsv_adjust) {
+	if (rsv_adjust) {
 		struct hstate *h = hstate_inode(inode);
 
 		hugetlb_acct_memory(h, 1);
-- 
1.8.3.1

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

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH v2] mm: remove unnecessary condition in remove_inode_hugepages
  2016-09-27 14:16 [PATCH v2] mm: remove unnecessary condition in remove_inode_hugepages zhongjiang
@ 2016-09-27 19:23 ` Mike Kravetz
  2016-09-28 23:55   ` Mike Kravetz
  2016-09-28  5:48 ` Naoya Horiguchi
  1 sibling, 1 reply; 5+ messages in thread
From: Mike Kravetz @ 2016-09-27 19:23 UTC (permalink / raw)
  To: zhongjiang, mhocko, akpm; +Cc: linux-mm, n-horiguchi

On 09/27/2016 07:16 AM, zhongjiang wrote:
> From: zhong jiang <zhongjiang@huawei.com>
> 
> when the huge page is added to the page cahce (huge_add_to_page_cache),
> the page private flag will be cleared. since this code
> (remove_inode_hugepages) will only be called for pages in the
> page cahce, PagePrivate(page) will always be false.
> 
> The patch remove the code without any functional change.
> 
> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
> ---
>  fs/hugetlbfs/inode.c    | 11 +++++------
>  include/linux/hugetlb.h |  2 +-
>  mm/hugetlb.c            |  4 ++--
>  3 files changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
> index 4ea71eb..40d0afe 100644
> --- a/fs/hugetlbfs/inode.c
> +++ b/fs/hugetlbfs/inode.c
> @@ -458,18 +458,17 @@ static void remove_inode_hugepages(struct inode *inode, loff_t lstart,
>  			 * cache (remove_huge_page) BEFORE removing the
>  			 * region/reserve map (hugetlb_unreserve_pages).  In
>  			 * rare out of memory conditions, removal of the
> -			 * region/reserve map could fail.  Before free'ing
> -			 * the page, note PagePrivate which is used in case
> -			 * of error.
> +			 * region/reserve map could fail. Correspondingly,
> +			 * the subpool and global reserve usage count can need
> +			 * to be adjusted.
>  			 */
> -			rsv_on_error = !PagePrivate(page);
> +			VM_BUG_ON(PagePrivate(page));
>  			remove_huge_page(page);
>  			freed++;
>  			if (!truncate_op) {
>  				if (unlikely(hugetlb_unreserve_pages(inode,
>  							next, next + 1, 1)))
> -					hugetlb_fix_reserve_counts(inode,
> -								rsv_on_error);
> +					hugetlb_fix_reserve_counts(inode);
>  			}
>  
>  			unlock_page(page);
> diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
> index c26d463..d2e0fc5 100644
> --- a/include/linux/hugetlb.h
> +++ b/include/linux/hugetlb.h
> @@ -90,7 +90,7 @@ int dequeue_hwpoisoned_huge_page(struct page *page);
>  bool isolate_huge_page(struct page *page, struct list_head *list);
>  void putback_active_hugepage(struct page *page);
>  void free_huge_page(struct page *page);
> -void hugetlb_fix_reserve_counts(struct inode *inode, bool restore_reserve);
> +void hugetlb_fix_reserve_counts(struct inode *inode);
>  extern struct mutex *hugetlb_fault_mutex_table;
>  u32 hugetlb_fault_mutex_hash(struct hstate *h, struct mm_struct *mm,
>  				struct vm_area_struct *vma,
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 87e11d8..28a079a 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -567,13 +567,13 @@ retry:
>   * appear as a "reserved" entry instead of simply dangling with incorrect
>   * counts.
>   */
> -void hugetlb_fix_reserve_counts(struct inode *inode, bool restore_reserve)
> +void hugetlb_fix_reserve_counts(struct inode *inode)
>  {
>  	struct hugepage_subpool *spool = subpool_inode(inode);
>  	long rsv_adjust;
>  
>  	rsv_adjust = hugepage_subpool_get_pages(spool, 1);
> -	if (restore_reserve && rsv_adjust) {
> +	if (rsv_adjust) {
>  		struct hstate *h = hstate_inode(inode);
>  
>  		hugetlb_acct_memory(h, 1);
> 

Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>
Tested-by: Mike Kravetz <mike.kravetz@oracle.com>

-- 
Mike Kravetz

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

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH v2] mm: remove unnecessary condition in remove_inode_hugepages
  2016-09-27 14:16 [PATCH v2] mm: remove unnecessary condition in remove_inode_hugepages zhongjiang
  2016-09-27 19:23 ` Mike Kravetz
@ 2016-09-28  5:48 ` Naoya Horiguchi
  1 sibling, 0 replies; 5+ messages in thread
From: Naoya Horiguchi @ 2016-09-28  5:48 UTC (permalink / raw)
  To: zhongjiang; +Cc: mike.kravetz, mhocko, akpm, linux-mm

On Tue, Sep 27, 2016 at 10:16:26PM +0800, zhongjiang wrote:
> From: zhong jiang <zhongjiang@huawei.com>
> 
> when the huge page is added to the page cahce (huge_add_to_page_cache),
> the page private flag will be cleared. since this code
> (remove_inode_hugepages) will only be called for pages in the
> page cahce, PagePrivate(page) will always be false.
> 
> The patch remove the code without any functional change.
> 
> Signed-off-by: zhong jiang <zhongjiang@huawei.com>

Looks good to me.

Reviewed-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
--
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>

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH v2] mm: remove unnecessary condition in remove_inode_hugepages
  2016-09-27 19:23 ` Mike Kravetz
@ 2016-09-28 23:55   ` Mike Kravetz
  2016-09-29  1:31     ` zhong jiang
  0 siblings, 1 reply; 5+ messages in thread
From: Mike Kravetz @ 2016-09-28 23:55 UTC (permalink / raw)
  To: zhongjiang, mhocko, akpm; +Cc: linux-mm, n-horiguchi

On 09/27/2016 12:23 PM, Mike Kravetz wrote:
> On 09/27/2016 07:16 AM, zhongjiang wrote:
>> From: zhong jiang <zhongjiang@huawei.com>
>>
>> when the huge page is added to the page cahce (huge_add_to_page_cache),
>> the page private flag will be cleared. since this code
>> (remove_inode_hugepages) will only be called for pages in the
>> page cahce, PagePrivate(page) will always be false.
>>
>> The patch remove the code without any functional change.
>>
>> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
>> ---
>>  fs/hugetlbfs/inode.c    | 11 +++++------
>>  include/linux/hugetlb.h |  2 +-
>>  mm/hugetlb.c            |  4 ++--
>>  3 files changed, 8 insertions(+), 9 deletions(-)
>>
>> diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
>> index 4ea71eb..40d0afe 100644
>> --- a/fs/hugetlbfs/inode.c
>> +++ b/fs/hugetlbfs/inode.c
>> @@ -458,18 +458,17 @@ static void remove_inode_hugepages(struct inode *inode, loff_t lstart,
>>  			 * cache (remove_huge_page) BEFORE removing the
>>  			 * region/reserve map (hugetlb_unreserve_pages).  In
>>  			 * rare out of memory conditions, removal of the
>> -			 * region/reserve map could fail.  Before free'ing
>> -			 * the page, note PagePrivate which is used in case
>> -			 * of error.
>> +			 * region/reserve map could fail. Correspondingly,
>> +			 * the subpool and global reserve usage count can need
>> +			 * to be adjusted.
>>  			 */
>> -			rsv_on_error = !PagePrivate(page);

You also need to remove the definition of rsv_on_error.

Sorry, I missed that on the review.
-- 
Mike Kravetz

>> +			VM_BUG_ON(PagePrivate(page));
>>  			remove_huge_page(page);
>>  			freed++;
>>  			if (!truncate_op) {
>>  				if (unlikely(hugetlb_unreserve_pages(inode,
>>  							next, next + 1, 1)))
>> -					hugetlb_fix_reserve_counts(inode,
>> -								rsv_on_error);
>> +					hugetlb_fix_reserve_counts(inode);
>>  			}
>>  
>>  			unlock_page(page);
>> diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
>> index c26d463..d2e0fc5 100644
>> --- a/include/linux/hugetlb.h
>> +++ b/include/linux/hugetlb.h
>> @@ -90,7 +90,7 @@ int dequeue_hwpoisoned_huge_page(struct page *page);
>>  bool isolate_huge_page(struct page *page, struct list_head *list);
>>  void putback_active_hugepage(struct page *page);
>>  void free_huge_page(struct page *page);
>> -void hugetlb_fix_reserve_counts(struct inode *inode, bool restore_reserve);
>> +void hugetlb_fix_reserve_counts(struct inode *inode);
>>  extern struct mutex *hugetlb_fault_mutex_table;
>>  u32 hugetlb_fault_mutex_hash(struct hstate *h, struct mm_struct *mm,
>>  				struct vm_area_struct *vma,
>> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
>> index 87e11d8..28a079a 100644
>> --- a/mm/hugetlb.c
>> +++ b/mm/hugetlb.c
>> @@ -567,13 +567,13 @@ retry:
>>   * appear as a "reserved" entry instead of simply dangling with incorrect
>>   * counts.
>>   */
>> -void hugetlb_fix_reserve_counts(struct inode *inode, bool restore_reserve)
>> +void hugetlb_fix_reserve_counts(struct inode *inode)
>>  {
>>  	struct hugepage_subpool *spool = subpool_inode(inode);
>>  	long rsv_adjust;
>>  
>>  	rsv_adjust = hugepage_subpool_get_pages(spool, 1);
>> -	if (restore_reserve && rsv_adjust) {
>> +	if (rsv_adjust) {
>>  		struct hstate *h = hstate_inode(inode);
>>  
>>  		hugetlb_acct_memory(h, 1);
>>
> 
> Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>
> Tested-by: Mike Kravetz <mike.kravetz@oracle.com>
> 

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

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH v2] mm: remove unnecessary condition in remove_inode_hugepages
  2016-09-28 23:55   ` Mike Kravetz
@ 2016-09-29  1:31     ` zhong jiang
  0 siblings, 0 replies; 5+ messages in thread
From: zhong jiang @ 2016-09-29  1:31 UTC (permalink / raw)
  To: Mike Kravetz; +Cc: mhocko, akpm, linux-mm, n-horiguchi

On 2016/9/29 7:55, Mike Kravetz wrote:
> On 09/27/2016 12:23 PM, Mike Kravetz wrote:
>> On 09/27/2016 07:16 AM, zhongjiang wrote:
>>> From: zhong jiang <zhongjiang@huawei.com>
>>>
>>> when the huge page is added to the page cahce (huge_add_to_page_cache),
>>> the page private flag will be cleared. since this code
>>> (remove_inode_hugepages) will only be called for pages in the
>>> page cahce, PagePrivate(page) will always be false.
>>>
>>> The patch remove the code without any functional change.
>>>
>>> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
>>> ---
>>>  fs/hugetlbfs/inode.c    | 11 +++++------
>>>  include/linux/hugetlb.h |  2 +-
>>>  mm/hugetlb.c            |  4 ++--
>>>  3 files changed, 8 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
>>> index 4ea71eb..40d0afe 100644
>>> --- a/fs/hugetlbfs/inode.c
>>> +++ b/fs/hugetlbfs/inode.c
>>> @@ -458,18 +458,17 @@ static void remove_inode_hugepages(struct inode *inode, loff_t lstart,
>>>  			 * cache (remove_huge_page) BEFORE removing the
>>>  			 * region/reserve map (hugetlb_unreserve_pages).  In
>>>  			 * rare out of memory conditions, removal of the
>>> -			 * region/reserve map could fail.  Before free'ing
>>> -			 * the page, note PagePrivate which is used in case
>>> -			 * of error.
>>> +			 * region/reserve map could fail. Correspondingly,
>>> +			 * the subpool and global reserve usage count can need
>>> +			 * to be adjusted.
>>>  			 */
>>> -			rsv_on_error = !PagePrivate(page);
> You also need to remove the definition of rsv_on_error.
>
> Sorry, I missed that on the review.
  Thanks,  I will remove it now.

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

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-09-29  1:39 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-27 14:16 [PATCH v2] mm: remove unnecessary condition in remove_inode_hugepages zhongjiang
2016-09-27 19:23 ` Mike Kravetz
2016-09-28 23:55   ` Mike Kravetz
2016-09-29  1:31     ` zhong jiang
2016-09-28  5:48 ` Naoya Horiguchi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.