[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <fceb603c-6072-2941-15d5-56c8a4b4c32c@loongson.cn>
Date: Mon, 23 Jun 2025 17:33:08 +0800
From: Tianyang Zhang <zhangtianyang@...ngson.cn>
To: Thomas Gleixner <tglx@...utronix.de>, chenhuacai@...nel.org,
kernel@...0n.name, corbet@....net, alexs@...nel.org, si.yanteng@...ux.dev,
jiaxun.yang@...goat.com, peterz@...radead.org, wangliupu@...ngson.cn,
lvjianmin@...ngson.cn, maobibo@...ngson.cn, siyanteng@...oftware.com.cn,
gaosong@...ngson.cn, yangtiezhu@...ngson.cn
Cc: loongarch@...ts.linux.dev, linux-doc@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 2/2] irq/irq-loongarch-ir:Add Redirect irqchip support
Hi, Thomas
在 2025/6/23 下午4:27, Thomas Gleixner 写道:
> On Mon, Jun 23 2025 at 10:45, Tianyang Zhang wrote:
>> 在 2025/6/13 下午11:20, Thomas Gleixner 写道:
>>> On Tue, Jun 10 2025 at 19:42, Tianyang Zhang wrote:
>>>
>>>> + tail = (tail + 1) % INVALID_QUEUE_SIZE;
>>> Why is this before the barrier? The barrier does not do anything about
>>> this and you can simplify this. See below.
>>>
>>> And as there is no rmb() counterpart you want to explain that this is
>>> serializing against the hardware.
>>>> + */
>>>> + wmb();
>>>> +
>>>> + write_queue_tail(rqueue->node, tail);
>>> write_queue_tail(rqueue->node, (tail + 1) & INVALID_QUEUE_MASK);
>>>
>>> No?
>> The reason fo coding here is that during testing, it was found that a
>> barrier is needed between the update of temporary variable 'tail' and
>> the operation of register with 'write_queue_tail' , otherwise
>> write_queue_tail will probabilistically fail to obtain the correct
>> value.
> How so?
>
> tail is the software managed part of the ringbuffer which is shared with
> the hardware, right?
>
> So even if the compiler would be allowed to reevalutate tail after the
> barrier (it is NOT), then tail would still contain the same value as
> before, no?
>
> The wmb() is required to ensure that the hardware can observe the full
> write of the command _before_ it can observe the update to the tail
> index.
>
> Anything else is voodoo.
>
> Thanks,
>
> tglx
In my previous understanding, tail'value is actually a part of 'full
write of the command '.
We must ensure that tail is updated to the correct value first, and then
write this value
into the register (perhaps by adding wmb in write_queue_tail ).
In other words, this is originally to prevent the write register
instruction from being executed
out of order before updating tail.
The above is just my personal understanding
Thanks
Tianyang
Powered by blists - more mailing lists