All of lore.kernel.org
 help / color / mirror / Atom feed
From: will.deacon@arm.com (Will Deacon)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] ARM: change definition of cpu_relax() for ARM11MPCore
Date: Tue, 9 Mar 2010 17:59:40 -0000	[thread overview]
Message-ID: <001001cabfb2$4a75d2c0$df617840$@deacon@arm.com> (raw)
In-Reply-To: <20100309164926.GC17251@n2100.arm.linux.org.uk>

> > Ok. I was going by the comments in Documentation/volatile-considered-harmful.txt
> > where cpu_relax() is also used as a memory barrier.
> 
> I thought you might; I've just submitted a patch for that to akpm, lkml
> and linux-arch.

Cheers, I'll take a read.

> > In the KGDB case [where this cropped up], if cpu_relax() is left as it is, then
> > an smp_mb() is required in the architecture independent code. This also seems wrong
> > because it's only needed for the ARM11MPCore. There may also potentially be other
> > situations in the Kernel which are prone to deadlock because it is assumed that the
> > write buffer will always drain.
> 
> Why is KGDB being special about this?  Ah yes, it's being brain dead:

<snip>

> Clearly, kgdb is using atomic_set()/atomic_read() in a way which does not
> match this documentation - it's certainly missing the barriers as required
> by the above quoted paragraphs.
> 
> Let me repeat: atomic_set() and atomic_read() are NOT atomic.  There's
> nothing atomic about them.  All they do is provide a pair of accessors
> to the underlying value in the atomic type.  They are no different to
> declaring a volatile int and reading/writing it directly.

Indeed. I'm not familiar enough with KGDB internals to dive in and look at all the
potential barrier conflicts, so I'll submit a patch that addresses the one that's
bitten me so far. Maybe it will motivate somebody else to take a closer look!

Cheers,

Will

  reply	other threads:[~2010-03-09 17:59 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-03-09 16:06 [PATCH] ARM: change definition of cpu_relax() for ARM11MPCore Will Deacon
2010-03-09 16:22 ` Russell King - ARM Linux
2010-03-09 16:35   ` Will Deacon
2010-03-09 16:49     ` Russell King - ARM Linux
2010-03-09 17:59       ` Will Deacon [this message]
2010-03-10 22:06         ` [Kgdb-bugreport] " Jason Wessel
2010-03-11  2:47           ` DDD
2010-03-11 13:53             ` Will Deacon
2010-03-11 13:29           ` Will Deacon
2010-03-11 14:51           ` Will Deacon
2010-03-16 17:26           ` Will Deacon
2010-03-16 18:52             ` Jason Wessel
  -- strict thread matches above, loose matches on Subject: below --
2010-04-12 17:23 Will Deacon
2010-04-12 17:32 ` Russell King - ARM Linux
2010-04-19 14:39 ` Will Deacon
2010-05-27 15:11 Will Deacon
2010-05-27 15:20 ` Shilimkar, Santosh
2010-05-27 16:53   ` Will Deacon
2010-05-28  4:33     ` Shilimkar, Santosh

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='001001cabfb2$4a75d2c0$df617840$@deacon@arm.com' \
    --to=will.deacon@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /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.