bdbuf: Disk I/O buffering

Tue Jul 5 14:22:22 UTC 2005

Kirspel, Kevin {Engineering - Osmetech CCD} wrote:
> The patch fixes the swapout() problem. As you mention earlier, I now
> have other issues that I will have to work out.  If anything is
> relevant, I will update this thread.

Should this patch be considered trusted at this point and applied
to CVS?

> Kevin Kirspel
> Osmetech 
> 235 Hembree Park Drive
> Roswell GA, 30076
> 770-510-4444 x568
> 
> 
> -----Original Message-----
> From: Eugeny S. Mints [mailto:emints at ru.mvista.com] 
> Sent: Monday, July 04, 2005 7:38 AM
> To: Eugeny S. Mints
> Cc: Kirspel, Kevin {Engineering - Osmetech CCD}; rtems-users at rtems.com
> Subject: Re: bdbuf: Disk I/O buffering
> 
> forgot to mention:
> 
> first, sorry for the patch format, second the patch is agains 4.6.0 and 
> the third I've never compiled/tested it.
> 
> 	Eugeny
> 
> Eugeny S. Mints wrote:
> 
>>Hello Kevin,
>>
>>Kirspel, Kevin {Engineering - Osmetech CCD} wrote:
>>
>>
>>>I have built an rtems 4.6.0 release and it has worked beautifully for
> 
> 2
> 
>>>years.  I added compact flash support to my build by using the IDE
>>>interface in rtems.  Every thing works great as long as I keep the
>>>compact flash card in the system. I wanted to add support to rtems so
>>>that I may remove and insert the compact flash card whenever I want.
> 
> I
> 
>>>now probe() the compact flash device to make sure it is in place
> 
> before
> 
>>>I perform any ata_ioctl() commands. If the device is not in place, I
>>>return -1 with errno = ENODEV. This causes the bdbuf_swapout_task()
>>>function to enter an indefinite loop because it keeps on trying to
> 
> call
> 
>>>the ata_ioctl() routine with no success.
>>>
>>>The following code is taken from the bdbuf_swapout_task() function in
>>>bdbuf.c:
>>>
>>>/* transfer_sema initialized when bd_buf inserted in the mod chain
>>>   first time */
>>>result = dd->ioctl(dd->phys_dev->dev, BLKIO_REQUEST, &req);
>>>
>>>rtems_disk_release(dd);
>>>
>>>if (result == -1)
>>>{
>>>  bd_buf->status = RTEMS_IO_ERROR;
>>>  bd_buf->error = errno;
>>>  /* Release tasks waiting on syncing this buffer */
>>>  _CORE_mutex_Flush(&bd_buf->transfer_sema, NULL,
>>>CORE_MUTEX_STATUS_SUCCESSFUL);
>>>}
>>>else
>>>{
>>>  if (bd_buf->in_progress)
>>>  {
>>>    rtems_interrupt_disable(level);
>>>    _CORE_mutex_Seize(&bd_buf->transfer_sema, 0, TRUE, 0, level);
>>>  }
>>>}
>>>
>>>I am not a disk buffering guru, but what can I do to the
>>>bdbuf_swapout_task() ( or other functions ) to make it process the
> 
> error
> 
>>>so that at the higher levels and error code is returned.
>>
>>Apart from that there is a bug that you discovered, the higher levels 
>>are aware about status/error of this low level I/O via bd_buf->status 
>>and bd_buf->error fields which are filled either in "if (result ==
> 
> -1)" 
> 
>>branch above (the swapout task algo is: a higher level task is
> 
> awaiting 
> 
>>for a buffer blocking on transfer sema; the buffer is removed from 
>>swapout task's queue of buffers to process at Chain_Get() and at 
>>transfer sema release time higher level task gets control on a buffer 
>>back with bd_buf status and error fields set appropriately) or at 
>>bdbuf_write_transfer_done() execution time.
>>
>>The bug you discovered is the following:
>>at the time ioclt returns "-1 " bd_buf->modified field is set. But a
> 
> bit 
> 
>>below at
>>
>>        if (bd_buf->use_count == 0)
>>        {
>>            if (bd_buf->modified)
>>            {
>>                Chain_Append(&bd_ctx.mod, &bd_buf->link);
>>                rc = rtems_semaphore_release(bd_ctx.flush_sema);
>>            }
>>            else
>>            {
>>                Chain_Append(&bd_pool->lru, &bd_buf->link);
>>                rc = rtems_semaphore_release(bd_pool->bufget_sema);
>>            }
>>        }
>>
>>since bd_buf->modified is not re-set (at normal path this field is
> 
> reset 
> 
>>at bdbuf_write_transfer_done() before this part of code execution) the
> 
> 
>>rtems_semaphore_release(bd_ctx.flush_sema) leads to the infinit
> 
> cycling 
> 
>>in swapout task.
>>
>>Please let us know whether the patch attached fixes your problem (at 
>>least inifinit cycling).
>>
>>
>>>My current scenario is this:
>>>
>>>1. I boot the system with the compact flash card in place.
>>>2. After task initialization, my main task initializes the ide
>>>partitions, mounts the drive, opens a file, writes data to the file,
>>>closes the file, and unmounts the drive. This works great.
>>>3. I then remove the compact flash and repeat the process.
>>>4. During the mount process, the system hangs in bdbuf_swapout_task()
> 
> do
> 
>>>to the fact that the dd->ioctl() returns -1;
>>
>>Based on your scenario I'm concerned about whether the bug in swapout 
>>taks is the main issue.
>>My understanding is that the scenario with removed flash shoud end up
> 
> in 
> 
>>mount() call (more exaclty in 
>>
> 
> msdos_initialize_support()->fat_init_volume_info()->rtems_disk_lookup() 
> 
>>(if you use fat fs ) since rtems_disk_lookup() should fail if there is
> 
> 
>>no actual disk) and has not to have any deal with swapout task at all.
>>
>>    Eugeny
>>
>>
>>>Kevin Kirspel
>>>Osmetech 235 Hembree Park Drive
>>>Roswell GA, 30076
>>>770-510-4444 x568
>>>
>>>
>>>
>>>
>>
>>
>>
> ------------------------------------------------------------------------
> 
>>--- bdbuf.old.c	2005-07-04 15:21:52.000000000 +0400
>>+++ bdbuf.c	2005-07-04 15:23:28.000000000 +0400
>>@@ -1545,7 +1545,7 @@
>>          * and release semaphore */
>>         if (bd_buf->use_count == 0)
>>         {
>>-            if (bd_buf->modified)
>>+            if ((bd_buf->status != RTEMS_IO_ERROR) &&
> 
> (bd_buf->modified))
> 
>>             {
>>                 Chain_Append(&bd_ctx.mod, &bd_buf->link);
>>                 rc = rtems_semaphore_release(bd_ctx.flush_sema);
> 
> 
> 

-- 
Joel Sherrill, Ph.D.             Director of Research & Development
joel at OARcorp.com                 On-Line Applications Research
Ask me about RTEMS: a free RTOS  Huntsville AL 35805
    Support Available             (256) 722-9985