On Fri, 1 May 2015 03:29:40 +0100 Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote: > On Fri, May 01, 2015 at 12:23:33PM +1000, NeilBrown wrote: > > > What kind of consistency warranties do callers expect, BTW? You do realize > > > that between iterate_dir() and callbacks an entry might have been removed > > > and/or replaced? > > > > For READDIR_PLUS, lookup_one_len is called on each name and it requires > > i_mutex, so the code currently holds i_mutex over the whole sequence. > > This is triggering a deadlock. > > Yes, I've seen the context. However, you are _not_ holding it between > actual iterate_dir() and those callbacks, which opens a window when > directory might have been changed. > > Again, what kind of consistency is expected by callers? Are they ready to > cope with "there's no such entry anymore" or "inumber is nothing like > what we'd put in ->ino, since it's no the same object" or "->d_type is > completely unrelated to what we'd found, since the damn thing had been > removed and created from scratch"? Ah, sorry. Yes, the callers are prepared for "there's no such entry anymore". They don't use d_type, so don't care if it might be meaningless. NFSv4 doesn't use ino either, but NFSv3 does and isn't properly cautious about ino changing. In nfs3xdr, we should probably pass 'ino' to encode_entryplus_baggage() and thence to compose_entry_fh() and it should report failure if dchild->d_inode->i_ino doesn't match. Simply not returning the extra attributes is perfectly acceptable in NFSv3. So it looks like we are mostly OK here - we don't really need i_mutex to be held for very long. NeilBrown
Attachment:
pgpe73Gsqo6s5.pgp
Description: OpenPGP digital signature
