bdbuf: Disk I/O buffering

Eugeny S. Mints emints at ru.mvista.com
Tue Jul 5 14:57:13 UTC 2005


Joel Sherrill  wrote:
> 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?
I believe so - it definitly fixes one bug.
	Eugeny
> 
>> 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);
>>
>>
>>
>>
> 
> 





More information about the users mailing list