linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: linux@arm.linux.org.uk (Russell King - ARM Linux)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 01/11] usb: chipidea: Add power management support
Date: Tue, 15 Oct 2013 12:15:13 +0100	[thread overview]
Message-ID: <20131015111512.GA25034@n2100.arm.linux.org.uk> (raw)
In-Reply-To: <20131015021814.GA3254@shlinux1.ap.freescale.net>

On Tue, Oct 15, 2013 at 10:18:15AM +0800, Peter Chen wrote:
> So, the lessons for this topic are:
> 
> - If one atomic variable's operation only includes one instruction like
> atomic_read and atomic_set, it is not meaningful for using atomic
> operation, we can just use bool instead of it.

The lesson here is that these are 100% equivalent as far as safety from
races is concerned:

	a = atomic_read(&v);		a = v->counter;

	atomic_set(&v, b);		v->counter = b;

and in general, whenever atomic_read() gets used it's almost certainly
a sign of a bug.

Consider this (similar has been submitted):

	a = atomic_read(&v);
	if (a != 0)
		a += 1;

	atomic_set(&v, a);

and people have thought that somehow this is magically safe from races
because they're using atomic_t, and somehow that saves the universe.
The above is in fact no safer than:

	a = *v;
	if (a != 0)
		a += 1;
	*v = a;

The only thing that using atomic_* does is add a false sense of security
and a level of obfuscation to catch the unwary reviewer.

The reason is quite simple: a single access read in itself is atomic.
Either it has read the value, or it hasn't.  A single access store is
itself atomic.  Either the data has been written, or it hasn't.  The
issue is _always_ what you do around it.

  reply	other threads:[~2013-10-15 11:15 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-12  9:35 [PATCH 00/11] Add power management support for chipidea Peter Chen
2013-10-12  9:35 ` [PATCH 01/11] usb: chipidea: Add power management support Peter Chen
2013-10-14  8:04   ` Lothar Waßmann
2013-10-14  7:55     ` Peter Chen
2013-10-14  8:42       ` Sascha Hauer
2013-10-14  9:04         ` Peter Chen
2013-10-14 10:23           ` Sascha Hauer
2013-10-14 10:46           ` Russell King - ARM Linux
2013-10-14 10:44       ` Russell King - ARM Linux
2013-10-14 11:01   ` Russell King - ARM Linux
2013-10-15  2:18     ` Peter Chen
2013-10-15 11:15       ` Russell King - ARM Linux [this message]
2013-10-12  9:35 ` [PATCH 02/11] usb: chipidea: imx: add " Peter Chen
2013-10-12  9:35 ` [PATCH 03/11] usb: chipidea: usbmisc_imx: remove the controller's clock information Peter Chen
2013-10-12  9:35 ` [PATCH 04/11] usb: chipidea: add wakeup interrupt handler Peter Chen
2013-10-12  9:35 ` [PATCH 05/11] usb: chipidea: usbmisc_imx: add set_wakup API Peter Chen
2013-10-12  9:35 ` [PATCH 06/11] usb: chipidea: imx: call set_wakeup when necessary Peter Chen
2013-10-12  9:35 ` [PATCH 07/11] usb: chipidea: host: add quirk for ehci operation Peter Chen
2013-10-12  9:35 ` [PATCH 08/11] usb: chipidea: host: add ehci quirk for imx controller Peter Chen
2013-10-12  9:35 ` [PATCH 09/11] usb: chipidea: imx: Enable CI_HDRC_IMX_EHCI_QUIRK if the phy has notify APIs Peter Chen
2013-10-12  9:35 ` [PATCH 10/11] usb: chipidea: imx: add binding for supporting runtime pm Peter Chen
2013-10-12 14:40   ` Alan Stern
2013-10-14  1:22     ` Peter Chen
2013-10-14  1:39       ` Marek Vasut
2013-10-14  1:33         ` Peter Chen
2013-10-12  9:35 ` [PATCH 11/11] ARM: dts: imx6qdl-sabresd: Enable runtime pm for usbotg and usb host 1 Peter Chen

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=20131015111512.GA25034@n2100.arm.linux.org.uk \
    --to=linux@arm.linux.org.uk \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).