[PATCH 0/4] mm: fix checkpatch.pl warnings in memcg v1 code

[PATCH 0/4] mm: fix checkpatch.pl warnings in memcg v1 code

[Keren Sun]

The patch series fixes 1 error and 27 warnings found by checkpatch.pl in the
memcg1 code.

Keren Sun (4):
  mm: fix quoted strings spliting across lines
  mm: Fix minor formatting issues for mm control
  mm: Prefer 'unsigned int' to bare use of 'unsigned'
  mm: Replace simple_strtoul() with kstrtoul()

 mm/memcontrol-v1.c | 63 +++++++++++++++++-----------------------------
 1 file changed, 23 insertions(+), 40 deletions(-)

-- 
2.47.0.163.g1226f6d8fa-goog

[PATCH 1/4] mm: fix quoted strings spliting across lines

[Keren Sun]

This patch fixes quoted strings splitting across lines in
pr_warn_once() by putting them into one line.

Signed-off-by: Keren Sun <kerensun@google.com>
---
 mm/memcontrol-v1.c | 33 ++++++++-------------------------
 1 file changed, 8 insertions(+), 25 deletions(-)

diff --git a/mm/memcontrol-v1.c b/mm/memcontrol-v1.c
index 81d8819f13cd..3951538bd73f 100644
--- a/mm/memcontrol-v1.c
+++ b/mm/memcontrol-v1.c
@@ -602,9 +602,7 @@ static int mem_cgroup_move_charge_write(struct cgroup_subsys_state *css,
 {
 	struct mem_cgroup *memcg = mem_cgroup_from_css(css);
 
-	pr_warn_once("Cgroup memory moving (move_charge_at_immigrate) is deprecated. "
-		     "Please report your usecase to linux-mm@kvack.org if you "
-		     "depend on this functionality.\n");
+	pr_warn_once("Cgroup memory moving (move_charge_at_immigrate) is deprecated. Please report your usecase to linux-mm@kvack.org if you depend on this functionality.\n");
 
 	if (val & ~MOVE_MASK)
 		return -EINVAL;
@@ -1994,15 +1992,11 @@ static ssize_t memcg_write_event_control(struct kernfs_open_file *of,
 		event->register_event = mem_cgroup_usage_register_event;
 		event->unregister_event = mem_cgroup_usage_unregister_event;
 	} else if (!strcmp(name, "memory.oom_control")) {
-		pr_warn_once("oom_control is deprecated and will be removed. "
-			     "Please report your usecase to linux-mm-@kvack.org"
-			     " if you depend on this functionality. \n");
+		pr_warn_once("oom_control is deprecated and will be removed. Please report your usecase to linux-mm-@kvack.org if you depend on this functionality.\n");
 		event->register_event = mem_cgroup_oom_register_event;
 		event->unregister_event = mem_cgroup_oom_unregister_event;
 	} else if (!strcmp(name, "memory.pressure_level")) {
-		pr_warn_once("pressure_level is deprecated and will be removed. "
-			     "Please report your usecase to linux-mm-@kvack.org "
-			     "if you depend on this functionality. \n");
+		pr_warn_once("pressure_level is deprecated and will be removed. Please report your usecase to linux-mm-@kvack.org if you depend on this functionality.\n");
 		event->register_event = vmpressure_register_event;
 		event->unregister_event = vmpressure_unregister_event;
 	} else if (!strcmp(name, "memory.memsw.usage_in_bytes")) {
@@ -2408,9 +2402,7 @@ static int mem_cgroup_hierarchy_write(struct cgroup_subsys_state *css,
 	if (val == 1)
 		return 0;
 
-	pr_warn_once("Non-hierarchical mode is deprecated. "
-		     "Please report your usecase to linux-mm@kvack.org if you "
-		     "depend on this functionality.\n");
+	pr_warn_once("Non-hierarchical mode is deprecated. Please report your usecase to linux-mm@kvack.org if you depend on this functionality.\n");
 
 	return -EINVAL;
 }
@@ -2533,16 +2525,11 @@ static ssize_t mem_cgroup_write(struct kernfs_open_file *of,
 			ret = mem_cgroup_resize_max(memcg, nr_pages, true);
 			break;
 		case _KMEM:
-			pr_warn_once("kmem.limit_in_bytes is deprecated and will be removed. "
-				     "Writing any value to this file has no effect. "
-				     "Please report your usecase to linux-mm@kvack.org if you "
-				     "depend on this functionality.\n");
+			pr_warn_once("kmem.limit_in_bytes is deprecated and will be removed. Writing any value to this file has no effect. Please report your usecase to linux-mm@kvack.org if you depend on this functionality.\n");
 			ret = 0;
 			break;
 		case _TCP:
-			pr_warn_once("kmem.tcp.limit_in_bytes is deprecated and will be removed. "
-				     "Please report your usecase to linux-mm@kvack.org if you "
-				     "depend on this functionality.\n");
+			pr_warn_once("kmem.tcp.limit_in_bytes is deprecated and will be removed. Please report your usecase to linux-mm@kvack.org if you depend on this functionality.\n");
 			ret = memcg_update_tcp_max(memcg, nr_pages);
 			break;
 		}
@@ -2551,9 +2538,7 @@ static ssize_t mem_cgroup_write(struct kernfs_open_file *of,
 		if (IS_ENABLED(CONFIG_PREEMPT_RT)) {
 			ret = -EOPNOTSUPP;
 		} else {
-			pr_warn_once("soft_limit_in_bytes is deprecated and will be removed. "
-				     "Please report your usecase to linux-mm@kvack.org if you "
-				     "depend on this functionality.\n");
+			pr_warn_once("soft_limit_in_bytes is deprecated and will be removed. Please report your usecase to linux-mm@kvack.org if you depend on this functionality.\n");
 			WRITE_ONCE(memcg->soft_limit, nr_pages);
 			ret = 0;
 		}
@@ -2847,9 +2832,7 @@ static int mem_cgroup_oom_control_write(struct cgroup_subsys_state *css,
 {
 	struct mem_cgroup *memcg = mem_cgroup_from_css(css);
 
-	pr_warn_once("oom_control is deprecated and will be removed. "
-		     "Please report your usecase to linux-mm-@kvack.org if you "
-		     "depend on this functionality. \n");
+	pr_warn_once("oom_control is deprecated and will be removed. Please report your usecase to linux-mm-@kvack.org if you depend on this functionality.\n");
 
 	/* cannot set to root cgroup and only 0 and 1 are allowed */
 	if (mem_cgroup_is_root(memcg) || !((val == 0) || (val == 1)))
-- 
2.47.0.163.g1226f6d8fa-goog

[PATCH 2/4] mm: Fix minor formatting issues for mm control

[Keren Sun]

Add a line after declaration as it's missing after DEFINE_WAIT(),
replace the spaces with tabs for indent, and remove the non-useful else
after a break in a if statement.

Signed-off-by: Keren Sun <kerensun@google.com>
---
 mm/memcontrol-v1.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/mm/memcontrol-v1.c b/mm/memcontrol-v1.c
index 3951538bd73f..5f9d3d6d443c 100644
--- a/mm/memcontrol-v1.c
+++ b/mm/memcontrol-v1.c
@@ -460,6 +460,7 @@ bool memcg1_wait_acct_move(struct mem_cgroup *memcg)
 	if (mc.moving_task && current != mc.moving_task) {
 		if (mem_cgroup_under_move(memcg)) {
 			DEFINE_WAIT(wait);
+
 			prepare_to_wait(&mc.waitq, &wait, TASK_INTERRUPTIBLE);
 			/* moving charge context might have finished. */
 			if (mc.moving_task)
@@ -490,7 +491,7 @@ void folio_memcg_lock(struct folio *folio)
 	 * The RCU lock is held throughout the transaction.  The fast
 	 * path can get away without acquiring the memcg->move_lock
 	 * because page moving starts with an RCU grace period.
-         */
+	 */
 	rcu_read_lock();
 
 	if (mem_cgroup_disabled())
@@ -2096,8 +2097,8 @@ static bool mem_cgroup_oom_trylock(struct mem_cgroup *memcg)
 			failed = iter;
 			mem_cgroup_iter_break(memcg, iter);
 			break;
-		} else
-			iter->oom_lock = true;
+		}
+		iter->oom_lock = true;
 	}
 
 	if (failed) {
-- 
2.47.0.163.g1226f6d8fa-goog

[PATCH 3/4] mm: Prefer 'unsigned int' to bare use of 'unsigned'

[Keren Sun]

Change the param 'mode' from type 'unsigned' to 'unsigned int' in
memcg_event_wake() and memcg_oom_wake_function(), and for the param
'nid' in VM_BUG_ON().

Signed-off-by: Keren Sun <kerensun@google.com>
---
 mm/memcontrol-v1.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/mm/memcontrol-v1.c b/mm/memcontrol-v1.c
index 5f9d3d6d443c..5e1854623824 100644
--- a/mm/memcontrol-v1.c
+++ b/mm/memcontrol-v1.c
@@ -1851,7 +1851,7 @@ static void memcg_event_remove(struct work_struct *work)
  *
  * Called with wqh->lock held and interrupts disabled.
  */
-static int memcg_event_wake(wait_queue_entry_t *wait, unsigned mode,
+static int memcg_event_wake(wait_queue_entry_t *wait, unsigned int mode,
 			    int sync, void *key)
 {
 	struct mem_cgroup_event *event =
@@ -2165,7 +2165,7 @@ struct oom_wait_info {
 };
 
 static int memcg_oom_wake_function(wait_queue_entry_t *wait,
-	unsigned mode, int sync, void *arg)
+	unsigned int mode, int sync, void *arg)
 {
 	struct mem_cgroup *wake_memcg = (struct mem_cgroup *)arg;
 	struct mem_cgroup *oom_wait_memcg;
@@ -2598,7 +2598,7 @@ static unsigned long mem_cgroup_node_nr_lru_pages(struct mem_cgroup *memcg,
 	unsigned long nr = 0;
 	enum lru_list lru;
 
-	VM_BUG_ON((unsigned)nid >= nr_node_ids);
+	VM_BUG_ON((unsigned int)nid >= nr_node_ids);
 
 	for_each_lru(lru) {
 		if (!(BIT(lru) & lru_mask))
-- 
2.47.0.163.g1226f6d8fa-goog

[PATCH 4/4] mm: Replace simple_strtoul() with kstrtoul()

[Keren Sun]

simple_strtoul() has caveat and is obsolete, use kstrtoul() instead in mmcg.

Signed-off-by: Keren Sun <kerensun@google.com>
---
 mm/memcontrol-v1.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/mm/memcontrol-v1.c b/mm/memcontrol-v1.c
index 5e1854623824..260b356cea5a 100644
--- a/mm/memcontrol-v1.c
+++ b/mm/memcontrol-v1.c
@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 
+#include "linux/kstrtox.h"
 #include <linux/memcontrol.h>
 #include <linux/swap.h>
 #include <linux/mm_inline.h>
@@ -1922,17 +1923,15 @@ static ssize_t memcg_write_event_control(struct kernfs_open_file *of,
 
 	buf = strstrip(buf);
 
-	efd = simple_strtoul(buf, &endp, 10);
-	if (*endp != ' ')
+	kstrtoul(buf, 10, efd);
+	if (*buf != ' ')
 		return -EINVAL;
-	buf = endp + 1;
+	buf++;
 
-	cfd = simple_strtoul(buf, &endp, 10);
-	if (*endp == '\0')
-		buf = endp;
-	else if (*endp == ' ')
-		buf = endp + 1;
-	else
+	kstrtoul(buf, 10, cfd);
+	if (*buf == ' ')
+		buf++;
+	else if (*buf != '\0')
 		return -EINVAL;
 
 	event = kzalloc(sizeof(*event), GFP_KERNEL);
-- 
2.47.0.163.g1226f6d8fa-goog

Re: [PATCH 0/4] mm: fix checkpatch.pl warnings in memcg v1 code

[Matthew Wilcox]

On Mon, Nov 04, 2024 at 02:27:33PM -0800, Keren Sun wrote:
> The patch series fixes 1 error and 27 warnings found by checkpatch.pl in the
> memcg1 code.

Please do not do this.  Fixing checkpatch messages in existing code
is counterproductive.  Only run checkpatch on new code, and even then
take its suggestions with a grain of salt.

Re: [PATCH 0/4] mm: fix checkpatch.pl warnings in memcg v1 code

[Roman Gushchin]

On Tue, Nov 05, 2024 at 12:31:36AM +0000, Matthew Wilcox wrote:
> On Mon, Nov 04, 2024 at 02:27:33PM -0800, Keren Sun wrote:
> > The patch series fixes 1 error and 27 warnings found by checkpatch.pl in the
> > memcg1 code.
> 
> Please do not do this.  Fixing checkpatch messages in existing code
> is counterproductive.  Only run checkpatch on new code, and even then
> take its suggestions with a grain of salt.

I do agree in general, but not in this case. These are nice cleanups
no matter what checkpatch.pl thinks of it :)

Thanks!

Re: [PATCH 1/4] mm: fix quoted strings spliting across lines

[Shakeel Butt]

On Mon, Nov 04, 2024 at 02:27:34PM -0800, Keren Sun wrote:
> This patch fixes quoted strings splitting across lines in
> pr_warn_once() by putting them into one line.
> 
> Signed-off-by: Keren Sun <kerensun@google.com>

Other patches seems fine but personally I don't like this patch. I
would prefer a multi line string over a very long single line of string.

Re: [PATCH 4/4] mm: Replace simple_strtoul() with kstrtoul()

[kernel test robot]

Hi Keren,

kernel test robot noticed the following build warnings:

[auto build test WARNING on v6.12-rc6]
[also build test WARNING on linus/master]
[cannot apply to akpm-mm/mm-everything next-20241104]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Keren-Sun/mm-fix-quoted-strings-spliting-across-lines/20241105-063007
base:   v6.12-rc6
patch link:    https://lore.kernel.org/r/20241104222737.298130-5-kerensun%40google.com
patch subject: [PATCH 4/4] mm: Replace simple_strtoul() with kstrtoul()
config: arc-randconfig-001-20241105 (https://download.01.org/0day-ci/archive/20241105/202411051219.uj1XBcp1-lkp@intel.com/config)
compiler: arceb-elf-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241105/202411051219.uj1XBcp1-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202411051219.uj1XBcp1-lkp@intel.com/

All warnings (new ones prefixed by >>):

   mm/memcontrol-v1.c: In function 'memcg_write_event_control':
>> mm/memcontrol-v1.c:1926:27: warning: passing argument 3 of 'kstrtoul' makes pointer from integer without a cast [-Wint-conversion]
    1926 |         kstrtoul(buf, 10, efd);
         |                           ^~~
         |                           |
         |                           unsigned int
   In file included from mm/memcontrol-v1.c:3:
   include/linux/kstrtox.h:30:90: note: expected 'long unsigned int *' but argument is of type 'unsigned int'
      30 | static inline int __must_check kstrtoul(const char *s, unsigned int base, unsigned long *res)
         |                                                                           ~~~~~~~~~~~~~~~^~~
   mm/memcontrol-v1.c:1931:27: warning: passing argument 3 of 'kstrtoul' makes pointer from integer without a cast [-Wint-conversion]
    1931 |         kstrtoul(buf, 10, cfd);
         |                           ^~~
         |                           |
         |                           unsigned int
   include/linux/kstrtox.h:30:90: note: expected 'long unsigned int *' but argument is of type 'unsigned int'
      30 | static inline int __must_check kstrtoul(const char *s, unsigned int base, unsigned long *res)
         |                                                                           ~~~~~~~~~~~~~~~^~~
>> mm/memcontrol-v1.c:1918:15: warning: unused variable 'endp' [-Wunused-variable]
    1918 |         char *endp;
         |               ^~~~
>> mm/memcontrol-v1.c:1926:9: warning: ignoring return value of 'kstrtoul' declared with attribute 'warn_unused_result' [-Wunused-result]
    1926 |         kstrtoul(buf, 10, efd);
         |         ^~~~~~~~~~~~~~~~~~~~~~
   mm/memcontrol-v1.c:1931:9: warning: ignoring return value of 'kstrtoul' declared with attribute 'warn_unused_result' [-Wunused-result]
    1931 |         kstrtoul(buf, 10, cfd);
         |         ^~~~~~~~~~~~~~~~~~~~~~
>> mm/memcontrol-v1.c:1926:9: warning: 'efd' is used uninitialized [-Wuninitialized]
    1926 |         kstrtoul(buf, 10, efd);
         |         ^~~~~~~~~~~~~~~~~~~~~~
   mm/memcontrol-v1.c:1913:22: note: 'efd' was declared here
    1913 |         unsigned int efd, cfd;
         |                      ^~~


vim +/kstrtoul +1926 mm/memcontrol-v1.c

  1897	
  1898	/*
  1899	 * DO NOT USE IN NEW FILES.
  1900	 *
  1901	 * Parse input and register new cgroup event handler.
  1902	 *
  1903	 * Input must be in format '<event_fd> <control_fd> <args>'.
  1904	 * Interpretation of args is defined by control file implementation.
  1905	 */
  1906	static ssize_t memcg_write_event_control(struct kernfs_open_file *of,
  1907						 char *buf, size_t nbytes, loff_t off)
  1908	{
  1909		struct cgroup_subsys_state *css = of_css(of);
  1910		struct mem_cgroup *memcg = mem_cgroup_from_css(css);
  1911		struct mem_cgroup_event *event;
  1912		struct cgroup_subsys_state *cfile_css;
  1913		unsigned int efd, cfd;
  1914		struct fd efile;
  1915		struct fd cfile;
  1916		struct dentry *cdentry;
  1917		const char *name;
> 1918		char *endp;
  1919		int ret;
  1920	
  1921		if (IS_ENABLED(CONFIG_PREEMPT_RT))
  1922			return -EOPNOTSUPP;
  1923	
  1924		buf = strstrip(buf);
  1925	
> 1926		kstrtoul(buf, 10, efd);
  1927		if (*buf != ' ')
  1928			return -EINVAL;
  1929		buf++;
  1930	
> 1931		kstrtoul(buf, 10, cfd);
  1932		if (*buf == ' ')
  1933			buf++;
  1934		else if (*buf != '\0')
  1935			return -EINVAL;
  1936	
  1937		event = kzalloc(sizeof(*event), GFP_KERNEL);
  1938		if (!event)
  1939			return -ENOMEM;
  1940	
  1941		event->memcg = memcg;
  1942		INIT_LIST_HEAD(&event->list);
  1943		init_poll_funcptr(&event->pt, memcg_event_ptable_queue_proc);
  1944		init_waitqueue_func_entry(&event->wait, memcg_event_wake);
  1945		INIT_WORK(&event->remove, memcg_event_remove);
  1946	
  1947		efile = fdget(efd);
  1948		if (!fd_file(efile)) {
  1949			ret = -EBADF;
  1950			goto out_kfree;
  1951		}
  1952	
  1953		event->eventfd = eventfd_ctx_fileget(fd_file(efile));
  1954		if (IS_ERR(event->eventfd)) {
  1955			ret = PTR_ERR(event->eventfd);
  1956			goto out_put_efile;
  1957		}
  1958	
  1959		cfile = fdget(cfd);
  1960		if (!fd_file(cfile)) {
  1961			ret = -EBADF;
  1962			goto out_put_eventfd;
  1963		}
  1964	
  1965		/* the process need read permission on control file */
  1966		/* AV: shouldn't we check that it's been opened for read instead? */
  1967		ret = file_permission(fd_file(cfile), MAY_READ);
  1968		if (ret < 0)
  1969			goto out_put_cfile;
  1970	
  1971		/*
  1972		 * The control file must be a regular cgroup1 file. As a regular cgroup
  1973		 * file can't be renamed, it's safe to access its name afterwards.
  1974		 */
  1975		cdentry = fd_file(cfile)->f_path.dentry;
  1976		if (cdentry->d_sb->s_type != &cgroup_fs_type || !d_is_reg(cdentry)) {
  1977			ret = -EINVAL;
  1978			goto out_put_cfile;
  1979		}
  1980	
  1981		/*
  1982		 * Determine the event callbacks and set them in @event.  This used
  1983		 * to be done via struct cftype but cgroup core no longer knows
  1984		 * about these events.  The following is crude but the whole thing
  1985		 * is for compatibility anyway.
  1986		 *
  1987		 * DO NOT ADD NEW FILES.
  1988		 */
  1989		name = cdentry->d_name.name;
  1990	
  1991		if (!strcmp(name, "memory.usage_in_bytes")) {
  1992			event->register_event = mem_cgroup_usage_register_event;
  1993			event->unregister_event = mem_cgroup_usage_unregister_event;
  1994		} else if (!strcmp(name, "memory.oom_control")) {
  1995			pr_warn_once("oom_control is deprecated and will be removed. Please report your usecase to linux-mm-@kvack.org if you depend on this functionality.\n");
  1996			event->register_event = mem_cgroup_oom_register_event;
  1997			event->unregister_event = mem_cgroup_oom_unregister_event;
  1998		} else if (!strcmp(name, "memory.pressure_level")) {
  1999			pr_warn_once("pressure_level is deprecated and will be removed. Please report your usecase to linux-mm-@kvack.org if you depend on this functionality.\n");
  2000			event->register_event = vmpressure_register_event;
  2001			event->unregister_event = vmpressure_unregister_event;
  2002		} else if (!strcmp(name, "memory.memsw.usage_in_bytes")) {
  2003			event->register_event = memsw_cgroup_usage_register_event;
  2004			event->unregister_event = memsw_cgroup_usage_unregister_event;
  2005		} else {
  2006			ret = -EINVAL;
  2007			goto out_put_cfile;
  2008		}
  2009	
  2010		/*
  2011		 * Verify @cfile should belong to @css.  Also, remaining events are
  2012		 * automatically removed on cgroup destruction but the removal is
  2013		 * asynchronous, so take an extra ref on @css.
  2014		 */
  2015		cfile_css = css_tryget_online_from_dir(cdentry->d_parent,
  2016						       &memory_cgrp_subsys);
  2017		ret = -EINVAL;
  2018		if (IS_ERR(cfile_css))
  2019			goto out_put_cfile;
  2020		if (cfile_css != css) {
  2021			css_put(cfile_css);
  2022			goto out_put_cfile;
  2023		}
  2024	
  2025		ret = event->register_event(memcg, event->eventfd, buf);
  2026		if (ret)
  2027			goto out_put_css;
  2028	
  2029		vfs_poll(fd_file(efile), &event->pt);
  2030	
  2031		spin_lock_irq(&memcg->event_list_lock);
  2032		list_add(&event->list, &memcg->event_list);
  2033		spin_unlock_irq(&memcg->event_list_lock);
  2034	
  2035		fdput(cfile);
  2036		fdput(efile);
  2037	
  2038		return nbytes;
  2039	
  2040	out_put_css:
  2041		css_put(css);
  2042	out_put_cfile:
  2043		fdput(cfile);
  2044	out_put_eventfd:
  2045		eventfd_ctx_put(event->eventfd);
  2046	out_put_efile:
  2047		fdput(efile);
  2048	out_kfree:
  2049		kfree(event);
  2050	
  2051		return ret;
  2052	}
  2053	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 4/4] mm: Replace simple_strtoul() with kstrtoul()

[kernel test robot]

Hi Keren,

kernel test robot noticed the following build errors:

[auto build test ERROR on v6.12-rc6]
[also build test ERROR on linus/master]
[cannot apply to akpm-mm/mm-everything next-20241104]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Keren-Sun/mm-fix-quoted-strings-spliting-across-lines/20241105-063007
base:   v6.12-rc6
patch link:    https://lore.kernel.org/r/20241104222737.298130-5-kerensun%40google.com
patch subject: [PATCH 4/4] mm: Replace simple_strtoul() with kstrtoul()
config: m68k-allmodconfig (https://download.01.org/0day-ci/archive/20241105/202411051248.C33eJ43V-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 14.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241105/202411051248.C33eJ43V-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202411051248.C33eJ43V-lkp@intel.com/

All errors (new ones prefixed by >>):

   mm/memcontrol-v1.c: In function 'memcg_write_event_control':
>> mm/memcontrol-v1.c:1926:27: error: passing argument 3 of 'kstrtoul' makes pointer from integer without a cast [-Wint-conversion]
    1926 |         kstrtoul(buf, 10, efd);
         |                           ^~~
         |                           |
         |                           unsigned int
   In file included from mm/memcontrol-v1.c:3:
   include/linux/kstrtox.h:30:90: note: expected 'long unsigned int *' but argument is of type 'unsigned int'
      30 | static inline int __must_check kstrtoul(const char *s, unsigned int base, unsigned long *res)
         |                                                                           ~~~~~~~~~~~~~~~^~~
   mm/memcontrol-v1.c:1931:27: error: passing argument 3 of 'kstrtoul' makes pointer from integer without a cast [-Wint-conversion]
    1931 |         kstrtoul(buf, 10, cfd);
         |                           ^~~
         |                           |
         |                           unsigned int
   include/linux/kstrtox.h:30:90: note: expected 'long unsigned int *' but argument is of type 'unsigned int'
      30 | static inline int __must_check kstrtoul(const char *s, unsigned int base, unsigned long *res)
         |                                                                           ~~~~~~~~~~~~~~~^~~
   mm/memcontrol-v1.c:1918:15: warning: unused variable 'endp' [-Wunused-variable]
    1918 |         char *endp;
         |               ^~~~
   mm/memcontrol-v1.c:1926:9: warning: ignoring return value of 'kstrtoul' declared with attribute 'warn_unused_result' [-Wunused-result]
    1926 |         kstrtoul(buf, 10, efd);
         |         ^~~~~~~~~~~~~~~~~~~~~~
   mm/memcontrol-v1.c:1931:9: warning: ignoring return value of 'kstrtoul' declared with attribute 'warn_unused_result' [-Wunused-result]
    1931 |         kstrtoul(buf, 10, cfd);
         |         ^~~~~~~~~~~~~~~~~~~~~~


vim +/kstrtoul +1926 mm/memcontrol-v1.c

  1897	
  1898	/*
  1899	 * DO NOT USE IN NEW FILES.
  1900	 *
  1901	 * Parse input and register new cgroup event handler.
  1902	 *
  1903	 * Input must be in format '<event_fd> <control_fd> <args>'.
  1904	 * Interpretation of args is defined by control file implementation.
  1905	 */
  1906	static ssize_t memcg_write_event_control(struct kernfs_open_file *of,
  1907						 char *buf, size_t nbytes, loff_t off)
  1908	{
  1909		struct cgroup_subsys_state *css = of_css(of);
  1910		struct mem_cgroup *memcg = mem_cgroup_from_css(css);
  1911		struct mem_cgroup_event *event;
  1912		struct cgroup_subsys_state *cfile_css;
  1913		unsigned int efd, cfd;
  1914		struct fd efile;
  1915		struct fd cfile;
  1916		struct dentry *cdentry;
  1917		const char *name;
  1918		char *endp;
  1919		int ret;
  1920	
  1921		if (IS_ENABLED(CONFIG_PREEMPT_RT))
  1922			return -EOPNOTSUPP;
  1923	
  1924		buf = strstrip(buf);
  1925	
> 1926		kstrtoul(buf, 10, efd);
  1927		if (*buf != ' ')
  1928			return -EINVAL;
  1929		buf++;
  1930	
  1931		kstrtoul(buf, 10, cfd);
  1932		if (*buf == ' ')
  1933			buf++;
  1934		else if (*buf != '\0')
  1935			return -EINVAL;
  1936	
  1937		event = kzalloc(sizeof(*event), GFP_KERNEL);
  1938		if (!event)
  1939			return -ENOMEM;
  1940	
  1941		event->memcg = memcg;
  1942		INIT_LIST_HEAD(&event->list);
  1943		init_poll_funcptr(&event->pt, memcg_event_ptable_queue_proc);
  1944		init_waitqueue_func_entry(&event->wait, memcg_event_wake);
  1945		INIT_WORK(&event->remove, memcg_event_remove);
  1946	
  1947		efile = fdget(efd);
  1948		if (!fd_file(efile)) {
  1949			ret = -EBADF;
  1950			goto out_kfree;
  1951		}
  1952	
  1953		event->eventfd = eventfd_ctx_fileget(fd_file(efile));
  1954		if (IS_ERR(event->eventfd)) {
  1955			ret = PTR_ERR(event->eventfd);
  1956			goto out_put_efile;
  1957		}
  1958	
  1959		cfile = fdget(cfd);
  1960		if (!fd_file(cfile)) {
  1961			ret = -EBADF;
  1962			goto out_put_eventfd;
  1963		}
  1964	
  1965		/* the process need read permission on control file */
  1966		/* AV: shouldn't we check that it's been opened for read instead? */
  1967		ret = file_permission(fd_file(cfile), MAY_READ);
  1968		if (ret < 0)
  1969			goto out_put_cfile;
  1970	
  1971		/*
  1972		 * The control file must be a regular cgroup1 file. As a regular cgroup
  1973		 * file can't be renamed, it's safe to access its name afterwards.
  1974		 */
  1975		cdentry = fd_file(cfile)->f_path.dentry;
  1976		if (cdentry->d_sb->s_type != &cgroup_fs_type || !d_is_reg(cdentry)) {
  1977			ret = -EINVAL;
  1978			goto out_put_cfile;
  1979		}
  1980	
  1981		/*
  1982		 * Determine the event callbacks and set them in @event.  This used
  1983		 * to be done via struct cftype but cgroup core no longer knows
  1984		 * about these events.  The following is crude but the whole thing
  1985		 * is for compatibility anyway.
  1986		 *
  1987		 * DO NOT ADD NEW FILES.
  1988		 */
  1989		name = cdentry->d_name.name;
  1990	
  1991		if (!strcmp(name, "memory.usage_in_bytes")) {
  1992			event->register_event = mem_cgroup_usage_register_event;
  1993			event->unregister_event = mem_cgroup_usage_unregister_event;
  1994		} else if (!strcmp(name, "memory.oom_control")) {
  1995			pr_warn_once("oom_control is deprecated and will be removed. Please report your usecase to linux-mm-@kvack.org if you depend on this functionality.\n");
  1996			event->register_event = mem_cgroup_oom_register_event;
  1997			event->unregister_event = mem_cgroup_oom_unregister_event;
  1998		} else if (!strcmp(name, "memory.pressure_level")) {
  1999			pr_warn_once("pressure_level is deprecated and will be removed. Please report your usecase to linux-mm-@kvack.org if you depend on this functionality.\n");
  2000			event->register_event = vmpressure_register_event;
  2001			event->unregister_event = vmpressure_unregister_event;
  2002		} else if (!strcmp(name, "memory.memsw.usage_in_bytes")) {
  2003			event->register_event = memsw_cgroup_usage_register_event;
  2004			event->unregister_event = memsw_cgroup_usage_unregister_event;
  2005		} else {
  2006			ret = -EINVAL;
  2007			goto out_put_cfile;
  2008		}
  2009	
  2010		/*
  2011		 * Verify @cfile should belong to @css.  Also, remaining events are
  2012		 * automatically removed on cgroup destruction but the removal is
  2013		 * asynchronous, so take an extra ref on @css.
  2014		 */
  2015		cfile_css = css_tryget_online_from_dir(cdentry->d_parent,
  2016						       &memory_cgrp_subsys);
  2017		ret = -EINVAL;
  2018		if (IS_ERR(cfile_css))
  2019			goto out_put_cfile;
  2020		if (cfile_css != css) {
  2021			css_put(cfile_css);
  2022			goto out_put_cfile;
  2023		}
  2024	
  2025		ret = event->register_event(memcg, event->eventfd, buf);
  2026		if (ret)
  2027			goto out_put_css;
  2028	
  2029		vfs_poll(fd_file(efile), &event->pt);
  2030	
  2031		spin_lock_irq(&memcg->event_list_lock);
  2032		list_add(&event->list, &memcg->event_list);
  2033		spin_unlock_irq(&memcg->event_list_lock);
  2034	
  2035		fdput(cfile);
  2036		fdput(efile);
  2037	
  2038		return nbytes;
  2039	
  2040	out_put_css:
  2041		css_put(css);
  2042	out_put_cfile:
  2043		fdput(cfile);
  2044	out_put_eventfd:
  2045		eventfd_ctx_put(event->eventfd);
  2046	out_put_efile:
  2047		fdput(efile);
  2048	out_kfree:
  2049		kfree(event);
  2050	
  2051		return ret;
  2052	}
  2053	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 4/4] mm: Replace simple_strtoul() with kstrtoul()

[kernel test robot]

Hi Keren,

kernel test robot noticed the following build errors:

[auto build test ERROR on v6.12-rc6]
[also build test ERROR on linus/master]
[cannot apply to akpm-mm/mm-everything next-20241104]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Keren-Sun/mm-fix-quoted-strings-spliting-across-lines/20241105-063007
base:   v6.12-rc6
patch link:    https://lore.kernel.org/r/20241104222737.298130-5-kerensun%40google.com
patch subject: [PATCH 4/4] mm: Replace simple_strtoul() with kstrtoul()
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20241105/202411051411.1N0Ziqjh-lkp@intel.com/config)
compiler: clang version 19.1.3 (https://github.com/llvm/llvm-project ab51eccf88f5321e7c60591c5546b254b6afab99)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241105/202411051411.1N0Ziqjh-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202411051411.1N0Ziqjh-lkp@intel.com/

All error/warnings (new ones prefixed by >>):

   In file included from mm/memcontrol-v1.c:4:
   In file included from include/linux/memcontrol.h:21:
   In file included from include/linux/mm.h:2213:
   include/linux/vmstat.h:504:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     504 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     505 |                            item];
         |                            ~~~~
   include/linux/vmstat.h:511:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     511 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     512 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
   include/linux/vmstat.h:518:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
     518 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
         |                               ~~~~~~~~~~~ ^ ~~~
   include/linux/vmstat.h:524:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     524 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     525 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
   In file included from mm/memcontrol-v1.c:6:
   include/linux/mm_inline.h:47:41: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
      47 |         __mod_lruvec_state(lruvec, NR_LRU_BASE + lru, nr_pages);
         |                                    ~~~~~~~~~~~ ^ ~~~
   include/linux/mm_inline.h:49:22: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
      49 |                                 NR_ZONE_LRU_BASE + lru, nr_pages);
         |                                 ~~~~~~~~~~~~~~~~ ^ ~~~
>> mm/memcontrol-v1.c:1926:20: error: incompatible integer to pointer conversion passing 'unsigned int' to parameter of type 'unsigned long *' [-Wint-conversion]
    1926 |         kstrtoul(buf, 10, efd);
         |                           ^~~
   include/linux/kstrtox.h:30:90: note: passing argument to parameter 'res' here
      30 | static inline int __must_check kstrtoul(const char *s, unsigned int base, unsigned long *res)
         |                                                                                          ^
>> mm/memcontrol-v1.c:1926:2: warning: ignoring return value of function declared with 'warn_unused_result' attribute [-Wunused-result]
    1926 |         kstrtoul(buf, 10, efd);
         |         ^~~~~~~~ ~~~~~~~~~~~~
   mm/memcontrol-v1.c:1931:20: error: incompatible integer to pointer conversion passing 'unsigned int' to parameter of type 'unsigned long *' [-Wint-conversion]
    1931 |         kstrtoul(buf, 10, cfd);
         |                           ^~~
   include/linux/kstrtox.h:30:90: note: passing argument to parameter 'res' here
      30 | static inline int __must_check kstrtoul(const char *s, unsigned int base, unsigned long *res)
         |                                                                                          ^
   mm/memcontrol-v1.c:1931:2: warning: ignoring return value of function declared with 'warn_unused_result' attribute [-Wunused-result]
    1931 |         kstrtoul(buf, 10, cfd);
         |         ^~~~~~~~ ~~~~~~~~~~~~
   mm/memcontrol-v1.c:1918:8: warning: unused variable 'endp' [-Wunused-variable]
    1918 |         char *endp;
         |               ^~~~
   mm/memcontrol-v1.c:2606:48: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
    2606 |                         nr += lruvec_page_state(lruvec, NR_LRU_BASE + lru);
         |                                                         ~~~~~~~~~~~ ^ ~~~
   mm/memcontrol-v1.c:2608:54: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
    2608 |                         nr += lruvec_page_state_local(lruvec, NR_LRU_BASE + lru);
         |                                                               ~~~~~~~~~~~ ^ ~~~
   mm/memcontrol-v1.c:2624:46: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
    2624 |                         nr += memcg_page_state(memcg, NR_LRU_BASE + lru);
         |                                                       ~~~~~~~~~~~ ^ ~~~
   mm/memcontrol-v1.c:2626:52: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
    2626 |                         nr += memcg_page_state_local(memcg, NR_LRU_BASE + lru);
         |                                                             ~~~~~~~~~~~ ^ ~~~
   13 warnings and 2 errors generated.


vim +1926 mm/memcontrol-v1.c

  1897	
  1898	/*
  1899	 * DO NOT USE IN NEW FILES.
  1900	 *
  1901	 * Parse input and register new cgroup event handler.
  1902	 *
  1903	 * Input must be in format '<event_fd> <control_fd> <args>'.
  1904	 * Interpretation of args is defined by control file implementation.
  1905	 */
  1906	static ssize_t memcg_write_event_control(struct kernfs_open_file *of,
  1907						 char *buf, size_t nbytes, loff_t off)
  1908	{
  1909		struct cgroup_subsys_state *css = of_css(of);
  1910		struct mem_cgroup *memcg = mem_cgroup_from_css(css);
  1911		struct mem_cgroup_event *event;
  1912		struct cgroup_subsys_state *cfile_css;
  1913		unsigned int efd, cfd;
  1914		struct fd efile;
  1915		struct fd cfile;
  1916		struct dentry *cdentry;
  1917		const char *name;
  1918		char *endp;
  1919		int ret;
  1920	
  1921		if (IS_ENABLED(CONFIG_PREEMPT_RT))
  1922			return -EOPNOTSUPP;
  1923	
  1924		buf = strstrip(buf);
  1925	
> 1926		kstrtoul(buf, 10, efd);
  1927		if (*buf != ' ')
  1928			return -EINVAL;
  1929		buf++;
  1930	
  1931		kstrtoul(buf, 10, cfd);
  1932		if (*buf == ' ')
  1933			buf++;
  1934		else if (*buf != '\0')
  1935			return -EINVAL;
  1936	
  1937		event = kzalloc(sizeof(*event), GFP_KERNEL);
  1938		if (!event)
  1939			return -ENOMEM;
  1940	
  1941		event->memcg = memcg;
  1942		INIT_LIST_HEAD(&event->list);
  1943		init_poll_funcptr(&event->pt, memcg_event_ptable_queue_proc);
  1944		init_waitqueue_func_entry(&event->wait, memcg_event_wake);
  1945		INIT_WORK(&event->remove, memcg_event_remove);
  1946	
  1947		efile = fdget(efd);
  1948		if (!fd_file(efile)) {
  1949			ret = -EBADF;
  1950			goto out_kfree;
  1951		}
  1952	
  1953		event->eventfd = eventfd_ctx_fileget(fd_file(efile));
  1954		if (IS_ERR(event->eventfd)) {
  1955			ret = PTR_ERR(event->eventfd);
  1956			goto out_put_efile;
  1957		}
  1958	
  1959		cfile = fdget(cfd);
  1960		if (!fd_file(cfile)) {
  1961			ret = -EBADF;
  1962			goto out_put_eventfd;
  1963		}
  1964	
  1965		/* the process need read permission on control file */
  1966		/* AV: shouldn't we check that it's been opened for read instead? */
  1967		ret = file_permission(fd_file(cfile), MAY_READ);
  1968		if (ret < 0)
  1969			goto out_put_cfile;
  1970	
  1971		/*
  1972		 * The control file must be a regular cgroup1 file. As a regular cgroup
  1973		 * file can't be renamed, it's safe to access its name afterwards.
  1974		 */
  1975		cdentry = fd_file(cfile)->f_path.dentry;
  1976		if (cdentry->d_sb->s_type != &cgroup_fs_type || !d_is_reg(cdentry)) {
  1977			ret = -EINVAL;
  1978			goto out_put_cfile;
  1979		}
  1980	
  1981		/*
  1982		 * Determine the event callbacks and set them in @event.  This used
  1983		 * to be done via struct cftype but cgroup core no longer knows
  1984		 * about these events.  The following is crude but the whole thing
  1985		 * is for compatibility anyway.
  1986		 *
  1987		 * DO NOT ADD NEW FILES.
  1988		 */
  1989		name = cdentry->d_name.name;
  1990	
  1991		if (!strcmp(name, "memory.usage_in_bytes")) {
  1992			event->register_event = mem_cgroup_usage_register_event;
  1993			event->unregister_event = mem_cgroup_usage_unregister_event;
  1994		} else if (!strcmp(name, "memory.oom_control")) {
  1995			pr_warn_once("oom_control is deprecated and will be removed. Please report your usecase to linux-mm-@kvack.org if you depend on this functionality.\n");
  1996			event->register_event = mem_cgroup_oom_register_event;
  1997			event->unregister_event = mem_cgroup_oom_unregister_event;
  1998		} else if (!strcmp(name, "memory.pressure_level")) {
  1999			pr_warn_once("pressure_level is deprecated and will be removed. Please report your usecase to linux-mm-@kvack.org if you depend on this functionality.\n");
  2000			event->register_event = vmpressure_register_event;
  2001			event->unregister_event = vmpressure_unregister_event;
  2002		} else if (!strcmp(name, "memory.memsw.usage_in_bytes")) {
  2003			event->register_event = memsw_cgroup_usage_register_event;
  2004			event->unregister_event = memsw_cgroup_usage_unregister_event;
  2005		} else {
  2006			ret = -EINVAL;
  2007			goto out_put_cfile;
  2008		}
  2009	
  2010		/*
  2011		 * Verify @cfile should belong to @css.  Also, remaining events are
  2012		 * automatically removed on cgroup destruction but the removal is
  2013		 * asynchronous, so take an extra ref on @css.
  2014		 */
  2015		cfile_css = css_tryget_online_from_dir(cdentry->d_parent,
  2016						       &memory_cgrp_subsys);
  2017		ret = -EINVAL;
  2018		if (IS_ERR(cfile_css))
  2019			goto out_put_cfile;
  2020		if (cfile_css != css) {
  2021			css_put(cfile_css);
  2022			goto out_put_cfile;
  2023		}
  2024	
  2025		ret = event->register_event(memcg, event->eventfd, buf);
  2026		if (ret)
  2027			goto out_put_css;
  2028	
  2029		vfs_poll(fd_file(efile), &event->pt);
  2030	
  2031		spin_lock_irq(&memcg->event_list_lock);
  2032		list_add(&event->list, &memcg->event_list);
  2033		spin_unlock_irq(&memcg->event_list_lock);
  2034	
  2035		fdput(cfile);
  2036		fdput(efile);
  2037	
  2038		return nbytes;
  2039	
  2040	out_put_css:
  2041		css_put(css);
  2042	out_put_cfile:
  2043		fdput(cfile);
  2044	out_put_eventfd:
  2045		eventfd_ctx_put(event->eventfd);
  2046	out_put_efile:
  2047		fdput(efile);
  2048	out_kfree:
  2049		kfree(event);
  2050	
  2051		return ret;
  2052	}
  2053	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 2/4] mm: Fix minor formatting issues for mm control

[Shakeel Butt]

On Mon, Nov 04, 2024 at 02:27:35PM -0800, Keren Sun wrote:
> Add a line after declaration as it's missing after DEFINE_WAIT(),
> replace the spaces with tabs for indent, and remove the non-useful else
> after a break in a if statement.
> 
> Signed-off-by: Keren Sun <kerensun@google.com>

Acked-by: Shakeel Butt <shakeel.butt@linux.dev>

Re: [PATCH 3/4] mm: Prefer 'unsigned int' to bare use of 'unsigned'

[Shakeel Butt]

On Mon, Nov 04, 2024 at 02:27:36PM -0800, Keren Sun wrote:
> Change the param 'mode' from type 'unsigned' to 'unsigned int' in
> memcg_event_wake() and memcg_oom_wake_function(), and for the param
> 'nid' in VM_BUG_ON().
> 
> Signed-off-by: Keren Sun <kerensun@google.com>

Acked-by: Shakeel Butt <shakeel.butt@linux.dev>

Re: [PATCH 4/4] mm: Replace simple_strtoul() with kstrtoul()

[Shakeel Butt]

On Mon, Nov 04, 2024 at 02:27:37PM -0800, Keren Sun wrote:
> simple_strtoul() has caveat and is obsolete, use kstrtoul() instead in mmcg.

Did you test this patch? I don't think kstrtoul() can be used here as it
expects a string containing a single number.

> 
> Signed-off-by: Keren Sun <kerensun@google.com>
> ---
>  mm/memcontrol-v1.c | 17 ++++++++---------
>  1 file changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/mm/memcontrol-v1.c b/mm/memcontrol-v1.c
> index 5e1854623824..260b356cea5a 100644
> --- a/mm/memcontrol-v1.c
> +++ b/mm/memcontrol-v1.c
> @@ -1,5 +1,6 @@
>  // SPDX-License-Identifier: GPL-2.0-or-later
>  
> +#include "linux/kstrtox.h"
>  #include <linux/memcontrol.h>
>  #include <linux/swap.h>
>  #include <linux/mm_inline.h>
> @@ -1922,17 +1923,15 @@ static ssize_t memcg_write_event_control(struct kernfs_open_file *of,
>  
>  	buf = strstrip(buf);
>  
> -	efd = simple_strtoul(buf, &endp, 10);
> -	if (*endp != ' ')
> +	kstrtoul(buf, 10, efd);
> +	if (*buf != ' ')
>  		return -EINVAL;
> -	buf = endp + 1;
> +	buf++;
>  
> -	cfd = simple_strtoul(buf, &endp, 10);
> -	if (*endp == '\0')
> -		buf = endp;
> -	else if (*endp == ' ')
> -		buf = endp + 1;
> -	else
> +	kstrtoul(buf, 10, cfd);
> +	if (*buf == ' ')
> +		buf++;
> +	else if (*buf != '\0')
>  		return -EINVAL;
>  
>  	event = kzalloc(sizeof(*event), GFP_KERNEL);
> -- 
> 2.47.0.163.g1226f6d8fa-goog
> 

Re: [PATCH 4/4] mm: Replace simple_strtoul() with kstrtoul()

[Keren Sun]

[-- Attachment #1: Type: text/plain, Size: 1815 bytes --]

Sorry I wasn't sure which test to test with. I'll revert this patch then.

On Tue, Nov 5, 2024 at 8:38 AM Shakeel Butt <shakeel.butt@linux.dev> wrote:

> On Mon, Nov 04, 2024 at 02:27:37PM -0800, Keren Sun wrote:
> > simple_strtoul() has caveat and is obsolete, use kstrtoul() instead in
> mmcg.
>
> Did you test this patch? I don't think kstrtoul() can be used here as it
> expects a string containing a single number.
>
> >
> > Signed-off-by: Keren Sun <kerensun@google.com>
> > ---
> >  mm/memcontrol-v1.c | 17 ++++++++---------
> >  1 file changed, 8 insertions(+), 9 deletions(-)
> >
> > diff --git a/mm/memcontrol-v1.c b/mm/memcontrol-v1.c
> > index 5e1854623824..260b356cea5a 100644
> > --- a/mm/memcontrol-v1.c
> > +++ b/mm/memcontrol-v1.c
> > @@ -1,5 +1,6 @@
> >  // SPDX-License-Identifier: GPL-2.0-or-later
> >
> > +#include "linux/kstrtox.h"
> >  #include <linux/memcontrol.h>
> >  #include <linux/swap.h>
> >  #include <linux/mm_inline.h>
> > @@ -1922,17 +1923,15 @@ static ssize_t memcg_write_event_control(struct
> kernfs_open_file *of,
> >
> >       buf = strstrip(buf);
> >
> > -     efd = simple_strtoul(buf, &endp, 10);
> > -     if (*endp != ' ')
> > +     kstrtoul(buf, 10, efd);
> > +     if (*buf != ' ')
> >               return -EINVAL;
> > -     buf = endp + 1;
> > +     buf++;
> >
> > -     cfd = simple_strtoul(buf, &endp, 10);
> > -     if (*endp == '\0')
> > -             buf = endp;
> > -     else if (*endp == ' ')
> > -             buf = endp + 1;
> > -     else
> > +     kstrtoul(buf, 10, cfd);
> > +     if (*buf == ' ')
> > +             buf++;
> > +     else if (*buf != '\0')
> >               return -EINVAL;
> >
> >       event = kzalloc(sizeof(*event), GFP_KERNEL);
> > --
> > 2.47.0.163.g1226f6d8fa-goog
> >
>

[-- Attachment #2: Type: text/html, Size: 2595 bytes --]

Re: [PATCH 1/4] mm: fix quoted strings spliting across lines

[Keren Sun]

[-- Attachment #1: Type: text/plain, Size: 505 bytes --]

Gotcha, will drop this one then.

On Mon, Nov 4, 2024 at 5:56 PM Shakeel Butt <shakeel.butt@linux.dev> wrote:

> On Mon, Nov 04, 2024 at 02:27:34PM -0800, Keren Sun wrote:
> > This patch fixes quoted strings splitting across lines in
> > pr_warn_once() by putting them into one line.
> >
> > Signed-off-by: Keren Sun <kerensun@google.com>
>
> Other patches seems fine but personally I don't like this patch. I
> would prefer a multi line string over a very long single line of string.
>
>

[-- Attachment #2: Type: text/html, Size: 880 bytes --]

Re: [PATCH 4/4] mm: Replace simple_strtoul() with kstrtoul()

[Roman Gushchin]

On Mon, Nov 04, 2024 at 02:27:37PM -0800, Keren Sun wrote:
> simple_strtoul() has caveat and is obsolete, use kstrtoul() instead in mmcg.
                                                                         ^^^^
									  ?
Btw, did you test this code? Shakeel was poiting out that kstrtoul() might not
work here, is it false?

Thanks!

Re: [PATCH 4/4] mm: Replace simple_strtoul() with kstrtoul()

[Keren Sun]

[-- Attachment #1: Type: text/plain, Size: 766 bytes --]

I'm trying to come up with a test. I did look into the code and saw that
the pointer to the string moves as it's being parsed, and the parsing stops
when the string is no longer a number. So I think it's good to use this
function to replace simple_strtoul(), as kstrtoul() has an overflow check
as a plus.

On Thu, Nov 7, 2024 at 9:20 AM Roman Gushchin <roman.gushchin@linux.dev>
wrote:

> On Mon, Nov 04, 2024 at 02:27:37PM -0800, Keren Sun wrote:
> > simple_strtoul() has caveat and is obsolete, use kstrtoul() instead in
> mmcg.
>
>  ^^^^
>                                                                           ?
> Btw, did you test this code? Shakeel was poiting out that kstrtoul() might
> not
> work here, is it false?
>
> Thanks!
>

[-- Attachment #2: Type: text/html, Size: 1210 bytes --]