2021-09-24 08:26:06

by Shaopeng Tan

[permalink] [raw]
Subject: [PATCH] x86/resctrl: Fix return code in mkdir_rdt_prepare()

When kzalloc fails, we should return ENOMEM instead of ENOSPC.

Signed-off-by: Shaopeng Tan <[email protected]>
---
Hello,

I just noticed this when I read the code.

Thanks,

arch/x86/kernel/cpu/resctrl/rdtgroup.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index b57b3db9a6a7..a92d047476f6 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -2854,7 +2854,7 @@ static int mkdir_rdt_prepare(struct kernfs_node *parent_kn,
/* allocate the rdtgroup. */
rdtgrp = kzalloc(sizeof(*rdtgrp), GFP_KERNEL);
if (!rdtgrp) {
- ret = -ENOSPC;
+ ret = -ENOMEM;
rdt_last_cmd_puts("Kernel out of memory\n");
goto out_unlock;
}
--
2.27.0


2021-09-24 22:14:46

by Reinette Chatre

[permalink] [raw]
Subject: Re: [PATCH] x86/resctrl: Fix return code in mkdir_rdt_prepare()

Hi Shaopeng Tan,

On 9/24/2021 1:17 AM, Shaopeng Tan wrote:
> When kzalloc fails, we should return ENOMEM instead of ENOSPC.
>
> Signed-off-by: Shaopeng Tan <[email protected]>
> ---
> Hello,
>
> I just noticed this when I read the code.
>
> Thanks,
>
> arch/x86/kernel/cpu/resctrl/rdtgroup.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> index b57b3db9a6a7..a92d047476f6 100644
> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> @@ -2854,7 +2854,7 @@ static int mkdir_rdt_prepare(struct kernfs_node *parent_kn,
> /* allocate the rdtgroup. */
> rdtgrp = kzalloc(sizeof(*rdtgrp), GFP_KERNEL);
> if (!rdtgrp) {
> - ret = -ENOSPC;
> + ret = -ENOMEM;
> rdt_last_cmd_puts("Kernel out of memory\n");
> goto out_unlock;
> }
>

I do not know the original motivation for using ENOSPC but from what I
can tell this error is propagated all the way to user space. This change
thus has the consequence that any interface built on top of resctrl
could be impacted.

Is there a specific issue that you are aiming to fix here?

Reinette

2021-09-28 04:48:29

by Shaopeng Tan (Fujitsu)

[permalink] [raw]
Subject: RE: [PATCH] x86/resctrl: Fix return code in mkdir_rdt_prepare()

Hi Reinette,

> On 9/24/2021 1:17 AM, Shaopeng Tan wrote:
> > When kzalloc fails, we should return ENOMEM instead of ENOSPC.
> >
> > Signed-off-by: Shaopeng Tan <[email protected]>
> > ---
> > Hello,
> >
> > I just noticed this when I read the code.
> >
> > Thanks,
> >
> > arch/x86/kernel/cpu/resctrl/rdtgroup.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> > b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> > index b57b3db9a6a7..a92d047476f6 100644
> > --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> > +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> > @@ -2854,7 +2854,7 @@ static int mkdir_rdt_prepare(struct kernfs_node
> *parent_kn,
> > /* allocate the rdtgroup. */
> > rdtgrp = kzalloc(sizeof(*rdtgrp), GFP_KERNEL);
> > if (!rdtgrp) {
> > - ret = -ENOSPC;
> > + ret = -ENOMEM;
> > rdt_last_cmd_puts("Kernel out of memory\n");
> > goto out_unlock;
> > }
> >
>
> I do not know the original motivation for using ENOSPC but from what I can tell
> this error is propagated all the way to user space. This change thus has the
> consequence that any interface built on top of resctrl could be impacted.
>
> Is there a specific issue that you are aiming to fix here?

I didn't detect any problem on machine, I noticed it by reading at the code.

Elsewhere in resctrl code, if kzalloc fails, it returns ENOMEM.
For example, in the same file(rdtgroup.c),
2107 s = kzalloc(sizeof(*s), GFP_KERNEL);
2108 if (!s)
2109 return -ENOMEM;
so, I thought it wasn't intentionally to return ENOSPC.

As you said it maybe impact user space,
so, please ignore this patch if you don't think it needs to be changed.

Thanks,
Tan Shaopeng