[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] xen/p2m: m2p_find_override: use list_for_each_entry_safe
On Fri, 20 Apr 2012, Dan Carpenter wrote:
> On Fri, Apr 20, 2012 at 12:23:21PM +0100, Stefano Stabellini wrote:
> > On Fri, 20 Apr 2012, Dan Carpenter wrote:
> > > Hi Stefano,
> > >
> > > I had a question about 8f2854c74ff4: "xen/p2m: m2p_find_override: use
> > > list_for_each_entry_safe".
> > >
> > > I think there is a misunderstanding about what the
> > > list_for_each_entry_safe() macro does. It has nothing to do with
> > > locking, so the spinlock is still needed. Without the lock ->next could
> > > point to an element which has been deleted in another thread. Probably
> > > the patch should be reverted.
> > I thought that list_for_each_entry_safe is safe against deletion, is it
> > not?
> > It doesn't matter whether we get up to date entries or old entries
> > here as long as walking through the list doesn't break if a concurrent
> > thread adds or removes items.
> It's safe against deletion in the same thread. But not against
> deletion from another thread.
> At the beginning of the loop it stores a pointer to the next
> element. If you delete the element you are on, no problem because
> you already have a pointer to the next one. But if another thread
> deletes the next element, now you have a pointer which is wrong.
The problem is not that the next element is wrong because we should be
able to cope with that.
The problem is that the next->next pointer would be set LIST_POISON1.
Maybe replacing our call to list_del with __list_del would be sufficient
to solve the problem?
Probably it is best to revert the patch at this stage.
Xen-devel mailing list