Re: Locks used in the FAT file system are non-atomic and in fact, don't work on SMP systems

Jeff Merkey (jmerkey@timpanogas.com)
Thu, 26 Aug 1999 12:22:53 -0600


Alan,

I code reviewed sleep_on and wake_up in the linux kernel. I wrote the SMP
kernel for NetWare 4, and discovered that unless you are holding the lock
over the context switch, there is always a very small window between
something going to sleep and waking up where A) you can end up not waking up
anyone or B) two guys can get through at the same time. The logic doesn't
seem to take this into account in Linus' code. The window is also probably
so small that most folks won't see it occur. There also appears to be
assumptions about ordering and the code paths are non-preemptive, so what's
there is probably ok. I have seen two guys inside a lock at the same time
(they didn't go to sleep as expected in sleep_on). We are hammering the
shit out of this system, more so that would happen in a real world scenario.
Normally you need to hold a "stack" lock of some kind when processess
**MOVE** between processors during context switch to avoid this happening.
In Netware we would zero out the stack pointer in the process control block
and would wait until it was non-zero before allowing other processors to run
that process (this would indicate that the process on the other processor
had actually left the previous context and saved off his stack pointer).
This ensured it was really "asleep" during sleep/post operations. There are
cases where someone is going to sleep (but not completely asleep) where a
post operation will allow more than one guy to wake up in such a scenario.
It has to do with how Intel's pipelines work and assumptions about cached
data. MESI really is "messy" to deal with in some cases on SMP.

Jeff

----- Original Message -----
From: Jeff Merkey <jmerkey@timpanogas.com>
To: Alan Cox <alan@lxorguk.ukuu.org.uk>
Cc: <linux-kernel@vger.rutgers.edu>
Sent: Thursday, August 26, 1999 12:07 PM
Subject: Re: Locks used in the FAT file system are non-atomic and in fact,
don't work on SMP systems

> Alan,
>
> Already tried this -- it seems to work (this is what the buffer cache is
> using). The other versions of "lock" however are probably broken. Up()
and
> Down() also work great. I think the FAT file system probably needs to do
> what you suggest so it wont be broken.
>
> Jeff
>
> ----- Original Message -----
> From: Alan Cox <alan@lxorguk.ukuu.org.uk>
> To: Jeff Merkey <jmerkey@timpanogas.com>
> Cc: <linux-kernel@vger.rutgers.edu>
> Sent: Thursday, August 26, 1999 12:01 PM
> Subject: Re: Locks used in the FAT file system are non-atomic and in fact,
> don't work on SMP systems
>
>
> > > Lock()
> > > {
> > > while (lock) sleep_on(&wait);
> > > lock =3D 1;
> > > }
> > >
> > > Unlock()
> > > {
> > > lock =3D 0;
> > > wake_up(&wait);
> > > }
> > >
> > > Two processes can enter Lock() while lock is equal to 0, and both set
=
> > > it. We have seen this occur, and it seems broken. =20
> >
> > Really
> >
> > CPU 0 CPU 1
> > while(lock) - its 0 while(lock) - its 0
> > lock=1 lock=1
> >
> > am I missing something. I think you want to be using atomic test and set
> > operations. (test_and_set_bit)
> >
> > Alan
> >
> >
>

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/