On Apr 11, 2017, at 12:48 AM, Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote: > On Mon, Apr 10, 2017 at 11:10:19PM -0700, Linus Torvalds wrote: > >> It looks odd because the lock part is >> >> if (flags & LOOKUP_RCU) >> rcu_read_lock(); >> >> ie it's locked conditionally, and the code in between does not seem to >> return every time LOOKUP_RCU is clear. >> >> So mind giving this a look? Is it as obviously buggy as I think it is, >> or is there something I'm missing? > > It's more obscure than I would like, and can grow into a bug one day, but... > nd_jump_root() can only return non-zero if you have LOOKUP_RCU. So without > LOOKUP_RCU in flags, this > if (flags & LOOKUP_RCU) > rcu_read_lock(); > set_root(nd); > if (likely(!nd_jump_root(nd))) > return s; > nd->root.mnt = NULL; > rcu_read_unlock(); > won't get to that rcu_read_unlock() at all - it'll get zero from nd_jump_root() > and proceed to return s; So possibly a comment like the following would be helpful: rcu_read_unlock(); /* nd_jump_root() returns if !LOOKUP_RCU */ so that us mere mortals have a chance to understand this in the future? Cheers, Andreas
Attachment:
signature.asc
Description: Message signed with OpenPGP
