[git pull] vfs pile 1.5

[git pull] vfs pile 1.5

[Al Viro]

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(-)

Re: [git pull] vfs pile 1.5

[Markus Trippelsdorf]

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

Re: [git pull] vfs pile 1.5

[Linus Torvalds]

[-- 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;
 }

Re: [git pull] vfs pile 1.5

[Stephen Rothwell]

[-- 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 --]

Re: [git pull] vfs pile 1.5

[Markus Trippelsdorf]

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

Re: [git pull] vfs pile 1.5

[Steven Liu]

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

Re: [git pull] vfs pile 1.5

[Markus Trippelsdorf]

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

[PATCH] xfs: Fix build breakage in xfs_iops.c when CONFIG_FS_POSIX_ACL is not set

[Markus Trippelsdorf]

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

Re: [PATCH] xfs: Fix build breakage in xfs_iops.c when CONFIG_FS_POSIX_ACL is not set

[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.

Re: [PATCH] xfs: Fix build breakage in xfs_iops.c when CONFIG_FS_POSIX_ACL is not set

[Markus Trippelsdorf]

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

Re: [PATCH] xfs: Fix build breakage in xfs_iops.c when CONFIG_FS_POSIX_ACL is not set

[Christoph Hellwig]

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.

Re: [git pull] vfs pile 1.5

[Linus Torvalds]

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

Re: [git pull] vfs pile 1.5

[Markus Trippelsdorf]

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

Re: [git pull] vfs pile 1.5

[Linus Torvalds]

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

Re: [PATCH] xfs: Fix build breakage in xfs_iops.c when CONFIG_FS_POSIX_ACL is not set

[Linus Torvalds]

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

Re: [git pull] vfs pile 1.5

[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.

Re: [git pull] vfs pile 1.5

[Linus Torvalds]

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

Re: [PATCH] xfs: Fix build breakage in xfs_iops.c when CONFIG_FS_POSIX_ACL is not set

[Ingo Molnar]

* 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

Re: [PATCH] xfs: Fix build breakage in xfs_iops.c when CONFIG_FS_POSIX_ACL is not set

[Alex Elder]

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

Re: [git pull] vfs pile 1.5

[Ali Bahar]

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