On Wed, Jun 19, 2019 at 01:18:58AM -0700, Christoph Hellwig wrote: > > mutex_lock(&hmm->lock); > > - list_del(&range->list); > > + list_del_init(&range->list); > > mutex_unlock(&hmm->lock); > > I don't see the point why this is a list_del_init - that just > reinitializeѕ range->list, but doesn't change anything for the list > head it was removed from. (and if the list_del_init was intended > a later patch in your branch reverts it to plain list_del..) Just following the instructions: /** * list_empty_careful - tests whether a list is empty and not being modified * @head: the list to test * * Description: * tests whether a list is empty _and_ checks that no other CPU might be * in the process of modifying either member (next or prev) * * NOTE: using list_empty_careful() without synchronization * can only be safe if the only activity that can happen * to the list entry is list_del_init(). Eg. it cannot be used * if another CPU could re-list_add() it. */ Agree it doesn't seem obvious why this is relevant when checking the list head.. Maybe the comment is a bit misleading? Jason
