Al Viro <viro@xxxxxxxxxxxxxxxxxx> writes: > On Thu, Dec 18, 2014 at 01:24:26PM -0600, Eric W. Biederman wrote: >> Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> writes: >> >> > On Thu, Dec 18, 2014 at 9:07 AM, Linus Torvalds >> > <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote: >> >> Why is this piece of code using its own made up and buggy list handling in >> >> the first place? We have list functions for these things, exactly so that >> >> people shouldn't write buggy stuff by hand. >> > >> > Oh. Ok, I see what's going on. We have "list_splice()", but we don't >> > have the equivalent "hlist_splice()". So it's doing that by hand, and >> > did it badly. >> > >> > Al, this is your bug. I guess I can take the "manual hlist_splice" fix >> > from Eric, but I'm not really happy with it. There's a few other >> > places in that same commit where the list splice operation has been >> > open-coded. >> > >> > Mind taking a look? >> >> It looks like we can pretty easily use mnt_list instead of mnt_hash, >> see below (note: the code is only compile tested). >> >> While converting this to ordinary list helpers I found something >> strange. >> >> In __propagate_umount we currently add the child to be unmounted in a >> different location in the list then we did before the conversion of >> mnt_hash to a hlist for rcu's accesses benefit. >> >> Now maybe propagate_next handles this (I still need to read and >> understand that code) if not it looks like I may have found another bug, >> as it looks like today we can add a node to our list without propogating >> the unmount from the node. > > Er... Why would we want to reprocess it? The loop goes through all nodes > getting events from ours; everything that gets events from _them_ included > at the same time they are. We might have wanted to because before your change to an hlist that is what the code did. Having read through the code of propagate_next it does look like it iterates through the entire propagation hierarchy so there shouldn't be a need to visit mounts that we have placed on the propagation list. Any thoughts on using mnt_list instead of mnt_hash to allow the use of list_splice and list_move? Eric -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
