On Sat, Jul 23, 2011 at 03:35:34PM +0200, Christoph Hellwig wrote: > On Fri, Jul 22, 2011 at 08:55:48PM -0700, Linus Torvalds wrote: > > If something sets dentry->d_inode without going through __d_instantiate, > > that misses it. I'm looking at d_obtain_alias(), and wondering, for > > example. > > As pointed out in my last mail d_obtain_alias will absolute need setting > up these flags as well. ... and dentry_unlink_inode() - clearing them (note that we already are clearing DCACHE_CANT_MOUNT there, so additional price will be trivial). > I don't think we need to re-add exec_permission for it. By checking > DCACHE_OP_PERMISSION instead of inode->i_op->permission you'll always > got directly to generic_permission in inode_permission, and in there > acl_permission_check -> check_acl should simply do the right thing with > your your patch to take the ACL cache checking into common code. Checking DCACHE_OP_PERMISSION on _what_? inode_permission() gets inode, not dentry... Now, we could mirror those into struct inode itself (and that might make more sense, actually), but I'd rather not go through the equivalent of d_set_d_op() patchset for i_op, with no hope for per-sb defaults reducing the size of mess this time. Calculating those flags at __d_instantiate/d_obtain_alias still looks like the least painful way to do it... Come to think of that, reintroducing exec_permission() and making it take dentry has a problem - we would be asking for trouble with the "clean the flags" side of things. If we race with e.g. rmdir(), dentry *can* become negative under us on those calls. Inode will remain allocated and, with MAY_NOT_BLOCK, the actual ->permission() will hopefully not rely on anything that might have been freed in ->evict_inode() (we do need to document and verify that, BTW), but dentry flags could be cleared... -- 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
