2008-08-06 17:22:51

by Rakib Mullick

Subject: [PATCH] cpuset : Handling improper memory allocation.

This patch makes sure that , if kmalloc fails in
"rebuild_sched_domains", then we've a place to go. If anything else,
please let me know.

Thanks.

Signed-off-by: Md.Rakib H. Mullick ([email protected])

--- linux-2.6.27-rc2.orig/kernel/cpuset.c 2008-08-06 16:23:26.000000000 +0600
+++ linux-2.6.27-rc2/kernel/cpuset.c 2008-08-06 23:07:59.504441992 +0600
@@ -615,7 +615,8 @@ void rebuild_sched_domains(void)
if (dattr) {
*dattr = SD_ATTR_INIT;
update_domain_attr_tree(dattr, &top_cpuset);
- }
+ } else
+ goto rebuild;
*doms = top_cpuset.cpus_allowed;
goto rebuild;
}
@@ -685,6 +686,8 @@ restart:
if (!doms)
goto rebuild;
dattr = kmalloc(ndoms * sizeof(struct sched_domain_attr), GFP_KERNEL);
+ if (!dattr)
+ goto rebuild;

for (nslot = 0, i = 0; i < csn; i++) {
struct cpuset *a = csa[i];

2008-08-06 20:13:45

by Max Krasnyansky

Subject: Re: [PATCH] cpuset : Handling improper memory allocation.

Rakib Mullick wrote:
> This patch makes sure that , if kmalloc fails in
> "rebuild_sched_domains", then we've a place to go. If anything else,
> please let me know.
>

partition_sched_domains() can deal with attrs==NULL case. So we do not
have to abort domain building.

Max


> Thanks.
>
> Signed-off-by: Md.Rakib H. Mullick ([email protected])
>
> --- linux-2.6.27-rc2.orig/kernel/cpuset.c 2008-08-06 16:23:26.000000000 +0600
> +++ linux-2.6.27-rc2/kernel/cpuset.c 2008-08-06 23:07:59.504441992 +0600
> @@ -615,7 +615,8 @@ void rebuild_sched_domains(void)
> if (dattr) {
> *dattr = SD_ATTR_INIT;
> update_domain_attr_tree(dattr, &top_cpuset);
> - }
> + } else
> + goto rebuild;
> *doms = top_cpuset.cpus_allowed;
> goto rebuild;
> }
> @@ -685,6 +686,8 @@ restart:
> if (!doms)
> goto rebuild;
> dattr = kmalloc(ndoms * sizeof(struct sched_domain_attr), GFP_KERNEL);
> + if (!dattr)
> + goto rebuild;
>
> for (nslot = 0, i = 0; i < csn; i++) {
> struct cpuset *a = csa[i];
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>

2008-08-06 20:37:22

by Paul Jackson

Subject: Re: [PATCH] cpuset : Handling improper memory allocation.

Max wrote:
> partition_sched_domains() can deal with attrs==NULL case. So we do not
> have to abort domain building.

This might be a good place for a comment, since unchecked kmalloc()
calls look dangerous:

/* Convert <csn, csa> to <ndoms, doms> */
doms = kmalloc(ndoms * sizeof(cpumask_t), GFP_KERNEL);
if (!doms)
goto rebuild;
dattr = kmalloc(ndoms * sizeof(struct sched_domain_attr), GFP_KERNEL);
/* dattr == NULL is ok; partition_sched_domains() can cope with it. */

--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <[email protected]> 1.940.382.4214

2008-08-06 20:44:22

by Max Krasnyansky

Subject: Re: [PATCH] cpuset : Handling improper memory allocation.

Paul Jackson wrote:
> Max wrote:
>> partition_sched_domains() can deal with attrs==NULL case. So we do not
>> have to abort domain building.
>
> This might be a good place for a comment, since unchecked kmalloc()
> calls look dangerous:
>
> /* Convert <csn, csa> to <ndoms, doms> */
> doms = kmalloc(ndoms * sizeof(cpumask_t), GFP_KERNEL);
> if (!doms)
> goto rebuild;
> dattr = kmalloc(ndoms * sizeof(struct sched_domain_attr), GFP_KERNEL);
> /* dattr == NULL is ok; partition_sched_domains() can cope with it. */

Makes sense. I'll add it, since I'm touching that area anyway.

Max