2023-01-11 04:22:45

by Li Lingfeng

Subject: [PATCH-next RESEND v2] lib: parser: optimize match_NUMER apis to use local array

Memory will be allocated to store substring_t in match_strdup(), which means
the caller of match_strdup() may need to be scheduled out to wait for reclaiming
memory.

Using local array to store substring_t to remove the restriction.

Link: https://lore.kernel.org/all/[email protected]/
Signed-off-by: Li Lingfeng <[email protected]>
Acked-by: Tejun Heo <[email protected]>
---
lib/parser.c | 42 +++++++++++++++++++++++-------------------
1 file changed, 23 insertions(+), 19 deletions(-)

diff --git a/lib/parser.c b/lib/parser.c
index bcb23484100e..0d00f0adb063 100644
--- a/lib/parser.c
+++ b/lib/parser.c
@@ -11,6 +11,15 @@
#include <linux/slab.h>
#include <linux/string.h>

+/*
+ * max size needed by diffrent bases to express U64
+ * HEX: "0xFFFFFFFFFFFFFFFF" --> 18
+ * DEC: "18446744073709551615" --> 20
+ * OCT: "01777777777777777777777" --> 23
+ * pick the max one to define NUMBER_BUF_LEN
+ */
+#define NUMBER_BUF_LEN 24
+
/**
* match_one - Determines if a string matches a simple pattern
* @s: the string to examine for presence of the pattern
@@ -129,14 +138,13 @@ EXPORT_SYMBOL(match_token);
static int match_number(substring_t *s, int *result, int base)
{
char *endp;
- char *buf;
+ char buf[NUMBER_BUF_LEN];
int ret;
long val;

- buf = match_strdup(s);
- if (!buf)
- return -ENOMEM;
-
+ if ((s->to - s->from) >= NUMBER_BUF_LEN)
+ return -ERANGE;
+ match_strlcpy(buf, s, NUMBER_BUF_LEN);
ret = 0;
val = simple_strtol(buf, &endp, base);
if (endp == buf)
@@ -145,7 +153,6 @@ static int match_number(substring_t *s, int *result, int base)
ret = -ERANGE;
else
*result = (int) val;
- kfree(buf);
return ret;
}

@@ -163,18 +170,16 @@ static int match_number(substring_t *s, int *result, int base)
*/
static int match_u64int(substring_t *s, u64 *result, int base)
{
- char *buf;
+ char buf[NUMBER_BUF_LEN];
int ret;
u64 val;

- buf = match_strdup(s);
- if (!buf)
- return -ENOMEM;
-
+ if ((s->to - s->from) >= NUMBER_BUF_LEN)
+ return -ERANGE;
+ match_strlcpy(buf, s, NUMBER_BUF_LEN);
ret = kstrtoull(buf, base, &val);
if (!ret)
*result = val;
- kfree(buf);
return ret;
}

@@ -206,14 +211,13 @@ EXPORT_SYMBOL(match_int);
*/
int match_uint(substring_t *s, unsigned int *result)
{
- int err = -ENOMEM;
- char *buf = match_strdup(s);
+ char buf[NUMBER_BUF_LEN];

- if (buf) {
- err = kstrtouint(buf, 10, result);
- kfree(buf);
- }
- return err;
+ if ((s->to - s->from) >= NUMBER_BUF_LEN)
+ return -ERANGE;
+ match_strlcpy(buf, s, NUMBER_BUF_LEN);
+
+ return kstrtouint(buf, 10, result);
}
EXPORT_SYMBOL(match_uint);

--
2.31.1

2023-01-11 07:33:22

by Eric Biggers

Subject: Re: [PATCH v2] lib: parser: optimize match_NUMER apis to use local array

On Wed, Jan 11, 2023 at 12:22:15PM +0800, Li Lingfeng wrote:
> /**
> * match_one - Determines if a string matches a simple pattern
> * @s: the string to examine for presence of the pattern
> @@ -129,14 +138,13 @@ EXPORT_SYMBOL(match_token);
> static int match_number(substring_t *s, int *result, int base)
> {
> char *endp;
> - char *buf;
> + char buf[NUMBER_BUF_LEN];
> int ret;
> long val;
>
> - buf = match_strdup(s);
> - if (!buf)
> - return -ENOMEM;
> -
> + if ((s->to - s->from) >= NUMBER_BUF_LEN)
> + return -ERANGE;
> + match_strlcpy(buf, s, NUMBER_BUF_LEN);

Instead of doing '(s->to - s->from) >= NUMBER_BUF_LEN', it would be simpler to
check the return value of match_strlcpy():

if (match_strlcpy(buf, s, NUMBER_BUF_LEN) >= NUMBER_BUF_LEN)
return -ERANGE;

Likewise in match_u64int() and match_uint().

- Eric

2023-01-19 19:11:51

by Tejun Heo

Subject: Re: [PATCH-next RESEND v2] lib: parser: optimize match_NUMER apis to use local array

On Wed, Jan 11, 2023 at 12:22:15PM +0800, Li Lingfeng wrote:
> Memory will be allocated to store substring_t in match_strdup(), which means
> the caller of match_strdup() may need to be scheduled out to wait for reclaiming
> memory.
>
> Using local array to store substring_t to remove the restriction.
>
> Link: https://lore.kernel.org/all/[email protected]/
> Signed-off-by: Li Lingfeng <[email protected]>
> Acked-by: Tejun Heo <[email protected]>

This will fix a sleep-in-atomic in #linus, so it'd be great if we can get it
merged. Can you please apply the suggested changes and repost? I think we
can route this through block too as the current breakage is there. So,
please cc Jens too.

Thanks.

--
tejun