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

[Mike Frysinger <vapier@gentoo.org>]

On Tuesday, August 23, 2011 14:39:48 Wolfgang Denk wrote:
> Anton Staaf wrote:
> > >> 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. =A0Please 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
> 
> As Mike pointed out, this information is of course available: the
> bufer was on the stack before, so it disappears upon return from this
> function.
> 
> > 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
> 
> Do we really need malloc at all? Why cannot we just use a slightly
> larger buffer on the stack and align the pointer into it?  We're
> talking about cache line sizes here, i. e. a few tens of bytes -
> that is probably way less than the code you add here.

i think we want to make this easy to support and hard to screw up, otherwise 
people are going to screw it up and cause more problems :).

if get_dcache_line_size() were a static inline, and we allow flexible array 
initializers (is that c99?  i forget), we could do:
#define DMA_ALIGN_SIZE(size) \
	(((size) + get_dcache_line_size() - 1) & ~(get_dcache_line_size() - 1))
#define DMA_DECLARE_BUFFER(type, name, size) \
	void *__##name[DMA_ALIGN_SIZE(size)]; \
	type name = __##name;

then people would simply do:
	DMA_DECLARE_BUFFER(char, sec_buf, SECTOR_SIZE);

but i'm not sure this is better than the proposed:
	char *sec_buf = dma_buffer_alloca(SECTOR_SIZE);

i'd have to look at the code gcc generates to see if it is simply the same ... 
in which case i'd stick with the latter as it's more natural to people.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20110823/fef1feec/attachment.pgp