Opened on 10/28/09 at 15:45:39
Closed on 12/28/09 at 11:21:26
#1457 closed defect (fixed)
Unnecessary read from device driver in case we have READ_AHEAD entry in CACHE
Reported by: | Oleg | Owned by: | Sebastian Huber |
---|---|---|---|
Priority: | normal | Milestone: | 4.10 |
Component: | fs | Version: | 4.10 |
Severity: | normal | Keywords: | |
Cc: | thomas.doerfler@…, chrisj@…, nbkolchin@…, sebastian.huber@…, Oleg.Kravtsov@… | Blocked By: | |
Blocking: |
Description
When we turn on read ahead feature and read blocks in sequence, the code does not use already pre-read blocks and start reading them from driver, which might reduce performance.
According to rtems_bdbuf_get_buffer() function, it can return a buffer in READ_AHEAD state that keeps valid data, and that should be returned by rtems_bdbuf_read() function, BUT instead of this rtems_bdbuf_read() initiates a new data transfer!
The problem is because we should have checked if a block is in either CACHED, MODIFIED, or READ_AHEAD state, but right now the code checks if a block is in CACHED or MODIFIED states only.
Actually I can't understand why READ_AHEAD state exists, it is actually not a state but rather attribute, because all READ_AHEAD buffers are the same as buffers in CACHED state (i.e. keep valid data), but the difference is that READ_AHEAD buffers are in ready list, and CACHED not there.
I.e. I believe that READ_AHEAD state should be removed and use a single CACHED state for it, but the way how it is handled will be indirectly show that it is read ahead, i.e. javing a CACHED buffer in ready list means that it was read ahead. I do not see any speciality or usefulness for READ_AHEAD state, but this is the matter of another Bug.
Simple test:
- enable read ahead in the image;
- read block #N (as the result this will read ahead some more blocks - depending on the configuration);
- read block #N+1 (there should be no access to the media), BUT right now there is access.
Attachments (1)
Change History (7)
Changed on 10/28/09 at 15:45:39 by Oleg
Attachment: | bdbuf_read_ahead.patch added |
---|
comment:1 Changed on 11/02/09 at 08:55:01 by Oleg
Cc: | nbkolchin@… Oleg.Kravtsov@… added |
---|
comment:2 Changed on 11/04/09 at 14:59:42 by Oleg
Cc: | thomas.doerfler added |
---|
comment:3 Changed on 11/04/09 at 15:11:24 by thomas.doerfler
Cc: | Sebastian Huber added |
---|
comment:4 Changed on 11/04/09 at 15:21:38 by Sebastian Huber
Cc: | Chris Johns added |
---|
comment:5 Changed on 11/20/09 at 08:49:52 by Sebastian Huber
Owner: | changed from Joel Sherrill to Sebastian Huber |
---|
comment:6 Changed on 12/28/09 at 11:21:26 by Oleg
Resolution: | → fixed |
---|---|
Status: | new → closed |
The initial problem seems to be fixed. At least your re-worked version does not seem to have this bug.
Patch to fix exactly this problem