linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] mm/hugetable.c: align some prints
@ 2020-10-09 16:23 Hui Su
  2020-10-27  0:23 ` Mike Kravetz
  0 siblings, 1 reply; 3+ messages in thread
From: Hui Su @ 2020-10-09 16:23 UTC (permalink / raw)
  To: gregkh, rafael, mike.kravetz, akpm, linux-kernel, linux-mm

in old code, it shows like:
Node 0 ShmemHugePages:        0 kB
Node 0 ShmemPmdMapped:        0 kB
Node 0 FileHugePages:        0 kB
Node 0 FilePmdMapped:        0 kB
Node 0 HugePages_Total:     0
Node 0 HugePages_Free:      0
Node 0 HugePages_Surp:      0

which is not align. So we align it.

Signed-off-by: Hui Su <sh_def@163.com>
---
 drivers/base/node.c | 4 ++--
 mm/hugetlb.c        | 6 +++---
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/base/node.c b/drivers/base/node.c
index 50af16e68d98..b5453c372c5b 100644
--- a/drivers/base/node.c
+++ b/drivers/base/node.c
@@ -430,8 +430,8 @@ static ssize_t node_read_meminfo(struct device *dev,
 		       "Node %d AnonHugePages:  %8lu kB\n"
 		       "Node %d ShmemHugePages: %8lu kB\n"
 		       "Node %d ShmemPmdMapped: %8lu kB\n"
-		       "Node %d FileHugePages: %8lu kB\n"
-		       "Node %d FilePmdMapped: %8lu kB\n"
+		       "Node %d FileHugePages:  %8lu kB\n"
+		       "Node %d FilePmdMapped:  %8lu kB\n"
 #endif
 			,
 		       nid, K(node_page_state(pgdat, NR_FILE_DIRTY)),
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 67fc6383995b..077860ea2452 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -3588,9 +3588,9 @@ int hugetlb_report_node_meminfo(int nid, char *buf)
 	if (!hugepages_supported())
 		return 0;
 	return sprintf(buf,
-		"Node %d HugePages_Total: %5u\n"
-		"Node %d HugePages_Free:  %5u\n"
-		"Node %d HugePages_Surp:  %5u\n",
+		"Node %d HugePages_Total:%8u\n"
+		"Node %d HugePages_Free: %8u\n"
+		"Node %d HugePages_Surp: %8u\n",
 		nid, h->nr_huge_pages_node[nid],
 		nid, h->free_huge_pages_node[nid],
 		nid, h->surplus_huge_pages_node[nid]);
-- 
2.25.1




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

* Re: [PATCH] mm/hugetable.c: align some prints
  2020-10-09 16:23 [PATCH] mm/hugetable.c: align some prints Hui Su
@ 2020-10-27  0:23 ` Mike Kravetz
  2020-10-27  6:14   ` Greg KH
  0 siblings, 1 reply; 3+ messages in thread
From: Mike Kravetz @ 2020-10-27  0:23 UTC (permalink / raw)
  To: Hui Su, gregkh, rafael, akpm, linux-kernel, linux-mm

On 10/9/20 9:23 AM, Hui Su wrote:
> in old code, it shows like:
> Node 0 ShmemHugePages:        0 kB
> Node 0 ShmemPmdMapped:        0 kB
> Node 0 FileHugePages:        0 kB
> Node 0 FilePmdMapped:        0 kB
> Node 0 HugePages_Total:     0
> Node 0 HugePages_Free:      0
> Node 0 HugePages_Surp:      0
> 
> which is not align. So we align it.
> 
> Signed-off-by: Hui Su <sh_def@163.com>

Apologies for the late reply.

I assume you you just want to make the output look better.  Correct?

To be honest, I am not sure about the policy for changing the output
of sysfs files.  My preference would be to not change the output.  Why?
When the output is changed there is always the possibility that someone
may have written code that depends on the current format.  It looks like
the output has been misaligned since the day the code was first written.

This code was recently changed to use sysfs_emit_at() instead of
sprintf().  At that time Greg noted that this also violates the sysfs
rule of one value per file.  So, it appears there may be a bigger issue
than alignment.

Greg,
Is it OK to break up these sysfs files to be one value per file if they
contained multiple values from day 1 of their existence?  I would prefer
not to touch them in case some is depending on current format.

-- 
Mike Kravetz

> ---
>  drivers/base/node.c | 4 ++--
>  mm/hugetlb.c        | 6 +++---
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/base/node.c b/drivers/base/node.c
> index 50af16e68d98..b5453c372c5b 100644
> --- a/drivers/base/node.c
> +++ b/drivers/base/node.c
> @@ -430,8 +430,8 @@ static ssize_t node_read_meminfo(struct device *dev,
>  		       "Node %d AnonHugePages:  %8lu kB\n"
>  		       "Node %d ShmemHugePages: %8lu kB\n"
>  		       "Node %d ShmemPmdMapped: %8lu kB\n"
> -		       "Node %d FileHugePages: %8lu kB\n"
> -		       "Node %d FilePmdMapped: %8lu kB\n"
> +		       "Node %d FileHugePages:  %8lu kB\n"
> +		       "Node %d FilePmdMapped:  %8lu kB\n"
>  #endif
>  			,
>  		       nid, K(node_page_state(pgdat, NR_FILE_DIRTY)),
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 67fc6383995b..077860ea2452 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -3588,9 +3588,9 @@ int hugetlb_report_node_meminfo(int nid, char *buf)
>  	if (!hugepages_supported())
>  		return 0;
>  	return sprintf(buf,
> -		"Node %d HugePages_Total: %5u\n"
> -		"Node %d HugePages_Free:  %5u\n"
> -		"Node %d HugePages_Surp:  %5u\n",
> +		"Node %d HugePages_Total:%8u\n"
> +		"Node %d HugePages_Free: %8u\n"
> +		"Node %d HugePages_Surp: %8u\n",
>  		nid, h->nr_huge_pages_node[nid],
>  		nid, h->free_huge_pages_node[nid],
>  		nid, h->surplus_huge_pages_node[nid]);
> 


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

* Re: [PATCH] mm/hugetable.c: align some prints
  2020-10-27  0:23 ` Mike Kravetz
@ 2020-10-27  6:14   ` Greg KH
  0 siblings, 0 replies; 3+ messages in thread
From: Greg KH @ 2020-10-27  6:14 UTC (permalink / raw)
  To: Mike Kravetz; +Cc: Hui Su, rafael, akpm, linux-kernel, linux-mm

On Mon, Oct 26, 2020 at 05:23:43PM -0700, Mike Kravetz wrote:
> On 10/9/20 9:23 AM, Hui Su wrote:
> > in old code, it shows like:
> > Node 0 ShmemHugePages:        0 kB
> > Node 0 ShmemPmdMapped:        0 kB
> > Node 0 FileHugePages:        0 kB
> > Node 0 FilePmdMapped:        0 kB
> > Node 0 HugePages_Total:     0
> > Node 0 HugePages_Free:      0
> > Node 0 HugePages_Surp:      0
> > 
> > which is not align. So we align it.
> > 
> > Signed-off-by: Hui Su <sh_def@163.com>
> 
> Apologies for the late reply.
> 
> I assume you you just want to make the output look better.  Correct?
> 
> To be honest, I am not sure about the policy for changing the output
> of sysfs files.  My preference would be to not change the output.  Why?
> When the output is changed there is always the possibility that someone
> may have written code that depends on the current format.  It looks like
> the output has been misaligned since the day the code was first written.
> 
> This code was recently changed to use sysfs_emit_at() instead of
> sprintf().  At that time Greg noted that this also violates the sysfs
> rule of one value per file.  So, it appears there may be a bigger issue
> than alignment.
> 
> Greg,
> Is it OK to break up these sysfs files to be one value per file if they
> contained multiple values from day 1 of their existence?  I would prefer
> not to touch them in case some is depending on current format.

You should create multiple files, with a different name, and then remove
this file.  Any tool that uses sysfs should be able to handle a file
going away, don't change the format of the data in the file, otherwise
there's no way for anyone to know what is happening.

thanks,

greg k-h


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

end of thread, other threads:[~2020-10-27  6:14 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-09 16:23 [PATCH] mm/hugetable.c: align some prints Hui Su
2020-10-27  0:23 ` Mike Kravetz
2020-10-27  6:14   ` Greg KH

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