linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] posix_acl.h: define missing ACL functions on non-posix-acl build
@ 2020-11-30  1:44 Sergey Senozhatsky
  2020-11-30  2:00 ` Randy Dunlap
  2020-11-30  6:05 ` kernel test robot
  0 siblings, 2 replies; 10+ messages in thread
From: Sergey Senozhatsky @ 2020-11-30  1:44 UTC (permalink / raw)
  To: Christoph Hellwig, Gustavo A. R. Silva
  Cc: Namjae Jeon, linux-kernel, Sergey Senozhatsky

Some functions that are declared when CONFIG_POSIX_ACL is defined
are not declared when CONFIG_POSIX_ACL is not defined. Add the
missing ones:
  set_posix_acl(), posix_acl_update_mode(), get_cached_acl(),
  get_cached_acl_rcu(), set_cached_acl(), forget_cached_acl().

Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
 include/linux/posix_acl.h | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/include/linux/posix_acl.h b/include/linux/posix_acl.h
index 90797f1b421d..f6d206359da5 100644
--- a/include/linux/posix_acl.h
+++ b/include/linux/posix_acl.h
@@ -117,6 +117,39 @@ static inline int posix_acl_create(struct inode *inode, umode_t *mode,
 static inline void forget_all_cached_acls(struct inode *inode)
 {
 }
+
+static inline int set_posix_acl(struct inode *inode, int type,
+				struct posix_acl *acl)
+{
+	return 0;
+}
+
+static inline int posix_acl_update_mode(struct inode *, umode_t *,
+					struct posix_acl **)
+{
+	return 0;
+}
+
+static inline struct posix_acl *get_cached_acl(struct inode *inode,
+					       int type)
+{
+	return NULL;
+}
+
+static inline struct posix_acl *get_cached_acl_rcu(struct inode *inode,
+						   int type)
+{
+	return NULL;
+}
+
+static inline void set_cached_acl(struct inode *inode, int type,
+				  struct posix_acl *acl)
+{
+}
+
+static inline void forget_cached_acl(struct inode *inode, int type)
+{
+}
 #endif /* CONFIG_FS_POSIX_ACL */
 
 struct posix_acl *get_acl(struct inode *inode, int type);
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] posix_acl.h: define missing ACL functions on non-posix-acl build
  2020-11-30  1:44 [PATCH] posix_acl.h: define missing ACL functions on non-posix-acl build Sergey Senozhatsky
@ 2020-11-30  2:00 ` Randy Dunlap
  2020-11-30  3:15   ` Sergey Senozhatsky
  2020-11-30  6:05 ` kernel test robot
  1 sibling, 1 reply; 10+ messages in thread
From: Randy Dunlap @ 2020-11-30  2:00 UTC (permalink / raw)
  To: Sergey Senozhatsky, Christoph Hellwig, Gustavo A. R. Silva
  Cc: Namjae Jeon, linux-kernel

On 11/29/20 5:44 PM, Sergey Senozhatsky wrote:
> Some functions that are declared when CONFIG_POSIX_ACL is defined
> are not declared when CONFIG_POSIX_ACL is not defined. Add the
> missing ones:
>   set_posix_acl(), posix_acl_update_mode(), get_cached_acl(),
>   get_cached_acl_rcu(), set_cached_acl(), forget_cached_acl().
> 
> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>

Hi,

I can't find CONFIG_POSIX_ACL in the kernel source tree.
Should it be CONFIG_FS_POSIX_ACL ?

How did you test this?

> ---
>  include/linux/posix_acl.h | 33 +++++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 
> diff --git a/include/linux/posix_acl.h b/include/linux/posix_acl.h
> index 90797f1b421d..f6d206359da5 100644
> --- a/include/linux/posix_acl.h
> +++ b/include/linux/posix_acl.h
> @@ -117,6 +117,39 @@ static inline int posix_acl_create(struct inode *inode, umode_t *mode,
>  static inline void forget_all_cached_acls(struct inode *inode)
>  {
>  }
> +
> +static inline int set_posix_acl(struct inode *inode, int type,
> +				struct posix_acl *acl)
> +{
> +	return 0;
> +}
> +
> +static inline int posix_acl_update_mode(struct inode *, umode_t *,
> +					struct posix_acl **)
> +{
> +	return 0;
> +}
> +
> +static inline struct posix_acl *get_cached_acl(struct inode *inode,
> +					       int type)
> +{
> +	return NULL;
> +}
> +
> +static inline struct posix_acl *get_cached_acl_rcu(struct inode *inode,
> +						   int type)
> +{
> +	return NULL;
> +}
> +
> +static inline void set_cached_acl(struct inode *inode, int type,
> +				  struct posix_acl *acl)
> +{
> +}
> +
> +static inline void forget_cached_acl(struct inode *inode, int type)
> +{
> +}
>  #endif /* CONFIG_FS_POSIX_ACL */
>  
>  struct posix_acl *get_acl(struct inode *inode, int type);
> 

thanks.
-- 
~Randy


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] posix_acl.h: define missing ACL functions on non-posix-acl build
  2020-11-30  2:00 ` Randy Dunlap
@ 2020-11-30  3:15   ` Sergey Senozhatsky
  2020-11-30  3:17     ` Sergey Senozhatsky
  0 siblings, 1 reply; 10+ messages in thread
From: Sergey Senozhatsky @ 2020-11-30  3:15 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Sergey Senozhatsky, Christoph Hellwig, Gustavo A. R. Silva,
	Namjae Jeon, linux-kernel

On (20/11/29 18:00), Randy Dunlap wrote:
> On 11/29/20 5:44 PM, Sergey Senozhatsky wrote:
> > Some functions that are declared when CONFIG_POSIX_ACL is defined
> > are not declared when CONFIG_POSIX_ACL is not defined. Add the
> > missing ones:
> >   set_posix_acl(), posix_acl_update_mode(), get_cached_acl(),
> >   get_cached_acl_rcu(), set_cached_acl(), forget_cached_acl().
> > 
> > Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> 
> Hi,
> 
> I can't find CONFIG_POSIX_ACL in the kernel source tree.
> Should it be CONFIG_FS_POSIX_ACL ?

Oh, yes, CONFIG_POSIX_ACL. My bad.

> How did you test this?

You know what - scratch this patch. Sorry for the noise.

Some of the posix_acl.h functions are guarded by ifdef/ifndef
CONFIG_FS_POSIX_ACL, and some are not. This can break the build
if the code in question doesn't use ifdef CONFIG_FS_POSIX_ACL
(which happens with our code). But this patch is not enough,
apparently, we need to add ifdef CONFIG_FS_POSIX_ACL to our
code anyway, because of, for instance, posix_acl_alloc() which
is undefined for !FS_POSIX_ACL builds. Sorry for the noise.

	-ss

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] posix_acl.h: define missing ACL functions on non-posix-acl build
  2020-11-30  3:15   ` Sergey Senozhatsky
@ 2020-11-30  3:17     ` Sergey Senozhatsky
  2020-11-30  3:37       ` Sergey Senozhatsky
  0 siblings, 1 reply; 10+ messages in thread
From: Sergey Senozhatsky @ 2020-11-30  3:17 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Christoph Hellwig, Gustavo A. R. Silva, Namjae Jeon,
	linux-kernel, Sergey Senozhatsky

On (20/11/30 12:15), Sergey Senozhatsky wrote:
> On (20/11/29 18:00), Randy Dunlap wrote:
> > On 11/29/20 5:44 PM, Sergey Senozhatsky wrote:
> > > Some functions that are declared when CONFIG_POSIX_ACL is defined
> > > are not declared when CONFIG_POSIX_ACL is not defined. Add the
> > > missing ones:
> > >   set_posix_acl(), posix_acl_update_mode(), get_cached_acl(),
> > >   get_cached_acl_rcu(), set_cached_acl(), forget_cached_acl().
> > > 
> > > Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> > 
> > Hi,
> > 
> > I can't find CONFIG_POSIX_ACL in the kernel source tree.
> > Should it be CONFIG_FS_POSIX_ACL ?
> 
> Oh, yes, CONFIG_POSIX_ACL. My bad.

     ...   CONFIG_FS_POSIX_ACL. I did it again.

	-ss

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] posix_acl.h: define missing ACL functions on non-posix-acl build
  2020-11-30  3:17     ` Sergey Senozhatsky
@ 2020-11-30  3:37       ` Sergey Senozhatsky
  2020-11-30  4:00         ` Randy Dunlap
  0 siblings, 1 reply; 10+ messages in thread
From: Sergey Senozhatsky @ 2020-11-30  3:37 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Christoph Hellwig, Gustavo A. R. Silva, Namjae Jeon,
	linux-kernel, Sergey Senozhatsky

A quick question, shouldn't there be dummy definitions for
the EXPORT_SYMBOL-s? So that external modules can be modprobed
and used.

Some of posix_acl exported symbols have dummy definitions,
others don't.

E.g. posix_acl_create() is exported symbol and it's defined for
both FS_POSIX_ACL and !FS_POSIX_ACL. While exported set_posix_acl()
is defined only for FS_POSIX_ACL config.

---

diff --git a/include/linux/posix_acl.h b/include/linux/posix_acl.h
index 90797f1b421d..8a6c77a69761 100644
--- a/include/linux/posix_acl.h
+++ b/include/linux/posix_acl.h
@@ -59,19 +59,19 @@ posix_acl_release(struct posix_acl *acl)
 
 /* posix_acl.c */
 
+extern int posix_acl_permission(struct inode *, const struct posix_acl *, int);
+
+extern struct posix_acl *get_posix_acl(struct inode *, int);
+
+#ifdef CONFIG_FS_POSIX_ACL
 extern void posix_acl_init(struct posix_acl *, int);
 extern struct posix_acl *posix_acl_alloc(int, gfp_t);
-extern int posix_acl_valid(struct user_namespace *, const struct posix_acl *);
-extern int posix_acl_permission(struct inode *, const struct posix_acl *, int);
-extern struct posix_acl *posix_acl_from_mode(umode_t, gfp_t);
 extern int posix_acl_equiv_mode(const struct posix_acl *, umode_t *);
+extern struct posix_acl *posix_acl_from_mode(umode_t, gfp_t);
 extern int __posix_acl_create(struct posix_acl **, gfp_t, umode_t *);
 extern int __posix_acl_chmod(struct posix_acl **, gfp_t, umode_t);
-
-extern struct posix_acl *get_posix_acl(struct inode *, int);
 extern int set_posix_acl(struct inode *, int, struct posix_acl *);
-
-#ifdef CONFIG_FS_POSIX_ACL
+extern int posix_acl_valid(struct user_namespace *, const struct posix_acl *);
 extern int posix_acl_chmod(struct inode *, umode_t);
 extern int posix_acl_create(struct inode *, umode_t *, struct posix_acl **,
 		struct posix_acl **);
@@ -91,18 +91,61 @@ static inline void cache_no_acl(struct inode *inode)
 	inode->i_acl = NULL;
 	inode->i_default_acl = NULL;
 }
+
+struct posix_acl *get_acl(struct inode *inode, int type);
 #else
+static inline void posix_acl_init(struct posix_acl *, int)
+{
+}
+
+static inline struct posix_acl *posix_acl_alloc(int, gfp_t)
+{
+	return NULL;
+}
+
+static inline int posix_acl_valid(struct user_namespace *,
+				  const struct posix_acl *)
+{
+	return 0;
+}
+
+static inline int posix_acl_equiv_mode(const struct posix_acl *, umode_t *)
+{
+	return 0;
+}
+
+static inline struct posix_acl *posix_acl_from_mode(umode_t, gfp_t)
+{
+	return NULL;
+}
+
 static inline int posix_acl_chmod(struct inode *inode, umode_t mode)
 {
 	return 0;
 }
 
+static inline int set_posix_acl(struct inode *, int, struct posix_acl *)
+{
+	return 0;
+}
+
 #define simple_set_acl		NULL
 
 static inline int simple_acl_create(struct inode *dir, struct inode *inode)
 {
 	return 0;
 }
+
+static inline int __posix_acl_create(struct posix_acl **, gfp_t, umode_t *)
+{
+	return 0;
+}
+
+static inline int __posix_acl_chmod(struct posix_acl **, gfp_t, umode_t)
+{
+	return 0;
+}
+
 static inline void cache_no_acl(struct inode *inode)
 {
 }
@@ -117,8 +160,38 @@ static inline int posix_acl_create(struct inode *inode, umode_t *mode,
 static inline void forget_all_cached_acls(struct inode *inode)
 {
 }
+
+static inline struct posix_acl *get_cached_acl(struct inode *inode, int type)
+{
+	return NULL;
+}
+
+static inline struct posix_acl *get_cached_acl_rcu(struct inode *inode,
+						   int type)
+{
+	return NULL;
+}
+
+static inline void set_cached_acl(struct inode *inode, int type,
+				  struct posix_acl *acl)
+{
+}
+
+static inline void forget_cached_acl(struct inode *inode, int type)
+{
+}
+
+static inline struct posix_acl *get_acl(struct inode *inode, int type)
+{
+	return NULL;
+}
+
+static inline int posix_acl_update_mode(struct inode *, umode_t *,
+					struct posix_acl **)
+{
+	return 0;
+}
 #endif /* CONFIG_FS_POSIX_ACL */
 
-struct posix_acl *get_acl(struct inode *inode, int type);
 
 #endif  /* __LINUX_POSIX_ACL_H */

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] posix_acl.h: define missing ACL functions on non-posix-acl build
  2020-11-30  3:37       ` Sergey Senozhatsky
@ 2020-11-30  4:00         ` Randy Dunlap
  2020-11-30 13:09           ` Andreas Gruenbacher
  0 siblings, 1 reply; 10+ messages in thread
From: Randy Dunlap @ 2020-11-30  4:00 UTC (permalink / raw)
  To: Sergey Senozhatsky
  Cc: Christoph Hellwig, Gustavo A. R. Silva, Namjae Jeon,
	linux-kernel, Andreas Gruenbacher, Al Viro

On 11/29/20 7:37 PM, Sergey Senozhatsky wrote:
> A quick question, shouldn't there be dummy definitions for
> the EXPORT_SYMBOL-s? So that external modules can be modprobed
> and used.
> 
> Some of posix_acl exported symbols have dummy definitions,
> others don't.
> 
> E.g. posix_acl_create() is exported symbol and it's defined for
> both FS_POSIX_ACL and !FS_POSIX_ACL. While exported set_posix_acl()
> is defined only for FS_POSIX_ACL config.

Hi,

Currently CONFIG_FS_POSIX_ACL differences seem to be handled in
each source file as needed:

fs/inode.c:#ifdef CONFIG_FS_POSIX_ACL
fs/inode.c:#ifdef CONFIG_FS_POSIX_ACL
fs/namei.c:#ifdef CONFIG_FS_POSIX_ACL
fs/overlayfs/dir.c:     if (!IS_ENABLED(CONFIG_FS_POSIX_ACL) || !acl)
fs/overlayfs/inode.c:   if (!IS_ENABLED(CONFIG_FS_POSIX_ACL) || !IS_POSIXACL(realinode))
fs/overlayfs/inode.c:#ifdef CONFIG_FS_POSIX_ACL
fs/overlayfs/super.c:#ifdef CONFIG_FS_POSIX_ACL
fs/xattr.c:#ifdef CONFIG_FS_POSIX_ACL
include/linux/evm.h:#ifdef CONFIG_FS_POSIX_ACL
include/linux/fs.h:#ifdef CONFIG_FS_POSIX_ACL
include/linux/posix_acl.h:#ifdef CONFIG_FS_POSIX_ACL
include/linux/posix_acl.h:#endif /* CONFIG_FS_POSIX_ACL */
include/linux/posix_acl_xattr.h:#ifdef CONFIG_FS_POSIX_ACL

However, I have no objection to your patch.

I am adding Andreas & Al for their viewpoints.


> ---
> 
> diff --git a/include/linux/posix_acl.h b/include/linux/posix_acl.h
> index 90797f1b421d..8a6c77a69761 100644
> --- a/include/linux/posix_acl.h
> +++ b/include/linux/posix_acl.h
> @@ -59,19 +59,19 @@ posix_acl_release(struct posix_acl *acl)
>  
>  /* posix_acl.c */
>  
> +extern int posix_acl_permission(struct inode *, const struct posix_acl *, int);
> +
> +extern struct posix_acl *get_posix_acl(struct inode *, int);
> +
> +#ifdef CONFIG_FS_POSIX_ACL
>  extern void posix_acl_init(struct posix_acl *, int);
>  extern struct posix_acl *posix_acl_alloc(int, gfp_t);
> -extern int posix_acl_valid(struct user_namespace *, const struct posix_acl *);
> -extern int posix_acl_permission(struct inode *, const struct posix_acl *, int);
> -extern struct posix_acl *posix_acl_from_mode(umode_t, gfp_t);
>  extern int posix_acl_equiv_mode(const struct posix_acl *, umode_t *);
> +extern struct posix_acl *posix_acl_from_mode(umode_t, gfp_t);
>  extern int __posix_acl_create(struct posix_acl **, gfp_t, umode_t *);
>  extern int __posix_acl_chmod(struct posix_acl **, gfp_t, umode_t);
> -
> -extern struct posix_acl *get_posix_acl(struct inode *, int);
>  extern int set_posix_acl(struct inode *, int, struct posix_acl *);
> -
> -#ifdef CONFIG_FS_POSIX_ACL
> +extern int posix_acl_valid(struct user_namespace *, const struct posix_acl *);
>  extern int posix_acl_chmod(struct inode *, umode_t);
>  extern int posix_acl_create(struct inode *, umode_t *, struct posix_acl **,
>  		struct posix_acl **);
> @@ -91,18 +91,61 @@ static inline void cache_no_acl(struct inode *inode)
>  	inode->i_acl = NULL;
>  	inode->i_default_acl = NULL;
>  }
> +
> +struct posix_acl *get_acl(struct inode *inode, int type);
>  #else
> +static inline void posix_acl_init(struct posix_acl *, int)
> +{
> +}
> +
> +static inline struct posix_acl *posix_acl_alloc(int, gfp_t)
> +{
> +	return NULL;
> +}
> +
> +static inline int posix_acl_valid(struct user_namespace *,
> +				  const struct posix_acl *)
> +{
> +	return 0;
> +}
> +
> +static inline int posix_acl_equiv_mode(const struct posix_acl *, umode_t *)
> +{
> +	return 0;
> +}
> +
> +static inline struct posix_acl *posix_acl_from_mode(umode_t, gfp_t)
> +{
> +	return NULL;
> +}
> +
>  static inline int posix_acl_chmod(struct inode *inode, umode_t mode)
>  {
>  	return 0;
>  }
>  
> +static inline int set_posix_acl(struct inode *, int, struct posix_acl *)
> +{
> +	return 0;
> +}
> +
>  #define simple_set_acl		NULL
>  
>  static inline int simple_acl_create(struct inode *dir, struct inode *inode)
>  {
>  	return 0;
>  }
> +
> +static inline int __posix_acl_create(struct posix_acl **, gfp_t, umode_t *)
> +{
> +	return 0;
> +}
> +
> +static inline int __posix_acl_chmod(struct posix_acl **, gfp_t, umode_t)
> +{
> +	return 0;
> +}
> +
>  static inline void cache_no_acl(struct inode *inode)
>  {
>  }
> @@ -117,8 +160,38 @@ static inline int posix_acl_create(struct inode *inode, umode_t *mode,
>  static inline void forget_all_cached_acls(struct inode *inode)
>  {
>  }
> +
> +static inline struct posix_acl *get_cached_acl(struct inode *inode, int type)
> +{
> +	return NULL;
> +}
> +
> +static inline struct posix_acl *get_cached_acl_rcu(struct inode *inode,
> +						   int type)
> +{
> +	return NULL;
> +}
> +
> +static inline void set_cached_acl(struct inode *inode, int type,
> +				  struct posix_acl *acl)
> +{
> +}
> +
> +static inline void forget_cached_acl(struct inode *inode, int type)
> +{
> +}
> +
> +static inline struct posix_acl *get_acl(struct inode *inode, int type)
> +{
> +	return NULL;
> +}
> +
> +static inline int posix_acl_update_mode(struct inode *, umode_t *,
> +					struct posix_acl **)
> +{
> +	return 0;
> +}
>  #endif /* CONFIG_FS_POSIX_ACL */
>  
> -struct posix_acl *get_acl(struct inode *inode, int type);
>  
>  #endif  /* __LINUX_POSIX_ACL_H */
> 


-- 
~Randy


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] posix_acl.h: define missing ACL functions on non-posix-acl build
  2020-11-30  1:44 [PATCH] posix_acl.h: define missing ACL functions on non-posix-acl build Sergey Senozhatsky
  2020-11-30  2:00 ` Randy Dunlap
@ 2020-11-30  6:05 ` kernel test robot
  1 sibling, 0 replies; 10+ messages in thread
From: kernel test robot @ 2020-11-30  6:05 UTC (permalink / raw)
  To: Sergey Senozhatsky, Christoph Hellwig, Gustavo A. R. Silva
  Cc: kbuild-all, clang-built-linux, Namjae Jeon, linux-kernel,
	Sergey Senozhatsky

[-- Attachment #1: Type: text/plain, Size: 3319 bytes --]

Hi Sergey,

I love your patch! Perhaps something to improve:

[auto build test WARNING on hch-configfs/for-next]
[also build test WARNING on linux/master linus/master v5.10-rc6 next-20201127]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Sergey-Senozhatsky/posix_acl-h-define-missing-ACL-functions-on-non-posix-acl-build/20201130-095018
base:   git://git.infradead.org/users/hch/configfs.git for-next
config: s390-randconfig-r005-20201130 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project dfcf1acf13226be0f599a7ab6b395b66dc9545d6)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install s390 cross compiling tool for clang build
        # apt-get install binutils-s390x-linux-gnu
        # https://github.com/0day-ci/linux/commit/377ebc564d08d21a4bc83fecc7f1238e342823d1
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Sergey-Senozhatsky/posix_acl-h-define-missing-ACL-functions-on-non-posix-acl-build/20201130-095018
        git checkout 377ebc564d08d21a4bc83fecc7f1238e342823d1
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=s390 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from fs/overlayfs/super.c:16:
   In file included from include/linux/posix_acl_xattr.h:15:
   include/linux/posix_acl.h:121:19: error: static declaration of 'set_posix_acl' follows non-static declaration
   static inline int set_posix_acl(struct inode *inode, int type,
                     ^
   include/linux/posix_acl.h:72:12: note: previous declaration is here
   extern int set_posix_acl(struct inode *, int, struct posix_acl *);
              ^
>> include/linux/posix_acl.h:127:55: warning: omitting the parameter name in a function definition is a C2x extension [-Wc2x-extensions]
   static inline int posix_acl_update_mode(struct inode *, umode_t *,
                                                         ^
   include/linux/posix_acl.h:127:66: warning: omitting the parameter name in a function definition is a C2x extension [-Wc2x-extensions]
   static inline int posix_acl_update_mode(struct inode *, umode_t *,
                                                                    ^
   include/linux/posix_acl.h:128:25: warning: omitting the parameter name in a function definition is a C2x extension [-Wc2x-extensions]
                                           struct posix_acl **)
                                                              ^
   3 warnings and 1 error generated.

vim +127 include/linux/posix_acl.h

   126	
 > 127	static inline int posix_acl_update_mode(struct inode *, umode_t *,
   128						struct posix_acl **)
   129	{
   130		return 0;
   131	}
   132	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28305 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] posix_acl.h: define missing ACL functions on non-posix-acl build
  2020-11-30  4:00         ` Randy Dunlap
@ 2020-11-30 13:09           ` Andreas Gruenbacher
  2020-11-30 13:13             ` Andreas Gruenbacher
  2020-11-30 13:28             ` Sergey Senozhatsky
  0 siblings, 2 replies; 10+ messages in thread
From: Andreas Gruenbacher @ 2020-11-30 13:09 UTC (permalink / raw)
  To: Sergey Senozhatsky
  Cc: Randy Dunlap, Christoph Hellwig, Gustavo A. R. Silva,
	Namjae Jeon, LKML, Al Viro, Jan Kara

On Mon, Nov 30, 2020 at 5:29 AM Randy Dunlap <rdunlap@infradead.org> wrote:
> On 11/29/20 7:37 PM, Sergey Senozhatsky wrote:
> > A quick question, shouldn't there be dummy definitions for
> > the EXPORT_SYMBOL-s? So that external modules can be modprobed
> > and used.
> >
> > Some of posix_acl exported symbols have dummy definitions,
> > others don't.
> >
> > E.g. posix_acl_create() is exported symbol and it's defined for
> > both FS_POSIX_ACL and !FS_POSIX_ACL. While exported set_posix_acl()
> > is defined only for FS_POSIX_ACL config.

This is to keep the amount of ifdefs in the code reasonably low: by
defining posix_acl_create as a dummy inline function like that, inode
creation in filesystems can be implemented without any ifdefs as in
jffs2_init_acl_pre whether or not CONFIG_FS_POSIX_ACL is enabled, for
example. Have a look at different filesystems to see how they avoid
using POSIX ACL code when that feature is disabled.

Note that ext2 / ext4 could be built without POSIX ACL support in the
past. That's at least broken since the following two commits though:

  commit 59fed3bf8a461 ("ext2: cache NULL when both default_acl and
acl are NULL")
  commit 6fd941784b8ac ("ext4: cache NULL when both default_acl and
acl are NULL")

> Hi,
>
> Currently CONFIG_FS_POSIX_ACL differences seem to be handled in
> each source file as needed:
>
> fs/inode.c:#ifdef CONFIG_FS_POSIX_ACL
> fs/inode.c:#ifdef CONFIG_FS_POSIX_ACL
> fs/namei.c:#ifdef CONFIG_FS_POSIX_ACL
> fs/overlayfs/dir.c:     if (!IS_ENABLED(CONFIG_FS_POSIX_ACL) || !acl)
> fs/overlayfs/inode.c:   if (!IS_ENABLED(CONFIG_FS_POSIX_ACL) || !IS_POSIXACL(realinode))
> fs/overlayfs/inode.c:#ifdef CONFIG_FS_POSIX_ACL
> fs/overlayfs/super.c:#ifdef CONFIG_FS_POSIX_ACL
> fs/xattr.c:#ifdef CONFIG_FS_POSIX_ACL
> include/linux/evm.h:#ifdef CONFIG_FS_POSIX_ACL
> include/linux/fs.h:#ifdef CONFIG_FS_POSIX_ACL
> include/linux/posix_acl.h:#ifdef CONFIG_FS_POSIX_ACL
> include/linux/posix_acl.h:#endif /* CONFIG_FS_POSIX_ACL */
> include/linux/posix_acl_xattr.h:#ifdef CONFIG_FS_POSIX_ACL
>
> However, I have no objection to your patch.
>
> I am adding Andreas & Al for their viewpoints.

Sergey, what actual problem is your patch trying to solve? It sounds
like this is either theoretical and pointless, or you're trying to
build an external module that uses POSIX ACL functions that shouldn't
be needed when CONFIG_FS_POSIX_ACL is disabled. In the latter case,
the external module will just end up including dead code, so the
module should be fixed instead.

Thanks,
Andreas


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] posix_acl.h: define missing ACL functions on non-posix-acl build
  2020-11-30 13:09           ` Andreas Gruenbacher
@ 2020-11-30 13:13             ` Andreas Gruenbacher
  2020-11-30 13:28             ` Sergey Senozhatsky
  1 sibling, 0 replies; 10+ messages in thread
From: Andreas Gruenbacher @ 2020-11-30 13:13 UTC (permalink / raw)
  To: Sergey Senozhatsky
  Cc: Randy Dunlap, Christoph Hellwig, Gustavo A. R. Silva,
	Namjae Jeon, LKML, Al Viro, Jan Kara

On Mon, Nov 30, 2020 at 2:09 PM Andreas Gruenbacher <agruenba@redhat.com> wrote:
> Note that ext2 / ext4 could be built without POSIX ACL support in the
> past. That's at least broken since the following two commits though:
>
>   commit 59fed3bf8a461 ("ext2: cache NULL when both default_acl and
> acl are NULL")
>   commit 6fd941784b8ac ("ext4: cache NULL when both default_acl and
> acl are NULL")

Scratch that, this is in fs/ext[24]/acl.c which is only included when
CONFIG_FS_POSIX_ACL is defined.

Thanks,
Andreas


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] posix_acl.h: define missing ACL functions on non-posix-acl build
  2020-11-30 13:09           ` Andreas Gruenbacher
  2020-11-30 13:13             ` Andreas Gruenbacher
@ 2020-11-30 13:28             ` Sergey Senozhatsky
  1 sibling, 0 replies; 10+ messages in thread
From: Sergey Senozhatsky @ 2020-11-30 13:28 UTC (permalink / raw)
  To: Andreas Gruenbacher
  Cc: Sergey Senozhatsky, Randy Dunlap, Christoph Hellwig,
	Gustavo A. R. Silva, Namjae Jeon, LKML, Al Viro, Jan Kara

Hi,

On (20/11/30 14:09), Andreas Gruenbacher wrote:
> 
> Sergey, what actual problem is your patch trying to solve? It sounds
> like this is either theoretical and pointless, or you're trying to
> build an external module that uses POSIX ACL functions that shouldn't
> be needed when CONFIG_FS_POSIX_ACL is disabled.

It's an external module, that OpenWRT folks build with !FS_POSIX_ACL.
It compiles just fine, but modprobe fails because there are several
exported ACL symbols that don't provide dummy definitions (which the
module in question didn't guard with ifdef-s).

> In the latter case, the external module will just end up including dead
> code, so the module should be fixed instead.

ifdef-s work. But since posix_acl.h already provides some dummy
definitions for exported symbols, I thought that that list can
be extended (become complete).

	-ss

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-11-30 13:28 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-30  1:44 [PATCH] posix_acl.h: define missing ACL functions on non-posix-acl build Sergey Senozhatsky
2020-11-30  2:00 ` Randy Dunlap
2020-11-30  3:15   ` Sergey Senozhatsky
2020-11-30  3:17     ` Sergey Senozhatsky
2020-11-30  3:37       ` Sergey Senozhatsky
2020-11-30  4:00         ` Randy Dunlap
2020-11-30 13:09           ` Andreas Gruenbacher
2020-11-30 13:13             ` Andreas Gruenbacher
2020-11-30 13:28             ` Sergey Senozhatsky
2020-11-30  6:05 ` kernel test robot

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