LinuxLists.cc - More than 8 CPUs detected and CONFIG_X86

[permalink] [raw]

Subject: Re: More than 8 CPUs detected and CONFIG_X86_PC cannot handle it on 2.6.16-rc6

Krzysztof Oledzki <[email protected]> wrote:
>
> After upgrading to 2.6.16-rc6 I noticed this strange message:
>
> More than 8 CPUs detected and CONFIG_X86_PC cannot handle it.
> Use CONFIG_X86_GENERICARCH or CONFIG_X86_BIGSMP.
>
> This is a Dell PowerEdge SC1425 with two P4 Xeons with HT enabled (so with
> totoal of 4 logical CPUs).

Please send full dmesg output for the failing kernel, thanks.

Which is the most-recently-tested kernel which behaved correctly?

2006-03-12 11:04:32

[permalink] [raw]

Subject: Re: More than 8 CPUs detected and CONFIG_X86_PC cannot handle it on 2.6.16-rc6

On Sat, 11 Mar 2006, Andrew Morton wrote:

> Krzysztof Oledzki <[email protected]> wrote:
>>
>> After upgrading to 2.6.16-rc6 I noticed this strange message:
>>
>> More than 8 CPUs detected and CONFIG_X86_PC cannot handle it.
>> Use CONFIG_X86_GENERICARCH or CONFIG_X86_BIGSMP.
>>
>> This is a Dell PowerEdge SC1425 with two P4 Xeons with HT enabled (so with
>> totoal of 4 logical CPUs).
>
> Please send full dmesg output for the failing kernel, thanks.
Attached.

> Which is the most-recently-tested kernel which behaved correctly?
2.6.15.6

Best regards,

Krzysztof Ol?dzki

Attachments:

dmesg-2.6.16-rc6 (22.69 kB)

2006-03-12 11:27:49

[permalink] [raw]

Subject: Re: More than 8 CPUs detected and CONFIG_X86_PC cannot handle it on 2.6.16-rc6

Krzysztof Oledzki <[email protected]> wrote:
>
> On Sat, 11 Mar 2006, Andrew Morton wrote:
>
> > Krzysztof Oledzki <[email protected]> wrote:
> >>
> >> After upgrading to 2.6.16-rc6 I noticed this strange message:
> >>
> >> More than 8 CPUs detected and CONFIG_X86_PC cannot handle it.
> >> Use CONFIG_X86_GENERICARCH or CONFIG_X86_BIGSMP.
> >>
> >> This is a Dell PowerEdge SC1425 with two P4 Xeons with HT enabled (so with
> >> totoal of 4 logical CPUs).
> >
> > Please send full dmesg output for the failing kernel, thanks.
> Attached.
>
> > Which is the most-recently-tested kernel which behaved correctly?
> 2.6.15.6

OK, thanks. I assume the machine's working OK?

>From my reading, you have CONFIG_HOTPLUG_CPU enabled and the machine has an
APIC. I'd expect that lots of people would hit that warning but for some
reason they don't - possibly because most APICs don't have sufficiently
high version numbers?

Anyway, various people cc'ed. I _think_ it's harmless, although the way in
which def_to_bigsmp propagates into the DMI and APIC code might be a
problem, depending upon config options.

Certainly the warning is incorrect, but I'm not sure what is the best thing
to do about it?

2006-03-12 13:05:17

[permalink] [raw]

Subject: Re: More than 8 CPUs detected and CONFIG_X86_PC cannot handle it on 2.6.16-rc6

On Sun, 12 Mar 2006, Andrew Morton wrote:

> Krzysztof Oledzki <[email protected]> wrote:
>>
>> On Sat, 11 Mar 2006, Andrew Morton wrote:
>>
>> > Krzysztof Oledzki <[email protected]> wrote:
>> >>
>> >> After upgrading to 2.6.16-rc6 I noticed this strange message:
>> >>
>> >> More than 8 CPUs detected and CONFIG_X86_PC cannot handle it.
>> >> Use CONFIG_X86_GENERICARCH or CONFIG_X86_BIGSMP.
>> >>
>> >> This is a Dell PowerEdge SC1425 with two P4 Xeons with HT enabled (so with
>> >> totoal of 4 logical CPUs).
>> >
>> > Please send full dmesg output for the failing kernel, thanks.
>> Attached.
>>
>> > Which is the most-recently-tested kernel which behaved correctly?
>> 2.6.15.6
>
> OK, thanks. I assume the machine's working OK?

Yes. So far no problems, only this warning.

> From my reading, you have CONFIG_HOTPLUG_CPU enabled and the machine has an
> APIC.
That is correct.

> I'd expect that lots of people would hit that warning but for some
> reason they don't - possibly because most APICs don't have sufficiently
> high version numbers?
>
> Anyway, various people cc'ed. I _think_ it's harmless, although the way in
> which def_to_bigsmp propagates into the DMI and APIC code might be a
> problem, depending upon config options.
>
> Certainly the warning is incorrect, but I'm not sure what is the best thing
> to do about it?

OK. Thank you.

Best regards,

Krzysztof Ol?dzki

2006-03-12 15:35:48

by Pallipadi, Venkatesh

[permalink] [raw]

Subject: Re: More than 8 CPUs detected and CONFIG_X86_PC cannot handle it on 2.6.16-rc6

On Sun, Mar 12, 2006 at 02:05:00PM +0100, Krzysztof Oledzki wrote:
>
>
> On Sun, 12 Mar 2006, Andrew Morton wrote:
>
> > Krzysztof Oledzki <[email protected]> wrote:
> >>
> >> On Sat, 11 Mar 2006, Andrew Morton wrote:
> >>
> >> > Krzysztof Oledzki <[email protected]> wrote:
> >> >>
> >> >> After upgrading to 2.6.16-rc6 I noticed this strange message:
> >> >>
> >> >> More than 8 CPUs detected and CONFIG_X86_PC cannot handle it.
> >> >> Use CONFIG_X86_GENERICARCH or CONFIG_X86_BIGSMP.
> >> >>
> >> >> This is a Dell PowerEdge SC1425 with two P4 Xeons with HT enabled (so with
> >> >> totoal of 4 logical CPUs).
> >> >
> >> > Please send full dmesg output for the failing kernel, thanks.
> >> Attached.
> >>
> >> > Which is the most-recently-tested kernel which behaved correctly?
> >> 2.6.15.6
> >
> > OK, thanks. I assume the machine's working OK?
>
> Yes. So far no problems, only this warning.
>
> > From my reading, you have CONFIG_HOTPLUG_CPU enabled and the machine has an
> > APIC.
> That is correct.
>
> > I'd expect that lots of people would hit that warning but for some
> > reason they don't - possibly because most APICs don't have sufficiently
> > high version numbers?
> >

Actually, this warning should be seen on many other systems on well. We
use the bigsmp when there _or_ more than 8 CPUs or CPU_HOTPLUG is used.
So, in that sense the message is wrong, it should also have CPU_HOTPLUG in
there. Or we should make CPU_HOTPLUG depend on GENERIC_ARCH or auto select
GENERIC_ARCH with hotplug at the CONFIG level.

Will defer to Ashok for a proper fix.

Thanks,
Venki

2006-03-12 21:14:15

[permalink] [raw]

Subject: Re: More than 8 CPUs detected and CONFIG_X86_PC cannot handle it on 2.6.16-rc6

On Sun, 12 Mar 2006, Venkatesh Pallipadi wrote:

> On Sun, Mar 12, 2006 at 02:05:00PM +0100, Krzysztof Oledzki wrote:
>>
>>
>> On Sun, 12 Mar 2006, Andrew Morton wrote:
>>
>>> Krzysztof Oledzki <[email protected]> wrote:
>>>>
>>>> On Sat, 11 Mar 2006, Andrew Morton wrote:
>>>>
>>>>> Krzysztof Oledzki <[email protected]> wrote:
>>>>>>
>>>>>> After upgrading to 2.6.16-rc6 I noticed this strange message:
>>>>>>
>>>>>> More than 8 CPUs detected and CONFIG_X86_PC cannot handle it.
>>>>>> Use CONFIG_X86_GENERICARCH or CONFIG_X86_BIGSMP.
>>>>>>
>>>>>> This is a Dell PowerEdge SC1425 with two P4 Xeons with HT enabled (so with
>>>>>> totoal of 4 logical CPUs).
>>>>>
>>>>> Please send full dmesg output for the failing kernel, thanks.
>>>> Attached.
>>>>
>>>>> Which is the most-recently-tested kernel which behaved correctly?
>>>> 2.6.15.6
>>>
>>> OK, thanks. I assume the machine's working OK?
>>
>> Yes. So far no problems, only this warning.
>>
>>> From my reading, you have CONFIG_HOTPLUG_CPU enabled and the machine has an
>>> APIC.
>> That is correct.
>>
>>> I'd expect that lots of people would hit that warning but for some
>>> reason they don't - possibly because most APICs don't have sufficiently
>>> high version numbers?
>>>
>
> Actually, this warning should be seen on many other systems on well. We
> use the bigsmp when there _or_ more than 8 CPUs or CPU_HOTPLUG is used.
> So, in that sense the message is wrong, it should also have CPU_HOTPLUG in
> there. Or we should make CPU_HOTPLUG depend on GENERIC_ARCH or auto select
> GENERIC_ARCH with hotplug at the CONFIG level.

Why? I have exactly 4 HT CPUs (2 cores), no more. I use CPU hotplug so I
can disable or enable any of them when I want to. So, this is a classic
SMP system and 2.6.15 is totally happy with this. Or is there any other
(better?) way to disable/enable CPU (especially second logical CPU from
HT) on running systems?

Best regards,

Krzysztof Ol?dzki

2006-03-12 22:33:20

[permalink] [raw]

Subject: Re: More than 8 CPUs detected and CONFIG_X86_PC cannot handle it on 2.6.16-rc6

Krzysztof Oledzki <[email protected]> wrote:
>
> > Actually, this warning should be seen on many other systems on well. We
> > use the bigsmp when there _or_ more than 8 CPUs or CPU_HOTPLUG is used.
> > So, in that sense the message is wrong, it should also have CPU_HOTPLUG in
> > there. Or we should make CPU_HOTPLUG depend on GENERIC_ARCH or auto select
> > GENERIC_ARCH with hotplug at the CONFIG level.
>
> Why? I have exactly 4 HT CPUs (2 cores), no more. I use CPU hotplug so I
> can disable or enable any of them when I want to. So, this is a classic
> SMP system and 2.6.15 is totally happy with this. Or is there any other
> (better?) way to disable/enable CPU (especially second logical CPU from
> HT) on running systems?

Maybe we should have:

if (num_possible_cpus() <= 8)
dont_do_any_of_that_stuff();

That's assuming that hotplug-cpu-capable platforms are correctly setting
cpu_possible_map. Do they?

2006-03-13 19:36:38

[permalink] [raw]

Subject: Re: More than 8 CPUs detected and CONFIG_X86_PC cannot handle it on 2.6.16-rc6

On Sun, Mar 12, 2006 at 02:30:53PM -0800, Andrew Morton wrote:
>
> Maybe we should have:
>
> if (num_possible_cpus() <= 8)
> dont_do_any_of_that_stuff();
>
> That's assuming that hotplug-cpu-capable platforms are correctly setting
> cpu_possible_map. Do they?

That wont work, since we use HOTPLUG_CPU to suspend/resume as well. We
switched to using bigsmp (that uses physflat for IPI's) just to avoid
sending IPI's to offline CPUs. When we use logical flat we use shortcuts
that have ill effects on CPUs that are offline.

Think making CONFIG_HOTPLUG_CPU depend on X86_GENERICARCH, or X86_BIGSMP
seems like a better choice.

--
Cheers,
Ashok Raj
- Open Source Technology Center

When CONFIG_HOTPLUG_CPU is turned on we always use physflat mode (bigsmp) even
when #of CPUs are less than 8 to avoid sending IPI to offline processors.

Without having BIGSMP on it spits out a warning during boot on systems that
seems misleading, since it complains even on systems that have less
than 8 cpus.

Signed-off-by: Ashok Raj <[email protected]>
---------------------------------------------------------

arch/i386/Kconfig | 2 +-
1 files changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6.16-rc6-mm1/arch/i386/Kconfig
===================================================================
--- linux-2.6.16-rc6-mm1.orig/arch/i386/Kconfig
+++ linux-2.6.16-rc6-mm1/arch/i386/Kconfig
@@ -760,7 +760,7 @@ config PHYSICAL_START

config HOTPLUG_CPU
bool "Support for hot-pluggable CPUs (EXPERIMENTAL)"
- depends on SMP && HOTPLUG && EXPERIMENTAL && !X86_VOYAGER
+ depends on SMP && HOTPLUG && EXPERIMENTAL && !X86_VOYAGER && (X86_GENERICARCH || X86_BIGSMP)
---help---
Say Y here to experiment with turning CPUs off and on. CPUs
can be controlled through /sys/devices/system/cpu.

2006-03-13 19:54:39

[permalink] [raw]

Subject: Re: More than 8 CPUs detected and CONFIG_X86_PC cannot handle it on 2.6.16-rc6

Ashok Raj <[email protected]> wrote:
>
> When CONFIG_HOTPLUG_CPU is turned on we always use physflat mode (bigsmp) even
> when #of CPUs are less than 8 to avoid sending IPI to offline processors.
>
> Without having BIGSMP on it spits out a warning during boot on systems that
> seems misleading, since it complains even on systems that have less
> than 8 cpus.
>
> ...
>
> --- linux-2.6.16-rc6-mm1.orig/arch/i386/Kconfig
> +++ linux-2.6.16-rc6-mm1/arch/i386/Kconfig
> @@ -760,7 +760,7 @@ config PHYSICAL_START
>
> config HOTPLUG_CPU
> bool "Support for hot-pluggable CPUs (EXPERIMENTAL)"
> - depends on SMP && HOTPLUG && EXPERIMENTAL && !X86_VOYAGER
> + depends on SMP && HOTPLUG && EXPERIMENTAL && !X86_VOYAGER && (X86_GENERICARCH || X86_BIGSMP)
> ---help---
> Say Y here to experiment with turning CPUs off and on. CPUs
> can be controlled through /sys/devices/system/cpu.

One of the main reasons for turning on CONFIG_HOTPLUG_CPU on x86 is
actually for suspend-to-disk on SMP. I don't think it's desirable to force
all those little machines to use X86_GENERICARCH || X86_BIGSMP. And it'd
be good to make that warning go away for 2.6.16.

2006-03-13 20:06:45

[permalink] [raw]

Subject: Re: More than 8 CPUs detected and CONFIG_X86_PC cannot handle it on 2.6.16-rc6

On Mon, Mar 13, 2006 at 11:51:55AM -0800, Andrew Morton wrote:
>
> One of the main reasons for turning on CONFIG_HOTPLUG_CPU on x86 is
> actually for suspend-to-disk on SMP. I don't think it's desirable to force
> all those little machines to use X86_GENERICARCH || X86_BIGSMP. And it'd
> be good to make that warning go away for 2.6.16.

But we cant use X86_PC since it uses logical flat mode for IPI's that could
cause hangup's if we deliver IPI's using IPI broadcast shortcut.

In i386 we do have an alternate that would use mask value to deliver IPI's
but Andi's recommendataion was to use flat physical mode just like what we
do for X86_64.

Other than the IPI mode, are there any other things that hurt small systems
by choosing bigsmp mode?

Venki suggested we could make it !X86_PC instead of listing
GENERICARCH or BIGSMP separately.

--
Cheers,
Ashok Raj
- Open Source Technology Center

When CONFIG_HOTPLUG_CPU is turned on we always use physflat mode (bigsmp) even
when #of CPUs are less than 8 to avoid sending IPI to offline processors.

Without having BIGSMP on it spits out a warning during boot on systems that
seems misleading, since it complains even on systems that have less
than 8 cpus.

Signed-off-by: Ashok Raj <[email protected]>
---------------------------------------------------------

arch/i386/Kconfig | 2 +-
1 files changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6.16-rc6-mm1/arch/i386/Kconfig
===================================================================
--- linux-2.6.16-rc6-mm1.orig/arch/i386/Kconfig
+++ linux-2.6.16-rc6-mm1/arch/i386/Kconfig
@@ -760,7 +760,7 @@ config PHYSICAL_START

config HOTPLUG_CPU
bool "Support for hot-pluggable CPUs (EXPERIMENTAL)"
- depends on SMP && HOTPLUG && EXPERIMENTAL && !X86_VOYAGER
+ depends on SMP && HOTPLUG && EXPERIMENTAL && !X86_VOYAGER && !X86_PC
---help---
Say Y here to experiment with turning CPUs off and on. CPUs
can be controlled through /sys/devices/system/cpu.

2006-03-13 22:25:00

[permalink] [raw]

Subject: Re: More than 8 CPUs detected and CONFIG_X86_PC cannot handle it on 2.6.16-rc6

Ashok Raj <[email protected]> wrote:
>
>
>
> When CONFIG_HOTPLUG_CPU is turned on we always use physflat mode (bigsmp) even
> when #of CPUs are less than 8 to avoid sending IPI to offline processors.
>
> Without having BIGSMP on it spits out a warning during boot on systems that
> seems misleading, since it complains even on systems that have less
> than 8 cpus.
>
> Signed-off-by: Ashok Raj <[email protected]>
> ---------------------------------------------------------
>
> arch/i386/Kconfig | 2 +-
> 1 files changed, 1 insertion(+), 1 deletion(-)
>
> Index: linux-2.6.16-rc6-mm1/arch/i386/Kconfig
> ===================================================================
> --- linux-2.6.16-rc6-mm1.orig/arch/i386/Kconfig
> +++ linux-2.6.16-rc6-mm1/arch/i386/Kconfig
> @@ -760,7 +760,7 @@ config PHYSICAL_START
>
> config HOTPLUG_CPU
> bool "Support for hot-pluggable CPUs (EXPERIMENTAL)"
> - depends on SMP && HOTPLUG && EXPERIMENTAL && !X86_VOYAGER
> + depends on SMP && HOTPLUG && EXPERIMENTAL && !X86_VOYAGER && !X86_PC
> ---help---
> Say Y here to experiment with turning CPUs off and on. CPUs
> can be controlled through /sys/devices/system/cpu.

Still seems wrong. People _do_ use HOTPLUG_CPU on X86_PCs so they can get
software suspend. The number of people who do this are probably 100000x
the number of people who have physically hotpluggable CPUs. And I don't
think we can churn their config requirements this much so late in the game.

So for now I suggest we're best off simply killing the printk (or doing
something smarter, like comparing cpu_online-map with cpu_possible_map
(which isn't right)).

Longer term, it appears that we need to do some Kconfig and C work to
separate out the HOTPLUG_CPU infrastructure which swsusp needs from actual
CPU hotplugging.

What _is_ this IPI problem anyway? Can't send point-to-point IPIs to
offlined CPUs? (Don't do that then?) Or do broadcast IPIs go wrong, or
what?

And does it affect pretend-x86-hotplug, or is it only affecting real hotplug?

Thanks.

2006-03-13 23:05:30

[permalink] [raw]

Subject: Re: More than 8 CPUs detected and CONFIG_X86_PC cannot handle it on 2.6.16-rc6

On Mon, Mar 13, 2006 at 02:22:23PM -0800, Andrew Morton wrote:
> > config HOTPLUG_CPU
> > bool "Support for hot-pluggable CPUs (EXPERIMENTAL)"
> > - depends on SMP && HOTPLUG && EXPERIMENTAL && !X86_VOYAGER
> > + depends on SMP && HOTPLUG && EXPERIMENTAL && !X86_VOYAGER && !X86_PC
> > ---help---
> > Say Y here to experiment with turning CPUs off and on. CPUs
> > can be controlled through /sys/devices/system/cpu.
>
> Longer term, it appears that we need to do some Kconfig and C work to
> separate out the HOTPLUG_CPU infrastructure which swsusp needs from actual
> CPU hotplugging.

The needs are not any different. Both (swsusp and cpu hotplug) both require
logical cpu offlining which is what CONFIG_HOTPLUG_CPU does.

Physical cpu hotplug is enabled by CONFIG_ACPI_HOTPLUG_CPU.

>
> What _is_ this IPI problem anyway? Can't send point-to-point IPIs to
> offlined CPUs? (Don't do that then?) Or do broadcast IPIs go wrong, or
> what?

Its not the point-to-point..we do that only to wake a CPU, but thats done
in flat physical mode always.

When we do smp_call_function() under X86_PC we use logical flat mode.
This sends a broadcast IPI by using a shortcut message. This is bad, since
the offline cpu may also receive it and process just when we bring the cpu
online.

send_IPI_allbutself() and send_IPI_all() versions that use the shortcut
values are the ones to avoid.

>
> And does it affect pretend-x86-hotplug, or is it only affecting real hotplug?
>
its no more pretend-x86, in the past we used to put the cpu in idle(),
now we do put the cpu in halt and bring back by another startup ipi, just like
boot sequence, both for x86 and x86_64.

--
Cheers,
Ashok Raj
- Open Source Technology Center

2006-03-15 05:41:38

by Nathan Lynch

[permalink] [raw]

Subject: Re: More than 8 CPUs detected and CONFIG_X86_PC cannot handle it on 2.6.16-rc6

Ashok Raj wrote:
> On Mon, Mar 13, 2006 at 02:22:23PM -0800, Andrew Morton wrote:
> >
> > And does it affect pretend-x86-hotplug, or is it only affecting real hotplug?
> >
> its no more pretend-x86, in the past we used to put the cpu in idle(),
> now we do put the cpu in halt and bring back by another startup ipi, just like
> boot sequence, both for x86 and x86_64.

That's actually broken since 2.6.14 (at least on my P3 box); please
see:

Subject: i386 cpu hotplug bug - instant reboot when onlining secondary

http://lkml.org/lkml/2006/2/19/186

2006-03-15 06:20:15

by Shaohua Li

[permalink] [raw]

Subject: Re: More than 8 CPUs detected and CONFIG_X86_PC cannot handle it on 2.6.16-rc6

On Wed, 2006-03-15 at 13:44 +0800, Nathan Lynch wrote:
> Ashok Raj wrote:
> > On Mon, Mar 13, 2006 at 02:22:23PM -0800, Andrew Morton wrote:
> > >
> > > And does it affect pretend-x86-hotplug, or is it only affecting
> real hotplug?
> > >
> > its no more pretend-x86, in the past we used to put the cpu in
> idle(),
> > now we do put the cpu in halt and bring back by another startup ipi,
> just like
> > boot sequence, both for x86 and x86_64.
>
> That's actually broken since 2.6.14 (at least on my P3 box); please
> see:
>
> Subject: i386 cpu hotplug bug - instant reboot when onlining secondary
>
> http://lkml.org/lkml/2006/2/19/186
Works for me. But I saw a warning.

---

linux-2.6.15-root/arch/i386/kernel/cpu/common.c | 2 +-
1 files changed, 1 insertion(+), 1 deletion(-)

diff -puN arch/i386/kernel/cpu/common.c~cpuhp arch/i386/kernel/cpu/common.c
--- linux-2.6.15/arch/i386/kernel/cpu/common.c~cpuhp 2006-03-14 12:13:43.000000000 +0800
+++ linux-2.6.15-root/arch/i386/kernel/cpu/common.c 2006-03-14 12:14:12.000000000 +0800
@@ -605,7 +605,7 @@ void __devinit cpu_init(void)
/* alloc_bootmem_pages panics on failure, so no check */
memset(gdt, 0, PAGE_SIZE);
} else {
- gdt = (struct desc_struct *)get_zeroed_page(GFP_KERNEL);
+ gdt = (struct desc_struct *)get_zeroed_page(GFP_ATOMIC);
if (unlikely(!gdt)) {
printk(KERN_CRIT "CPU%d failed to allocate GDT\n", cpu);
for (;;)
_

2006-03-15 07:34:30

[permalink] [raw]

Subject: Re: More than 8 CPUs detected and CONFIG_X86_PC cannot handle it on 2.6.16-rc6

Shaohua Li <[email protected]> wrote:
>
> On Wed, 2006-03-15 at 13:44 +0800, Nathan Lynch wrote:
> > Ashok Raj wrote:
> > > On Mon, Mar 13, 2006 at 02:22:23PM -0800, Andrew Morton wrote:
> > > >
> > > > And does it affect pretend-x86-hotplug, or is it only affecting
> > real hotplug?
> > > >
> > > its no more pretend-x86, in the past we used to put the cpu in
> > idle(),
> > > now we do put the cpu in halt and bring back by another startup ipi,
> > just like
> > > boot sequence, both for x86 and x86_64.
> >
> > That's actually broken since 2.6.14 (at least on my P3 box); please
> > see:
> >
> > Subject: i386 cpu hotplug bug - instant reboot when onlining secondary
> >
> > http://lkml.org/lkml/2006/2/19/186
> Works for me. But I saw a warning.

Guys, will you please stop being so cryptic? What worked for you? What
warning? wtf is going on? Who owns this problem, whatever it is?
<head spins>

> linux-2.6.15-root/arch/i386/kernel/cpu/common.c | 2 +-
> 1 files changed, 1 insertion(+), 1 deletion(-)
>
> diff -puN arch/i386/kernel/cpu/common.c~cpuhp arch/i386/kernel/cpu/common.c
> --- linux-2.6.15/arch/i386/kernel/cpu/common.c~cpuhp 2006-03-14 12:13:43.000000000 +0800
> +++ linux-2.6.15-root/arch/i386/kernel/cpu/common.c 2006-03-14 12:14:12.000000000 +0800
> @@ -605,7 +605,7 @@ void __devinit cpu_init(void)
> /* alloc_bootmem_pages panics on failure, so no check */
> memset(gdt, 0, PAGE_SIZE);
> } else {
> - gdt = (struct desc_struct *)get_zeroed_page(GFP_KERNEL);
> + gdt = (struct desc_struct *)get_zeroed_page(GFP_ATOMIC);

That would be rather a sad thing to have to do. OK if it's during initial
bootup, less OK if it's during CPU hot-add.

2006-03-15 09:37:24

[permalink] [raw]

Subject: [PATCH] No need to protect current->group_info in sys_getgroups(), in_group_p() and in_egroup_p()

--- a/kernel/sys.c 2006-03-15 10:14:37.000000000 +0100
+++ b/kernel/sys.c 2006-03-15 10:15:55.000000000 +0100
@@ -1433,7 +1433,6 @@
return -EINVAL;

/* no need to grab task_lock here; it cannot change */
- get_group_info(current->group_info);
i = current->group_info->ngroups;
if (gidsetsize) {
if (i > gidsetsize) {
@@ -1446,7 +1445,6 @@
}
}
out:
- put_group_info(current->group_info);
return i;
}

@@ -1487,9 +1485,7 @@
{
int retval = 1;
if (grp != current->fsgid) {
- get_group_info(current->group_info);
retval = groups_search(current->group_info, grp);
- put_group_info(current->group_info);
}
return retval;
}
@@ -1500,9 +1496,7 @@
{
int retval = 1;
if (grp != current->egid) {
- get_group_info(current->group_info);
retval = groups_search(current->group_info, grp);
- put_group_info(current->group_info);
}
return retval;
}

Attachments:

kernel_sys.patch (884.00 B)

2006-03-15 10:57:02

by Chuck Ebbert

[permalink] [raw]

Subject: Re: More than 8 CPUs detected and CONFIG_X86_PC cannot handle it on 2.6.16-rc6

In-Reply-To: <[email protected]>

On Sun, 12 Mar 2006 03:04:40 +0100, Krzysztof Oledzki wrote:

> After upgrading to 2.6.16-rc6 I noticed this strange message:
>
> More than 8 CPUs detected and CONFIG_X86_PC cannot handle it.
> Use CONFIG_X86_GENERICARCH or CONFIG_X86_BIGSMP.
>
> This is a Dell PowerEdge SC1425 with two P4 Xeons with HT enabled (so with
> totoal of 4 logical CPUs).

In a later message, you wrote:

> ACPI: LAPIC (acpi_id[0x01] lapic_id[0x00] enabled)
> Processor #0 15:4 APIC version 20
> ACPI: LAPIC (acpi_id[0x02] lapic_id[0x06] enabled)
> Processor #6 15:4 APIC version 20
^
> ACPI: LAPIC (acpi_id[0x03] lapic_id[0x01] enabled)
> Processor #1 15:4 APIC version 20
> ACPI: LAPIC (acpi_id[0x04] lapic_id[0x07] enabled)
> Processor #7 15:4 APIC version 20
^

What processor numbers did you get on 2.6.15.x?
Does /proc/cpuinfo show all four CPUs?
If you start four CPU-hungry processes, do all four show 100% utilization in top(1)?

--
Chuck
"Penguins don't come from next door, they come from the Antarctic!"

2006-03-15 15:44:22

[permalink] [raw]

Subject: Re: More than 8 CPUs detected and CONFIG_X86_PC cannot handle it on 2.6.16-rc6

On Wed, 15 Mar 2006, Chuck Ebbert wrote:

> In-Reply-To: <[email protected]>
>
> On Sun, 12 Mar 2006 03:04:40 +0100, Krzysztof Oledzki wrote:
>
>> After upgrading to 2.6.16-rc6 I noticed this strange message:
>>
>> More than 8 CPUs detected and CONFIG_X86_PC cannot handle it.
>> Use CONFIG_X86_GENERICARCH or CONFIG_X86_BIGSMP.
>>
>> This is a Dell PowerEdge SC1425 with two P4 Xeons with HT enabled (so with
>> totoal of 4 logical CPUs).
>
> In a later message, you wrote:
>
>> ACPI: LAPIC (acpi_id[0x01] lapic_id[0x00] enabled)
>> Processor #0 15:4 APIC version 20
>> ACPI: LAPIC (acpi_id[0x02] lapic_id[0x06] enabled)
>> Processor #6 15:4 APIC version 20
> ^
>> ACPI: LAPIC (acpi_id[0x03] lapic_id[0x01] enabled)
>> Processor #1 15:4 APIC version 20
>> ACPI: LAPIC (acpi_id[0x04] lapic_id[0x07] enabled)
>> Processor #7 15:4 APIC version 20
> ^
>
> What processor numbers did you get on 2.6.15.x?

Mar 6 00:08:03 fw2 kernel: Processor #0 15:4 APIC version 20
Mar 6 00:08:03 fw2 kernel: Processor #6 15:4 APIC version 20
Mar 6 00:08:03 fw2 kernel: Processor #1 15:4 APIC version 20
Mar 6 00:08:03 fw2 kernel: Processor #7 15:4 APIC version 20

> Does /proc/cpuinfo show all four CPUs?
Yes.

# cat /proc/cpuinfo
processor : 0
vendor_id : GenuineIntel
cpu family : 15
model : 4
model name : Intel(R) Xeon(TM) CPU 3.20GHz
stepping : 3
cpu MHz : 3200.000
cache size : 2048 KB
physical id : 0
siblings : 2
core id : 0
cpu cores : 1
fdiv_bug : no
hlt_bug : no
f00f_bug : no
coma_bug : no
fpu : yes
fpu_exception : yes
cpuid level : 5
wp : yes
flags : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe nx lm constant_tsc pni monitor ds_cpl cid cx16 xtpr
bogomips : 6404.87

processor : 1
vendor_id : GenuineIntel
cpu family : 15
model : 4
model name : Intel(R) Xeon(TM) CPU 3.20GHz
stepping : 3
cpu MHz : 3200.000
cache size : 2048 KB
physical id : 0
siblings : 2
core id : 0
cpu cores : 1
fdiv_bug : no
hlt_bug : no
f00f_bug : no
coma_bug : no
fpu : yes
fpu_exception : yes
cpuid level : 5
wp : yes
flags : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe nx lm constant_tsc pni monitor ds_cpl cid cx16 xtpr
bogomips : 6400.28

processor : 2
vendor_id : GenuineIntel
cpu family : 15
model : 4
model name : Intel(R) Xeon(TM) CPU 3.20GHz
stepping : 3
cpu MHz : 2800.000
cache size : 2048 KB
physical id : 3
siblings : 2
core id : 0
cpu cores : 1
fdiv_bug : no
hlt_bug : no
f00f_bug : no
coma_bug : no
fpu : yes
fpu_exception : yes
cpuid level : 5
wp : yes
flags : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe nx lm constant_tsc pni monitor ds_cpl cid cx16 xtpr
bogomips : 6400.31

processor : 3
vendor_id : GenuineIntel
cpu family : 15
model : 4
model name : Intel(R) Xeon(TM) CPU 3.20GHz
stepping : 3
cpu MHz : 2800.000
cache size : 2048 KB
physical id : 3
siblings : 2
core id : 0
cpu cores : 1
fdiv_bug : no
hlt_bug : no
f00f_bug : no
coma_bug : no
fpu : yes
fpu_exception : yes
cpuid level : 5
wp : yes
flags : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe nx lm constant_tsc pni monitor ds_cpl cid cx16 xtpr
bogomips : 6400.32

> If you start four CPU-hungry processes, do all four show 100% utilization in top(1)?

# cd /usr/src/linux
# /usr/bin/time make -j10
(...)

Cpu0 : 94.4% us, 5.3% sy, 0.0% ni, 0.3% id, 0.0% wa, 0.0% hi, 0.0% si
Cpu1 : 92.4% us, 5.3% sy, 0.0% ni, 2.3% id, 0.0% wa, 0.0% hi, 0.0% si
Cpu2 : 91.7% us, 6.6% sy, 0.0% ni, 0.3% id, 1.3% wa, 0.0% hi, 0.0% si
Cpu3 : 94.7% us, 5.3% sy, 0.0% ni, 0.0% id, 0.0% wa, 0.0% hi, 0.0% si

Best regards,

Krzysztof Ol?dzki

2006-03-15 18:11:26

[permalink] [raw]

Subject: Re: More than 8 CPUs detected and CONFIG_X86_PC cannot handle it on 2.6.16-rc6

On Tue, Mar 14, 2006 at 11:31:38PM -0800, Andrew Morton wrote:
> > > see:
> > >
> > > Subject: i386 cpu hotplug bug - instant reboot when onlining secondary
> > >
> > > http://lkml.org/lkml/2006/2/19/186
> > Works for me. But I saw a warning.
>
> Guys, will you please stop being so cryptic? What worked for you? What
> warning? wtf is going on? Who owns this problem, whatever it is?

Nathan's problem is different, its nothing related to this thread.

Appears that a PIII box had trouble to bring a CPU back online after it was
just offlined. Iam not able to reproduce it with the systems i have here.
I have tried a PIII box itself, and also a x86_64 system booting a i386 kernel
and all seems to work ok.

Zwane was attempting to trace Nathan's issue with some experimental patches
but dont think it went far along yet.

> > - gdt = (struct desc_struct *)get_zeroed_page(GFP_KERNEL);
> > + gdt = (struct desc_struct *)get_zeroed_page(GFP_ATOMIC);
>

It might help to post with the actual warning, so we can understand why this
fix is necessary.

--
Cheers,
Ashok Raj
- Open Source Technology Center

2006-03-20 19:10:10

[permalink] [raw]

Subject: [PATCH] Use unsigned int types for a faster bsearch

--- a/kernel/sys.c 2006-03-20 18:42:41.000000000 +0100
+++ b/kernel/sys.c 2006-03-20 19:00:43.000000000 +0100
@@ -1375,7 +1375,7 @@
/* a simple bsearch */
int groups_search(struct group_info *group_info, gid_t grp)
{
- int left, right;
+ unsigned int left, right;

if (!group_info)
return 0;
@@ -1383,7 +1383,7 @@
left = 0;
right = group_info->ngroups;
while (left < right) {
- int mid = (left+right)/2;
+ unsigned int mid = (left+right)/2;
int cmp = grp - GROUP_AT(group_info, mid);
if (cmp > 0)
left = mid + 1;

Attachments:

groups_search.patch (540.00 B)

2006-03-22 05:06:33

[permalink] [raw]

Subject: [PATCH] Use __read_mostly on some hot fs variables

--- a/fs/dcache.c 2006-03-21 13:48:13.000000000 +0100
+++ b/fs/dcache.c 2006-03-21 13:55:00.000000000 +0100
@@ -36,7 +36,7 @@

/* #define DCACHE_DEBUG 1 */

-int sysctl_vfs_cache_pressure = 100;
+int sysctl_vfs_cache_pressure __read_mostly = 100;
EXPORT_SYMBOL_GPL(sysctl_vfs_cache_pressure);

__cacheline_aligned_in_smp DEFINE_SPINLOCK(dcache_lock);
@@ -44,7 +44,7 @@

EXPORT_SYMBOL(dcache_lock);

-static kmem_cache_t *dentry_cache;
+static kmem_cache_t *dentry_cache __read_mostly;

#define DNAME_INLINE_LEN (sizeof(struct dentry)-offsetof(struct dentry,d_iname))

@@ -59,9 +59,9 @@
#define D_HASHBITS d_hash_shift
#define D_HASHMASK d_hash_mask

-static unsigned int d_hash_mask;
-static unsigned int d_hash_shift;
-static struct hlist_head *dentry_hashtable;
+static unsigned int d_hash_mask __read_mostly;
+static unsigned int d_hash_shift __read_mostly;
+static struct hlist_head *dentry_hashtable __read_mostly;
static LIST_HEAD(dentry_unused);

/* Statistics gathering. */
@@ -1706,10 +1706,10 @@
}

/* SLAB cache for __getname() consumers */
-kmem_cache_t *names_cachep;
+kmem_cache_t *names_cachep __read_mostly;

/* SLAB cache for file structures */
-kmem_cache_t *filp_cachep;
+kmem_cache_t *filp_cachep __read_mostly;

EXPORT_SYMBOL(d_genocide);

--- a/fs/inode.c 2006-03-21 13:50:19.000000000 +0100
+++ b/fs/inode.c 2006-03-21 13:54:39.000000000 +0100
@@ -56,8 +56,8 @@
#define I_HASHBITS i_hash_shift
#define I_HASHMASK i_hash_mask

-static unsigned int i_hash_mask;
-static unsigned int i_hash_shift;
+static unsigned int i_hash_mask __read_mostly;
+static unsigned int i_hash_shift __read_mostly;

/*
* Each inode can be on two separate lists. One is
@@ -73,7 +73,7 @@

LIST_HEAD(inode_in_use);
LIST_HEAD(inode_unused);
-static struct hlist_head *inode_hashtable;
+static struct hlist_head *inode_hashtable __read_mostly;

/*
* A simple spinlock to protect the list manipulations.
@@ -98,7 +98,7 @@
*/
struct inodes_stat_t inodes_stat;

-static kmem_cache_t * inode_cachep;
+static kmem_cache_t * inode_cachep __read_mostly;

static struct inode *alloc_inode(struct super_block *sb)
{
--- a/fs/namespace.c 2006-03-22 05:20:33.000000000 +0100
+++ b/fs/namespace.c 2006-03-22 05:23:40.000000000 +0100
@@ -43,9 +43,9 @@

static int event;

-static struct list_head *mount_hashtable;
+static struct list_head *mount_hashtable __read_mostly;
static int hash_mask __read_mostly, hash_bits __read_mostly;
-static kmem_cache_t *mnt_cache;
+static kmem_cache_t *mnt_cache __read_mostly;
static struct rw_semaphore namespace_sem;

/* /sys/fs */
--- a/fs/dcookies.c 2006-03-22 05:35:46.000000000 +0100
+++ b/fs/dcookies.c 2006-03-22 05:36:55.000000000 +0100
@@ -37,9 +37,9 @@

static LIST_HEAD(dcookie_users);
static DECLARE_MUTEX(dcookie_sem);
-static kmem_cache_t * dcookie_cache;
-static struct list_head * dcookie_hashtable;
-static size_t hash_size;
+static kmem_cache_t * dcookie_cache __read_mostly;
+static struct list_head * dcookie_hashtable __read_mostly;
+static size_t hash_size __read_mostly;

static inline int is_live(void)
{
--- a/fs/fcntl.c 2006-03-22 05:37:40.000000000 +0100
+++ b/fs/fcntl.c 2006-03-22 05:43:49.000000000 +0100
@@ -413,7 +413,7 @@

/* Table to convert sigio signal codes into poll band bitmaps */

-static long band_table[NSIGPOLL] = {
+static const long band_table[NSIGPOLL] = {
POLLIN | POLLRDNORM, /* POLL_IN */
POLLOUT | POLLWRNORM | POLLWRBAND, /* POLL_OUT */
POLLIN | POLLRDNORM | POLLMSG, /* POLL_MSG */
@@ -532,7 +532,7 @@
}

static DEFINE_RWLOCK(fasync_lock);
-static kmem_cache_t *fasync_cache;
+static kmem_cache_t *fasync_cache __read_mostly;

/*
* fasync_helper() is used by some character device drivers (mainly mice)
--- a/fs/eventpoll.c 2006-03-22 05:39:06.000000000 +0100
+++ b/fs/eventpoll.c 2006-03-22 05:43:49.000000000 +0100
@@ -280,13 +280,13 @@
static struct poll_safewake psw;

/* Slab cache used to allocate "struct epitem" */
-static kmem_cache_t *epi_cache;
+static kmem_cache_t *epi_cache __read_mostly;

/* Slab cache used to allocate "struct eppoll_entry" */
-static kmem_cache_t *pwq_cache;
+static kmem_cache_t *pwq_cache __read_mostly;

/* Virtual fs used to allocate inodes for eventpoll files */
-static struct vfsmount *eventpoll_mnt;
+static struct vfsmount *eventpoll_mnt __read_mostly;

/* File callbacks that implement the eventpoll file behaviour */
static struct file_operations eventpoll_fops = {
--- a/fs/inotify.c 2006-03-22 05:40:44.000000000 +0100
+++ b/fs/inotify.c 2006-03-22 05:43:49.000000000 +0100
@@ -40,15 +40,15 @@
static atomic_t inotify_cookie;
static atomic_t inotify_watches;

-static kmem_cache_t *watch_cachep;
-static kmem_cache_t *event_cachep;
+static kmem_cache_t *watch_cachep __read_mostly;
+static kmem_cache_t *event_cachep __read_mostly;

-static struct vfsmount *inotify_mnt;
+static struct vfsmount *inotify_mnt __read_mostly;

/* these are configurable via /proc/sys/fs/inotify/ */
-int inotify_max_user_instances;
-int inotify_max_user_watches;
-int inotify_max_queued_events;
+int inotify_max_user_instances __read_mostly;
+int inotify_max_user_watches __read_mostly;
+int inotify_max_queued_events __read_mostly;

/*
* Lock ordering:
--- a/fs/locks.c 2006-03-22 05:43:34.000000000 +0100
+++ b/fs/locks.c 2006-03-22 05:43:49.000000000 +0100
@@ -145,7 +145,7 @@

static LIST_HEAD(blocked_list);

-static kmem_cache_t *filelock_cache;
+static kmem_cache_t *filelock_cache __read_mostly;

/* Allocate an empty lock structure. */
static struct file_lock *locks_alloc_lock(void)
--- a/fs/bio.c 2006-03-22 05:44:20.000000000 +0100
+++ b/fs/bio.c 2006-03-22 05:50:37.000000000 +0100
@@ -29,7 +29,7 @@

#define BIO_POOL_SIZE 256

-static kmem_cache_t *bio_slab;
+static kmem_cache_t *bio_slab __read_mostly;

#define BIOVEC_NR_POOLS 6

@@ -38,7 +38,7 @@
* basically we just need to survive
*/
#define BIO_SPLIT_ENTRIES 8
-mempool_t *bio_split_pool;
+mempool_t *bio_split_pool __read_mostly;

struct biovec_slab {
int nr_vecs;
--- a/fs/dnotify.c 2006-03-22 05:46:33.000000000 +0100
+++ b/fs/dnotify.c 2006-03-22 05:50:37.000000000 +0100
@@ -21,9 +21,9 @@
#include <linux/spinlock.h>
#include <linux/slab.h>

-int dir_notify_enable = 1;
+int dir_notify_enable __read_mostly = 1;

-static kmem_cache_t *dn_cache;
+static kmem_cache_t *dn_cache __read_mostly;

static void redo_inode_mask(struct inode *inode)
{
--- a/fs/block_dev.c 2006-03-22 05:48:29.000000000 +0100
+++ b/fs/block_dev.c 2006-03-22 05:50:37.000000000 +0100
@@ -238,7 +238,7 @@
*/

static __cacheline_aligned_in_smp DEFINE_SPINLOCK(bdev_lock);
-static kmem_cache_t * bdev_cachep;
+static kmem_cache_t * bdev_cachep __read_mostly;

static struct inode *bdev_alloc_inode(struct super_block *sb)
{
@@ -312,7 +312,7 @@
.kill_sb = kill_anon_super,
};

-static struct vfsmount *bd_mnt;
+static struct vfsmount *bd_mnt __read_mostly;
struct super_block *blockdev_superblock;

void __init bdev_cache_init(void)
--- a/fs/pipe.c 2006-03-22 05:51:16.000000000 +0100
+++ b/fs/pipe.c 2006-03-22 05:54:11.000000000 +0100
@@ -676,7 +676,7 @@
return NULL;
}

-static struct vfsmount *pipe_mnt;
+static struct vfsmount *pipe_mnt __read_mostly;
static int pipefs_delete_dentry(struct dentry *dentry)
{
return 1;

Attachments:

fs_readmostly.patch (7.16 kB)

2006-03-22 05:15:50

by Nick Piggin

[permalink] [raw]

Subject: Re: [PATCH] Use __read_mostly on some hot fs variables

Eric Dumazet wrote:

>
> Before someone asks, it is valid to declare a pointer as 'read
> mostly', even if the data pointed by the pointer is heavily modified.
> hash table pointers and kmem_cache pointers are setup at boot time, so
> they are perfect candidates to 'read_mostly' section. Same apply for
> 'struct vfsmount *'
>

Yes... why wouldn't it be if the variable is only written to once? This
is _exactly_ what __read_mostly section is for, isn't it?

Patch looks good though.

Nick
---
Send instant messages to your online friends http://au.messenger.yahoo.com

2006-03-22 06:23:39

[permalink] [raw]

Subject: [RFC, PATCH] avoid some atomics in open()/close() for monothreaded processes

--- a/include/linux/file.h 2006-03-22 06:23:02.000000000 +0100
+++ b/include/linux/file.h 2006-03-22 07:11:08.000000000 +0100
@@ -42,6 +42,11 @@
spinlock_t file_lock; /* Protects concurrent writers. Nests inside tsk->alloc_lock */
};

+static inline int files_multithreaded(const struct files_struct *files)
+{
+ return sizeof(files->file_lock) > 0 && atomic_read(&files->count) > 1;
+}
+
#define files_fdtable(files) (rcu_dereference((files)->fdt))

extern void FASTCALL(__fput(struct file *));
--- a/fs/open.c.orig 2006-03-22 06:24:34.000000000 +0100
+++ b/fs/open.c 2006-03-22 06:30:54.000000000 +0100
@@ -1050,11 +1050,17 @@
{
struct files_struct *files = current->files;
struct fdtable *fdt;
- spin_lock(&files->file_lock);
+ int fl_locked = 0;
+
+ if (files_multithreaded(files)) {
+ spin_lock(&files->file_lock);
+ fl_locked = 1;
+ }
fdt = files_fdtable(files);
BUG_ON(fdt->fd[fd] != NULL);
rcu_assign_pointer(fdt->fd[fd], file);
- spin_unlock(&files->file_lock);
+ if (fl_locked)
+ spin_unlock(&files->file_lock);
}

EXPORT_SYMBOL(fd_install);
@@ -1147,8 +1153,12 @@
struct file * filp;
struct files_struct *files = current->files;
struct fdtable *fdt;
+ int fl_locked = 0;

- spin_lock(&files->file_lock);
+ if (files_multithreaded(files)) {
+ spin_lock(&files->file_lock);
+ fl_locked = 1;
+ }
fdt = files_fdtable(files);
if (fd >= fdt->max_fds)
goto out_unlock;
@@ -1158,11 +1168,13 @@
rcu_assign_pointer(fdt->fd[fd], NULL);
FD_CLR(fd, fdt->close_on_exec);
__put_unused_fd(files, fd);
- spin_unlock(&files->file_lock);
+ if (fl_locked)
+ spin_unlock(&files->file_lock);
return filp_close(filp, files);

out_unlock:
- spin_unlock(&files->file_lock);
+ if (fl_locked)
+ spin_unlock(&files->file_lock);
return -EBADF;
}

Attachments:

files_multithreaded.patch (1.75 kB)

2006-03-22 06:46:13

[permalink] [raw]

Subject: Re: [RFC, PATCH] avoid some atomics in open()/close() for monothreaded processes

Eric Dumazet <[email protected]> wrote:
>
> Goal : Avoid some locking/unlocking 'struct files_struct'->file_lock for mono
> threaded processes.
>
> We define files_multithreaded() function .
>
> static inline int files_multithreaded(const struct files_struct *files)
> {
> return sizeof(files->file_lock) > 0 && atomic_read(&files->count) > 1;
> }

That's bascially sizeof(spinlock_t). That's architecture dependent and
varies wildly according to the day of week.

It _might_ work in all situations - probably you checked that. But I still
wouldn't do it because it might break in the future. Let's be explicit and
stick the appropriate ifdefs in there.

I'd also consider renaming it to files_shared() - processes are
multithreaded, not data structures.

Once you're done with that we should change fget_light() and fput_light() to
use this helper. Separate patch.

2006-03-22 06:59:20

[permalink] [raw]

Subject: Re: [RFC, PATCH] avoid some atomics in open()/close() for monothreaded processes

Andrew Morton a ?crit :
> Eric Dumazet <[email protected]> wrote:
>> Goal : Avoid some locking/unlocking 'struct files_struct'->file_lock for mono
>> threaded processes.
>>
>> We define files_multithreaded() function .
>>
>> static inline int files_multithreaded(const struct files_struct *files)
>> {
>> return sizeof(files->file_lock) > 0 && atomic_read(&files->count) > 1;
>> }
>
> That's bascially sizeof(spinlock_t). That's architecture dependent and
> varies wildly according to the day of week.

I used sizeof(files->file_lock) instead of sizeof(spinlock_t) because I found
it more explicit , while not using ugly ifdefs.

>
> It _might_ work in all situations - probably you checked that. But I still
> wouldn't do it because it might break in the future. Let's be explicit and
> stick the appropriate ifdefs in there.
>
> I'd also consider renaming it to files_shared() - processes are
> multithreaded, not data structures.

Thanks for the feedback, I will redo the patch and test it on various
platforms before resubmit (including performance data :) )

>
> Once you're done with that we should change fget_light() and fput_light() to
> use this helper. Separate patch.

Hum... this discussion is not relevant with fget_light() unless I mistaken.

Nowadays, this function doesnt take spinlock thanks to RCU

Eric

2006-03-22 07:07:01