Re: patch fr1-2.17 does not work with kernel 2.4.30

"Also sprach Stephan von Krawczynski:"
> On Sat, 9 Apr 2005 00:59:12 +0200 (MET DST)
> "Peter T. Breuer" <[email protected]> wrote:
> 
> > "Also sprach ptb:"
> > > "Also sprach ptb:"
> > > I'm afraid I can't see a UML patch for anything more recent than
> > > 2.4.27. I can try that. Meanwhile I suggest the printk route!
> > 
> > And it looks like the 2.4.27 UML patch will not compile in my libc
> > environment. I even think I've tried that before now.
> > 
> > Maybe somebody can give me a 2.4.27 uml kernel? Better, 2.4.30.
> 
> via debugging I found out your raid1.c patch is in trouble regarding spinlocks:

Are you running SMP? Preemptive?

(I have managed to patch 2.4.30 for UML, but it is not as yet very
stable - still, I was able to confirm that the FR1 patch does not oops,
so you must be running SMP).

> At this point (around line 2050) point 8 is reached but not 9
> 
> #if defined(CONFIG_MD_FR1) || defined(CONFIG_MD_FR1_MODULE)
>         printk (KERN_WARNING "raid1.c: 8\n");
>         /* printout info from time to time */
>         spin_lock_irq(&conf->segment_lock);
>         printk (KERN_WARNING "raid1.c: 9\n");
> 
> I guess the cause of it all is this (around line 1930)
> 
>         conf->cnt_ready++;
> #if defined(CONFIG_MD_FR1) || defined(CONFIG_MD_FR1_MODULE)
>         /* PTB hold the segment lock a bit longer for more setup ...*/
>         printk (KERN_WARNING "raid1.c: 1\n");
> #else
>         spin_unlock_irq(&conf->segment_lock);  
> #endif /* CONFIG_MD_FR1 */
> 
> In fact you seem to never release this lock again later on...

Ummmmm. That could well be it. Well done.

> 
> Where/how did you intend to release it? As I have no insider knowledge about

It's supposed to be released ... well, it looks as though it can be
released at once, now.  The device lock is now taken instead in the
code, in order to prevent the configuration from changing. 

I would simply remove that 1-line hunk of the patch that had erased the
spin_unlock. The device lck is taken immediately afterwards.

Either that or release it just after block_not_dirty is set.

Releasing it earlier avoids taking one spinlock under another (which is
risky because ne must always take them in the same order everywhere, or
one creates a deadlock window). Releasing it later guarantees that
there is no window of opportunity to change the config outside of
spinlock control.

> the code itself I cannot just place some unlock somewhere, not knowing what it
> really protects...

It's not clear what. I presume it was protecting the loop lower down
that counted the number of live disks in the array. But that's better
protected by the device_lock, and that lock seems to be used more now,
so let the device lock have it ...

Well done!

Peter