[U-Boot] [PATCH v2] ext2: Cache line aligned partial sector bounce buffer

[Anton Staaf]

On Tue, Aug 23, 2011 at 10:23 AM, Mike Frysinger <vapier at gentoo.org> wrote:
> On Monday, August 22, 2011 17:48:47 Anton Staaf wrote:
>> On Mon, Aug 22, 2011 at 2:42 PM, Wolfgang Denk wrote:
>> > Anton Staaf wrote:
>> >> Currently, if a device read request is done that does not begin or end
>> >> on a sector boundary a stack allocated bounce buffer is used to perform
>> >> the read, and then just the part of the sector that is needed is copied
>> >> into the users buffer.  This stack allocation can mean that the bounce
>> >> buffer will not be aligned to the dcache line size.  This is a problem
>> >> when caches are enabled because unaligned cache invalidates are not
>> >> safe.
>> >>
>> >> This patch allocates a cache line size aligned sector sized bounce
>> >> buffer the first time that ext2fs_devread is called.
>> >
>> > ...and never frees ist, which is a bad thing.  Please fix.
>>
>> That was actually intentional.  To free the buffer the code would need
>> to know when it was done doing ext2 accesses.  This information isn't
>> really available.  And it would be a performance hit to allocate and
>> free the buffer every time a read was performed, instead this patch
>> re-uses the same allocated buffer every time that the read is called.
>> Would you prefer that I allocate and free the buffer each time?  I can
>> see an argument for that since it would mean that the code could also
>> be called from multiple threads simultaneously, not that we have any
>> such thing to worry about at the moment.
>
> i'm not sure i follow ... the current code always frees it upon func exit.
> why cant yours do the same ?

I certainly could.  But as I mentioned it would be a performance hit
to do so.  The devread function is called many times.  And there is no
way of knowing when the last one finishes.  And since it's likely that
a kernel will be loaded shortly it seems better to be fast than to
free this buffer.  But I would be happy to change this if people
disagree (which it sounds like they do).  An alternative would be to
allocate the buffer the first time it is needed in the devread
function and then free it in the ext2fs_close function.  Or if we know
that ext2fs_mount will always be called first we could allocate the
buffer there.

Thanks,
    Anton

> -mike
>