From: Zhu Yanjun <yanjun.zhu@linux.dev>
To: linke li <lilinke99@qq.com>, leon@kernel.org
Cc: bmt@zurich.ibm.com, jgg@ziepe.ca, linux-kernel@vger.kernel.org,
linux-rdma@vger.kernel.org
Subject: Re: [PATCH] RDMA/siw: Reuse value read using READ_ONCE instead of re-reading it
Date: Mon, 11 Mar 2024 06:11:58 +0100 [thread overview]
Message-ID: <0c7d1d79-2372-4bae-ba9b-e7b6070af14c@linux.dev> (raw)
In-Reply-To: <tencent_03614198A34E56D038455012AA31022D9C06@qq.com>
在 2024/3/11 3:34, linke li 写道:
>> If value can change between subsequent reads, then you need to use locks
>> to make sure that it doesn't happen. Using READ_ONCE() doesn't solve the
>> concurrency issue, but makes sure that compiler doesn't reorder reads
>> and writes.
>
> This code do not need to prevent other thread from writing on the flags.
>
> This topic got quite a bit of discussion [1], quote from it:
>
> (READ_ONCE and WRITE_ONCE)
> That's often useful - lots of code doesn't really care if you get the
> old or the new value, but the code *does* care that it gets *one*
> value, and not some random mix of "I tested one value for validity,
> then it got reloaded due to register pressure, and I actually used
> another value".
>
> And not some "I read one value, and it was a mix of two other values".
>
> From the original code, the first read seems to do the same things. So
> READ_ONCE is probably ok here.
>
> I just want to make sure the flags stored to wqe->sqe.flags is consistent
> with the read used in the if condition.
Sure. Follow Leon's advice, to make this ("wqe->sqe.flags is consistent
with the read used in the if condition") happen, you need a lock to
ensure it. The lock can be spin lock or mutex lock depens on its
sleeping or not.
From the original source code, wqe->sqe.flags should be a volatile
variable. It should be read from the original source, not from cache.
Zhu Yanjun
>
> [1]https://lore.kernel.org/lkml/CAHk-=wgG6Dmt1JTXDbrbXh_6s2yLjL=9pHo7uv0==LHFD+aBtg@mail.gmail.com/
>
next prev parent reply other threads:[~2024-03-11 5:12 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-09 12:27 [PATCH] RDMA/siw: Reuse value read using READ_ONCE instead of re-reading it linke li
2024-03-10 4:53 ` Zhu Yanjun
2024-03-10 12:36 ` linke li
2024-03-10 17:00 ` Greg Sword
2024-03-11 2:57 ` linke li
2024-03-11 8:17 ` Zhu Yanjun
2024-03-10 11:33 ` Leon Romanovsky
2024-03-10 12:15 ` linke li
2024-03-10 19:19 ` Leon Romanovsky
2024-03-11 2:34 ` linke li
2024-03-11 5:11 ` Zhu Yanjun [this message]
2024-03-10 17:02 ` Greg Sword
2024-03-11 14:14 ` Bernard Metzler
2024-03-12 1:30 ` linke li
2024-03-12 7:57 ` Leon Romanovsky
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=0c7d1d79-2372-4bae-ba9b-e7b6070af14c@linux.dev \
--to=yanjun.zhu@linux.dev \
--cc=bmt@zurich.ibm.com \
--cc=jgg@ziepe.ca \
--cc=leon@kernel.org \
--cc=lilinke99@qq.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rdma@vger.kernel.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.