From: Will Deacon <will.deacon@arm.com> To: Russell King - ARM Linux <linux@armlinux.org.uk> Cc: John Garry <john.garry@huawei.com>, Mark Rutland <mark.rutland@arm.com>, dikshit.n@huawei.com, anurupvasu@gmail.com, gabriele.paoloni@huawei.com, huangdaode@hisilicon.com, shyju.pv@huawei.com, linux-kernel@vger.kernel.org, xuwei5@hisilicon.com, linuxarm@huawei.com, Shaokun Zhang <zhangshaokun@hisilicon.com>, sanil.kumar@hisilicon.com, linux-arm-kernel@lists.infradead.org, shiju.jose@huawei.com, tanxiaojun@huawei.com, anurup.m@huawei.com Subject: Re: [PATCH v8 6/9] drivers: perf: hisi: Add support for Hisilicon Djtag driver Date: Wed, 14 Jun 2017 12:06:03 +0100 [thread overview] Message-ID: <20170614110603.GM16190@arm.com> (raw) In-Reply-To: <20170614104806.GF4902@n2100.armlinux.org.uk> On Wed, Jun 14, 2017 at 11:48:07AM +0100, Russell King - ARM Linux 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 > > I think the delay is to counter this: > > Agent 1 Agent 2 > read flag > not locked > read flag > not locked > write unique ID > read back > not locked by someone else > write unique ID > read back > not locked by someone else > > With the delay present, this becomes: > > Agent 1 Agent 2 > read flag > not locked > read flag > not locked > write unique ID > delay > write unique ID > delay > read back > locked by agent 2 > read back > not locked by someone else > > For this to work, the delay has to be guaranteed to be greater than > the maximum duration that any agent takes between the initial read > and the write of its unique ID. The delay doesn't even have to be > identical between each agent, it just has to satisfy that condition. I think that it also needs to account for write propagation delays. > The key thing though is that the reads and writes must happen when > the program intends them to, so I don't think the _relaxed variants > should be used here. If they're buffered, then the delay doesn't > have the desired effect. If buffering is a concern, then I think the non-relaxed write has the barrier on the wrong side, so relaxed + mb() would be better. Will
WARNING: multiple messages have this Message-ID (diff)
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:06:03 +0100 [thread overview] Message-ID: <20170614110603.GM16190@arm.com> (raw) In-Reply-To: <20170614104806.GF4902@n2100.armlinux.org.uk> On Wed, Jun 14, 2017 at 11:48:07AM +0100, Russell King - ARM Linux 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 > > I think the delay is to counter this: > > Agent 1 Agent 2 > read flag > not locked > read flag > not locked > write unique ID > read back > not locked by someone else > write unique ID > read back > not locked by someone else > > With the delay present, this becomes: > > Agent 1 Agent 2 > read flag > not locked > read flag > not locked > write unique ID > delay > write unique ID > delay > read back > locked by agent 2 > read back > not locked by someone else > > For this to work, the delay has to be guaranteed to be greater than > the maximum duration that any agent takes between the initial read > and the write of its unique ID. The delay doesn't even have to be > identical between each agent, it just has to satisfy that condition. I think that it also needs to account for write propagation delays. > The key thing though is that the reads and writes must happen when > the program intends them to, so I don't think the _relaxed variants > should be used here. If they're buffered, then the delay doesn't > have the desired effect. If buffering is a concern, then I think the non-relaxed write has the barrier on the wrong side, so relaxed + mb() would be better. Will
next prev parent reply other threads:[~2017-06-14 11:05 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 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 [this message] 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=20170614110603.GM16190@arm.com \ --to=will.deacon@arm.com \ --cc=anurup.m@huawei.com \ --cc=anurupvasu@gmail.com \ --cc=dikshit.n@huawei.com \ --cc=gabriele.paoloni@huawei.com \ --cc=huangdaode@hisilicon.com \ --cc=john.garry@huawei.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux@armlinux.org.uk \ --cc=linuxarm@huawei.com \ --cc=mark.rutland@arm.com \ --cc=sanil.kumar@hisilicon.com \ --cc=shiju.jose@huawei.com \ --cc=shyju.pv@huawei.com \ --cc=tanxiaojun@huawei.com \ --cc=xuwei5@hisilicon.com \ --cc=zhangshaokun@hisilicon.com \ /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: linkBe 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.