* [PATCH 01/10] hugetlb: make hugetlb_cma_check() static
2022-08-26 9:24 [PATCH 00/10] A few cleanup patches for hugetlb Miaohe Lin
@ 2022-08-26 9:24 ` Miaohe Lin
2022-08-27 1:40 ` Muchun Song
2022-08-26 9:24 ` [PATCH 02/10] hugetlb: Use helper macro SZ_1K Miaohe Lin
` (8 subsequent siblings)
9 siblings, 1 reply; 27+ messages in thread
From: Miaohe Lin @ 2022-08-26 9:24 UTC (permalink / raw)
To: akpm, mike.kravetz, songmuchun; +Cc: linux-mm, linux-kernel, linmiaohe
Make hugetlb_cma_check() static as it's only used inside mm/hugetlb.c.
Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
include/linux/hugetlb.h | 4 ----
mm/hugetlb.c | 10 +++++++++-
2 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index 852f911d676e..c2abbd7950de 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -1155,14 +1155,10 @@ static inline spinlock_t *huge_pte_lock(struct hstate *h,
#if defined(CONFIG_HUGETLB_PAGE) && defined(CONFIG_CMA)
extern void __init hugetlb_cma_reserve(int order);
-extern void __init hugetlb_cma_check(void);
#else
static inline __init void hugetlb_cma_reserve(int order)
{
}
-static inline __init void hugetlb_cma_check(void)
-{
-}
#endif
bool want_pmd_share(struct vm_area_struct *vma, unsigned long addr);
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index f1dc80f47dd6..10d84abb1ca7 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -4058,6 +4058,14 @@ static void hugetlb_register_all_nodes(void) { }
#endif
+#ifdef CONFIG_CMA
+static void __init hugetlb_cma_check(void);
+#else
+static inline __init void hugetlb_cma_check(void)
+{
+}
+#endif
+
static int __init hugetlb_init(void)
{
int i;
@@ -7546,7 +7554,7 @@ void __init hugetlb_cma_reserve(int order)
hugetlb_cma_size = 0;
}
-void __init hugetlb_cma_check(void)
+static void __init hugetlb_cma_check(void)
{
if (!hugetlb_cma_size || cma_reserve_called)
return;
--
2.23.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH 02/10] hugetlb: Use helper macro SZ_1K
2022-08-26 9:24 [PATCH 00/10] A few cleanup patches for hugetlb Miaohe Lin
2022-08-26 9:24 ` [PATCH 01/10] hugetlb: make hugetlb_cma_check() static Miaohe Lin
@ 2022-08-26 9:24 ` Miaohe Lin
2022-08-27 1:40 ` Muchun Song
2022-08-26 9:24 ` [PATCH 03/10] hugetlb: Use LIST_HEAD() to define a list head Miaohe Lin
` (7 subsequent siblings)
9 siblings, 1 reply; 27+ messages in thread
From: Miaohe Lin @ 2022-08-26 9:24 UTC (permalink / raw)
To: akpm, mike.kravetz, songmuchun; +Cc: linux-mm, linux-kernel, linmiaohe
Use helper macro SZ_1K to do the size conversion to make code more
consistent in this file. Minor readability improvement.
Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
mm/hugetlb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 10d84abb1ca7..24f580ddf130 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -4165,7 +4165,7 @@ void __init hugetlb_add_hstate(unsigned int order)
h->next_nid_to_alloc = first_memory_node;
h->next_nid_to_free = first_memory_node;
snprintf(h->name, HSTATE_NAME_LEN, "hugepages-%lukB",
- huge_page_size(h)/1024);
+ huge_page_size(h)/SZ_1K);
parsed_hstate = h;
}
--
2.23.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH 03/10] hugetlb: Use LIST_HEAD() to define a list head
2022-08-26 9:24 [PATCH 00/10] A few cleanup patches for hugetlb Miaohe Lin
2022-08-26 9:24 ` [PATCH 01/10] hugetlb: make hugetlb_cma_check() static Miaohe Lin
2022-08-26 9:24 ` [PATCH 02/10] hugetlb: Use helper macro SZ_1K Miaohe Lin
@ 2022-08-26 9:24 ` Miaohe Lin
2022-08-27 1:47 ` Muchun Song
2022-08-26 9:24 ` [PATCH 04/10] hugetlb: Use sizeof() to get the array size Miaohe Lin
` (6 subsequent siblings)
9 siblings, 1 reply; 27+ messages in thread
From: Miaohe Lin @ 2022-08-26 9:24 UTC (permalink / raw)
To: akpm, mike.kravetz, songmuchun; +Cc: linux-mm, linux-kernel, linmiaohe
We can avoid unneeded WRITE_ONCE() overhead by using LIST_HEAD() to define
a list head.
Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
mm/hugetlb.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 24f580ddf130..b3e6592247a3 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -459,14 +459,12 @@ static int allocate_file_region_entries(struct resv_map *resv,
int regions_needed)
__must_hold(&resv->lock)
{
- struct list_head allocated_regions;
+ LIST_HEAD(allocated_regions);
int to_allocate = 0, i = 0;
struct file_region *trg = NULL, *rg = NULL;
VM_BUG_ON(regions_needed < 0);
- INIT_LIST_HEAD(&allocated_regions);
-
/*
* Check for sufficient descriptors in the cache to accommodate
* the number of in progress add operations plus regions_needed.
@@ -2352,7 +2350,7 @@ struct page *alloc_huge_page_vma(struct hstate *h, struct vm_area_struct *vma,
static int gather_surplus_pages(struct hstate *h, long delta)
__must_hold(&hugetlb_lock)
{
- struct list_head surplus_list;
+ LIST_HEAD(surplus_list);
struct page *page, *tmp;
int ret;
long i;
@@ -2367,7 +2365,6 @@ static int gather_surplus_pages(struct hstate *h, long delta)
}
allocated = 0;
- INIT_LIST_HEAD(&surplus_list);
ret = -ENOMEM;
retry:
--
2.23.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH 03/10] hugetlb: Use LIST_HEAD() to define a list head
2022-08-26 9:24 ` [PATCH 03/10] hugetlb: Use LIST_HEAD() to define a list head Miaohe Lin
@ 2022-08-27 1:47 ` Muchun Song
2022-08-27 2:27 ` Miaohe Lin
0 siblings, 1 reply; 27+ messages in thread
From: Muchun Song @ 2022-08-27 1:47 UTC (permalink / raw)
To: Miaohe Lin
Cc: Andrew Morton, mike.kravetz, Muchun Song, linux-mm, linux-kernel
> On Aug 26, 2022, at 17:24, Miaohe Lin <linmiaohe@huawei.com> wrote:
>
> We can avoid unneeded WRITE_ONCE() overhead by using LIST_HEAD() to define
> a list head.
IIUC, the overhead doesn’t change. Right?
I’m fine with your changes.
Reviewed-by: Muchun Song <songmuchun@bytedance.com>
Thanks.
>
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> ---
> mm/hugetlb.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 24f580ddf130..b3e6592247a3 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -459,14 +459,12 @@ static int allocate_file_region_entries(struct resv_map *resv,
> int regions_needed)
> __must_hold(&resv->lock)
> {
> - struct list_head allocated_regions;
> + LIST_HEAD(allocated_regions);
> int to_allocate = 0, i = 0;
> struct file_region *trg = NULL, *rg = NULL;
>
> VM_BUG_ON(regions_needed < 0);
>
> - INIT_LIST_HEAD(&allocated_regions);
> -
> /*
> * Check for sufficient descriptors in the cache to accommodate
> * the number of in progress add operations plus regions_needed.
> @@ -2352,7 +2350,7 @@ struct page *alloc_huge_page_vma(struct hstate *h, struct vm_area_struct *vma,
> static int gather_surplus_pages(struct hstate *h, long delta)
> __must_hold(&hugetlb_lock)
> {
> - struct list_head surplus_list;
> + LIST_HEAD(surplus_list);
> struct page *page, *tmp;
> int ret;
> long i;
> @@ -2367,7 +2365,6 @@ static int gather_surplus_pages(struct hstate *h, long delta)
> }
>
> allocated = 0;
> - INIT_LIST_HEAD(&surplus_list);
>
> ret = -ENOMEM;
> retry:
> --
> 2.23.0
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 03/10] hugetlb: Use LIST_HEAD() to define a list head
2022-08-27 1:47 ` Muchun Song
@ 2022-08-27 2:27 ` Miaohe Lin
2022-08-27 2:48 ` Muchun Song
0 siblings, 1 reply; 27+ messages in thread
From: Miaohe Lin @ 2022-08-27 2:27 UTC (permalink / raw)
To: Muchun Song
Cc: Andrew Morton, mike.kravetz, Muchun Song, linux-mm, linux-kernel
On 2022/8/27 9:47, Muchun Song wrote:
>
>
>> On Aug 26, 2022, at 17:24, Miaohe Lin <linmiaohe@huawei.com> wrote:
>>
>> We can avoid unneeded WRITE_ONCE() overhead by using LIST_HEAD() to define
>> a list head.
>
> IIUC, the overhead doesn’t change. Right?
I think the overhead is changed. LIST_HEAD is initialized without using WRITE_ONCE():
#define LIST_HEAD_INIT(name) { &(name), &(name) }
#define LIST_HEAD(name) \
struct list_head name = LIST_HEAD_INIT(name)
while INIT_LIST_HEAD has:
static inline void INIT_LIST_HEAD(struct list_head *list)
{
WRITE_ONCE(list->next, list);
WRITE_ONCE(list->prev, list);
}
Or am I miss something?
>
> I’m fine with your changes.
>
> Reviewed-by: Muchun Song <songmuchun@bytedance.com>
Many thanks for your review and comment. :)
Thanks,
Miaohe Lin
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 03/10] hugetlb: Use LIST_HEAD() to define a list head
2022-08-27 2:27 ` Miaohe Lin
@ 2022-08-27 2:48 ` Muchun Song
2022-08-27 6:38 ` Miaohe Lin
0 siblings, 1 reply; 27+ messages in thread
From: Muchun Song @ 2022-08-27 2:48 UTC (permalink / raw)
To: Miaohe Lin
Cc: Andrew Morton, mike.kravetz, Muchun Song, linux-mm, linux-kernel
> On Aug 27, 2022, at 10:27, Miaohe Lin <linmiaohe@huawei.com> wrote:
>
> On 2022/8/27 9:47, Muchun Song wrote:
>>
>>
>>> On Aug 26, 2022, at 17:24, Miaohe Lin <linmiaohe@huawei.com> wrote:
>>>
>>> We can avoid unneeded WRITE_ONCE() overhead by using LIST_HEAD() to define
>>> a list head.
>>
>> IIUC, the overhead doesn’t change. Right?
>
> I think the overhead is changed. LIST_HEAD is initialized without using WRITE_ONCE():
I think there is no special difference with "WRITE_ONCE(var, 0)" vs "var = 0” in
assembly code. Both code of line will be compiled to a mov or movq instruction.
I didn’t confirm if the assembly code is different (I tend to think it is similar).
Just some analysis from me.
>
> #define LIST_HEAD_INIT(name) { &(name), &(name) }
>
> #define LIST_HEAD(name) \
> struct list_head name = LIST_HEAD_INIT(name)
>
> while INIT_LIST_HEAD has:
>
> static inline void INIT_LIST_HEAD(struct list_head *list)
> {
> WRITE_ONCE(list->next, list);
> WRITE_ONCE(list->prev, list);
> }
>
> Or am I miss something?
>
>>
>> I’m fine with your changes.
>>
>> Reviewed-by: Muchun Song <songmuchun@bytedance.com>
>
> Many thanks for your review and comment. :)
>
> Thanks,
> Miaohe Lin
>
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 03/10] hugetlb: Use LIST_HEAD() to define a list head
2022-08-27 2:48 ` Muchun Song
@ 2022-08-27 6:38 ` Miaohe Lin
0 siblings, 0 replies; 27+ messages in thread
From: Miaohe Lin @ 2022-08-27 6:38 UTC (permalink / raw)
To: Muchun Song
Cc: Andrew Morton, mike.kravetz, Muchun Song, linux-mm, linux-kernel
On 2022/8/27 10:48, Muchun Song wrote:
>
>
>> On Aug 27, 2022, at 10:27, Miaohe Lin <linmiaohe@huawei.com> wrote:
>>
>> On 2022/8/27 9:47, Muchun Song wrote:
>>>
>>>
>>>> On Aug 26, 2022, at 17:24, Miaohe Lin <linmiaohe@huawei.com> wrote:
>>>>
>>>> We can avoid unneeded WRITE_ONCE() overhead by using LIST_HEAD() to define
>>>> a list head.
>>>
>>> IIUC, the overhead doesn’t change. Right?
>>
>> I think the overhead is changed. LIST_HEAD is initialized without using WRITE_ONCE():
>
> I think there is no special difference with "WRITE_ONCE(var, 0)" vs "var = 0” in
It's not write var to 0 indeed. But there seems are no special difference.
> assembly code. Both code of line will be compiled to a mov or movq instruction.
> I didn’t confirm if the assembly code is different (I tend to think it is similar).
> Just some analysis from me.
I checked the generated code in x86, they're almost same. And in aarch64, there's difference
between one "stp" instruction vs two "str" instruction. So I think you're right. Thanks for
pointing this out. I should tweak the commit log in next version.
Thanks a lot,
Miaohe Lin
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 04/10] hugetlb: Use sizeof() to get the array size
2022-08-26 9:24 [PATCH 00/10] A few cleanup patches for hugetlb Miaohe Lin
` (2 preceding siblings ...)
2022-08-26 9:24 ` [PATCH 03/10] hugetlb: Use LIST_HEAD() to define a list head Miaohe Lin
@ 2022-08-26 9:24 ` Miaohe Lin
2022-08-27 1:43 ` Muchun Song
2022-08-26 9:24 ` [PATCH 05/10] hugetlb: Use helper {huge_pte|pmd}_lock() Miaohe Lin
` (5 subsequent siblings)
9 siblings, 1 reply; 27+ messages in thread
From: Miaohe Lin @ 2022-08-26 9:24 UTC (permalink / raw)
To: akpm, mike.kravetz, songmuchun; +Cc: linux-mm, linux-kernel, linmiaohe
It's better to use sizeof() to get the array size instead of manual
calculation. Minor readability improvement.
Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
mm/hugetlb.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index b3e6592247a3..06293da96112 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -4177,11 +4177,11 @@ static void __init hugepages_clear_pages_in_node(void)
if (!hugetlb_max_hstate) {
default_hstate_max_huge_pages = 0;
memset(default_hugepages_in_node, 0,
- MAX_NUMNODES * sizeof(unsigned int));
+ sizeof(default_hugepages_in_node));
} else {
parsed_hstate->max_huge_pages = 0;
memset(parsed_hstate->max_huge_pages_node, 0,
- MAX_NUMNODES * sizeof(unsigned int));
+ sizeof(parsed_hstate->max_huge_pages_node));
}
}
--
2.23.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH 05/10] hugetlb: Use helper {huge_pte|pmd}_lock()
2022-08-26 9:24 [PATCH 00/10] A few cleanup patches for hugetlb Miaohe Lin
` (3 preceding siblings ...)
2022-08-26 9:24 ` [PATCH 04/10] hugetlb: Use sizeof() to get the array size Miaohe Lin
@ 2022-08-26 9:24 ` Miaohe Lin
2022-08-27 1:48 ` Muchun Song
2022-08-26 9:24 ` [PATCH 06/10] hugetlb: pass NULL to kobj_to_hstate() if nid is unused Miaohe Lin
` (4 subsequent siblings)
9 siblings, 1 reply; 27+ messages in thread
From: Miaohe Lin @ 2022-08-26 9:24 UTC (permalink / raw)
To: akpm, mike.kravetz, songmuchun; +Cc: linux-mm, linux-kernel, linmiaohe
Use helper huge_pte_lock and pmd_lock to simplify the code. No functional
change intended.
Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
mm/hugetlb.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 06293da96112..6ea9f73aea84 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -6094,8 +6094,7 @@ int hugetlb_mcopy_atomic_pte(struct mm_struct *dst_mm,
page_in_pagecache = true;
}
- ptl = huge_pte_lockptr(h, dst_mm, dst_pte);
- spin_lock(ptl);
+ ptl = huge_pte_lock(h, dst_mm, dst_pte);
/*
* We allow to overwrite a pte marker: consider when both MISSING|WP
@@ -7176,8 +7175,7 @@ follow_huge_pmd(struct mm_struct *mm, unsigned long address,
return NULL;
retry:
- ptl = pmd_lockptr(mm, pmd);
- spin_lock(ptl);
+ ptl = pmd_lock(mm, pmd);
/*
* make sure that the address range covered by this pmd is not
* unmapped from other threads.
--
2.23.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH 06/10] hugetlb: pass NULL to kobj_to_hstate() if nid is unused
2022-08-26 9:24 [PATCH 00/10] A few cleanup patches for hugetlb Miaohe Lin
` (4 preceding siblings ...)
2022-08-26 9:24 ` [PATCH 05/10] hugetlb: Use helper {huge_pte|pmd}_lock() Miaohe Lin
@ 2022-08-26 9:24 ` Miaohe Lin
2022-08-27 1:49 ` Muchun Song
2022-08-26 9:24 ` [PATCH 07/10] hugetlb: kill hugetlbfs_pagecache_page() Miaohe Lin
` (3 subsequent siblings)
9 siblings, 1 reply; 27+ messages in thread
From: Miaohe Lin @ 2022-08-26 9:24 UTC (permalink / raw)
To: akpm, mike.kravetz, songmuchun; +Cc: linux-mm, linux-kernel, linmiaohe
We can pass NULL to kobj_to_hstate() directly when nid is unused to
simplify the code. No functional change intended.
Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
mm/hugetlb.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 6ea9f73aea84..58eeae934e09 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -3779,8 +3779,7 @@ HSTATE_ATTR_WO(demote);
static ssize_t demote_size_show(struct kobject *kobj,
struct kobj_attribute *attr, char *buf)
{
- int nid;
- struct hstate *h = kobj_to_hstate(kobj, &nid);
+ struct hstate *h = kobj_to_hstate(kobj, NULL);
unsigned long demote_size = (PAGE_SIZE << h->demote_order) / SZ_1K;
return sysfs_emit(buf, "%lukB\n", demote_size);
@@ -3793,7 +3792,6 @@ static ssize_t demote_size_store(struct kobject *kobj,
struct hstate *h, *demote_hstate;
unsigned long demote_size;
unsigned int demote_order;
- int nid;
demote_size = (unsigned long)memparse(buf, NULL);
@@ -3805,7 +3803,7 @@ static ssize_t demote_size_store(struct kobject *kobj,
return -EINVAL;
/* demote order must be smaller than hstate order */
- h = kobj_to_hstate(kobj, &nid);
+ h = kobj_to_hstate(kobj, NULL);
if (demote_order >= h->order)
return -EINVAL;
--
2.23.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH 07/10] hugetlb: kill hugetlbfs_pagecache_page()
2022-08-26 9:24 [PATCH 00/10] A few cleanup patches for hugetlb Miaohe Lin
` (5 preceding siblings ...)
2022-08-26 9:24 ` [PATCH 06/10] hugetlb: pass NULL to kobj_to_hstate() if nid is unused Miaohe Lin
@ 2022-08-26 9:24 ` Miaohe Lin
2022-08-27 1:52 ` Muchun Song
2022-08-26 9:24 ` [PATCH 08/10] hugetlb: remove unneeded SetHPageVmemmapOptimized() Miaohe Lin
` (2 subsequent siblings)
9 siblings, 1 reply; 27+ messages in thread
From: Miaohe Lin @ 2022-08-26 9:24 UTC (permalink / raw)
To: akpm, mike.kravetz, songmuchun; +Cc: linux-mm, linux-kernel, linmiaohe
Fold hugetlbfs_pagecache_page() into its sole caller to remove some
duplicated code. No functional change intended.
Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
mm/hugetlb.c | 15 +--------------
1 file changed, 1 insertion(+), 14 deletions(-)
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 58eeae934e09..7934188bbed0 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -5467,19 +5467,6 @@ static vm_fault_t hugetlb_wp(struct mm_struct *mm, struct vm_area_struct *vma,
return ret;
}
-/* Return the pagecache page at a given address within a VMA */
-static struct page *hugetlbfs_pagecache_page(struct hstate *h,
- struct vm_area_struct *vma, unsigned long address)
-{
- struct address_space *mapping;
- pgoff_t idx;
-
- mapping = vma->vm_file->f_mapping;
- idx = vma_hugecache_offset(h, vma, address);
-
- return find_lock_page(mapping, idx);
-}
-
/*
* Return whether there is a pagecache page to back given address within VMA.
* Caller follow_hugetlb_page() holds page_table_lock so we cannot lock_page.
@@ -5885,7 +5872,7 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma,
/* Just decrements count, does not deallocate */
vma_end_reservation(h, vma, haddr);
- pagecache_page = hugetlbfs_pagecache_page(h, vma, haddr);
+ pagecache_page = find_lock_page(mapping, idx);
}
ptl = huge_pte_lock(h, mm, ptep);
--
2.23.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH 08/10] hugetlb: remove unneeded SetHPageVmemmapOptimized()
2022-08-26 9:24 [PATCH 00/10] A few cleanup patches for hugetlb Miaohe Lin
` (6 preceding siblings ...)
2022-08-26 9:24 ` [PATCH 07/10] hugetlb: kill hugetlbfs_pagecache_page() Miaohe Lin
@ 2022-08-26 9:24 ` Miaohe Lin
2022-08-27 1:35 ` Muchun Song
2022-08-26 9:24 ` [PATCH 09/10] hugetlb: remove meaningless BUG_ON(huge_pte_none()) Miaohe Lin
2022-08-26 9:24 ` [PATCH 10/10] hugetlb: make hugetlb selects SYSFS if !SYSCTL Miaohe Lin
9 siblings, 1 reply; 27+ messages in thread
From: Miaohe Lin @ 2022-08-26 9:24 UTC (permalink / raw)
To: akpm, mike.kravetz, songmuchun; +Cc: linux-mm, linux-kernel, linmiaohe
If code reaches here, it's guaranteed that HPageVmemmapOptimized is set
for the hugetlb page (or VM_BUG_ON_PAGE() will complain about it). It's
unnecessary to set it again.
Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
mm/hugetlb.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 7934188bbed0..b432a00061e3 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -1520,7 +1520,6 @@ static void add_hugetlb_page(struct hstate *h, struct page *page,
set_compound_page_dtor(page, HUGETLB_PAGE_DTOR);
set_page_private(page, 0);
- SetHPageVmemmapOptimized(page);
/*
* This page is about to be managed by the hugetlb allocator and
--
2.23.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH 08/10] hugetlb: remove unneeded SetHPageVmemmapOptimized()
2022-08-26 9:24 ` [PATCH 08/10] hugetlb: remove unneeded SetHPageVmemmapOptimized() Miaohe Lin
@ 2022-08-27 1:35 ` Muchun Song
2022-08-27 2:18 ` Miaohe Lin
0 siblings, 1 reply; 27+ messages in thread
From: Muchun Song @ 2022-08-27 1:35 UTC (permalink / raw)
To: Miaohe Lin
Cc: Andrew Morton, Mike Kravetz, Muchun Song, Linux MM, linux-kernel
> On Aug 26, 2022, at 17:24, Miaohe Lin <linmiaohe@huawei.com> wrote:
>
> If code reaches here, it's guaranteed that HPageVmemmapOptimized is set
> for the hugetlb page (or VM_BUG_ON_PAGE() will complain about it). It's
Hi Miaohe,
Right.
> unnecessary to set it again.
No, I suppose you didn’t test this patch since it does not work. The
HPageVmemmapOptimized is cleared in the above code of line
(set_page_private(page, 0)). So NAck.
Thanks,
Muchun
>
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> ---
> mm/hugetlb.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 7934188bbed0..b432a00061e3 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -1520,7 +1520,6 @@ static void add_hugetlb_page(struct hstate *h, struct page *page,
>
> set_compound_page_dtor(page, HUGETLB_PAGE_DTOR);
> set_page_private(page, 0);
> - SetHPageVmemmapOptimized(page);
>
> /*
> * This page is about to be managed by the hugetlb allocator and
> --
> 2.23.0
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 08/10] hugetlb: remove unneeded SetHPageVmemmapOptimized()
2022-08-27 1:35 ` Muchun Song
@ 2022-08-27 2:18 ` Miaohe Lin
2022-08-27 2:56 ` Muchun Song
0 siblings, 1 reply; 27+ messages in thread
From: Miaohe Lin @ 2022-08-27 2:18 UTC (permalink / raw)
To: Muchun Song
Cc: Andrew Morton, Mike Kravetz, Muchun Song, Linux MM, linux-kernel
On 2022/8/27 9:35, Muchun Song wrote:
>
>
>> On Aug 26, 2022, at 17:24, Miaohe Lin <linmiaohe@huawei.com> wrote:
>>
>> If code reaches here, it's guaranteed that HPageVmemmapOptimized is set
>> for the hugetlb page (or VM_BUG_ON_PAGE() will complain about it). It's
>
> Hi Miaohe,
>
> Right.
>
>> unnecessary to set it again.
>
> No, I suppose you didn’t test this patch since it does not work. The
> HPageVmemmapOptimized is cleared in the above code of line
> (set_page_private(page, 0)). So NAck.
Sorry, I missed that and thanks for pointing this out. Does this code deserves a
comment like below?
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index d0617d64d718..3374c1d2b52e 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -1522,6 +1522,10 @@ static void add_hugetlb_page(struct hstate *h, struct page *page,
set_compound_page_dtor(page, HUGETLB_PAGE_DTOR);
set_page_private(page, 0);
+ /*
+ * We have to set HPageVmemmapOptimized again as above
+ * set_page_private(page, 0) cleared it.
+ */
SetHPageVmemmapOptimized(page);
/*
Thanks,
Miaohe Lin
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH 08/10] hugetlb: remove unneeded SetHPageVmemmapOptimized()
2022-08-27 2:18 ` Miaohe Lin
@ 2022-08-27 2:56 ` Muchun Song
0 siblings, 0 replies; 27+ messages in thread
From: Muchun Song @ 2022-08-27 2:56 UTC (permalink / raw)
To: Miaohe Lin
Cc: Andrew Morton, Mike Kravetz, Muchun Song, Linux MM, linux-kernel
> On Aug 27, 2022, at 10:18, Miaohe Lin <linmiaohe@huawei.com> wrote:
>
> On 2022/8/27 9:35, Muchun Song wrote:
>>
>>
>>> On Aug 26, 2022, at 17:24, Miaohe Lin <linmiaohe@huawei.com> wrote:
>>>
>>> If code reaches here, it's guaranteed that HPageVmemmapOptimized is set
>>> for the hugetlb page (or VM_BUG_ON_PAGE() will complain about it). It's
>>
>> Hi Miaohe,
>>
>> Right.
>>
>>> unnecessary to set it again.
>>
>> No, I suppose you didn’t test this patch since it does not work. The
>> HPageVmemmapOptimized is cleared in the above code of line
>> (set_page_private(page, 0)). So NAck.
>
> Sorry, I missed that and thanks for pointing this out. Does this code deserves a
> comment like below?
>
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index d0617d64d718..3374c1d2b52e 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -1522,6 +1522,10 @@ static void add_hugetlb_page(struct hstate *h, struct page *page,
>
> set_compound_page_dtor(page, HUGETLB_PAGE_DTOR);
> set_page_private(page, 0);
> + /*
> + * We have to set HPageVmemmapOptimized again as above
> + * set_page_private(page, 0) cleared it.
> + */
> SetHPageVmemmapOptimized(page);
I’m fine with this comment.
Reviewed-by: Muchun Song <songmuchun@bytedance.com>
>
> /*
>
>
> Thanks,
> Miaohe Lin
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 09/10] hugetlb: remove meaningless BUG_ON(huge_pte_none())
2022-08-26 9:24 [PATCH 00/10] A few cleanup patches for hugetlb Miaohe Lin
` (7 preceding siblings ...)
2022-08-26 9:24 ` [PATCH 08/10] hugetlb: remove unneeded SetHPageVmemmapOptimized() Miaohe Lin
@ 2022-08-26 9:24 ` Miaohe Lin
2022-08-27 2:03 ` Muchun Song
2022-08-26 9:24 ` [PATCH 10/10] hugetlb: make hugetlb selects SYSFS if !SYSCTL Miaohe Lin
9 siblings, 1 reply; 27+ messages in thread
From: Miaohe Lin @ 2022-08-26 9:24 UTC (permalink / raw)
To: akpm, mike.kravetz, songmuchun; +Cc: linux-mm, linux-kernel, linmiaohe
When code reaches here, invalid page would have been accessed if huge pte
is none. So this BUG_ON(huge_pte_none()) is meaningless. Remove it.
Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
mm/hugetlb.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index b432a00061e3..0ef26dbdc7f8 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -5374,7 +5374,6 @@ static vm_fault_t hugetlb_wp(struct mm_struct *mm, struct vm_area_struct *vma,
u32 hash;
put_page(old_page);
- BUG_ON(huge_pte_none(pte));
/*
* Drop hugetlb_fault_mutex and vma_lock before
* unmapping. unmapping needs to hold vma_lock
--
2.23.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH 10/10] hugetlb: make hugetlb selects SYSFS if !SYSCTL
2022-08-26 9:24 [PATCH 00/10] A few cleanup patches for hugetlb Miaohe Lin
` (8 preceding siblings ...)
2022-08-26 9:24 ` [PATCH 09/10] hugetlb: remove meaningless BUG_ON(huge_pte_none()) Miaohe Lin
@ 2022-08-26 9:24 ` Miaohe Lin
2022-08-27 1:59 ` Muchun Song
9 siblings, 1 reply; 27+ messages in thread
From: Miaohe Lin @ 2022-08-26 9:24 UTC (permalink / raw)
To: akpm, mike.kravetz, songmuchun; +Cc: linux-mm, linux-kernel, linmiaohe
If CONFIG_SYSFS and CONFIG_SYSCTL are both undefined, hugetlb doesn't work
now as there's no way to set max huge pages. Make sure at least one of the
above configs is defined to make hugetlb works as expected.
Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
fs/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/fs/Kconfig b/fs/Kconfig
index a547307c1ae8..b266e7b5de85 100644
--- a/fs/Kconfig
+++ b/fs/Kconfig
@@ -235,6 +235,7 @@ config ARCH_SUPPORTS_HUGETLBFS
config HUGETLBFS
bool "HugeTLB file system support"
depends on X86 || IA64 || SPARC64 || ARCH_SUPPORTS_HUGETLBFS || BROKEN
+ select SYSFS if !SYSCTL
help
hugetlbfs is a filesystem backing for HugeTLB pages, based on
ramfs. For architectures that support it, say Y here and read
--
2.23.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH 10/10] hugetlb: make hugetlb selects SYSFS if !SYSCTL
2022-08-26 9:24 ` [PATCH 10/10] hugetlb: make hugetlb selects SYSFS if !SYSCTL Miaohe Lin
@ 2022-08-27 1:59 ` Muchun Song
2022-08-27 2:34 ` Miaohe Lin
0 siblings, 1 reply; 27+ messages in thread
From: Muchun Song @ 2022-08-27 1:59 UTC (permalink / raw)
To: Miaohe Lin
Cc: Andrew Morton, Mike Kravetz, Muchun Song, linux-mm, linux-kernel
> On Aug 26, 2022, at 17:24, Miaohe Lin <linmiaohe@huawei.com> wrote:
>
> If CONFIG_SYSFS and CONFIG_SYSCTL are both undefined, hugetlb doesn't work
> now as there's no way to set max huge pages. Make sure at least one of the
> above configs is defined to make hugetlb works as expected.
Right. I think the majority of the people are usually use /proc knob to
reserve HugeTLB pages than /sys fs. So I suggest to add a “depends on SYSCTL”
item. What’s your thoughts?
Thanks.
>
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> ---
> fs/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/fs/Kconfig b/fs/Kconfig
> index a547307c1ae8..b266e7b5de85 100644
> --- a/fs/Kconfig
> +++ b/fs/Kconfig
> @@ -235,6 +235,7 @@ config ARCH_SUPPORTS_HUGETLBFS
> config HUGETLBFS
> bool "HugeTLB file system support"
> depends on X86 || IA64 || SPARC64 || ARCH_SUPPORTS_HUGETLBFS || BROKEN
> + select SYSFS if !SYSCTL
> help
> hugetlbfs is a filesystem backing for HugeTLB pages, based on
> ramfs. For architectures that support it, say Y here and read
> --
> 2.23.0
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 10/10] hugetlb: make hugetlb selects SYSFS if !SYSCTL
2022-08-27 1:59 ` Muchun Song
@ 2022-08-27 2:34 ` Miaohe Lin
0 siblings, 0 replies; 27+ messages in thread
From: Miaohe Lin @ 2022-08-27 2:34 UTC (permalink / raw)
To: Muchun Song
Cc: Andrew Morton, Mike Kravetz, Muchun Song, linux-mm, linux-kernel
On 2022/8/27 9:59, Muchun Song wrote:
>
>
>> On Aug 26, 2022, at 17:24, Miaohe Lin <linmiaohe@huawei.com> wrote:
>>
>> If CONFIG_SYSFS and CONFIG_SYSCTL are both undefined, hugetlb doesn't work
>> now as there's no way to set max huge pages. Make sure at least one of the
>> above configs is defined to make hugetlb works as expected.
>
> Right. I think the majority of the people are usually use /proc knob to
> reserve HugeTLB pages than /sys fs. So I suggest to add a “depends on SYSCTL”
> item. What’s your thoughts?
It seems some functions, e.g. demotion, is exported via SYSFS instead of SYSCTL.
So maybe "depends on SYSFS || SYSCTL", i.e.
depends on (X86 || IA64 || SPARC64 || ARCH_SUPPORTS_HUGETLBFS || BROKEN) && (SYSFS || SYSCTL)
will be more suitable?
Thanks,
Miaohe Lin
^ permalink raw reply [flat|nested] 27+ messages in thread