From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45110) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VDv94-000667-8H for qemu-devel@nongnu.org; Mon, 26 Aug 2013 07:39:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VDv8v-0006DO-H8 for qemu-devel@nongnu.org; Mon, 26 Aug 2013 07:39:42 -0400 Received: from mail-wi0-x22d.google.com ([2a00:1450:400c:c05::22d]:59136) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VDv8v-0006DF-BU for qemu-devel@nongnu.org; Mon, 26 Aug 2013 07:39:33 -0400 Received: by mail-wi0-f173.google.com with SMTP id ey11so671978wid.12 for ; Mon, 26 Aug 2013 04:39:32 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <521B3E6F.4080008@redhat.com> Date: Mon, 26 Aug 2013 13:39:27 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1377371209-2017-1-git-send-email-ncmike@ncultra.org> <5219A4E8.60802@redhat.com> <87vc2tapj5.fsf@pixel.localdomain> In-Reply-To: <87vc2tapj5.fsf@pixel.localdomain> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC PATCH] Introduce RCU-enabled DQs (v2) List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Mike Day Cc: Paul Mckenney , Mathew Desnoyers , qemu-devel@nongnu.org, Anthony Liguori Il 25/08/2013 15:06, Mike Day ha scritto: > > Paolo Bonzini writes: > >> Just a couple of questions, one of them on the new macro... >> >>> +/* prior to publication of the elm->prev->next value, some list >>> + * readers may still see the removed element when following >>> + * the antecedent's next pointer. >>> + */ >>> + >>> +#define QLIST_REMOVE_RCU(elm, field) do { \ >>> + if ((elm)->field.le_next != NULL) { \ >>> + (elm)->field.le_next->field.le_prev = \ >>> + (elm)->field.le_prev; \ >>> + } \ >>> + atomic_rcu_set((elm)->field.le_prev, (elm)->field.le_next); \ >>> +} while (/*CONSTCOND*/0) >> >> Why is the barrier needed here, but not in Linux's list_del_rcu? >> >> I think it is not needed because all involved elements have already been >> published and just have their pointers shuffled. > > I read this as more than shuffling pointers. The intent here is > that the previous element's next pointer is being updated to omit the > current element from the list. Sorry if I were too concise... by "having their pointers shuffled" I meant that all assigned values were already present in the list. The important point is that no new node has to be published in the list. The importance of the write barrier with RCU is to ensure an item is fully ready before it is added to a data structure. For this to be true, all writes to the item must be complete before a pointer to the item is first written in memory. Here, the pointer had already been written in memory, so there's nothing to complete. > atomic_set always deferences the pointer passed to it, and > (field)->le_pre is a double pointer. So looking at the macro: > > #define atomic_set(ptr, i) ((*(__typeof__(*ptr) *volatile) (ptr)) = (i)) > > It translates to: > > ( ( * (__typeof(*elm->field.le_prev) *volatile) (elm)->field.le_prev) = > elm->field.le_next; ) > > Which is: > > *((struct *elm) *volatile)(elm)->field.le_prev = elm->field.le_next; > > Which is: > > *(elm)->field.le_prev = elm->field.le_next; > > Because field.le_prev is a double pointer that has previously been set > to &prev (the address of the previous list element) this is assiging the > *previous* element's next pointer, the way I read it. Correct. > The Linux list_del_rcu is dealing with a singly linked list and > therefore does not set a value in the previous node's element. Note that Linux list_head is a circular list; hlist is a singly-linked list. list_del_rcu still modifies the previous pointer via __list_del_entry: static inline void __list_del_entry(struct list_head *entry) { __list_del(entry->prev, entry->next); } static inline void __list_del(struct list_head * prev, struct list_head * next) { next->prev = prev; prev->next = next; } > But I'm still unclear on whether or not the memory barrier is needed > because the deleted element won't be reclaimed right away. Right. That memory barrier is not needed here, it is included in the implementation of synchronize_rcu. Paolo