Skip to contentSkip to conversation

randconfig broken on choice

PATCH Matthieu CASTET 2 participants 6 messages Sep 6, 2011 – Sep 9, 2011

Subsystems (2)
Kbuild Kconfig
lore
Share & tools
lore

Matthieu CASTET

author
#0 lore raw

Hi,

I am using v3.1-rc4 and make randconfig look broken for choice.
The choice config is always the same or choice entry can have more than one entry.

After some debugging, it seems in randomize_choice_values, we don't clean SYMBOL_VALID for choice entry.
And we don't take "sym->def[S_DEF_USER].tri" but the "default sym->cur.tri".

The following patch seems to fix the problem.

Matthieu

scripts/kconfig/confdata.c +4 -0
index 59b667c..08331f8 100644
@@ -1045,6 +1045,10 @@ static void randomize_choice_values(struct symbol *csym)
 		else {
 			sym->def[S_DEF_USER].tri = no;
 		}
+		sym->flags |= SYMBOL_DEF_USER;
+		/* clear VALID to get value calculated */
+		sym->flags &= ~(SYMBOL_VALID);
+
 	}
 	csym->flags |= SYMBOL_DEF_USER;
 	/* clear VALID to get value calculated */
#1 lore raw

Hi,

Quoted 5 lines · Matthieu CASTET

Hi,

I am using v3.1-rc4 and make randconfig look broken for choice.
The choice config is always the same or choice entry can have more than one entry.

Could you includes testcases showing that behavior, please ?

Btw, try also to include patch inlined, not multipart. That makes review easier. I know, gmail is not making that easy, but git send-email works just fine.

Thanks,
- Arnaud

Quoted 10 lines

After some debugging, it seems in randomize_choice_values, we don't clean SYMBOL_VALID for choice entry.
And we don't take "sym->def[S_DEF_USER].tri" but the "default sym->cur.tri".

The following patch seems to fix the problem.

Matthieu

author
#2 lore raw

Hi,

Quoted 10 lines · Arnaud Lacombe

Hi,

Quoted 5 lines · Matthieu CASTET

Hi,

I am using v3.1-rc4 and make randconfig look broken for choice.
The choice config is always the same or choice entry can have more than one entry.

Could you includes testcases showing that behavior, please ?

$ make randconfig
$ grep -e SLUB -e SLAB -e SLOB .config
# CONFIG_SLAB is not set
CONFIG_SLUB=y
CONFIG_SLOB=y
# CONFIG_FAILSLAB is not set

We have 2 choice selected (SLUB and SLOB). You may run randconfig (more than one time to get this result).

Matthieu

#3 lore raw

Hi,

Quoted 15 lines · Matthieu CASTET

Hi,

Quoted 10 lines · Arnaud Lacombe

Hi,

Quoted 5 lines · Matthieu CASTET

Hi,

I am using v3.1-rc4 and make randconfig look broken for choice.
The choice config is always the same or choice entry can have more than one entry.

Could you includes testcases showing that behavior, please ?

$ make randconfig
Quoted 9 lines
$ grep -e SLUB -e SLAB -e SLOB .config
# CONFIG_SLAB is not set
CONFIG_SLUB=y
CONFIG_SLOB=y
# CONFIG_FAILSLAB is not set

We have 2 choice selected (SLUB and SLOB). You may run randconfig (more than one time to get this result).

ok, I reproduced it with:

while true; do
make randconfig
CONFIG_SLAB=; CONFIG_SLOB=; CONFIG_SLUB=
source .config
if [ "${CONFIG_SLAB}${CONFIG_SLOB}${CONFIG_SLUB}" != "y" ]; then
break
fi
sleep 1
done

I wonder what set the flags originally...

- Arnaud

Quoted 2 lines

Matthieu

#4 lore raw

Hi,

Quoted 40 lines · Arnaud Lacombe

Hi,

Quoted 15 lines · Matthieu CASTET

Hi,

Quoted 10 lines · Arnaud Lacombe

Hi,

Quoted 5 lines · Matthieu CASTET

Hi,

I am using v3.1-rc4 and make randconfig look broken for choice.
The choice config is always the same or choice entry can have more than one entry.

Could you includes testcases showing that behavior, please ?

$ make randconfig
Quoted 9 lines
$ grep -e SLUB -e SLAB -e SLOB .config
# CONFIG_SLAB is not set
CONFIG_SLUB=y
CONFIG_SLOB=y
# CONFIG_FAILSLAB is not set

We have 2 choice selected (SLUB and SLOB). You may run randconfig (more than one time to get this result).

ok, I reproduced it with:

while true; do
   make randconfig
   CONFIG_SLAB=; CONFIG_SLOB=; CONFIG_SLUB=
   source .config
   if [ "${CONFIG_SLAB}${CONFIG_SLOB}${CONFIG_SLUB}" != "y" ]; then break
   fi
   sleep 1
done

Quoted 2 lines

I wonder what set the flags originally...

This is interesting; I'm always getting the following:

CONFIG_SLUB=y
CONFIG_SLOB=y

never CONFIG_SLAB and another one. Actually, the default of the choice ends up being selected along with the random one. Is this what you mean by "we don't take "sym->def[S_DEF_USER].tri" but the "default sym->cur.tri"" ? However, I do not see this behavior with the following reduced testcase:

config EXPERT
bool
default y

choice
prompt "Choose SLAB allocator"
default SLUB

config SLAB
bool "SLAB"

config SLUB
bool "SLUB (Unqueued Allocator)"

config SLOB
depends on EXPERT
bool "SLOB (Simple Allocator)"

endchoice

Btw, you say that "we don't clean SYMBOL_VALID for choice entry", but AFAIU the code, we explicitly set the not-randomly-chosen choice value to 'no', so it's value should be valid.

 - Arnaud

author
#5 lore raw

Hi Arnaud,

Quoted 28 lines · Arnaud Lacombe

Hi,

Quoted 21 lines · Arnaud Lacombe

Hi,

Quoted 5 lines · Matthieu CASTET

Hi,

Quoted 2 lines · Arnaud Lacombe

Hi,

ok, I reproduced it with:

while true; do
make randconfig
CONFIG_SLAB=; CONFIG_SLOB=; CONFIG_SLUB=
source .config
if [ "${CONFIG_SLAB}${CONFIG_SLOB}${CONFIG_SLUB}" != "y" ]; then
break
fi
sleep 1
done

Quoted 2 lines

I wonder what set the flags originally...

This is interesting; I'm always getting the following:

Quoted 7 lines

CONFIG_SLUB=y
CONFIG_SLOB=y

never CONFIG_SLAB and another one. Actually, the default of the choice ends up being selected along with the random one. Is this what you mean by "we don't take "sym->def[S_DEF_USER].tri" but the "default sym->cur.tri"" ?

Yes, what I mean is that we don't take the value set by randomize_choice_values,
but some default values.
I wonder why the other one is random (I don't search who set cur value).

You can see the problem with the attached patch

and do make randconfig | grep -e "SL[UAO]B,"

However, I do not see this behavior with the

Quoted 3 lines

following reduced testcase:

Yes that's weird.

Quoted 4 lines

Btw, you say that "we don't clean SYMBOL_VALID for choice entry", but AFAIU the code, we explicitly set the not-randomly-chosen choice value to 'no', so it's value should be valid.

I don't understand what you mean.

What I saw is that randomize_choice_values modify sym->def[S_DEF_USER], but because it is already marked valid, we never use sym->def[S_DEF_USER] in the code writing the .config.

Matthieu

scripts/kconfig/lkc.h +3 -0
index b633bdb..eecb7af 100644
@@ -147,6 +147,9 @@ struct property *sym_get_env_prop(struct symbol *sym);
 
 static inline tristate sym_get_tristate_value(struct symbol *sym)
 {
+	printf("name %s, cur=%d(%s) user=%d(%s)\n", sym->name,
+			sym->curr.tri, sym->flags & SYMBOL_VALID?"valid":"invalid",
+			sym->def[S_DEF_USER].tri, sym->flags & SYMBOL_DEF_USER?"valid":"invalid");
 	return sym->curr.tri;
 }