From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751429AbeAZDUH (ORCPT ); Thu, 25 Jan 2018 22:20:07 -0500 Received: from mx1.redhat.com ([209.132.183.28]:58344 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751174AbeAZDUG (ORCPT ); Thu, 25 Jan 2018 22:20:06 -0500 Subject: Re: [PATCH net-next 03/12] ptr_ring: READ/WRITE_ONCE for __ptr_ring_empty To: "Michael S. Tsirkin" Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org, John Fastabend , David Miller References: <1516923320-16959-1-git-send-email-mst@redhat.com> <1516923320-16959-4-git-send-email-mst@redhat.com> <2b7d17c0-ef6c-0aad-9b47-0fb8ad78cc3a@redhat.com> <20180126044231-mutt-send-email-mst@kernel.org> From: Jason Wang Message-ID: <51d01ba2-56f9-c68e-ec19-6799d2c87d21@redhat.com> Date: Fri, 26 Jan 2018 11:19:58 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-Version: 1.0 In-Reply-To: <20180126044231-mutt-send-email-mst@kernel.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2018年01月26日 10:44, Michael S. Tsirkin wrote: > On Fri, Jan 26, 2018 at 10:37:58AM +0800, Jason Wang wrote: >> >> On 2018年01月26日 07:36, Michael S. Tsirkin wrote: >>> Lockless __ptr_ring_empty requires that consumer head is read and >>> written at once, atomically. Annotate accordingly to make sure compiler >>> does it correctly. Switch locked callers to __ptr_ring_peek which does >>> not support the lockless operation. >>> >>> Signed-off-by: Michael S. Tsirkin >>> --- >>> include/linux/ptr_ring.h | 11 ++++++++--- >>> 1 file changed, 8 insertions(+), 3 deletions(-) >>> >>> diff --git a/include/linux/ptr_ring.h b/include/linux/ptr_ring.h >>> index 8594c7b..9a72d8f 100644 >>> --- a/include/linux/ptr_ring.h >>> +++ b/include/linux/ptr_ring.h >>> @@ -196,7 +196,9 @@ static inline void *__ptr_ring_peek(struct ptr_ring *r) >>> */ >>> static inline bool __ptr_ring_empty(struct ptr_ring *r) >>> { >>> - return !__ptr_ring_peek(r); >>> + if (likely(r->size)) >>> + return !r->queue[READ_ONCE(r->consumer_head)]; >>> + return true; >>> } >> So after patch 8, __ptr_ring_peek() did: >> >> static inline void *__ptr_ring_peek(struct ptr_ring *r) >> { >>     if (likely(r->size)) >>         return READ_ONCE(r->queue[r->consumer_head]); >>     return NULL; >> } >> >> Looks like a duplication. >> >> Thanks > Nope - they are different. > > The reason is that __ptr_ring_peek does not need to read the consumer_head once > since callers have a lock, I get this. > and __ptr_ring_empty does not need to read > the queue once since it merely compares it to 0. > Do this still work if it was called inside a loop? Thanks