On Tue, Jul 26, 2011 at 09:10:02AM -0700, Linus Torvalds wrote: > However, if we *do* have POSIX ACL's enabled, but we just have the XFS > ACL's disabled, then just returning NULL from xfs_get_acl() is > actually a horrible idea, because then the VFS code will always call > that function and it never gets cached as "I have no ACL's". Which is > why I didn't like the inline function approach. Well, at least not the > one that only returns NULL - we also want to fill in the cache. Even getting to check_acl() requires MS_POSIXACL to be set on the superblock, which it won't be if CONFIG_XFS_ACL isn't set. These days we can probably kill CONFIG_FOOFS_ACL given that everyone now uses the generic code, and due to the deeper VFS interaction is more or less forced to anyway. Historically that wasn't the case. > I'm actually getting more and more convinced that we should just do > that "set_acl_cache()" in generic code for the "get_acl()" case. Even > without any locking it's no worse than all the current filesystems, > since they *also* do it with no locking. And then this kind of issue > wouldn't come up. While I agree with you that we should life it into the common code, the common caching is not related at all to this particular issue. The code that fails to compile is where XFS implements the default ACL inheritance when creating new inodes. Move filesystems do this a bit simpler and dumber and do the ACL-induced i_mode modifications after creating the inode. XFS tries to create the inode with the correct mode from the first moment and thus has a slightly broader interface between the "normal" inode ops and its ACL code. Due to various interactions with the fs transaction code the inheritance code is at the very end of my list of ACL pieces to move to common code. -- 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
