On Tuesday, August 23, 2011 13:58:01 Anton Staaf wrote: > On Tue, Aug 23, 2011 at 10:23 AM, Mike Frysinger 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.
and what do you do when there is no memory left in the malloc arena because you leaked it all and so can't service any new read requests ? if the malloc performance is poor, then why not fix that ? -mike
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
