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 v8 6/9] drivers: perf: hisi: Add support for Hisilicon Djtag driver
Date: Wed, 14 Jun 2017 12:01:41 +0100	[thread overview]
Message-ID: <20170614110141.GL16190@arm.com> (raw)
In-Reply-To: <20170614104230.GC6085@leverpostej>

On Wed, Jun 14, 2017 at 11:42:30AM +0100, Mark Rutland wrote:
> On Wed, Jun 14, 2017 at 11:06:58AM +0100, Will Deacon wrote:
> > Apologies, I misunderstood your algorithm (I thought step (a) was on one CPU
> > and step (b) was on another). Still, I don't understand the need for the
> > timeout. If you instead read back the flag immediately, wouldn't it still
> > work? e.g.
> > 
> > 
> > lock:
> >   Readl_relaxed flag
> >   if (locked)
> >     goto lock;
> > 
> >   Writel_relaxed unique ID to flag
> >   Readl flag
> >   if (locked by somebody else)
> >     goto lock;
> > 
> > <critical section>
> > 
> > unlock:
> >   Writel unlocked value to flag
> > 
> > 
> > Given that we're dealing with iomem, I think it will work, but I could be
> > missing something obvious.
> 
> Don't we have the race below where both threads can enter the critical
> section?
> 
> 	// flag f initial zero (unlocked)
> 
> 	// t1, flag 1			// t2, flag 2
> 	readl(f); // reads 0		l = readl(f); // reads 0
> 
> 	<thinks lock is free>		<thinks lock is free>
> 
> 	writel(1, f);
> 	readl(f); // reads 1
> 	<thinks lock owned>
> 					writel(2, f);
> 					readl(f) // reads 2
> 					<thinks lock owned>
> 
> 	<crticial section>		<critical section>

Urgh, yeah, of course and *that's* what the udelay is trying to avoid,
by "ensuring" that the <thinks lock is free> time and subsequent write
propagation is all over before we re-read the flag.

John -- how much space do you have on this device? Do you have, e.g. a byte
for each CPU?

Will

WARNING: multiple messages have this Message-ID (diff)
From: Will Deacon <will.deacon@arm.com>
To: Mark Rutland <mark.rutland@arm.com>
Cc: John Garry <john.garry@huawei.com>,
	Shaokun Zhang <zhangshaokun@hisilicon.com>,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, anurup.m@huawei.com,
	tanxiaojun@huawei.com, xuwei5@hisilicon.com,
	sanil.kumar@hisilicon.com, gabriele.paoloni@huawei.com,
	shiju.jose@huawei.com, huangdaode@hisilicon.com,
	linuxarm@huawei.com, dikshit.n@huawei.com, shyju.pv@huawei.com,
	anurupvasu@gmail.com
Subject: Re: [PATCH v8 6/9] drivers: perf: hisi: Add support for Hisilicon Djtag driver
Date: Wed, 14 Jun 2017 12:01:41 +0100	[thread overview]
Message-ID: <20170614110141.GL16190@arm.com> (raw)
In-Reply-To: <20170614104230.GC6085@leverpostej>

On Wed, Jun 14, 2017 at 11:42:30AM +0100, Mark Rutland wrote:
> On Wed, Jun 14, 2017 at 11:06:58AM +0100, Will Deacon wrote:
> > Apologies, I misunderstood your algorithm (I thought step (a) was on one CPU
> > and step (b) was on another). Still, I don't understand the need for the
> > timeout. If you instead read back the flag immediately, wouldn't it still
> > work? e.g.
> > 
> > 
> > lock:
> >   Readl_relaxed flag
> >   if (locked)
> >     goto lock;
> > 
> >   Writel_relaxed unique ID to flag
> >   Readl flag
> >   if (locked by somebody else)
> >     goto lock;
> > 
> > <critical section>
> > 
> > unlock:
> >   Writel unlocked value to flag
> > 
> > 
> > Given that we're dealing with iomem, I think it will work, but I could be
> > missing something obvious.
> 
> Don't we have the race below where both threads can enter the critical
> section?
> 
> 	// flag f initial zero (unlocked)
> 
> 	// t1, flag 1			// t2, flag 2
> 	readl(f); // reads 0		l = readl(f); // reads 0
> 
> 	<thinks lock is free>		<thinks lock is free>
> 
> 	writel(1, f);
> 	readl(f); // reads 1
> 	<thinks lock owned>
> 					writel(2, f);
> 					readl(f) // reads 2
> 					<thinks lock owned>
> 
> 	<crticial section>		<critical section>

Urgh, yeah, of course and *that's* what the udelay is trying to avoid,
by "ensuring" that the <thinks lock is free> time and subsequent write
propagation is all over before we re-read the flag.

John -- how much space do you have on this device? Do you have, e.g. a byte
for each CPU?

Will

  parent reply	other threads:[~2017-06-14 11:01 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-22 12:48 [PATCH v8 6/9] drivers: perf: hisi: Add support for Hisilicon Djtag driver Shaokun Zhang
2017-05-22 12:48 ` Shaokun Zhang
2017-06-08 16:35 ` Mark Rutland
2017-06-08 16:35   ` Mark Rutland
2017-06-09 14:18   ` John Garry
2017-06-09 14:18     ` John Garry
2017-06-09 14:30     ` Will Deacon
2017-06-09 14:30       ` Will Deacon
2017-06-09 15:10       ` John Garry
2017-06-09 15:10         ` John Garry
2017-06-14 10:06         ` Will Deacon
2017-06-14 10:06           ` Will Deacon
2017-06-14 10:42           ` Mark Rutland
2017-06-14 10:42             ` Mark Rutland
2017-06-14 10:50             ` Mark Rutland
2017-06-14 10:50               ` Mark Rutland
2017-06-14 11:01             ` Will Deacon [this message]
2017-06-14 11:01               ` Will Deacon
2017-06-14 11:35               ` John Garry
2017-06-14 11:35                 ` John Garry
2017-06-14 11:40                 ` Will Deacon
2017-06-14 11:40                   ` Will Deacon
2017-06-14 11:59                   ` John Garry
2017-06-14 11:59                     ` John Garry
2017-06-14 10:48           ` Russell King - ARM Linux
2017-06-14 10:48             ` Russell King - ARM Linux
2017-06-14 11:06             ` Will Deacon
2017-06-14 11:06               ` Will Deacon
2017-06-09 15:44     ` Mark Rutland
2017-06-09 15:44       ` Mark Rutland
2017-06-09 16:09       ` John Garry
2017-06-09 16:09         ` John Garry
2017-06-09 16:45         ` Mark Rutland
2017-06-09 16:45           ` Mark Rutland
2017-06-14  8:11   ` Zhangshaokun
2017-06-14  8:11     ` Zhangshaokun

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=20170614110141.GL16190@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.