Bottom halves being disabled

Bottom halves being disabled

[Ed Co]

Hi All,

In the NOR driver code, the spin locks are acquired
everywhere with bottom halves disabled using the
spin_lock_bh() API. My doubt is why are the bottom
halves disabled? (Doesn't that add to the real time
latency on the system!)

Hoping someone would clear the doubt

Ed


		
_______________________________
Do you Yahoo!?
Win 1 of 4,000 free domain names from Yahoo! Enter now.
http://promotions.yahoo.com/goldrush

Re: Bottom halves being disabled

[David Woodhouse]

On Tue, 2004-08-24 at 05:09 -0700, Ed Co wrote:
> Hi All,
> 
> In the NOR driver code, the spin locks are acquired
> everywhere with bottom halves disabled using the
> spin_lock_bh() API. My doubt is why are the bottom
> halves disabled? (Doesn't that add to the real time
> latency on the system!)
> 
> Hoping someone would clear the doubt

Because we intend the erase completion to be asynchronous, hence we lock
against the timers which may one day check for it. There's no real
reason we couldn't switch to spin_lock() for now.

-- 
dwmw2

Re: Bottom halves being disabled

[Linh Dang]

David Woodhouse <dwmw2@infradead.org> wrote:

> On Tue, 2004-08-24 at 05:09 -0700, Ed Co wrote:
>> Hi All,
>>
>> In the NOR driver code, the spin locks are acquired
>> everywhere with bottom halves disabled using the
>> spin_lock_bh() API. My doubt is why are the bottom
>> halves disabled? (Doesn't that add to the real time
>> latency on the system!)
>>
>> Hoping someone would clear the doubt
>
> Because we intend the erase completion to be asynchronous, hence we
> lock against the timers which may one day check for it. There's no
> real reason we couldn't switch to spin_lock() for now.

Is it safe to convert those spin_locks to mutexes?

-- 
Linh Dang

Re: Bottom halves being disabled

[David Woodhouse]

On Tue, 2004-08-24 at 22:36 -0400, Linh Dang wrote:
> Is it safe to convert those spin_locks to mutexes?

A spinlock is a mutex. Do you mean convert to semaphores? Why? 

-- 
dwmw2

Re: Bottom halves being disabled

[Linh Dang]

David Woodhouse <dwmw2@infradead.org> wrote:

> On Tue, 2004-08-24 at 22:36 -0400, Linh Dang wrote:
>> Is it safe to convert those spin_locks to mutexes?
>
> A spinlock is a mutex. Do you mean convert to semaphores?

yes

> Why?

reduce latency with preemptable kernel. I've tried that. In my test I
hammered jffs2 pretty hard (for typical flash usage, no kernel
compilation here) and everything still seemed ok.

--
Linh Dang

Re: Bottom halves being disabled

[David Woodhouse]

On Wed, 2004-08-25 at 06:48 -0400, Linh Dang wrote:
> reduce latency with preemptable kernel. I've tried that. In my test I
> hammered jffs2 pretty hard (for typical flash usage, no kernel
> compilation here) and everything still seemed ok.

Show numbers.

-- 
dwmw2

Re: Bottom halves being disabled

[Linh Dang]

David Woodhouse <dwmw2@infradead.org> wrote:

> On Wed, 2004-08-25 at 06:48 -0400, Linh Dang wrote:
>> reduce latency with preemptable kernel. I've tried that. In my test
>> I hammered jffs2 pretty hard (for typical flash usage, no kernel
>> compilation here) and everything still seemed ok.
>
> Show numbers.

On my card (AMD Am29LV640 NOR flash), preemption was often disabled
for 450usecs in cfi_cmdset_002.c's do_read_onechip().

using semaphore instead of spin_lock, preemption would NOT be disabled
at all!

Btw, JFFS2 is great!

-- 
Linh Dang

Hi Latency in do_read_onechip (was Re: Bottom halves being disabled)

[Linh Dang]

Hi David,

On our system, we have some slow AMD nor flash. The line

	map_copy_from(map, buf, adr, len);

in do_read_onechip (in cfi_cmdset_0002.c) is called while holding a
spin_lock (chip->mutex). This triggers 450usec non-preemptible periods
on our system.

1. Is there any plan to fix this?
2. Is it safe to replace the spin_lock with a semaphore?
3. while mtd has to lock softirq?

Thanx

-- 
Linh Dang

Re: Bottom halves being disabled

[Josh Boyer]

On Tue, 2004-08-24 at 07:09, Ed Co wrote:
> Hi All,
> 
> In the NOR driver code, the spin locks are acquired
> everywhere with bottom halves disabled using the
> spin_lock_bh() API. My doubt is why are the bottom
> halves disabled? (Doesn't that add to the real time
> latency on the system!)

What version of the code are you using, and which driver?  I seem to
remember dwmw2 doing a s/spin_lock_bh/spin_lock a while ago since MTD
doesn't even use bottom halves.

Latest CVS shows only cfi_cmdset_0020.c, amd_flash.c, and sharp.c as
using spin_lock_bh().  Of those three, two are obsolete and the other is
in need of fixing.

josh