* [git pull] vfs pile 1.5 @ 2011-07-25 18:56 Al Viro 2011-07-26 3:35 ` Markus Trippelsdorf 0 siblings, 1 reply; 20+ messages in thread From: Al Viro @ 2011-07-25 18:56 UTC (permalink / raw) To: Linus Torvalds; +Cc: linux-kernel, linux-fsdevel Regression fixes (devtmpfs race, cifs ->d_revalidate() breakage, fsync fallout), ACL stuff (me/Linus/Christoph), fixes for bugs found while wading through the mode_t handling. There'll be another pull request with assorted stuff from over the last cycle. Please, pull from git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs-2.6.git/ for-linus Shortlog: Al Viro (11): ocfs2_init_acl(): fix a leak 9p: close ACL leaks fix jffs2 ACLs on big-endian with 16bit mode_t asus-wmi: ->is_visible() can't return negative get rid of create_proc_entry() abuses - proc_mkdir() is there for purpose caam: don't pass bogus S_IFCHR to debugfs_create_...() fix devtmpfs race kill boilerplate around posix_acl_chmod_masq() generic_acl: no need to clone acl just to push it to set_cached_acl() kill boilerplates around posix_acl_create_masq() bury posix_acl_..._masq() variants Christoph Hellwig (4): 9p: do no return 0 from ->check_acl without actually checking xfs: cache negative ACLs if there is no attribute fork reiserfs: cache negative ACLs for v1 stat format fs: take the ACL checks to common code Linus Torvalds (1): vfs: move ACL cache lookup into generic code Markus Trippelsdorf (1): xfs: Fix wrong return value of xfs_file_aio_write Pavel Shilovsky (1): CIFS: Fix oops while mounting with prefixpath Tim Chen (1): VFS : mount lock scalability for internal mounts Diffstat: Documentation/filesystems/Locking | 4 +- Documentation/filesystems/porting | 7 +- Documentation/filesystems/vfs.txt | 2 +- arch/arm/mach-bcmring/dma.c | 2 +- drivers/base/devtmpfs.c | 3 +- drivers/crypto/caam/ctrl.c | 26 +++--- drivers/mtd/mtdchar.c | 2 +- drivers/platform/x86/asus-wmi.c | 2 +- drivers/staging/rtl8187se/r8180_core.c | 2 +- .../staging/rtl8192e/ieee80211/ieee80211_module.c | 2 +- drivers/staging/rtl8192e/r8192E_core.c | 6 +- .../staging/rtl8192u/ieee80211/ieee80211_module.c | 2 +- drivers/staging/rtl8192u/r8192U_core.c | 6 +- fs/9p/acl.c | 68 +++++--------- fs/9p/acl.h | 10 +- fs/9p/vfs_inode_dotl.c | 13 ++- fs/anon_inodes.c | 2 +- fs/btrfs/acl.c | 63 +++---------- fs/btrfs/ctree.h | 4 +- fs/btrfs/inode.c | 10 +- fs/cifs/dir.c | 2 +- fs/ext2/acl.c | 63 +++---------- fs/ext2/acl.h | 4 +- fs/ext2/file.c | 2 +- fs/ext2/namei.c | 4 +- fs/ext3/acl.c | 94 ++++++------------- fs/ext3/acl.h | 4 +- fs/ext3/file.c | 2 +- fs/ext3/namei.c | 4 +- fs/ext4/acl.c | 95 ++++++------------- fs/ext4/acl.h | 4 +- fs/ext4/file.c | 2 +- fs/ext4/namei.c | 4 +- fs/generic_acl.c | 62 +++---------- fs/gfs2/acl.c | 75 ++++------------ fs/gfs2/acl.h | 2 +- fs/gfs2/inode.c | 6 +- fs/hugetlbfs/inode.c | 1 + fs/jffs2/acl.c | 49 +++-------- fs/jffs2/acl.h | 6 +- fs/jffs2/dir.c | 2 +- fs/jffs2/file.c | 2 +- fs/jffs2/fs.c | 2 +- fs/jffs2/os-linux.h | 2 +- fs/jffs2/symlink.c | 2 +- fs/jfs/acl.c | 75 +++++----------- fs/jfs/file.c | 2 +- fs/jfs/jfs_acl.h | 2 +- fs/jfs/namei.c | 2 +- fs/namei.c | 54 +++++++++++- fs/namespace.c | 21 ++++- fs/nfs/nfs3acl.c | 10 +-- fs/ocfs2/acl.c | 77 ++++++----------- fs/ocfs2/acl.h | 2 +- fs/ocfs2/file.c | 4 +- fs/ocfs2/namei.c | 2 +- fs/pipe.c | 2 +- fs/posix_acl.c | 48 ++++++++-- fs/reiserfs/file.c | 2 +- fs/reiserfs/inode.c | 5 + fs/reiserfs/namei.c | 6 +- fs/reiserfs/xattr.c | 27 ------ fs/reiserfs/xattr_acl.c | 71 +++++---------- fs/xfs/linux-2.6/xfs_acl.c | 80 ++++------------- fs/xfs/linux-2.6/xfs_file.c | 5 +- fs/xfs/linux-2.6/xfs_iops.c | 14 ++- fs/xfs/linux-2.6/xfs_trace.h | 2 +- fs/xfs/xfs_acl.h | 2 - include/linux/fs.h | 3 +- include/linux/generic_acl.h | 1 - include/linux/posix_acl.h | 5 +- include/linux/reiserfs_acl.h | 6 +- include/linux/reiserfs_xattr.h | 2 - mm/shmem.c | 6 -- security/selinux/selinuxfs.c | 1 + sound/core/info.c | 2 +- 76 files changed, 477 insertions(+), 795 deletions(-) ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [git pull] vfs pile 1.5 2011-07-25 18:56 [git pull] vfs pile 1.5 Al Viro @ 2011-07-26 3:35 ` Markus Trippelsdorf 2011-07-26 4:52 ` Linus Torvalds 2011-07-31 3:45 ` Ali Bahar 0 siblings, 2 replies; 20+ messages in thread From: Markus Trippelsdorf @ 2011-07-26 3:35 UTC (permalink / raw) To: Al Viro; +Cc: Linus Torvalds, linux-kernel, linux-fsdevel On 2011.07.25 at 19:56 +0100, Al Viro wrote: > Regression fixes (devtmpfs race, cifs ->d_revalidate() breakage, fsync > fallout), ACL stuff (me/Linus/Christoph), fixes for bugs found while > Linus Torvalds (1): > vfs: move ACL cache lookup into generic code This breaks the build for "# CONFIG_FS_POSIX_ACL is not set" configs: CC fs/namei.o fs/namei.c: In function ‘check_acl’: fs/namei.c:191:10: error: implicit declaration of function ‘negative_cached_acl’ [-Werror=implicit-function-declaration] fs/namei.c:196:2: error: implicit declaration of function ‘get_cached_acl’ [-Werror=implicit-function-declaration] fs/namei.c:196:6: warning: assignment makes pointer from integer without a cast [enabled by default] fs/namei.c:212:11: error: implicit declaration of function ‘set_cached_acl’ [-Werror=implicit-function-declaration] cc1: some warnings being treated as errors -- Markus -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [git pull] vfs pile 1.5 2011-07-26 3:35 ` Markus Trippelsdorf @ 2011-07-26 4:52 ` Linus Torvalds 2011-07-26 5:25 ` Stephen Rothwell 2011-07-26 8:21 ` Markus Trippelsdorf 2011-07-31 3:45 ` Ali Bahar 1 sibling, 2 replies; 20+ messages in thread From: Linus Torvalds @ 2011-07-26 4:52 UTC (permalink / raw) To: Markus Trippelsdorf; +Cc: Al Viro, linux-kernel, linux-fsdevel [-- Attachment #1: Type: text/plain, Size: 402 bytes --] On Mon, Jul 25, 2011 at 8:35 PM, Markus Trippelsdorf <markus@trippelsdorf.de> wrote: > > This breaks the build for "# CONFIG_FS_POSIX_ACL is not set" configs: Of course it does. And as usual, my exhaustive testing was for the allyesconfig case, rather than doing both allyesconfig and allnoconfig like I should. Bah. Does the attached trivial patch fix everything for you? Linus [-- Attachment #2: patch.diff --] [-- Type: text/x-patch, Size: 511 bytes --] fs/namei.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/fs/namei.c b/fs/namei.c index ef00b984fb20..f8c69d373793 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -176,6 +176,7 @@ EXPORT_SYMBOL(putname); static int check_acl(struct inode *inode, int mask) { +#ifdef CONFIG_FS_POSIX_ACL struct posix_acl *acl; /* @@ -219,6 +220,7 @@ static int check_acl(struct inode *inode, int mask) posix_acl_release(acl); return error; } +#endif return -EAGAIN; } ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [git pull] vfs pile 1.5 2011-07-26 4:52 ` Linus Torvalds @ 2011-07-26 5:25 ` Stephen Rothwell 2011-07-26 8:21 ` Markus Trippelsdorf 1 sibling, 0 replies; 20+ messages in thread From: Stephen Rothwell @ 2011-07-26 5:25 UTC (permalink / raw) To: Linus Torvalds; +Cc: Markus Trippelsdorf, Al Viro, linux-kernel, linux-fsdevel [-- Attachment #1: Type: text/plain, Size: 753 bytes --] Hi Linus, On Mon, 25 Jul 2011 21:52:57 -0700 Linus Torvalds <torvalds@linux-foundation.org> wrote: > > On Mon, Jul 25, 2011 at 8:35 PM, Markus Trippelsdorf > <markus@trippelsdorf.de> wrote: > > > > This breaks the build for "# CONFIG_FS_POSIX_ACL is not set" configs: > > Of course it does. And as usual, my exhaustive testing was for the > allyesconfig case, rather than doing both allyesconfig and allnoconfig > like I should. > > Bah. > > Does the attached trivial patch fix everything for you? It fixes my allnoconfig builds in linux-next today (which were broken). Acked-by: Stephen Rothwell <sfr@canb.auug.org.au> -- Cheers, Stephen Rothwell sfr@canb.auug.org.au http://www.canb.auug.org.au/~sfr/ [-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [git pull] vfs pile 1.5 2011-07-26 4:52 ` Linus Torvalds 2011-07-26 5:25 ` Stephen Rothwell @ 2011-07-26 8:21 ` Markus Trippelsdorf 2011-07-26 8:52 ` Steven Liu 1 sibling, 1 reply; 20+ messages in thread From: Markus Trippelsdorf @ 2011-07-26 8:21 UTC (permalink / raw) To: Linus Torvalds; +Cc: Al Viro, linux-kernel, linux-fsdevel, Christoph Hellwig On 2011.07.25 at 21:52 -0700, Linus Torvalds wrote: > On Mon, Jul 25, 2011 at 8:35 PM, Markus Trippelsdorf > <markus@trippelsdorf.de> wrote: > > > > This breaks the build for "# CONFIG_FS_POSIX_ACL is not set" configs: > > Of course it does. And as usual, my exhaustive testing was for the > allyesconfig case, rather than doing both allyesconfig and allnoconfig > like I should. > > Bah. > > Does the attached trivial patch fix everything for you? Yes, thanks. But now xfs fails to build when CONFIG_FS_POSIX_ACL is not set: CC fs/xfs/linux-2.6/xfs_xattr.o fs/xfs/linux-2.6/xfs_iops.c:1025:14: error: ‘xfs_get_acl’ undeclared here (not in a function) -- Markus -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [git pull] vfs pile 1.5 2011-07-26 8:21 ` Markus Trippelsdorf @ 2011-07-26 8:52 ` Steven Liu 2011-07-26 8:58 ` Markus Trippelsdorf 0 siblings, 1 reply; 20+ messages in thread From: Steven Liu @ 2011-07-26 8:52 UTC (permalink / raw) To: Markus Trippelsdorf Cc: Linus Torvalds, Al Viro, linux-kernel, linux-fsdevel, Christoph Hellwig maybe some one have not use #ifdef CONFIG_FS_POSIX_ACL with fs/xfs/linux-2.6/xfs_iops.c:1025:14 :) 2011/7/26 Markus Trippelsdorf <markus@trippelsdorf.de>: > On 2011.07.25 at 21:52 -0700, Linus Torvalds wrote: >> On Mon, Jul 25, 2011 at 8:35 PM, Markus Trippelsdorf >> <markus@trippelsdorf.de> wrote: >> > >> > This breaks the build for "# CONFIG_FS_POSIX_ACL is not set" configs: >> >> Of course it does. And as usual, my exhaustive testing was for the >> allyesconfig case, rather than doing both allyesconfig and allnoconfig >> like I should. >> >> Bah. >> >> Does the attached trivial patch fix everything for you? > > Yes, thanks. > > But now xfs fails to build when CONFIG_FS_POSIX_ACL is not set: > > CC fs/xfs/linux-2.6/xfs_xattr.o > fs/xfs/linux-2.6/xfs_iops.c:1025:14: error: ‘xfs_get_acl’ undeclared here (not in a function) > > -- > Markus > -- > To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [git pull] vfs pile 1.5 2011-07-26 8:52 ` Steven Liu @ 2011-07-26 8:58 ` Markus Trippelsdorf 2011-07-26 9:15 ` [PATCH] xfs: Fix build breakage in xfs_iops.c when CONFIG_FS_POSIX_ACL is not set Markus Trippelsdorf 2011-07-26 15:52 ` [git pull] vfs pile 1.5 Linus Torvalds 0 siblings, 2 replies; 20+ messages in thread From: Markus Trippelsdorf @ 2011-07-26 8:58 UTC (permalink / raw) To: Steven Liu Cc: Linus Torvalds, Al Viro, linux-kernel, linux-fsdevel, Christoph Hellwig On 2011.07.26 at 16:52 +0800, Steven Liu wrote: > 2011/7/26 Markus Trippelsdorf <markus@trippelsdorf.de>: > > On 2011.07.25 at 21:52 -0700, Linus Torvalds wrote: > >> On Mon, Jul 25, 2011 at 8:35 PM, Markus Trippelsdorf > >> <markus@trippelsdorf.de> wrote: > >> > > >> > This breaks the build for "# CONFIG_FS_POSIX_ACL is not set" configs: > >> > >> Of course it does. And as usual, my exhaustive testing was for the > >> allyesconfig case, rather than doing both allyesconfig and allnoconfig > >> like I should. > >> > >> Bah. > >> > >> Does the attached trivial patch fix everything for you? > > > > Yes, thanks. > > > > But now xfs fails to build when CONFIG_FS_POSIX_ACL is not set: > > > > CC fs/xfs/linux-2.6/xfs_xattr.o > > fs/xfs/linux-2.6/xfs_iops.c:1025:14: error: ‘xfs_get_acl’ undeclared here (not in a function) > > maybe some one have not use #ifdef CONFIG_FS_POSIX_ACL > with fs/xfs/linux-2.6/xfs_iops.c:1025:14 #ifdefs are ugly. The following fixes the issue for me: diff --git a/fs/xfs/xfs_acl.h b/fs/xfs/xfs_acl.h index 2c656ef..39632d9 100644 --- a/fs/xfs/xfs_acl.h +++ b/fs/xfs/xfs_acl.h @@ -51,7 +51,10 @@ extern int posix_acl_default_exists(struct inode *inode); extern const struct xattr_handler xfs_xattr_acl_access_handler; extern const struct xattr_handler xfs_xattr_acl_default_handler; #else -# define xfs_get_acl(inode, type) NULL +static inline struct posix_acl *xfs_get_acl(struct inode *inode, int type) +{ + return NULL; +} # define xfs_inherit_acl(inode, default_acl) 0 # define xfs_acl_chmod(inode) 0 # define posix_acl_access_exists(inode) 0 -- Markus -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH] xfs: Fix build breakage in xfs_iops.c when CONFIG_FS_POSIX_ACL is not set 2011-07-26 8:58 ` Markus Trippelsdorf @ 2011-07-26 9:15 ` Markus Trippelsdorf 2011-07-26 9:34 ` Christoph Hellwig 2011-07-29 17:56 ` Alex Elder 2011-07-26 15:52 ` [git pull] vfs pile 1.5 Linus Torvalds 1 sibling, 2 replies; 20+ messages in thread From: Markus Trippelsdorf @ 2011-07-26 9:15 UTC (permalink / raw) To: Steven Liu Cc: Linus Torvalds, Al Viro, linux-kernel, linux-fsdevel, Christoph Hellwig commit 4e34e719e45, that takes the ACL checks to common code, accidentely broke the build when CONFIG_FS_POSIX_ACL is not set: CC fs/xfs/linux-2.6/xfs_iops.o fs/xfs/linux-2.6/xfs_iops.c:1025:14: error: ‘xfs_get_acl’ undeclared here (not in a function) Fix this by declaring xfs_get_acl a static inline function. Signed-off-by: Markus Trippelsdorf <markus@trippelsdorf.de> --- fs/xfs/xfs_acl.h | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/fs/xfs/xfs_acl.h b/fs/xfs/xfs_acl.h index 2c656ef..39632d9 100644 --- a/fs/xfs/xfs_acl.h +++ b/fs/xfs/xfs_acl.h @@ -51,7 +51,10 @@ extern int posix_acl_default_exists(struct inode *inode); extern const struct xattr_handler xfs_xattr_acl_access_handler; extern const struct xattr_handler xfs_xattr_acl_default_handler; #else -# define xfs_get_acl(inode, type) NULL +static inline struct posix_acl *xfs_get_acl(struct inode *inode, int type) +{ + return NULL; +} # define xfs_inherit_acl(inode, default_acl) 0 # define xfs_acl_chmod(inode) 0 # define posix_acl_access_exists(inode) 0 -- Markus -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH] xfs: Fix build breakage in xfs_iops.c when CONFIG_FS_POSIX_ACL is not set 2011-07-26 9:15 ` [PATCH] xfs: Fix build breakage in xfs_iops.c when CONFIG_FS_POSIX_ACL is not set Markus Trippelsdorf @ 2011-07-26 9:34 ` Christoph Hellwig 2011-07-26 9:47 ` Markus Trippelsdorf 2011-07-29 13:17 ` Ingo Molnar 2011-07-29 17:56 ` Alex Elder 1 sibling, 2 replies; 20+ messages in thread From: Christoph Hellwig @ 2011-07-26 9:34 UTC (permalink / raw) To: Markus Trippelsdorf Cc: Steven Liu, Linus Torvalds, Al Viro, linux-kernel, linux-fsdevel, Christoph Hellwig On Tue, Jul 26, 2011 at 11:15:20AM +0200, Markus Trippelsdorf wrote: > commit 4e34e719e45, that takes the ACL checks to common code, > accidentely broke the build when CONFIG_FS_POSIX_ACL is not set: > > CC fs/xfs/linux-2.6/xfs_iops.o > fs/xfs/linux-2.6/xfs_iops.c:1025:14: error: ???xfs_get_acl??? undeclared here (not in a function) > > Fix this by declaring xfs_get_acl a static inline function. Do you have OPTIMIZE_INLINING enabled? In general I dont think we can take the address of an inline function, so without this option I far the code might not compile, in which case we'll need ifdefs around the inode operation assignment in addition to your patch. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] xfs: Fix build breakage in xfs_iops.c when CONFIG_FS_POSIX_ACL is not set 2011-07-26 9:34 ` Christoph Hellwig @ 2011-07-26 9:47 ` Markus Trippelsdorf 2011-07-26 9:49 ` Christoph Hellwig 2011-07-29 13:17 ` Ingo Molnar 1 sibling, 1 reply; 20+ messages in thread From: Markus Trippelsdorf @ 2011-07-26 9:47 UTC (permalink / raw) To: Christoph Hellwig Cc: Steven Liu, Linus Torvalds, Al Viro, linux-kernel, linux-fsdevel On 2011.07.26 at 05:34 -0400, Christoph Hellwig wrote: > On Tue, Jul 26, 2011 at 11:15:20AM +0200, Markus Trippelsdorf wrote: > > commit 4e34e719e45, that takes the ACL checks to common code, > > accidentely broke the build when CONFIG_FS_POSIX_ACL is not set: > > > > CC fs/xfs/linux-2.6/xfs_iops.o > > fs/xfs/linux-2.6/xfs_iops.c:1025:14: error: ???xfs_get_acl??? undeclared here (not in a function) > > > > Fix this by declaring xfs_get_acl a static inline function. > > Do you have OPTIMIZE_INLINING enabled? In general I dont think we can > take the address of an inline function, so without this option I far > the code might not compile, in which case we'll need ifdefs around > the inode operation assignment in addition to your patch. No, "# CONFIG_OPTIMIZE_INLINING is not set". I've tested this patch with gcc 4.5.2, 4.6.1 and 4.7.0 and in all cases the kernel builds just fine. -- Markus ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] xfs: Fix build breakage in xfs_iops.c when CONFIG_FS_POSIX_ACL is not set 2011-07-26 9:47 ` Markus Trippelsdorf @ 2011-07-26 9:49 ` Christoph Hellwig 2011-07-26 16:12 ` Linus Torvalds 0 siblings, 1 reply; 20+ messages in thread From: Christoph Hellwig @ 2011-07-26 9:49 UTC (permalink / raw) To: Markus Trippelsdorf Cc: Christoph Hellwig, Steven Liu, Linus Torvalds, Al Viro, linux-kernel, linux-fsdevel On Tue, Jul 26, 2011 at 11:47:28AM +0200, Markus Trippelsdorf wrote: > No, "# CONFIG_OPTIMIZE_INLINING is not set". > I've tested this patch with gcc 4.5.2, 4.6.1 and 4.7.0 and in all cases > the kernel builds just fine. In that case we might as well just throw it in and try it. In the worst case we'll get one expansion per site that takes the address, but no-acl builds aren't really something worth optimizing for. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] xfs: Fix build breakage in xfs_iops.c when CONFIG_FS_POSIX_ACL is not set 2011-07-26 9:49 ` Christoph Hellwig @ 2011-07-26 16:12 ` Linus Torvalds 0 siblings, 0 replies; 20+ messages in thread From: Linus Torvalds @ 2011-07-26 16:12 UTC (permalink / raw) To: Christoph Hellwig Cc: Markus Trippelsdorf, Steven Liu, Al Viro, linux-kernel, linux-fsdevel On Tue, Jul 26, 2011 at 2:49 AM, Christoph Hellwig <hch@infradead.org> wrote: > > In that case we might as well just throw it in and try it. In the worst > case we'll get one expansion per site that takes the address, but no-acl > builds aren't really something worth optimizing for. If ACL's are enabled on a system level, but not in XFS, this is more than a small optimization. It kills the RCU walk entirely if you just return NULL without setting the cache. Linus -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] xfs: Fix build breakage in xfs_iops.c when CONFIG_FS_POSIX_ACL is not set 2011-07-26 9:34 ` Christoph Hellwig 2011-07-26 9:47 ` Markus Trippelsdorf @ 2011-07-29 13:17 ` Ingo Molnar 1 sibling, 0 replies; 20+ messages in thread From: Ingo Molnar @ 2011-07-29 13:17 UTC (permalink / raw) To: Christoph Hellwig Cc: Markus Trippelsdorf, Steven Liu, Linus Torvalds, Al Viro, linux-kernel, linux-fsdevel * Christoph Hellwig <hch@infradead.org> wrote: > On Tue, Jul 26, 2011 at 11:15:20AM +0200, Markus Trippelsdorf wrote: > > commit 4e34e719e45, that takes the ACL checks to common code, > > accidentely broke the build when CONFIG_FS_POSIX_ACL is not set: > > > > CC fs/xfs/linux-2.6/xfs_iops.o > > fs/xfs/linux-2.6/xfs_iops.c:1025:14: error: ???xfs_get_acl??? undeclared here (not in a function) > > > > Fix this by declaring xfs_get_acl a static inline function. > > Do you have OPTIMIZE_INLINING enabled? In general I dont think we > can take the address of an inline function, so without this option > I far the code might not compile, in which case we'll need ifdefs > around the inode operation assignment in addition to your patch. FYI, this build bug is still present upstream and triggers rather frequently in randconfig testing. Also, according to the timestamps of the guilty commit (4e34e719e45) it had near zero linux-next testing time before it was pushed upstream - could we improve on that please? Thanks, Ingo ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] xfs: Fix build breakage in xfs_iops.c when CONFIG_FS_POSIX_ACL is not set 2011-07-26 9:15 ` [PATCH] xfs: Fix build breakage in xfs_iops.c when CONFIG_FS_POSIX_ACL is not set Markus Trippelsdorf 2011-07-26 9:34 ` Christoph Hellwig @ 2011-07-29 17:56 ` Alex Elder 1 sibling, 0 replies; 20+ messages in thread From: Alex Elder @ 2011-07-29 17:56 UTC (permalink / raw) To: Linus Torvalds Cc: Steven Liu, Al Viro, linux-kernel, linux-fsdevel, Christoph Hellwig, Markus Trippelsdorf On Tue, 2011-07-26 at 11:15 +0200, Markus Trippelsdorf wrote: > commit 4e34e719e45, that takes the ACL checks to common code, > accidentely broke the build when CONFIG_FS_POSIX_ACL is not set: > > CC fs/xfs/linux-2.6/xfs_iops.o > fs/xfs/linux-2.6/xfs_iops.c:1025:14: error: ‘xfs_get_acl’ undeclared here (not in a function) > > Fix this by declaring xfs_get_acl a static inline function. > > Signed-off-by: Markus Trippelsdorf <markus@trippelsdorf.de> > --- > fs/xfs/xfs_acl.h | 5 ++++- > 1 files changed, 4 insertions(+), 1 deletions(-) > > diff --git a/fs/xfs/xfs_acl.h b/fs/xfs/xfs_acl.h > index 2c656ef..39632d9 100644 > --- a/fs/xfs/xfs_acl.h > +++ b/fs/xfs/xfs_acl.h > @@ -51,7 +51,10 @@ extern int posix_acl_default_exists(struct inode *inode); > extern const struct xattr_handler xfs_xattr_acl_access_handler; > extern const struct xattr_handler xfs_xattr_acl_default_handler; > #else > -# define xfs_get_acl(inode, type) NULL > +static inline struct posix_acl *xfs_get_acl(struct inode *inode, int type) > +{ > + return NULL; > +} > # define xfs_inherit_acl(inode, default_acl) 0 > # define xfs_acl_chmod(inode) 0 > # define posix_acl_access_exists(inode) 0 Linus, I have done basic testing of this patch (build, boot, and some simple XFS tests that touch ACLs) using your latest master branch (55f9c40). I tested three combinations: CONFIG_XFS_POSIX_ACL (and CONFIG_FS_POSIX_ACL) on CONFIG_XFS_POSIX_ACL off, CONFIG_FS_POSIX_ACL on CONFIG_XFS_POSIX_ACL and CONFIG_FS_POSIX_ACL both off I confirmed the build problem in the last two configs without this patch, and verified it went away with the patch applied. So all is well from my perspective. Please either take his original patch, or pull from here: git://oss.sgi.com/xfs/xfs for-linus Thanks. Signed-off-by: Alex Elder <aelder@sgi.com> -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [git pull] vfs pile 1.5 2011-07-26 8:58 ` Markus Trippelsdorf 2011-07-26 9:15 ` [PATCH] xfs: Fix build breakage in xfs_iops.c when CONFIG_FS_POSIX_ACL is not set Markus Trippelsdorf @ 2011-07-26 15:52 ` Linus Torvalds 2011-07-26 15:59 ` Markus Trippelsdorf 1 sibling, 1 reply; 20+ messages in thread From: Linus Torvalds @ 2011-07-26 15:52 UTC (permalink / raw) To: Markus Trippelsdorf Cc: Steven Liu, Al Viro, linux-kernel, linux-fsdevel, Christoph Hellwig On Tue, Jul 26, 2011 at 1:58 AM, Markus Trippelsdorf <markus@trippelsdorf.de> wrote: > > #ifdefs are ugly. The following fixes the issue for me: Actually, I think the proper fix is just the following (whitespace-damaged). It's better to just set ".get_acl" to NULL, and it's what we used to do back when it was called "check_acl" (see commit 4e34e719e45). Christoph? Linus --- fs/xfs/xfs_acl.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/fs/xfs/xfs_acl.h b/fs/xfs/xfs_acl.h index 2c656ef49473..b97a6edcab9d 100644 --- a/fs/xfs/xfs_acl.h +++ b/fs/xfs/xfs_acl.h @@ -51,7 +51,7 @@ extern int posix_acl_default_exists(struct inode *inode); extern const struct xattr_handler xfs_xattr_acl_access_handler; extern const struct xattr_handler xfs_xattr_acl_default_handler; #else -# define xfs_get_acl(inode, type) NULL +# define xfs_get_acl NULL # define xfs_inherit_acl(inode, default_acl) 0 # define xfs_acl_chmod(inode) 0 # define posix_acl_access_exists(inode) 0 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [git pull] vfs pile 1.5 2011-07-26 15:52 ` [git pull] vfs pile 1.5 Linus Torvalds @ 2011-07-26 15:59 ` Markus Trippelsdorf 2011-07-26 16:10 ` Linus Torvalds 0 siblings, 1 reply; 20+ messages in thread From: Markus Trippelsdorf @ 2011-07-26 15:59 UTC (permalink / raw) To: Linus Torvalds Cc: Steven Liu, Al Viro, linux-kernel, linux-fsdevel, Christoph Hellwig On 2011.07.26 at 08:52 -0700, Linus Torvalds wrote: > On Tue, Jul 26, 2011 at 1:58 AM, Markus Trippelsdorf > <markus@trippelsdorf.de> wrote: > > > > #ifdefs are ugly. The following fixes the issue for me: > > Actually, I think the proper fix is just the following > (whitespace-damaged). It's better to just set ".get_acl" to NULL, and > it's what we used to do back when it was called "check_acl" (see > commit 4e34e719e45). > Yes, that was my first idea, too. But unfortunately it doesn't work: CC fs/xfs/linux-2.6/xfs_iops.o fs/xfs/linux-2.6/xfs_iops.c: In function ‘xfs_vn_mknod’: fs/xfs/linux-2.6/xfs_iops.c:184:28: error: called object ‘0u’ is not a function make[2]: *** [fs/xfs/linux-2.6/xfs_iops.o] Error 1 -- Markus -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [git pull] vfs pile 1.5 2011-07-26 15:59 ` Markus Trippelsdorf @ 2011-07-26 16:10 ` Linus Torvalds 2011-07-26 16:18 ` Christoph Hellwig 0 siblings, 1 reply; 20+ messages in thread From: Linus Torvalds @ 2011-07-26 16:10 UTC (permalink / raw) To: Markus Trippelsdorf Cc: Steven Liu, Al Viro, linux-kernel, linux-fsdevel, Christoph Hellwig On Tue, Jul 26, 2011 at 8:59 AM, Markus Trippelsdorf <markus@trippelsdorf.de> wrote: > > Yes, that was my first idea, too. But unfortunately it doesn't work: > > CC fs/xfs/linux-2.6/xfs_iops.o > fs/xfs/linux-2.6/xfs_iops.c: In function ‘xfs_vn_mknod’: > fs/xfs/linux-2.6/xfs_iops.c:184:28: error: called object ‘0u’ is not a function > make[2]: *** [fs/xfs/linux-2.6/xfs_iops.o] Error 1 Ahh, we use that elsewhere too. 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. 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. Christoph, Al, comments? Linus -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [git pull] vfs pile 1.5 2011-07-26 16:10 ` Linus Torvalds @ 2011-07-26 16:18 ` Christoph Hellwig 2011-07-26 16:37 ` Linus Torvalds 0 siblings, 1 reply; 20+ messages in thread From: Christoph Hellwig @ 2011-07-26 16:18 UTC (permalink / raw) To: Linus Torvalds Cc: Markus Trippelsdorf, Steven Liu, Al Viro, linux-kernel, linux-fsdevel, Christoph Hellwig 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. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [git pull] vfs pile 1.5 2011-07-26 16:18 ` Christoph Hellwig @ 2011-07-26 16:37 ` Linus Torvalds 0 siblings, 0 replies; 20+ messages in thread From: Linus Torvalds @ 2011-07-26 16:37 UTC (permalink / raw) To: Christoph Hellwig Cc: Markus Trippelsdorf, Steven Liu, Al Viro, linux-kernel, linux-fsdevel On Tue, Jul 26, 2011 at 9:18 AM, Christoph Hellwig <hch@infradead.org> wrote: > > 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. Ahh, ok. That thing is well-hidden behind that IS_POSIXACL() thing.. And it turns out that now that I see it, I shudder. That's another of those potentially expensive inode indirect pointer accesses - but I haven't seen it because my normal test-workload is as the owner of the tree I'm traversing. > 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. Yeah. And these days pretty much every single distribution uses ACL's, which also didn't use to be the case (or even if they used them, it wasn't a requirement: now it tends to be). > 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. Yeah, I had missed the hidden MS_POSIX_ACL test. So I agree, the plain NULL return for "no ACL" works fine. Do we have a signed-off version with a changelog etc? Linus ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [git pull] vfs pile 1.5 2011-07-26 3:35 ` Markus Trippelsdorf 2011-07-26 4:52 ` Linus Torvalds @ 2011-07-31 3:45 ` Ali Bahar 1 sibling, 0 replies; 20+ messages in thread From: Ali Bahar @ 2011-07-31 3:45 UTC (permalink / raw) To: Markus Trippelsdorf; +Cc: Al Viro, Linus Torvalds, linux-kernel, linux-fsdevel On Tue, Jul 26, 2011 at 05:35:35AM +0200, Markus Trippelsdorf wrote: > On 2011.07.25 at 19:56 +0100, Al Viro wrote: > > Regression fixes (devtmpfs race, cifs ->d_revalidate() breakage, fsync > > fallout), ACL stuff (me/Linus/Christoph), fixes for bugs found while > > > Linus Torvalds (1): > > vfs: move ACL cache lookup into generic code > > This breaks the build for "# CONFIG_FS_POSIX_ACL is not set" configs: > > CC fs/namei.o > fs/namei.c: In function ‘check_acl’: > fs/namei.c:191:10: error: implicit declaration of function ‘negative_cached_acl’ [-Werror=implicit-function-declaration] > fs/namei.c:196:2: error: implicit declaration of function ‘get_cached_acl’ [-Werror=implicit-function-declaration] > fs/namei.c:196:6: warning: assignment makes pointer from integer without a cast [enabled by default] > fs/namei.c:212:11: error: implicit declaration of function ‘set_cached_acl’ [-Werror=implicit-function-declaration] > cc1: some warnings being treated as errors Ditto. (I'm using the staging-2.6 tree.) regards, ali > > -- > Markus -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2011-07-31 5:47 UTC | newest] Thread overview: 20+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-07-25 18:56 [git pull] vfs pile 1.5 Al Viro 2011-07-26 3:35 ` Markus Trippelsdorf 2011-07-26 4:52 ` Linus Torvalds 2011-07-26 5:25 ` Stephen Rothwell 2011-07-26 8:21 ` Markus Trippelsdorf 2011-07-26 8:52 ` Steven Liu 2011-07-26 8:58 ` Markus Trippelsdorf 2011-07-26 9:15 ` [PATCH] xfs: Fix build breakage in xfs_iops.c when CONFIG_FS_POSIX_ACL is not set Markus Trippelsdorf 2011-07-26 9:34 ` Christoph Hellwig 2011-07-26 9:47 ` Markus Trippelsdorf 2011-07-26 9:49 ` Christoph Hellwig 2011-07-26 16:12 ` Linus Torvalds 2011-07-29 13:17 ` Ingo Molnar 2011-07-29 17:56 ` Alex Elder 2011-07-26 15:52 ` [git pull] vfs pile 1.5 Linus Torvalds 2011-07-26 15:59 ` Markus Trippelsdorf 2011-07-26 16:10 ` Linus Torvalds 2011-07-26 16:18 ` Christoph Hellwig 2011-07-26 16:37 ` Linus Torvalds 2011-07-31 3:45 ` Ali Bahar
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).