bdbuf: Disk I/O buffering
Eugeny S. Mints
emints at ru.mvista.com
Tue Jul 5 14:55:09 UTC 2005
Kirspel, Kevin {Engineering - Osmetech CCD} wrote:
> Where do the "bd_buf->status" and "bd_buf->error" variables get check in
> the higher level functions?
wow! good question:)
those have to be checked every time by any function which is going to
rely on a bd_buf data. Thus for example after each rtems_bdbuf_read()
invocation and before bd_buf data utilization. Most probably there are
potential bugs related to lack of such verification. You are welcome..:)
Eugeny
> Kevin Kirspel
> Osmetech
> 235 Hembree Park Drive
> Roswell GA, 30076
> 770-510-4444 x568
>
>
> -----Original Message-----
> From: Kirspel, Kevin {Engineering - Osmetech CCD}
> [mailto:kevin.kirspel at osmetech.com]
> Sent: Tuesday, July 05, 2005 9:27 AM
> To: Eugeny S. Mints
> Cc: rtems-users at rtems.com
> Subject: RE: bdbuf: Disk I/O buffering
>
> 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.
>
> 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