Is it safe to spin lock list_for_each_entry_safe or list_for_each_entry?

Question

I want to add things to my list, and if something already exists in the list I want to remove it before re-adding it. This is just an exercise that I'm trying out for myself (I know, there is no practical purpose to doing something like this)

spin_lock(&mylock);
bool added = false
list_for_each_entry_safe(cur, nxt, &mylist.entries, entries)
{
    //Check to see if cur == the item I want to add, if it is add it and set my bool to true
}

//Check to see if my bool is true. If it's false, add the item to the list

spin_unlock(&mylock);

Is this unsafe? The problem is I cannot figure out how to put the spin locks anywhere else without the creating a race condition. For example if I did this

list_for_each_entry_safe(cur, nxt, &mylist.entries, entries)
{
spin_lock(&mylock);

    //Check to see if cur == the item I want to add, if it is add it and set my bool to true

spin_unlock(&mylock);
}

//race condition here

spin_lock(&mylock);

//Check to see if my bool is true. If it's false, add the item to the list 

spin_unlock(&mylock);

There could be a race condition where I commented "race condition here", because some other thread could add the item, and then I wouldn't know and the current thread would add the item again, so there would be a duplicate item in the list.

The issue is I don't know if it's ok or not to have an entire list_for_each_entry inside the critical section of a spin lock?

Answer 1

There is no problem in iterating over a list whilst holding a spinlock. After all, what list_for_each_entry does is merely a simple for loop. If you are asking whether list_for_each_entry can sleep then the answer is no. Your first snippet of code looks safe per se (one would have to also take a look at the rest of your code to be 100% sure that it is safe).