Al Viro <viro@xxxxxxxxxxxxxxxxxx> writes: > On Mon, May 19, 2025 at 11:11:10AM -0700, Linus Torvalds wrote: >> Another thing that is either purely syntactic, or shows that I >> *really* don't understand your patch. Why do you do this odd thing: >> >> // reduce the set until it's non-shifting >> for (m = first_candidate(); m; m = trim_one(m)) >> ; >> >> which seems to just walk the candidates list in a very non-obvious >> manner (this is one of those "I had to go back and forth to see what >> first_candidate() did and what lists it used" cases). >> >> It *seems* to be the same as >> >> list_for_each_entry_safe(m, tmp, &candidates, mnt_umounting) >> trim_one(m); >> >> because if I read that code right, 'trim_one()' will just always >> return the next entry in that candidate list. > > > Another variant would be to steal one more bit from mnt_flags, > set it for all candidates when collecting them, have is_candidate() check > that instead of list_empty(&m->mnt_umounting) and clean it where this > variant removes from the list; trim_one() would immediately return if > bit is not set. Then we could really do list_for_each_entry_safe(), > with another loop doing list removals afterwards. Extra work that way, > though, and I still think it's more confusing... I have only skimmed this so far, and I am a bit confused what we are using MNT_MARK for. I would think we should be able to use MNT_MARK instead of stealing another bit. Regardless I believe you said the goal is to make the code as readable as possible, so next time it needs to be audited a decade from now it won't be hard to figure out what is going on. To that end I think leaving everything on the candidate list, and flipping a bit when we decide that that the mount should be kept will be easier to understand. That way we can have all of the mostly naive algorithms examine a mount and see what we should do with it, in all of the various cases, and we don't have to be clever. The only way I can see to avoid difficult audits is to remove as much cleverness from the code as the problem domain allows. Eric
