suspend2 with smp

suspend2 with smp

[Pavel Machek]

Hi!

At some point I claimed that SMP support in suspend2 is "probably
broken". I guess I should post more data:

It is broken in theory.

+/*
+ * Save and restore processor state for secondary processors.
+ * IRQs (and therefore preemption) are already disabled
+ * when we enter here (IPI).
+ */
+
+void smp_suspend2_lowlevel(void * info)
+{
+       smp_mb();
+       barrier();
+       if (now_resuming) {
+               __asm__( "movl %%ecx,%%cr3\n" ::"c"(__pa(swsusp_pg_dir)));
+
+               kernel_fpu_begin();
+               atomic_inc(&suspend_cpu_counter);
+               smp_mb();
+               barrier();
#+               while ((software_suspend_state & SOFTWARE_SUSPEND_FREEZE_SMP) ||
#+                       (atomic_read(&suspend_cpu_counter) != smp_processor_id())) {
#+                       cpu_relax();
#+                       smp_mb();
+               }
+               my_saved_context = (unsigned char *) (suspend2_saved_contexts + smp_processor_id());
+               for (loop = sizeof(struct suspend2_saved_context); loop--; loop)
+                       *(((unsigned char *) &suspend2_saved_context) + loop - 1) = *(my_saved_context + loop - 1);
+               restore_processor_context();
+               FLUSH_LOCAL_TLB();
+               atomic_dec(&suspend_cpu_counter);

CPU is basically looping in loop marked by #, while its memory is
being overwriten. Now, the code probably works in practice, but it
should be really written in assembly so that compiler can not do
something stupid.

Compilers are not designed to deal with their stack (etc) randomly
overwritten, so compiler may do anything it wants here. I see that -O0
may help a lot here, but it simply is not the right thing to do.

At least /* FIXME: should be rewritten to assembly */ should be added there.

								Pavel
-- 
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!

Re: suspend2 with smp

[Nigel Cunningham]

Hi.

On Fri, 2004-08-13 at 07:56, Pavel Machek wrote:
> Hi!
> 
> At some point I claimed that SMP support in suspend2 is "probably
> broken". I guess I should post more data:
> 
> It is broken in theory.
> 
> CPU is basically looping in loop marked by #, while its memory is
> being overwriten. Now, the code probably works in practice, but it
> should be really written in assembly so that compiler can not do
> something stupid.
> 
> Compilers are not designed to deal with their stack (etc) randomly
> overwritten, so compiler may do anything it wants here. I see that -O0
> may help a lot here, but it simply is not the right thing to do.
> 
> At least /* FIXME: should be rewritten to assembly */ should be added there.

Ah, okay. So it's not that the code itself broken, but that you don't
trust the assembler to do the right thing with the code. I'll happily
include an inline asm routine if you'll code it for me (I don't know x86
assembly). In case I haven't said it already, feel free to take the
freezer changes and put them in your code. I'd only be submitting a
patch to do the same anyway.

Nigel
-- 
Nigel Cunningham
Christian Reformed Church of Tuggeranong
PO Box 1004, Tuggeranong, ACT 2901

Many today claim to be tolerant. But true tolerance can cope with others
being intolerant.