[Top] [All Lists]

Re: PATCH: sleeping while holding a lock in _pagebuf_free_bh()::page_buf

To: Stephen Lord <lord@xxxxxxx>
Subject: Re: PATCH: sleeping while holding a lock in _pagebuf_free_bh()::page_buf.c
From: Luben Tuikov <luben@xxxxxxxxxxxx>
Date: Tue, 22 Oct 2002 15:55:43 -0400
Cc: linux-xfs <linux-xfs@xxxxxxxxxxx>, Eric Sandeen <sandeen@xxxxxxx>
Organization: Splentec Ltd.
References: <3DB49424.9E4CAC0F@splentec.com> <1035289272.9684.13.camel@laptop.americas.sgi.com>
Sender: linux-xfs-bounce@xxxxxxxxxxx
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.1) Gecko/20020826
Stephen Lord wrote:
On Mon, 2002-10-21 at 18:56, Luben Tuikov wrote:

Problem: on an SMP system, BANG#@!, the unthinkable happens.
Solution: never sleep when holding a lock.

This patch applies to CVS code as of about 18:30 EDT
on Mon Oct 21 (today), and is self-explanatory.

This patch fixes the problem of the mount going into D state
indefinitely when the RAID is syncing and mount is run
right after mkfs.xfs (from shell script, no sleep between,
low system load, SMP).

If you know of similar incidents in other parts of the code
those should be fixed, probably ASAP.

Rethinking this, the wake_up does not actually sleep. All
this change does is change it from waking all waiters on this lock to waking one of them.


This is what wake_up_sync() does. wake_up() may reschedule
on an SMP system, since the other CPUs may be able to get
another task to run. This is why there are two different
implementations wake_up() and wake_up_sync().

If you have 4 CPUs, 3 idle and call wake_up() why should
the 3 idle CPUs _STAY_ idle, _until_ you (later) call schedule()
and _then_ to schedule another task on the idle CPUs, and
you to continue to go on, say if you were a SCHED_FIFO, or

BTW, that the _whole_ point of SMP -- _symmetric_ MP.

Bugs like this and the one recently found by Tom Wang
an me, are elementary OS 101 and Programming 101. Maybe a major
overhaul of the source should be done.

Anyway, if you really DO THINK that you're right, maybe you should
INSIST that the code is LEFT as it was, with a call to wake_up().


<Prev in Thread] Current Thread [Next in Thread>