From: Yunsheng Lin <linyunsheng@huawei.com>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: <davem@davemloft.net>, <kuba@kernel.org>, <jasowang@redhat.com>,
<brouer@redhat.com>, <paulmck@kernel.org>, <peterz@infradead.org>,
<will@kernel.org>, <shuah@kernel.org>,
<linux-kernel@vger.kernel.org>, <netdev@vger.kernel.org>,
<linux-kselftest@vger.kernel.org>, <linuxarm@openeuler.org>
Subject: Re: [PATCH net-next v2 2/2] ptr_ring: make __ptr_ring_empty() checking more reliable
Date: Mon, 28 Jun 2021 10:17:12 +0800 [thread overview]
Message-ID: <9519986f-165c-1afe-8d1d-dbea11908f00@huawei.com> (raw)
In-Reply-To: <20210627020132-mutt-send-email-mst@kernel.org>
On 2021/6/27 14:03, Michael S. Tsirkin wrote:
>>>>>
>>>>>
>>>>> So if now we need this to be reliable then
>>>>> we also need smp_wmb before writing r->queue[consumer_head],
>>>>> there could be other gotchas.
>>>>
>>>> Yes, This patch does not make it strictly reliable.
>>>> T think I could mention that in the commit log?
>>>
>>> OK so it's not that it makes it more reliable - this patch simply makes
>>> a possible false positive less likely while making a false negative
>>> more likely. Our assumption is that a false negative is cheaper then?
>>>
>>> How do we know that it is?
>>>
>>> And even if we prove the ptr_ring itself is faster now,
>>> how do we know what affects callers in a better way a
>>> false positive or a false negative?
>>>
>>> I would rather we worked on actually making it reliable
>>> e.g. if we can guarantee no false positives, that would be
>>> a net win.
>> I thought deeper about the case you mentioned above, it
>> seems for the above to happen, the consumer_head need to
>> be rolled back to zero and incremented to the point when
>> caller of __ptr_ring_empty() is still *not* able to see the
>> r->queue[] which has been set to NULL in __ptr_ring_discard_one().
>>
>> It seems smp_wmb() only need to be done once when consumer_head
>> is rolled back to zero, and maybe that is enough to make sure the
>> case you mentioned is fixed too?
>>
>> And the smp_wmb() is only done once in a round of producing/
>> consuming, so the performance impact should be minimized?(of
>> course we need to test it too).
>
>
> Sorry I don't really understand the question here.
> I think I agree it's enough to do one smp_wmb between
> the write of r->queue and write of consumer_head
> to help guarantee no false positives.
> What other code changes are necessary I can't yet say
> without more a deeper code review.
>
Ok, thanks for the reviewing.
Will add handling the case you mentioned above in V3 if there
is no noticable performanc impact for handling the above case.
next prev parent reply other threads:[~2021-06-28 2:17 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-25 3:18 [PATCH net-next v2 0/2] add benchmark selftest and optimization for ptr_ring Yunsheng Lin
2021-06-25 3:18 ` [PATCH net-next v2 1/2] selftests/ptr_ring: add benchmark application " Yunsheng Lin
2021-06-25 3:36 ` Jason Wang
2021-06-25 3:52 ` Yunsheng Lin
2021-06-27 6:09 ` Michael S. Tsirkin
2021-06-28 1:42 ` Yunsheng Lin
2021-06-25 6:37 ` Michael S. Tsirkin
2021-06-25 7:40 ` Yunsheng Lin
2021-06-25 3:18 ` [PATCH net-next v2 2/2] ptr_ring: make __ptr_ring_empty() checking more reliable Yunsheng Lin
2021-06-25 6:32 ` Michael S. Tsirkin
2021-06-25 7:21 ` Yunsheng Lin
2021-06-25 7:30 ` Michael S. Tsirkin
2021-06-25 8:33 ` Yunsheng Lin
2021-06-27 6:03 ` Michael S. Tsirkin
2021-06-28 2:17 ` Yunsheng Lin [this message]
2021-06-25 6:39 ` Michael S. Tsirkin
2021-06-25 9:20 ` Yunsheng Lin
2021-06-27 6:07 ` Michael S. Tsirkin
2021-06-28 2:11 ` [Linuxarm] " Yunsheng Lin
2021-06-25 6:42 ` [PATCH net-next v2 0/2] add benchmark selftest and optimization for ptr_ring Michael S. Tsirkin
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=9519986f-165c-1afe-8d1d-dbea11908f00@huawei.com \
--to=linyunsheng@huawei.com \
--cc=brouer@redhat.com \
--cc=davem@davemloft.net \
--cc=jasowang@redhat.com \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=linuxarm@openeuler.org \
--cc=mst@redhat.com \
--cc=netdev@vger.kernel.org \
--cc=paulmck@kernel.org \
--cc=peterz@infradead.org \
--cc=shuah@kernel.org \
--cc=will@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).