All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marek Vasut <marex@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] nios2: convert cache flush to use dm cpu data
Date: Sat, 17 Oct 2015 13:44:11 +0200	[thread overview]
Message-ID: <201510171344.11944.marex@denx.de> (raw)
In-Reply-To: <5621BF01.4090606@wytron.com.tw>

On Saturday, October 17, 2015 at 05:22:41 AM, Thomas Chou wrote:
> Hi Marek,

Hi!

> On 10/17/2015 07:03 AM, Marek Vasut wrote:
> >> I would suggest the "cache alignment check and skip" be removed from
> >> cache flush ops, and say out the DMA buffer allocation rule loudly in
> >> README, and enforce it by guardianship.
> > 
> > What exactly do you envision by this "guardianship" ?
> 
> I mean the reviews of custodians.

Wouldn't an automated check be better ? It's easier and costs almost nothing.
Besides, custodians are not perfect and cannot detect all the issues.

> >> Please allow me to restate the reasons,
> >> 
> >> 1. The cache flush ops are commonly used. Please refer to the "Cache and
> >> TLB Flushing Under Linux" doc, linux/Documentation/cachetlb.txt.
> >> Violating the defined interface is much worse than violating coding
> >> style. It will certainly impact the portability of u-boot. And might
> >> introduce more bug than resolve.
> > 
> > I agree with this one.
> > 
> >> 2. We all agree that enforcing DMA buffer allocation to cache aligned is
> >> the only real solution. Adding such "check and skip" to cache flush ops
> >> cannot prevent the flush or solve the problem.
> > 
> > We should probably check-scream-skip here.
> > 
> >> 3. Though the flush size of block device are usually aligned, the size
> >> of packet are not. Asking the packet drivers to adjust the flush size
> >> does not make sense. It is the job of cache flush ops. The debug probe
> >> should not override the original purpose. It should be spelled for
> >> common understanding.
> > 
> > The socket buffer(s) should be aligned, so network packets should be
> > fine.
> 
> While the start of socket buffer might be aligned, the size of the
> transfer might not for the send ops. It is depended on the net/tcp/ip
> packets size.

Aurgh :-( Now I see what you mean. This is purely bad, very bad. Here is
a real possibility for corruption of variables close to the allocated DMA
buffer, right ?

> For example, with tftp, there is a lot of unaligned end of packets.
> 
> tftp d1000000 u-boot-dtb.bin
> 
> flush unaligned d7ff7020-d7ff704e
> [repeat ..]
> 
> So, such an alarm may be false. And such a skip can be bug.
> 
> In fact, for my own projects, I have changed the memory allocation to
> always cache aligned. And I rarely worry about it ever after.
> 
> I look at the net.c of u-boot. There are packets buffer allocated on BSS
> and stack. I would suggest avoid such programming, and use aligned
> memory allocation stead.

The stack allocation there is used because it's slightly faster and you don't
need mallocator for that. I guess this is a topic for a broader discussion and
we should include Tom and others into it. Would you mind starting another thread
on the ML and CCing me, Tom Rini, Simon Glass etc please ?

Best regards,
Marek Vasut

  reply	other threads:[~2015-10-17 11:44 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-06  8:20 [U-Boot] [PATCH] nios2: convert cache flush to use dm cpu data Thomas Chou
2015-10-08 21:39 ` Marek Vasut
2015-10-09  2:49   ` Ley Foon Tan
2015-10-09  8:00     ` Thomas Chou
2015-10-09 14:42       ` Marek Vasut
2015-10-10  5:55         ` Thomas Chou
2015-10-10  6:32           ` Thomas Chou
2015-10-10 18:18             ` Marek Vasut
2015-10-11  0:38               ` Thomas Chou
2015-10-11 12:15                 ` Marek Vasut
2015-10-12  0:34                   ` Thomas Chou
2015-10-12 10:30                     ` Marek Vasut
2015-10-12 13:12                       ` Thomas Chou
2015-10-12 13:29                         ` Marek Vasut
2015-10-12 13:49                           ` Wolfgang Denk
2015-10-13  1:04                           ` Thomas Chou
2015-10-16 23:03                             ` Marek Vasut
2015-10-17  3:22                               ` Thomas Chou
2015-10-17 11:44                                 ` Marek Vasut [this message]
2015-10-10 18:12           ` Marek Vasut
2015-10-09 14:40     ` Marek Vasut
2015-10-09 12:58 ` [U-Boot] [PATCH v2] " Thomas Chou

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=201510171344.11944.marex@denx.de \
    --to=marex@denx.de \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.