lkml.org 
[lkml]   [2018]   [Feb]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] irqchip/gic-v3-its: handle rd_idx wrapping in its_wait_for_range_completion()
From
Date


On 2018/2/12 2:45, Marc Zyngier wrote:
Hi, Marc

Sorry for replying so late.
> On Sun, 11 Feb 2018 03:42:01 +0000,
> Yang Yingliang wrote:
>
> Hi Yang,
>
>> In direct case, rd_idx will wrap if other cpus post commands
>> that make rd_idx increase. When rd_idx wrapped, the driver prints
>> timeout messages but in fact the command is finished. To handle
>> this case by adding last_rd_id to check rd_idx whether wrapped.
>>
>> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
> Please always Cc LKML on irqchip related patches.
>
>> ---
>> drivers/irqchip/irq-gic-v3-its.c | 84 ++++++++++++++++++++++------------------
>> 1 file changed, 46 insertions(+), 38 deletions(-)
>>
>> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
>> index 06f025f..d7176d1 100644
>> --- a/drivers/irqchip/irq-gic-v3-its.c
>> +++ b/drivers/irqchip/irq-gic-v3-its.c
>> @@ -713,7 +713,8 @@ static void its_flush_cmd(struct its_node *its, struct its_cmd_block *cmd)
>>
>> static int its_wait_for_range_completion(struct its_node *its,
>> struct its_cmd_block *from,
>> - struct its_cmd_block *to)
>> + struct its_cmd_block *to,
>> + u64 last_rd_idx)
>> {
>> u64 rd_idx, from_idx, to_idx;
>> u32 count = 1000000; /* 1s! */
>> @@ -724,9 +725,14 @@ static int its_wait_for_range_completion(struct its_node *its,
>> while (1) {
>> rd_idx = readl_relaxed(its->base + GITS_CREADR);
>>
>> - /* Direct case */
>> - if (from_idx < to_idx && rd_idx >= to_idx)
>> - break;
>> +
>> + /*
>> + * Direct case. In this case, rd_idx may wrapped,
>> + * because other cpus may post commands that make
>> + * rd_idx increase.
>> + */
>> + if (from_idx < to_idx && (rd_idx >= to_idx || rd_idx < last_rd_idx))
>> + break;
>>
>> /* Wrapped case */
>> if (from_idx >= to_idx && rd_idx >= to_idx && rd_idx < from_idx)
>> @@ -746,40 +752,42 @@ static int its_wait_for_range_completion(struct its_node *its,
> [...]
>
>> + last_rd_idx = readl_relaxed(its->base + GITS_CREADR); \
> What is this last_rd_idx exactly? It is just some random sampling of
> the read pointer after we've posted our commands. It can still be in
> any position. And if the reader as wrapped because other CPUs are
> feeding more commands to the queue, it could just as well overtake
> last_rd_idx, making your new condition just as false. Yes, this is
> unlikely, but still.
>
> If you're going to harden the command queue wrapping, I'd suggest you
> implement a generation mechanism so that we can easily work out if
> we've seen the queue wrapping or not. THis would lift any kind of
> ambiguity once and for all.
I get a lot of timeout messages, when I am doing set affinity stress
testing which
will post many its commands. I will try another way to handle the
wrapped case.

Regards,
Yang
>
> Thanks,
>
> M.
>


\
 
 \ /
  Last update: 2018-03-01 02:56    [W:0.046 / U:0.040 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site