Why timer interrupt is disabled?

Why timer interrupt is disabled?

[andreev]

Hi, list.
I have an Intel CFI compatible chip on my MIPS machine, which is mapped
on to the physical memory .When i compiled linux-2.4.1 kernel with Intel
CFI support, and booted my MIPS, linux hanged when i tried to read data
from flash.

Then, i inserted debugging printk functions, and found that i read data
insteed of status register from flash. ( Because my chip must be
programmed with double word commands ).  In this case, driver must
return -EIO in do_read_1_by_16_onechip after time out. But it do not,
because all interrupts were disabled before do_ftl_request called. In
do_ftl_request i found commented out sti() function. Why it were
commented out? I had a look to the current sourse in your CVS, and i
found that sti() is still commented out. Where we have to enable
interrupts? If driver works under the closed interrupts, why does it use
the time_after and jiffies?
Can anybody tell me abouut it?

Re: Why timer interrupt is disabled?

[David Woodhouse]

andreev@niisi.msk.ru said:
>  In do_ftl_request i found commented out sti() function. Why it were
> commented out? I had a look to the current sourse in your CVS, and i
> found that sti() is still commented out. Where we have to enable
> interrupts? If driver works under the closed interrupts, why does it
> use the time_after and jiffies? Can anybody tell me abouut it?

You probably need to unlock the io_request_lock. But you probably shouldn't 
be using FTL anyway - why not just put JFFS or JFFS2 on the flash directly?

--
dwmw2

Re: Why timer interrupt is disabled?

[David Woodhouse]

andreev@niisi.msk.ru said:
> <br><br></pre></blockquote> I know that JFFS is the better choice, but
> I must use the FTL driver, because there is installed X-server on my 
> flash <br> partition... <br>

I don't see the logic. The Compaq iPAQ works quite well with the X-server 
in the JFFS2 filesystem.


>  <br> I think that unlocking of the io_request_lock inside ftl driver
> is not a good idea, because the do_ftl_request must be <br> atomic.

My understanding is that it's perfectly safe to drop the io_request_lock
as long as we only look at the request at the head of the queue. There's no 
need for do_ftl_request to be atomic - and in fact it cannot be atomic 
because the flash read and write calls may sleep.

> What about enabling all or only <br> timer interrupts inside
> do_ftl_request. Can you uncomment the sti() in ftl.c in your CVS
> version?<br> <br> <br> </body></html>

If you ensure that it's correct to do so, and provide a patch which 
releases the io_request_lock, then I'll certainly consider applying it.

--
dwmw2

Re: Why timer interrupt is disabled?

[Alexandr Andreev]

David Woodhouse wrote:

>andreev@niisi.msk.ru said:
>
>>I know that JFFS is the better choice, but
>>I must use the FTL driver, because there is installed X-server on my 
>>flash partition... 
>>
>I don't see the logic. The Compaq iPAQ works quite well with the X-server 
>in the JFFS2 filesystem.
>
I don`t want to format my flash partition write now. Besides it doesn`t 
matter. Of course i`ll use JFFS in the future. 

>>I think that unlocking of the io_request_lock inside ftl driver
>>is not a good idea, because the do_ftl_request must be atomic.
>>
>My understanding is that it's perfectly safe to drop the io_request_lock as long as we only look at the request at the head of the queue. There's no 
>need for do_ftl_request to be atomic - and in fact it cannot be atomic 
>because the flash read and write calls may sleep.
>
   I think, if write call sleeps, than we must return. Do i wrong?
   Locking of io_request_lock and disabling interrupts, are mading in 
block devices common driver. I read the kernel-api documentation in 
Documentation/DocBook/kernel-api/r10011.html: " A global spin lock 
$io_request_lock must be held while manipulating the requests on the 
request queue."
   Besides, if we unlock the io_request_lock and enable all interrupts 
it might to occur this thing:
While we are reading or writing data to the flash, the timer interrupt 
is happens. Next, the schedule() called, and an other process begins to 
write or read data to the flash. Can you guarantee, that flash is 
protected by locks and "chips->state" inside the CFI driver? I mean: if 
we write the READ_STATUS command to flash inside context of first 
process, and trying to read data inside context of second process, than 
we read status, but not data. 
   In all cases we need to enable timer ticks, anywhere in flash. It is 
the simplies way i think...

Re: Why timer interrupt is disabled?

[David Woodhouse]

andreev@niisi.msk.ru said:
>   Locking of io_request_lock and disabling interrupts, are mading in
> block devices common driver. I read the kernel-api documentation in
> Documentation/DocBook/kernel-api/r10011.html: " A global spin lock
> $io_request_lock must be held while manipulating the requests on the
> request queue."

Ah - but we're _not_ manipulating the requests on the request queue. We're 
servicing the request at the head of the queue, which is a special case - 
the queueing code knows that drivers are likely to be looking at the 
request at the head of the queue even while the io_request_lock is not held.


andreev@niisi.msk.ru said:
> While we are reading or writing data to the flash, the timer interrupt
>  is happens. Next, the schedule() called, and an other process begins
> to  write or read data to the flash.

The kernel is not preemptively scheduled. Unless the first process 
explicitly calls schedule(), it's not going to lose the CPU. Only processes 
in user mode are scheduled from the timer IRQ.

--
dwmw2

Re: Why timer interrupt is disabled?

[Herman Oosthuysen]

On Fri, 25 May 2001, David Woodhouse wrote:
> The kernel is not preemptively scheduled. Unless the first process 
> explicitly calls schedule(), it's not going to lose the CPU. Only processes 
> in user mode are scheduled from the timer IRQ.

Is it not dangerous to assume that the kernel is non-preemptive?
MTD is after all most likely to be used in a real-time system, which may very
well have a different scheduler from the common/garden variety Linux.

Cheers,
-- 
Herman Oosthuysen
Phone: 403+569-5687
E-mail: Herman@WirelessNetworksInc.com

Re: Why timer interrupt is disabled?

[Alexandr Andreev]

David Woodhouse wrote:

>Ah - but we're _not_ manipulating the requests on the request queue. We're 
>servicing the request at the head of the queue, which is a special case - 
>the queueing code knows that drivers are likely to be looking at the 
>request at the head of the queue even while the io_request_lock is not held.
>
>
>The kernel is not preemptively scheduled. Unless the first process 
>explicitly calls schedule(), it's not going to lose the CPU. Only processes 
>in user mode are scheduled from the timer IRQ.
>
Oh yeah, i've got it... you absolutely right, the flash driver will 
never support requests manipulating, so we can don't care about the 
io_request_lock holding, and kernel is really preemptively.
So, we can unlock the io_request_lock in ftl.c, but what about double 
unlocking? Or, do you want to lock it back after proceeding request :) 
Is there any conflicts, when we try to unlock already unlocked lock? 

Re: Why timer interrupt is disabled?

[David Woodhouse]

andreev@niisi.msk.ru said:
> reemptively. So, we can unlock the io_request_lock in ftl.c, but what
> about double  unlocking? Or, do you want to lock it back after
> proceeding request :)  Is there any conflicts, when we try to unlock
> already unlocked lock?  

You have to lock it again before returning. 

But you really shouldn't be taking my advice on how to write block devices. 
I've been trying to avoid it :)

--
dwmw2

Re: Why timer interrupt is disabled?

[Alexandr Andreev]

I had asked this question to linux-kernel@vger.kernel.org and there is 
the answer:

Jens Axboe wrote:

>On Fri, May 25 2001, Alexandr Andreev wrote:
>
>>Hi, list
>>In ll_rw_block.c, before calling block device specific request function 
>>( i mean do_hd_request, do_ftl_request, ... ) the io_request_lock is 
>>locking, and all interrupts are disabling. I know, that request handler 
>>routine have to be atomic, but when we read data from a flash device ( 
>>for example ) we use a timeouts. Where do we have to enable timer 
>>interrupts, or should we disable all interrupts?
>>
>
>Even with dropping io_request_lock, it's not recommended to sleep inside
>the request_fn. WIth plugging, you are basically preventing the other
>plugged queues from being run until you return.
>
>You could use a timer or similar to call you on a specified timeout
>instead.
>
... and nevertheless, this is a bad idea to unlock the io_request_lock 
inside the do_ftl_request. What about the SMP? It may happen that one 
CPU is reading and other CPU is writing data at the same time. So, the 
flash memory can return status value instead of data, for example. Does 
the chip->mutex lock provides  necessary synchronization?
I look at the other block device drivers code and found, that io_request 
handlers do sti() in some cases. If you still don`t want to do it, we 
could use a softirq. In the do_fd_request(), the "bottom halves" is used 
for example. We could use a "bottom halves" or even a tasklet in our 
case. What do you think about this?

Re: Why timer interrupt is disabled?

[David Woodhouse]

andreev@niisi.msk.ru said:
>  It may happen that one  CPU is reading and other CPU is writing data
> at the same time. So, the  flash memory can return status value
> instead of data, for example. Does  the chip->mutex lock provides
> necessary synchronization?

Yes. The flash driver is required to make that work correctly. The (blkdev)
request functions aren't required to be reentrant anyway, I believe.

> I look at the other block device drivers code and found, that
> io_request  handlers do sti() in some cases. If you still don`t want
> to do it, we  could use a softirq. In the do_fd_request(), the "bottom
> halves" is used  for example. We could use a "bottom halves" or even a
> tasklet in our  case. What do you think about this?

Take a look at the way it's done by the mtdblock device, with a kernel 
thread.

--
dwmw2