Messages in this thread | | | Date | Wed, 20 Apr 2016 05:11:13 +0100 | From | Al Viro <> | Subject | Re: [PATCH] devpts: Make each mount of devpts an independent filesystem. |
| |
On Tue, Apr 19, 2016 at 10:43:03PM -0500, Eric W. Biederman wrote: > >> + if (!d_can_lookup(parent)) > >> + return -ENOENT; > > > > And how, pray tell, would a parent of anything fail to be a directory? > > It is to make that function be visually distinct from path_parentat > which does something rather different.
Huh? I'm asking how can that condition ever turn out to be true. Unless you really advocate something like if (2 * 17 != 34) return -234567; // to make it visually distinct from foobar(), // which doesn't have such a test your reply doesn't seem to make any sense...
> >> + this.name = "pts"; > >> + this.len = 3; > >> + this.hash = full_name_hash(this.name, this.len); > >> + if (parent->d_flags & DCACHE_OP_HASH) { > >> + int err = parent->d_op->d_hash(parent, &this); > >> + if (err < 0) > >> + return err; > >> + } > >> + inode_lock(parent->d_inode); > > > > What the hell for? What does that lock on parent change for the > > dcache lookup you are doing here? > > Good point. That is overkill. As we know the dentry is a mount point and > must be in the dcache, the customary lock for performing a lookup from > the disk is not necessary.
Er... To avoid reader confusion: a) d_lookup() does *not* do a filesystem lookup b) it does not need inode_lock() c) it (and not a "lookup from the disk") is what's actually being called in the code in question.
| |