On Tue, 3 Aug 2021, Huang, Ying wrote: > > As Hugh pointed out, EINVAL isn't an appropriate error code for race > condition. After checking the code, I found that EEXIST is the error > code used for race condition. So I revise the patch as below. If Hugh > doesn't object, can you help to replace the patch with the below one? (I'm sorry that it's so hard to extract responses from me...) Yes, I'll go along with this version, or Matthew's better commented version, which Andrew has now taken into his tree. I won't go so far as to Ack this, because I still want to revert the original commit; but this will not do actual harm, and I'm too slow to mess you around further for 5.14. I'll just have to work through it and argue it later when/if I have time. I'll say more on that in answering your earlier mail in this thread. But should admit right now that I think have somewhat misled us all. Neither the EINVAL nor the -EINVAL were as dangerous as they looked: because they were followed immediately by "goto failed", and failed: if (!shmem_confirm_swap(mapping, index, swap)) error = -EEXIST; and in the case that get_swap_device() fails, all the swapping off has been done, so shmem_confirm_swap() will return false, and the error then be set to -EEXIST anyway. But let's pretend that I hadn't realized that: what's in Andrew's tree is better than what was there before. (And let's pretend that in writing those paragraphs, I did not realize that get_swap_device() could also fail if entry had got corrupted - should never happen, of course - and shmem then get stuck in a repeating -EEXIST loop. Maybe I'll want to do better for that case too, but not this time.) Hugh
