| Message ID | 20201205130224.81607-6-songmuchun@bytedance.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Convert all THP vmstat counters to pages | expand |
On Sat, Dec 05, 2020 at 09:02:20PM +0800, Muchun Song wrote: > Converrt NR_FILE_THPS account to pages. > > Signed-off-by: Muchun Song <songmuchun@bytedance.com> > --- > drivers/base/node.c | 3 +-- > fs/proc/meminfo.c | 2 +- > mm/filemap.c | 2 +- > mm/huge_memory.c | 3 ++- > mm/khugepaged.c | 2 +- > mm/memcontrol.c | 5 ++--- > 6 files changed, 8 insertions(+), 9 deletions(-) > > diff --git a/drivers/base/node.c b/drivers/base/node.c > index 05c369e93e16..f6a9521bbcf8 100644 > --- a/drivers/base/node.c > +++ b/drivers/base/node.c > @@ -466,8 +466,7 @@ static ssize_t node_read_meminfo(struct device *dev, > HPAGE_PMD_NR), > nid, K(node_page_state(pgdat, NR_SHMEM_PMDMAPPED) * > HPAGE_PMD_NR), > - nid, K(node_page_state(pgdat, NR_FILE_THPS) * > - HPAGE_PMD_NR), > + nid, K(node_page_state(pgdat, NR_FILE_THPS)), Again, is this changing a user-visable value?
On Sat, Dec 5, 2020 at 10:09 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > On Sat, Dec 05, 2020 at 09:02:20PM +0800, Muchun Song wrote: > > Converrt NR_FILE_THPS account to pages. > > > > Signed-off-by: Muchun Song <songmuchun@bytedance.com> > > --- > > drivers/base/node.c | 3 +-- > > fs/proc/meminfo.c | 2 +- > > mm/filemap.c | 2 +- > > mm/huge_memory.c | 3 ++- > > mm/khugepaged.c | 2 +- > > mm/memcontrol.c | 5 ++--- > > 6 files changed, 8 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/base/node.c b/drivers/base/node.c > > index 05c369e93e16..f6a9521bbcf8 100644 > > --- a/drivers/base/node.c > > +++ b/drivers/base/node.c > > @@ -466,8 +466,7 @@ static ssize_t node_read_meminfo(struct device *dev, > > HPAGE_PMD_NR), > > nid, K(node_page_state(pgdat, NR_SHMEM_PMDMAPPED) * > > HPAGE_PMD_NR), > > - nid, K(node_page_state(pgdat, NR_FILE_THPS) * > > - HPAGE_PMD_NR), > > + nid, K(node_page_state(pgdat, NR_FILE_THPS)), > > Again, is this changing a user-visable value? > Of course not. In the previous, the NR_FILE_THPS account is like below: __mod_lruvec_page_state(page, NR_FILE_THPS, 1); With this patch, it is: __mod_lruvec_page_state(page, NR_FILE_THPS, HPAGE_PMD_NR); So the result is not changed from the view of user space. Thanks. -- Yours, Muchun
On Sat, Dec 05, 2020 at 11:29:26PM +0800, Muchun Song wrote: > On Sat, Dec 5, 2020 at 10:09 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > > > On Sat, Dec 05, 2020 at 09:02:20PM +0800, Muchun Song wrote: > > > Converrt NR_FILE_THPS account to pages. > > > > > > Signed-off-by: Muchun Song <songmuchun@bytedance.com> > > > --- > > > drivers/base/node.c | 3 +-- > > > fs/proc/meminfo.c | 2 +- > > > mm/filemap.c | 2 +- > > > mm/huge_memory.c | 3 ++- > > > mm/khugepaged.c | 2 +- > > > mm/memcontrol.c | 5 ++--- > > > 6 files changed, 8 insertions(+), 9 deletions(-) > > > > > > diff --git a/drivers/base/node.c b/drivers/base/node.c > > > index 05c369e93e16..f6a9521bbcf8 100644 > > > --- a/drivers/base/node.c > > > +++ b/drivers/base/node.c > > > @@ -466,8 +466,7 @@ static ssize_t node_read_meminfo(struct device *dev, > > > HPAGE_PMD_NR), > > > nid, K(node_page_state(pgdat, NR_SHMEM_PMDMAPPED) * > > > HPAGE_PMD_NR), > > > - nid, K(node_page_state(pgdat, NR_FILE_THPS) * > > > - HPAGE_PMD_NR), > > > + nid, K(node_page_state(pgdat, NR_FILE_THPS)), > > > > Again, is this changing a user-visable value? > > > > Of course not. > > In the previous, the NR_FILE_THPS account is like below: > > __mod_lruvec_page_state(page, NR_FILE_THPS, 1); > > With this patch, it is: > > __mod_lruvec_page_state(page, NR_FILE_THPS, HPAGE_PMD_NR); > > So the result is not changed from the view of user space. So you "broke" it on the previous patch and "fixed" it on this one? Why not just do it all in one patch? Confused, greg k-h
On Sat, Dec 5, 2020 at 11:32 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > On Sat, Dec 05, 2020 at 11:29:26PM +0800, Muchun Song wrote: > > On Sat, Dec 5, 2020 at 10:09 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > > > > > On Sat, Dec 05, 2020 at 09:02:20PM +0800, Muchun Song wrote: > > > > Converrt NR_FILE_THPS account to pages. > > > > > > > > Signed-off-by: Muchun Song <songmuchun@bytedance.com> > > > > --- > > > > drivers/base/node.c | 3 +-- > > > > fs/proc/meminfo.c | 2 +- > > > > mm/filemap.c | 2 +- > > > > mm/huge_memory.c | 3 ++- > > > > mm/khugepaged.c | 2 +- > > > > mm/memcontrol.c | 5 ++--- > > > > 6 files changed, 8 insertions(+), 9 deletions(-) > > > > > > > > diff --git a/drivers/base/node.c b/drivers/base/node.c > > > > index 05c369e93e16..f6a9521bbcf8 100644 > > > > --- a/drivers/base/node.c > > > > +++ b/drivers/base/node.c > > > > @@ -466,8 +466,7 @@ static ssize_t node_read_meminfo(struct device *dev, > > > > HPAGE_PMD_NR), > > > > nid, K(node_page_state(pgdat, NR_SHMEM_PMDMAPPED) * > > > > HPAGE_PMD_NR), > > > > - nid, K(node_page_state(pgdat, NR_FILE_THPS) * > > > > - HPAGE_PMD_NR), > > > > + nid, K(node_page_state(pgdat, NR_FILE_THPS)), > > > > > > Again, is this changing a user-visable value? > > > > > > > Of course not. > > > > In the previous, the NR_FILE_THPS account is like below: > > > > __mod_lruvec_page_state(page, NR_FILE_THPS, 1); > > > > With this patch, it is: > > > > __mod_lruvec_page_state(page, NR_FILE_THPS, HPAGE_PMD_NR); > > > > So the result is not changed from the view of user space. > > So you "broke" it on the previous patch and "fixed" it on this one? Why > not just do it all in one patch? Sorry for the confusion. I mean that the "previous" is without all of this patch series. So this series is aimed to convert the unit of all different THP vmstat counters from HPAGE_PMD_NR to pages. Thanks. > > Confused, > > greg k-h
On Sat, Dec 05, 2020 at 11:39:24PM +0800, Muchun Song wrote: > On Sat, Dec 5, 2020 at 11:32 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > > > On Sat, Dec 05, 2020 at 11:29:26PM +0800, Muchun Song wrote: > > > On Sat, Dec 5, 2020 at 10:09 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > > > > > > > On Sat, Dec 05, 2020 at 09:02:20PM +0800, Muchun Song wrote: > > > > > Converrt NR_FILE_THPS account to pages. > > > > > > > > > > Signed-off-by: Muchun Song <songmuchun@bytedance.com> > > > > > --- > > > > > drivers/base/node.c | 3 +-- > > > > > fs/proc/meminfo.c | 2 +- > > > > > mm/filemap.c | 2 +- > > > > > mm/huge_memory.c | 3 ++- > > > > > mm/khugepaged.c | 2 +- > > > > > mm/memcontrol.c | 5 ++--- > > > > > 6 files changed, 8 insertions(+), 9 deletions(-) > > > > > > > > > > diff --git a/drivers/base/node.c b/drivers/base/node.c > > > > > index 05c369e93e16..f6a9521bbcf8 100644 > > > > > --- a/drivers/base/node.c > > > > > +++ b/drivers/base/node.c > > > > > @@ -466,8 +466,7 @@ static ssize_t node_read_meminfo(struct device *dev, > > > > > HPAGE_PMD_NR), > > > > > nid, K(node_page_state(pgdat, NR_SHMEM_PMDMAPPED) * > > > > > HPAGE_PMD_NR), > > > > > - nid, K(node_page_state(pgdat, NR_FILE_THPS) * > > > > > - HPAGE_PMD_NR), > > > > > + nid, K(node_page_state(pgdat, NR_FILE_THPS)), > > > > > > > > Again, is this changing a user-visable value? > > > > > > > > > > Of course not. > > > > > > In the previous, the NR_FILE_THPS account is like below: > > > > > > __mod_lruvec_page_state(page, NR_FILE_THPS, 1); > > > > > > With this patch, it is: > > > > > > __mod_lruvec_page_state(page, NR_FILE_THPS, HPAGE_PMD_NR); > > > > > > So the result is not changed from the view of user space. > > > > So you "broke" it on the previous patch and "fixed" it on this one? Why > > not just do it all in one patch? > > Sorry for the confusion. I mean that the "previous" is without all of this patch > series. So this series is aimed to convert the unit of all different THP vmstat > counters from HPAGE_PMD_NR to pages. Thanks. I'm sorry, I still do not understand. It looks to me that you are changing the number printed to userspace here. Where is the corrisponding change that changed the units for this function? Is it in this patch? If so, sorry, I did not see that at all... thanks, greg k-h
On Sun, Dec 6, 2020 at 12:32 AM Greg KH <gregkh@linuxfoundation.org> wrote: > > On Sat, Dec 05, 2020 at 11:39:24PM +0800, Muchun Song wrote: > > On Sat, Dec 5, 2020 at 11:32 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > > > > > On Sat, Dec 05, 2020 at 11:29:26PM +0800, Muchun Song wrote: > > > > On Sat, Dec 5, 2020 at 10:09 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > > > > > > > > > On Sat, Dec 05, 2020 at 09:02:20PM +0800, Muchun Song wrote: > > > > > > Converrt NR_FILE_THPS account to pages. > > > > > > > > > > > > Signed-off-by: Muchun Song <songmuchun@bytedance.com> > > > > > > --- > > > > > > drivers/base/node.c | 3 +-- > > > > > > fs/proc/meminfo.c | 2 +- > > > > > > mm/filemap.c | 2 +- > > > > > > mm/huge_memory.c | 3 ++- > > > > > > mm/khugepaged.c | 2 +- > > > > > > mm/memcontrol.c | 5 ++--- > > > > > > 6 files changed, 8 insertions(+), 9 deletions(-) > > > > > > > > > > > > diff --git a/drivers/base/node.c b/drivers/base/node.c > > > > > > index 05c369e93e16..f6a9521bbcf8 100644 > > > > > > --- a/drivers/base/node.c > > > > > > +++ b/drivers/base/node.c > > > > > > @@ -466,8 +466,7 @@ static ssize_t node_read_meminfo(struct device *dev, > > > > > > HPAGE_PMD_NR), > > > > > > nid, K(node_page_state(pgdat, NR_SHMEM_PMDMAPPED) * > > > > > > HPAGE_PMD_NR), > > > > > > - nid, K(node_page_state(pgdat, NR_FILE_THPS) * > > > > > > - HPAGE_PMD_NR), > > > > > > + nid, K(node_page_state(pgdat, NR_FILE_THPS)), > > > > > > > > > > Again, is this changing a user-visable value? > > > > > > > > > > > > > Of course not. > > > > > > > > In the previous, the NR_FILE_THPS account is like below: > > > > > > > > __mod_lruvec_page_state(page, NR_FILE_THPS, 1); > > > > > > > > With this patch, it is: > > > > > > > > __mod_lruvec_page_state(page, NR_FILE_THPS, HPAGE_PMD_NR); > > > > > > > > So the result is not changed from the view of user space. > > > > > > So you "broke" it on the previous patch and "fixed" it on this one? Why > > > not just do it all in one patch? > > > > Sorry for the confusion. I mean that the "previous" is without all of this patch > > series. So this series is aimed to convert the unit of all different THP vmstat > > counters from HPAGE_PMD_NR to pages. Thanks. > > I'm sorry, I still do not understand. It looks to me that you are > changing the number printed to userspace here. Where is the > corrisponding change that changed the units for this function? Is it in > this patch? If so, sorry, I did not see that at all... Sorry, actually, this patch does not change the number printed to userspace. It only changes the unit of the vmstat counter. Without this patch, every counter of NR_FILE_THPS represents NR_FILE_THPS pages. However, with this patch, every counter represents only one page. And why do I want to do this? Can reference to the cover letter. Thanks very much. > > thanks, > > greg k-h
On Sun, Dec 06, 2020 at 12:52:34AM +0800, Muchun Song wrote: > On Sun, Dec 6, 2020 at 12:32 AM Greg KH <gregkh@linuxfoundation.org> wrote: > > > > On Sat, Dec 05, 2020 at 11:39:24PM +0800, Muchun Song wrote: > > > On Sat, Dec 5, 2020 at 11:32 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > > > > > > > On Sat, Dec 05, 2020 at 11:29:26PM +0800, Muchun Song wrote: > > > > > On Sat, Dec 5, 2020 at 10:09 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > > > > > > > > > > > On Sat, Dec 05, 2020 at 09:02:20PM +0800, Muchun Song wrote: > > > > > > > Converrt NR_FILE_THPS account to pages. > > > > > > > > > > > > > > Signed-off-by: Muchun Song <songmuchun@bytedance.com> > > > > > > > --- > > > > > > > drivers/base/node.c | 3 +-- > > > > > > > fs/proc/meminfo.c | 2 +- > > > > > > > mm/filemap.c | 2 +- > > > > > > > mm/huge_memory.c | 3 ++- > > > > > > > mm/khugepaged.c | 2 +- > > > > > > > mm/memcontrol.c | 5 ++--- > > > > > > > 6 files changed, 8 insertions(+), 9 deletions(-) > > > > > > > > > > > > > > diff --git a/drivers/base/node.c b/drivers/base/node.c > > > > > > > index 05c369e93e16..f6a9521bbcf8 100644 > > > > > > > --- a/drivers/base/node.c > > > > > > > +++ b/drivers/base/node.c > > > > > > > @@ -466,8 +466,7 @@ static ssize_t node_read_meminfo(struct device *dev, > > > > > > > HPAGE_PMD_NR), > > > > > > > nid, K(node_page_state(pgdat, NR_SHMEM_PMDMAPPED) * > > > > > > > HPAGE_PMD_NR), > > > > > > > - nid, K(node_page_state(pgdat, NR_FILE_THPS) * > > > > > > > - HPAGE_PMD_NR), > > > > > > > + nid, K(node_page_state(pgdat, NR_FILE_THPS)), > > > > > > > > > > > > Again, is this changing a user-visable value? > > > > > > > > > > > > > > > > Of course not. > > > > > > > > > > In the previous, the NR_FILE_THPS account is like below: > > > > > > > > > > __mod_lruvec_page_state(page, NR_FILE_THPS, 1); > > > > > > > > > > With this patch, it is: > > > > > > > > > > __mod_lruvec_page_state(page, NR_FILE_THPS, HPAGE_PMD_NR); > > > > > > > > > > So the result is not changed from the view of user space. > > > > > > > > So you "broke" it on the previous patch and "fixed" it on this one? Why > > > > not just do it all in one patch? > > > > > > Sorry for the confusion. I mean that the "previous" is without all of this patch > > > series. So this series is aimed to convert the unit of all different THP vmstat > > > counters from HPAGE_PMD_NR to pages. Thanks. > > > > I'm sorry, I still do not understand. It looks to me that you are > > changing the number printed to userspace here. Where is the > > corrisponding change that changed the units for this function? Is it in > > this patch? If so, sorry, I did not see that at all... > > Sorry, actually, this patch does not change the number printed to > userspace. It only changes the unit of the vmstat counter. > > Without this patch, every counter of NR_FILE_THPS represents > NR_FILE_THPS pages. However, with this patch, every counter > represents only one page. And why do I want to do this? Can > reference to the cover letter. Thanks very much. Ah, I missed the change of the "ratio" value in the memory_stats[] array. That wasn't obvious at all, ugh. Sorry for the noise, greg k-h
diff --git a/drivers/base/node.c b/drivers/base/node.c index 05c369e93e16..f6a9521bbcf8 100644 --- a/drivers/base/node.c +++ b/drivers/base/node.c @@ -466,8 +466,7 @@ static ssize_t node_read_meminfo(struct device *dev, HPAGE_PMD_NR), nid, K(node_page_state(pgdat, NR_SHMEM_PMDMAPPED) * HPAGE_PMD_NR), - nid, K(node_page_state(pgdat, NR_FILE_THPS) * - HPAGE_PMD_NR), + nid, K(node_page_state(pgdat, NR_FILE_THPS)), nid, K(node_page_state(pgdat, NR_FILE_PMDMAPPED) * HPAGE_PMD_NR) #endif diff --git a/fs/proc/meminfo.c b/fs/proc/meminfo.c index f447ac191d24..9b2cb770326e 100644 --- a/fs/proc/meminfo.c +++ b/fs/proc/meminfo.c @@ -135,7 +135,7 @@ static int meminfo_proc_show(struct seq_file *m, void *v) show_val_kb(m, "ShmemPmdMapped: ", global_node_page_state(NR_SHMEM_PMDMAPPED) * HPAGE_PMD_NR); show_val_kb(m, "FileHugePages: ", - global_node_page_state(NR_FILE_THPS) * HPAGE_PMD_NR); + global_node_page_state(NR_FILE_THPS)); show_val_kb(m, "FilePmdMapped: ", global_node_page_state(NR_FILE_PMDMAPPED) * HPAGE_PMD_NR); #endif diff --git a/mm/filemap.c b/mm/filemap.c index 28e7309c29c8..c4dcb1144883 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -206,7 +206,7 @@ static void unaccount_page_cache_page(struct address_space *mapping, if (PageTransHuge(page)) __dec_lruvec_page_state(page, NR_SHMEM_THPS); } else if (PageTransHuge(page)) { - __dec_lruvec_page_state(page, NR_FILE_THPS); + __mod_lruvec_page_state(page, NR_FILE_THPS, -HPAGE_PMD_NR); filemap_nr_thps_dec(mapping); } diff --git a/mm/huge_memory.c b/mm/huge_memory.c index 1a13e1dab3ec..37840bdeaad0 100644 --- a/mm/huge_memory.c +++ b/mm/huge_memory.c @@ -2748,7 +2748,8 @@ int split_huge_page_to_list(struct page *page, struct list_head *list) if (PageSwapBacked(head)) __dec_lruvec_page_state(head, NR_SHMEM_THPS); else - __dec_lruvec_page_state(head, NR_FILE_THPS); + __mod_lruvec_page_state(head, NR_FILE_THPS, + -HPAGE_PMD_NR); } __split_huge_page(page, list, end); diff --git a/mm/khugepaged.c b/mm/khugepaged.c index ad316d2e1fee..1e1ced2208d0 100644 --- a/mm/khugepaged.c +++ b/mm/khugepaged.c @@ -1859,7 +1859,7 @@ static void collapse_file(struct mm_struct *mm, if (is_shmem) __inc_lruvec_page_state(new_page, NR_SHMEM_THPS); else { - __inc_lruvec_page_state(new_page, NR_FILE_THPS); + __mod_lruvec_page_state(new_page, NR_FILE_THPS, HPAGE_PMD_NR); filemap_nr_thps_inc(mapping); } diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 39cb7f1b00d3..dce76dddac61 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -1514,7 +1514,7 @@ static struct memory_stat memory_stats[] = { * constant(e.g. powerpc). */ { "anon_thp", PAGE_SIZE, NR_ANON_THPS }, - { "file_thp", 0, NR_FILE_THPS }, + { "file_thp", PAGE_SIZE, NR_FILE_THPS }, { "shmem_thp", 0, NR_SHMEM_THPS }, #endif { "inactive_anon", PAGE_SIZE, NR_INACTIVE_ANON }, @@ -1546,8 +1546,7 @@ static int __init memory_stats_init(void) for (i = 0; i < ARRAY_SIZE(memory_stats); i++) { #ifdef CONFIG_TRANSPARENT_HUGEPAGE - if (memory_stats[i].idx == NR_FILE_THPS || - memory_stats[i].idx == NR_SHMEM_THPS) + if (memory_stats[i].idx == NR_SHMEM_THPS) memory_stats[i].ratio = HPAGE_PMD_SIZE; #endif VM_BUG_ON(!memory_stats[i].ratio);
Converrt NR_FILE_THPS account to pages. Signed-off-by: Muchun Song <songmuchun@bytedance.com> --- drivers/base/node.c | 3 +-- fs/proc/meminfo.c | 2 +- mm/filemap.c | 2 +- mm/huge_memory.c | 3 ++- mm/khugepaged.c | 2 +- mm/memcontrol.c | 5 ++--- 6 files changed, 8 insertions(+), 9 deletions(-)