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

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

[Anton Staaf]

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.

Signed-off-by: Anton Staaf <robotboy@chromium.org>
Cc: Lukasz Majewski <l.majewski@samsung.com>
Cc: Mike Frysinger <vapier@gentoo.org>
Cc: Dave Liu <r63238@freescale.com>
Cc: Andy Fleming <afleming@gmail.com>
Cc: Albert ARIBAUD <albert.u.boot@aribaud.net>
---
v2: Remove C++ style comment

This patch depends on Lukasz Majewski's dcache line size patch sent to the
list in: http://patchwork.ozlabs.org/patch/110501/

 fs/ext2/dev.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/fs/ext2/dev.c b/fs/ext2/dev.c
index 78851d0..daac507 100644
--- a/fs/ext2/dev.c
+++ b/fs/ext2/dev.c
@@ -26,6 +26,7 @@
 
 #include <common.h>
 #include <config.h>
+#include <malloc.h>
 #include <ext2fs.h>
 
 static block_dev_desc_t *ext2fs_block_dev_desc;
@@ -52,9 +53,15 @@ int ext2fs_set_blk_dev(block_dev_desc_t *rbdd, int part)
 
 int ext2fs_devread(int sector, int byte_offset, int byte_len, char *buf)
 {
-	char sec_buf[SECTOR_SIZE];
+	static char *sec_buf;
 	unsigned sectors;
 
+	if (sec_buf == NULL)
+		sec_buf = memalign(get_dcache_line_size(), SECTOR_SIZE);
+
+	if (sec_buf == NULL)
+		return 0; /* -ENOMEM */
+
 	/*
 	 *  Check partition boundaries
 	 */
-- 
1.7.3.1

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

[Wolfgang Denk]

Dear Anton Staaf,

In message <1314043924-22130-1-git-send-email-robotboy@chromium.org> you 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.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
NOTE: The  Most  Fundamental  Particles  in  This  Product  Are  Held
Together  by  a  "Gluing" Force About Which Little is Currently Known
and Whose Adhesive Power Can Therefore Not Be Permanently Guaranteed.

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

[Anton Staaf]

On Mon, Aug 22, 2011 at 2:42 PM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Anton Staaf,
>
> In message <1314043924-22130-1-git-send-email-robotboy@chromium.org> you 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.

Thanks,
    Anton

> Best regards,
>
> Wolfgang Denk
>
> --
> DENX Software Engineering GmbH, ? ? MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
> NOTE: The ?Most ?Fundamental ?Particles ?in ?This ?Product ?Are ?Held
> Together ?by ?a ?"Gluing" Force About Which Little is Currently Known
> and Whose Adhesive Power Can Therefore Not Be Permanently Guaranteed.
>

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

[Mike Frysinger]

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 ?
-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/f3a88785/attachment.pgp 

[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@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
>

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

[Mike Frysinger]

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
-------------- 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/57745dfe/attachment.pgp 

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

[Anton Staaf]

On Tue, Aug 23, 2011 at 11:32 AM, Mike Frysinger <vapier@gentoo.org> wrote:
> 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 ?

I think you've miss-understood my patch.  The allocated buffer is
stored in a static variable.  So only the first time through is it
allocated.

> if the malloc performance is poor, then why not fix that ?
> -mike
>

I think our other thread has provided the correct solution here.

Thanks,
    Anton

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

[Mike Frysinger]

On Tuesday, August 23, 2011 14:48:24 Anton Staaf wrote:
> On Tue, Aug 23, 2011 at 11:32 AM, Mike Frysinger wrote:
> > 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 ?
> 
> I think you've miss-understood my patch.  The allocated buffer is
> stored in a static variable.  So only the first time through is it
> allocated.

yes, i missed that.  still no excuse for leaking though ! :)
-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/5e2150a7/attachment.pgp 

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

[Anton Staaf]

On Tue, Aug 23, 2011 at 11:55 AM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Tuesday, August 23, 2011 14:48:24 Anton Staaf wrote:
>> On Tue, Aug 23, 2011 at 11:32 AM, Mike Frysinger wrote:
>> > 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 ?
>>
>> I think you've miss-understood my patch. ?The allocated buffer is
>> stored in a static variable. ?So only the first time through is it
>> allocated.
>
> yes, i missed that. ?still no excuse for leaking though ! :)

Fair enough.  I'm happier not leaking as well.  I'll resubmit this
patch once the aligned alloca thread has come to a consensus.

Thanks,
    Anton

> -mike
>

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

[Wolfgang Denk]

Dear Anton Staaf,

In message <CAF6FioUoRx50_6nEkMc0MyAKf6rZwsbu9tONrC+4c2UWUH+ZxQ@mail.gmail.com> you wrote:
>
> > 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 ?
>
> I think you've miss-understood my patch.  The allocated buffer is
> stored in a static variable.  So only the first time through is it
> allocated.

Strictly speaking, only the _pointer_to_ the allocated buffer is
stored in a static variable. [I know you meant this, but we should try
and be exact.]

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
You have the capacity to learn from  mistakes.  You'll  learn  a  lot
today.

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

[Anton Staaf]

On Tue, Aug 23, 2011 at 1:18 PM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Anton Staaf,
>
> In message <CAF6FioUoRx50_6nEkMc0MyAKf6rZwsbu9tONrC+4c2UWUH+ZxQ@mail.gmail.com> you wrote:
>>
>> > 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 ?
>>
>> I think you've miss-understood my patch. ?The allocated buffer is
>> stored in a static variable. ?So only the first time through is it
>> allocated.
>
> Strictly speaking, only the _pointer_to_ the allocated buffer is
> stored in a static variable. [I know you meant this, but we should try
> and be exact.]

Yes, absolutely.

Thanks,
    Anton

> Best regards,
>
> Wolfgang Denk
>
> --
> DENX Software Engineering GmbH, ? ? MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
> You have the capacity to learn from ?mistakes. ?You'll ?learn ?a ?lot
> today.
>

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

[Wolfgang Denk]

Dear Anton Staaf,

In message <CAF6FioXyMnhK=gKbwDeGpvGGz-xMs-Hr6c4q3SsTtJ29CRsqCQ@mail.gmail.com> you 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.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
No question is too silly to ask. Of course, some  questions  are  too
silly to to answer...  - L. Wall & R. L. Schwartz, _Programming Perl_

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

[Mike Frysinger]

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 

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

[Anton Staaf]

On Tue, Aug 23, 2011 at 11:47 AM, Mike Frysinger <vapier@gentoo.org> wrote:
> 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);

Wow!  Yes, please let's not do this and let's just use the simpler
aligned alloca that we discussed in the other thread.  Also, the above
macro does not create an aligned buffer, it just creates a buffer that
is a multiple of the cache line size.  It there is nothing that
changes the alignment of the __##name array.

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

I would much prefer this solution.

Thanks,
    Anton

> 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
>

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

[Wolfgang Denk]

Dear Mike Frysinger,

In message <201108231447.39934.vapier@gentoo.org> you wrote:
>
> 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.

Even if the code is different the former is so ugly that chances for
it to go into mainline are _really_ small.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Just because your doctor has a name for your condition  doesn't  mean
he knows what it is.