Re: xen/p2m: m2p_find_override: use list_for_each_entry_safe

[Dan Carpenter <dan.carpenter@oracle.com>]

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.

regards,
dan carpenter