[PATCH 1/2] ext4: only set project inherit bit for directory

[PATCH 1/2] ext4: only set project inherit bit for directory

[Wang Shilong]

From: Wang Shilong <wshilong@ddn.com>

It doesn't make any sense to have project inherit bits
for regular files, even though this won't cause any
problem, but it is better fix this.

Cc: Andreas Dilger <adilger@dilger.ca>
Signed-off-by: Wang Shilong <wshilong@ddn.com>
---
 fs/ext4/ext4.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 1cb67859e051..ceb74093e138 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -421,7 +421,8 @@ struct flex_groups {
 			   EXT4_PROJINHERIT_FL | EXT4_CASEFOLD_FL)
 
 /* Flags that are appropriate for regular files (all but dir-specific ones). */
-#define EXT4_REG_FLMASK (~(EXT4_DIRSYNC_FL | EXT4_TOPDIR_FL | EXT4_CASEFOLD_FL))
+#define EXT4_REG_FLMASK (~(EXT4_DIRSYNC_FL | EXT4_TOPDIR_FL | EXT4_CASEFOLD_FL |\
+			   EXT4_PROJINHERIT_FL))
 
 /* Flags that are appropriate for non-directories/regular files. */
 #define EXT4_OTHER_FLMASK (EXT4_NODUMP_FL | EXT4_NOATIME_FL)
-- 
2.21.0

[PATCH 2/2] f2fs: only set project inherit bit for directory

[Wang Shilong]

From: Wang Shilong <wshilong@ddn.com>

It doesn't make any sense to have project inherit bits
for regular files, even though this won't cause any
problem, but it is better fix this.

Cc: Andreas Dilger <adilger@dilger.ca>
Signed-off-by: Wang Shilong <wshilong@ddn.com>
---
 fs/f2fs/f2fs.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 06b89a9862ab..f02ebecb68ea 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -2370,7 +2370,8 @@ static inline void f2fs_change_bit(unsigned int nr, char *addr)
 			   F2FS_PROJINHERIT_FL)
 
 /* Flags that are appropriate for regular files (all but dir-specific ones). */
-#define F2FS_REG_FLMASK		(~(F2FS_DIRSYNC_FL | F2FS_TOPDIR_FL))
+#define F2FS_REG_FLMASK 	(~(F2FS_DIRSYNC_FL | F2FS_TOPDIR_FL |\
+				   F2FS_PROJINHERIT_FL))
 
 /* Flags that are appropriate for non-directories/regular files. */
 #define F2FS_OTHER_FLMASK	(F2FS_NODUMP_FL | F2FS_NOATIME_FL)
-- 
2.21.0

Re: [PATCH 2/2] f2fs: only set project inherit bit for directory

[Chao Yu]

On 2019/6/6 12:32, Wang Shilong wrote:
> From: Wang Shilong <wshilong@ddn.com>
> 
> It doesn't make any sense to have project inherit bits
> for regular files, even though this won't cause any
> problem, but it is better fix this.
> 
> Cc: Andreas Dilger <adilger@dilger.ca>
> Signed-off-by: Wang Shilong <wshilong@ddn.com>
> ---
>  fs/f2fs/f2fs.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 06b89a9862ab..f02ebecb68ea 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -2370,7 +2370,8 @@ static inline void f2fs_change_bit(unsigned int nr, char *addr)
>  			   F2FS_PROJINHERIT_FL)
>  
>  /* Flags that are appropriate for regular files (all but dir-specific ones). */
> -#define F2FS_REG_FLMASK		(~(F2FS_DIRSYNC_FL | F2FS_TOPDIR_FL))
> +#define F2FS_REG_FLMASK 	(~(F2FS_DIRSYNC_FL | F2FS_TOPDIR_FL |\
> +				   F2FS_PROJINHERIT_FL))

Hi Shilong,

Could you please add below diff as ext4 did?

diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index efdafa886510..295ca5ed42d9 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -1759,6 +1759,9 @@ static int f2fs_ioc_getflags(struct file *filp, unsigned
long arg)

 	fsflags &= F2FS_GETTABLE_FS_FL;

+	if (S_ISREG(inode->i_mode))
+		fsflags &= ~FS_PROJINHERIT_FL;
+
 	return put_user(fsflags, (int __user *)arg);
 }

Thanks,

>  
>  /* Flags that are appropriate for non-directories/regular files. */
>  #define F2FS_OTHER_FLMASK	(F2FS_NODUMP_FL | F2FS_NOATIME_FL)
> 

Re: [f2fs-dev] [PATCH 2/2] f2fs: only set project inherit bit for directory

[Chao Yu]

On 2019/6/6 12:32, Wang Shilong wrote:
> From: Wang Shilong <wshilong@ddn.com>
> 
> It doesn't make any sense to have project inherit bits
> for regular files, even though this won't cause any
> problem, but it is better fix this.
> 
> Cc: Andreas Dilger <adilger@dilger.ca>
> Signed-off-by: Wang Shilong <wshilong@ddn.com>
> ---
>  fs/f2fs/f2fs.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 06b89a9862ab..f02ebecb68ea 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -2370,7 +2370,8 @@ static inline void f2fs_change_bit(unsigned int nr, char *addr)
>  			   F2FS_PROJINHERIT_FL)
>  
>  /* Flags that are appropriate for regular files (all but dir-specific ones). */
> -#define F2FS_REG_FLMASK		(~(F2FS_DIRSYNC_FL | F2FS_TOPDIR_FL))
> +#define F2FS_REG_FLMASK 	(~(F2FS_DIRSYNC_FL | F2FS_TOPDIR_FL |\
> +				   F2FS_PROJINHERIT_FL))

Hi Shilong,

Could you please add below diff as ext4 did?

diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index efdafa886510..295ca5ed42d9 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -1759,6 +1759,9 @@ static int f2fs_ioc_getflags(struct file *filp, unsigned
long arg)

 	fsflags &= F2FS_GETTABLE_FS_FL;

+	if (S_ISREG(inode->i_mode))
+		fsflags &= ~FS_PROJINHERIT_FL;
+
 	return put_user(fsflags, (int __user *)arg);
 }

Thanks,

>  
>  /* Flags that are appropriate for non-directories/regular files. */
>  #define F2FS_OTHER_FLMASK	(F2FS_NODUMP_FL | F2FS_NOATIME_FL)
> 


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

回复: [PATCH 2/2] f2fs: only set project inherit bit for directory

[Wang Shilong via Linux-f2fs-devel]

Hi Chao,

 I just sent a V2, but I think we'd better do that when reading inode, for two reasons:

1) not only F2FS_IOC_GETFLAGS need filter flags but also F2FS_IOC_FSGETXATTR need that, so the amended parts is not enough IMO.

2) doing that by reading inode give a benefit that we could correct on disk flags for regular file next dirtying inode happen.

________________________________________
发件人: Chao Yu <yuchao0@huawei.com>
发送时间: 2019年6月13日 14:36
收件人: Wang Shilong; linux-ext4@vger.kernel.org; linux-f2fs-devel@lists.sourceforge.net
抄送: Wang Shilong; Andreas Dilger
主题: Re: [PATCH 2/2] f2fs: only set project inherit bit for directory

On 2019/6/6 12:32, Wang Shilong wrote:
> From: Wang Shilong <wshilong@ddn.com>
>
> It doesn't make any sense to have project inherit bits
> for regular files, even though this won't cause any
> problem, but it is better fix this.
>
> Cc: Andreas Dilger <adilger@dilger.ca>
> Signed-off-by: Wang Shilong <wshilong@ddn.com>
> ---
>  fs/f2fs/f2fs.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 06b89a9862ab..f02ebecb68ea 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -2370,7 +2370,8 @@ static inline void f2fs_change_bit(unsigned int nr, char *addr)
>                          F2FS_PROJINHERIT_FL)
>
>  /* Flags that are appropriate for regular files (all but dir-specific ones). */
> -#define F2FS_REG_FLMASK              (~(F2FS_DIRSYNC_FL | F2FS_TOPDIR_FL))
> +#define F2FS_REG_FLMASK      (~(F2FS_DIRSYNC_FL | F2FS_TOPDIR_FL |\
> +                                F2FS_PROJINHERIT_FL))

Hi Shilong,

Could you please add below diff as ext4 did?

diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index efdafa886510..295ca5ed42d9 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -1759,6 +1759,9 @@ static int f2fs_ioc_getflags(struct file *filp, unsigned
long arg)

        fsflags &= F2FS_GETTABLE_FS_FL;

+       if (S_ISREG(inode->i_mode))
+               fsflags &= ~FS_PROJINHERIT_FL;
+
        return put_user(fsflags, (int __user *)arg);
 }

Thanks,

>
>  /* Flags that are appropriate for non-directories/regular files. */
>  #define F2FS_OTHER_FLMASK    (F2FS_NODUMP_FL | F2FS_NOATIME_FL)
>

_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[f2fs-dev] 回复: [PATCH 2/2] f2fs: only set project inherit bit for directory

[Wang Shilong via Linux-f2fs-devel]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="UTF-8", Size: 2484 bytes --]

Hi Chao,

 I just sent a V2, but I think we'd better do that when reading inode, for two reasons:

1) not only F2FS_IOC_GETFLAGS need filter flags but also F2FS_IOC_FSGETXATTR need that, so the amended parts is not enough IMO.

2) doing that by reading inode give a benefit that we could correct on disk flags for regular file next dirtying inode happen.

________________________________________
·¢¼þÈË: Chao Yu <yuchao0@huawei.com>
·¢ËÍʱ¼ä: 2019Äê6ÔÂ13ÈÕ 14:36
ÊÕ¼þÈË: Wang Shilong; linux-ext4@vger.kernel.org; linux-f2fs-devel@lists.sourceforge.net
³­ËÍ: Wang Shilong; Andreas Dilger
Ö÷Ìâ: Re: [PATCH 2/2] f2fs: only set project inherit bit for directory

On 2019/6/6 12:32, Wang Shilong wrote:
> From: Wang Shilong <wshilong@ddn.com>
>
> It doesn't make any sense to have project inherit bits
> for regular files, even though this won't cause any
> problem, but it is better fix this.
>
> Cc: Andreas Dilger <adilger@dilger.ca>
> Signed-off-by: Wang Shilong <wshilong@ddn.com>
> ---
>  fs/f2fs/f2fs.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 06b89a9862ab..f02ebecb68ea 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -2370,7 +2370,8 @@ static inline void f2fs_change_bit(unsigned int nr, char *addr)
>                          F2FS_PROJINHERIT_FL)
>
>  /* Flags that are appropriate for regular files (all but dir-specific ones). */
> -#define F2FS_REG_FLMASK              (~(F2FS_DIRSYNC_FL | F2FS_TOPDIR_FL))
> +#define F2FS_REG_FLMASK      (~(F2FS_DIRSYNC_FL | F2FS_TOPDIR_FL |\
> +                                F2FS_PROJINHERIT_FL))

Hi Shilong,

Could you please add below diff as ext4 did?

diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index efdafa886510..295ca5ed42d9 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -1759,6 +1759,9 @@ static int f2fs_ioc_getflags(struct file *filp, unsigned
long arg)

        fsflags &= F2FS_GETTABLE_FS_FL;

+       if (S_ISREG(inode->i_mode))
+               fsflags &= ~FS_PROJINHERIT_FL;
+
        return put_user(fsflags, (int __user *)arg);
 }

Thanks,

>
>  /* Flags that are appropriate for non-directories/regular files. */
>  #define F2FS_OTHER_FLMASK    (F2FS_NODUMP_FL | F2FS_NOATIME_FL)
>

_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: 回复: [PATCH 2/2] f2fs: only set project inherit bit for directory

[Chao Yu]

On 2019/6/13 15:34, Wang Shilong wrote:
> Hi Chao,
> 
>  I just sent a V2, but I think we'd better do that when reading inode, for two reasons:
> 
> 1) not only F2FS_IOC_GETFLAGS need filter flags but also F2FS_IOC_FSGETXATTR need that, so the amended parts is not enough IMO.
> 
> 2) doing that by reading inode give a benefit that we could correct on disk flags for regular file next dirtying inode happen.

Hi Shilong, agreed, I'm okay with that change. :)

Thanks,

Re: [f2fs-dev] 回复: [PATCH 2/2] f2fs: only set project inherit bit for directory

[Chao Yu]

On 2019/6/13 15:34, Wang Shilong wrote:
> Hi Chao,
> 
>  I just sent a V2, but I think we'd better do that when reading inode, for two reasons:
> 
> 1) not only F2FS_IOC_GETFLAGS need filter flags but also F2FS_IOC_FSGETXATTR need that, so the amended parts is not enough IMO.
> 
> 2) doing that by reading inode give a benefit that we could correct on disk flags for regular file next dirtying inode happen.

Hi Shilong, agreed, I'm okay with that change. :)

Thanks,


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [PATCH 1/2] ext4: only set project inherit bit for directory

[Darrick J. Wong]

On Thu, Jun 06, 2019 at 01:32:24PM +0900, Wang Shilong wrote:
> From: Wang Shilong <wshilong@ddn.com>
> 
> It doesn't make any sense to have project inherit bits
> for regular files, even though this won't cause any
> problem, but it is better fix this.
> 
> Cc: Andreas Dilger <adilger@dilger.ca>
> Signed-off-by: Wang Shilong <wshilong@ddn.com>

It's good to be maintaining consistent behavior with XFS.

Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

(applies to both ext4 & f2fs patches)

--D

> ---
>  fs/ext4/ext4.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 1cb67859e051..ceb74093e138 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -421,7 +421,8 @@ struct flex_groups {
>  			   EXT4_PROJINHERIT_FL | EXT4_CASEFOLD_FL)
>  
>  /* Flags that are appropriate for regular files (all but dir-specific ones). */
> -#define EXT4_REG_FLMASK (~(EXT4_DIRSYNC_FL | EXT4_TOPDIR_FL | EXT4_CASEFOLD_FL))
> +#define EXT4_REG_FLMASK (~(EXT4_DIRSYNC_FL | EXT4_TOPDIR_FL | EXT4_CASEFOLD_FL |\
> +			   EXT4_PROJINHERIT_FL))
>  
>  /* Flags that are appropriate for non-directories/regular files. */
>  #define EXT4_OTHER_FLMASK (EXT4_NODUMP_FL | EXT4_NOATIME_FL)
> -- 
> 2.21.0
> 

Re: [f2fs-dev] [PATCH 1/2] ext4: only set project inherit bit for directory

[Darrick J. Wong]

On Thu, Jun 06, 2019 at 01:32:24PM +0900, Wang Shilong wrote:
> From: Wang Shilong <wshilong@ddn.com>
> 
> It doesn't make any sense to have project inherit bits
> for regular files, even though this won't cause any
> problem, but it is better fix this.
> 
> Cc: Andreas Dilger <adilger@dilger.ca>
> Signed-off-by: Wang Shilong <wshilong@ddn.com>

It's good to be maintaining consistent behavior with XFS.

Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

(applies to both ext4 & f2fs patches)

--D

> ---
>  fs/ext4/ext4.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 1cb67859e051..ceb74093e138 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -421,7 +421,8 @@ struct flex_groups {
>  			   EXT4_PROJINHERIT_FL | EXT4_CASEFOLD_FL)
>  
>  /* Flags that are appropriate for regular files (all but dir-specific ones). */
> -#define EXT4_REG_FLMASK (~(EXT4_DIRSYNC_FL | EXT4_TOPDIR_FL | EXT4_CASEFOLD_FL))
> +#define EXT4_REG_FLMASK (~(EXT4_DIRSYNC_FL | EXT4_TOPDIR_FL | EXT4_CASEFOLD_FL |\
> +			   EXT4_PROJINHERIT_FL))
>  
>  /* Flags that are appropriate for non-directories/regular files. */
>  #define EXT4_OTHER_FLMASK (EXT4_NODUMP_FL | EXT4_NOATIME_FL)
> -- 
> 2.21.0
> 


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [PATCH 1/2] ext4: only set project inherit bit for directory

[Eric Biggers]

On Thu, Jun 06, 2019 at 01:32:24PM +0900, Wang Shilong wrote:
> From: Wang Shilong <wshilong@ddn.com>
> 
> It doesn't make any sense to have project inherit bits
> for regular files, even though this won't cause any
> problem, but it is better fix this.
> 
> Cc: Andreas Dilger <adilger@dilger.ca>
> Signed-off-by: Wang Shilong <wshilong@ddn.com>
> ---
>  fs/ext4/ext4.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 1cb67859e051..ceb74093e138 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -421,7 +421,8 @@ struct flex_groups {
>  			   EXT4_PROJINHERIT_FL | EXT4_CASEFOLD_FL)
>  
>  /* Flags that are appropriate for regular files (all but dir-specific ones). */
> -#define EXT4_REG_FLMASK (~(EXT4_DIRSYNC_FL | EXT4_TOPDIR_FL | EXT4_CASEFOLD_FL))
> +#define EXT4_REG_FLMASK (~(EXT4_DIRSYNC_FL | EXT4_TOPDIR_FL | EXT4_CASEFOLD_FL |\
> +			   EXT4_PROJINHERIT_FL))
>  
>  /* Flags that are appropriate for non-directories/regular files. */
>  #define EXT4_OTHER_FLMASK (EXT4_NODUMP_FL | EXT4_NOATIME_FL)
> -- 
> 2.21.0

Won't this break 'chattr' on files that already have this flag set?
FS_IOC_GETFLAGS will return this flag, so 'chattr' will pass it back to
FS_IOC_SETFLAGS which will return EOPNOTSUPP due to this:

	if (ext4_mask_flags(inode->i_mode, flags) != flags)
		return -EOPNOTSUPP;

- Eric

Re: [f2fs-dev] [PATCH 1/2] ext4: only set project inherit bit for directory

[Eric Biggers]

On Thu, Jun 06, 2019 at 01:32:24PM +0900, Wang Shilong wrote:
> From: Wang Shilong <wshilong@ddn.com>
> 
> It doesn't make any sense to have project inherit bits
> for regular files, even though this won't cause any
> problem, but it is better fix this.
> 
> Cc: Andreas Dilger <adilger@dilger.ca>
> Signed-off-by: Wang Shilong <wshilong@ddn.com>
> ---
>  fs/ext4/ext4.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 1cb67859e051..ceb74093e138 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -421,7 +421,8 @@ struct flex_groups {
>  			   EXT4_PROJINHERIT_FL | EXT4_CASEFOLD_FL)
>  
>  /* Flags that are appropriate for regular files (all but dir-specific ones). */
> -#define EXT4_REG_FLMASK (~(EXT4_DIRSYNC_FL | EXT4_TOPDIR_FL | EXT4_CASEFOLD_FL))
> +#define EXT4_REG_FLMASK (~(EXT4_DIRSYNC_FL | EXT4_TOPDIR_FL | EXT4_CASEFOLD_FL |\
> +			   EXT4_PROJINHERIT_FL))
>  
>  /* Flags that are appropriate for non-directories/regular files. */
>  #define EXT4_OTHER_FLMASK (EXT4_NODUMP_FL | EXT4_NOATIME_FL)
> -- 
> 2.21.0

Won't this break 'chattr' on files that already have this flag set?
FS_IOC_GETFLAGS will return this flag, so 'chattr' will pass it back to
FS_IOC_SETFLAGS which will return EOPNOTSUPP due to this:

	if (ext4_mask_flags(inode->i_mode, flags) != flags)
		return -EOPNOTSUPP;

- Eric


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

回复: [PATCH 1/2] ext4: only set project inherit bit for directory

[Wang Shilong via Linux-f2fs-devel]

Hi,

> --
> 2.21.0

Won't this break 'chattr' on files that already have this flag set?
FS_IOC_GETFLAGS will return this flag, so 'chattr' will pass it back to
FS_IOC_SETFLAGS which will return EOPNOTSUPP due to this:

        if (ext4_mask_flags(inode->i_mode, flags) != flags)
                return -EOPNOTSUPP;

>>>>

You are right for this and we also need take care of this in EXT4_IOC_FSSETXATTR/
this is a bit strange behavior as chattr read existed flags
but could not set them again, there are several possible ways that I could think
of to fix the issue?

1) change chattr to filter Project inherit bit before call FS_IOC_SETFLAGS

2) we automatically fixed the flag before mask check, something like:
if reg:
     flags &= ~PROJECT_INHERT;
      if (ext4_mask_flags(inode->i_mode, flags) != flags)
                return -EOPNOTSUPP;
But this might be not good..

I would prefer solution 1)
What do you think?

_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[f2fs-dev] 回复: [PATCH 1/2] ext4: only set project inherit bit for directory

[Wang Shilong via Linux-f2fs-devel]

Hi,

> --
> 2.21.0

Won't this break 'chattr' on files that already have this flag set?
FS_IOC_GETFLAGS will return this flag, so 'chattr' will pass it back to
FS_IOC_SETFLAGS which will return EOPNOTSUPP due to this:

        if (ext4_mask_flags(inode->i_mode, flags) != flags)
                return -EOPNOTSUPP;

>>>>

You are right for this and we also need take care of this in EXT4_IOC_FSSETXATTR/
this is a bit strange behavior as chattr read existed flags
but could not set them again, there are several possible ways that I could think
of to fix the issue?

1) change chattr to filter Project inherit bit before call FS_IOC_SETFLAGS

2) we automatically fixed the flag before mask check, something like:
if reg:
     flags &= ~PROJECT_INHERT;
      if (ext4_mask_flags(inode->i_mode, flags) != flags)
                return -EOPNOTSUPP;
But this might be not good..

I would prefer solution 1)
What do you think?

_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: 回复: [PATCH 1/2] ext4: only set project inherit bit for directory

[Eric Biggers]

On Fri, Jun 07, 2019 at 07:51:18AM +0000, Wang Shilong wrote:
> Hi,
> 
> > --
> > 2.21.0
> 
> Won't this break 'chattr' on files that already have this flag set?
> FS_IOC_GETFLAGS will return this flag, so 'chattr' will pass it back to
> FS_IOC_SETFLAGS which will return EOPNOTSUPP due to this:
> 
>         if (ext4_mask_flags(inode->i_mode, flags) != flags)
>                 return -EOPNOTSUPP;
> 
> >>>>
> 
> You are right for this and we also need take care of this in EXT4_IOC_FSSETXATTR/
> this is a bit strange behavior as chattr read existed flags
> but could not set them again, there are several possible ways that I could think
> of to fix the issue?
> 
> 1) change chattr to filter Project inherit bit before call FS_IOC_SETFLAGS
> 
> 2) we automatically fixed the flag before mask check, something like:
> if reg:
>      flags &= ~PROJECT_INHERT;
>       if (ext4_mask_flags(inode->i_mode, flags) != flags)
>                 return -EOPNOTSUPP;
> But this might be not good..
> 
> I would prefer solution 1)
> What do you think?

Existing versions of chattr can't be changed, and people don't necessarily
upgrade the kernel and e2fsprogs at the same time.  So (1) wouldn't really work.

A better solution might be to make FS_IOC_GETFLAGS and FS_IOC_FSGETXATTR never
return the project inherit flag on regular files.

- Eric

Re: [f2fs-dev] 回复: [PATCH 1/2] ext4: only set project inherit bit for directory

[Eric Biggers]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="UTF-8", Size: 1565 bytes --]

On Fri, Jun 07, 2019 at 07:51:18AM +0000, Wang Shilong wrote:
> Hi,
> 
> > --
> > 2.21.0
> 
> Won't this break 'chattr' on files that already have this flag set?
> FS_IOC_GETFLAGS will return this flag, so 'chattr' will pass it back to
> FS_IOC_SETFLAGS which will return EOPNOTSUPP due to this:
> 
>         if (ext4_mask_flags(inode->i_mode, flags) != flags)
>                 return -EOPNOTSUPP;
> 
> >>>>
> 
> You are right for this and we also need take care of this in EXT4_IOC_FSSETXATTR/
> this is a bit strange behavior as chattr read existed flags
> but could not set them again, there are several possible ways that I could think
> of to fix the issue?
> 
> 1) change chattr to filter Project inherit bit before call FS_IOC_SETFLAGS
> 
> 2) we automatically fixed the flag before mask check, something like:
> if reg:
>      flags &= ~PROJECT_INHERT;
>       if (ext4_mask_flags(inode->i_mode, flags) != flags)
>                 return -EOPNOTSUPP;
> But this might be not good..
> 
> I would prefer solution 1)
> What do you think?

Existing versions of chattr can't be changed, and people don't necessarily
upgrade the kernel and e2fsprogs at the same time.  So (1) wouldn't really work.

A better solution might be to make FS_IOC_GETFLAGS and FS_IOC_FSGETXATTR never
return the project inherit flag on regular files.

- Eric


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

回复: 回复: [PATCH 1/2] ext4: only set project inherit bit for directory

[Wang Shilong via Linux-f2fs-devel]

Hi,


> You are right for this and we also need take care of this in EXT4_IOC_FSSETXATTR/
> this is a bit strange behavior as chattr read existed flags
> but could not set them again, there are several possible ways that I could think
> of to fix the issue?
>
> 1) change chattr to filter Project inherit bit before call FS_IOC_SETFLAGS
>
> 2) we automatically fixed the flag before mask check, something like:
> if reg:
>      flags &= ~PROJECT_INHERT;
>       if (ext4_mask_flags(inode->i_mode, flags) != flags)
>                 return -EOPNOTSUPP;
> But this might be not good..
>
> I would prefer solution 1)
> What do you think?

Existing versions of chattr can't be changed, and people don't necessarily
upgrade the kernel and e2fsprogs at the same time.  So (1) wouldn't really work.

A better solution might be to make FS_IOC_GETFLAGS and FS_IOC_FSGETXATTR never
return the project inherit flag on regular files.

- Eric

>>>>>>

How about fix it in  __ext4_iget():

  ei->i_flags = le32_to_cpu(raw_inode->i_flags);
  if (S_ISREG(inode->i_mode))
       ei->i_flags &= ~EXT4_PROJINHERIT_FL;

This way will give a big chance flag will be automatically fixed
next time whenever inode is dirtied.

Thanks,
Shilong

[f2fs-dev] 回复: 回复: [PATCH 1/2] ext4: only set project inherit bit for directory

[Wang Shilong via Linux-f2fs-devel]

Hi,


> You are right for this and we also need take care of this in EXT4_IOC_FSSETXATTR/
> this is a bit strange behavior as chattr read existed flags
> but could not set them again, there are several possible ways that I could think
> of to fix the issue?
>
> 1) change chattr to filter Project inherit bit before call FS_IOC_SETFLAGS
>
> 2) we automatically fixed the flag before mask check, something like:
> if reg:
>      flags &= ~PROJECT_INHERT;
>       if (ext4_mask_flags(inode->i_mode, flags) != flags)
>                 return -EOPNOTSUPP;
> But this might be not good..
>
> I would prefer solution 1)
> What do you think?

Existing versions of chattr can't be changed, and people don't necessarily
upgrade the kernel and e2fsprogs at the same time.  So (1) wouldn't really work.

A better solution might be to make FS_IOC_GETFLAGS and FS_IOC_FSGETXATTR never
return the project inherit flag on regular files.

- Eric

>>>>>>

How about fix it in  __ext4_iget():

  ei->i_flags = le32_to_cpu(raw_inode->i_flags);
  if (S_ISREG(inode->i_mode))
       ei->i_flags &= ~EXT4_PROJINHERIT_FL;

This way will give a big chance flag will be automatically fixed
next time whenever inode is dirtied.

Thanks,
Shilong
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: 回复: [PATCH 1/2] ext4: only set project inherit bit for directory

[Theodore Ts'o]

On Fri, Jun 07, 2019 at 11:14:52AM -0700, Eric Biggers wrote:
> 
> Existing versions of chattr can't be changed, and people don't necessarily
> upgrade the kernel and e2fsprogs at the same time.  So (1) wouldn't really work.
> 
> A better solution might be to make FS_IOC_GETFLAGS and FS_IOC_FSGETXATTR never
> return the project inherit flag on regular files.

I've amended this patch by adding the following to fix it for
FS_IOC_GETFLAGS (which is chattr uses):

diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
index 7af835ac8d23..74648d42c69b 100644
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@ -779,6 +779,8 @@ long ext4_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 		return ext4_ioc_getfsmap(sb, (void __user *)arg);
 	case EXT4_IOC_GETFLAGS:
 		flags = ei->i_flags & EXT4_FL_USER_VISIBLE;
+		if (S_ISREG(inode->i_mode))
+			flags &= ~EXT4_PROJINHERIT_FL;
 		return put_user(flags, (int __user *) arg);
 	case EXT4_IOC_SETFLAGS: {
 		int err;

						- Ted

Re: [f2fs-dev] 回复: [PATCH 1/2] ext4: only set project inherit bit for directory

[Theodore Ts'o]

On Fri, Jun 07, 2019 at 11:14:52AM -0700, Eric Biggers wrote:
> 
> Existing versions of chattr can't be changed, and people don't necessarily
> upgrade the kernel and e2fsprogs at the same time.  So (1) wouldn't really work.
> 
> A better solution might be to make FS_IOC_GETFLAGS and FS_IOC_FSGETXATTR never
> return the project inherit flag on regular files.

I've amended this patch by adding the following to fix it for
FS_IOC_GETFLAGS (which is chattr uses):

diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
index 7af835ac8d23..74648d42c69b 100644
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@ -779,6 +779,8 @@ long ext4_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 		return ext4_ioc_getfsmap(sb, (void __user *)arg);
 	case EXT4_IOC_GETFLAGS:
 		flags = ei->i_flags & EXT4_FL_USER_VISIBLE;
+		if (S_ISREG(inode->i_mode))
+			flags &= ~EXT4_PROJINHERIT_FL;
 		return put_user(flags, (int __user *) arg);
 	case EXT4_IOC_SETFLAGS: {
 		int err;

						- Ted


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [PATCH 1/2] ext4: only set project inherit bit for directory

[Theodore Ts'o]

On Thu, Jun 06, 2019 at 01:32:24PM +0900, Wang Shilong wrote:
> From: Wang Shilong <wshilong@ddn.com>
> 
> It doesn't make any sense to have project inherit bits
> for regular files, even though this won't cause any
> problem, but it is better fix this.
> 
> Cc: Andreas Dilger <adilger@dilger.ca>
> Signed-off-by: Wang Shilong <wshilong@ddn.com>

Thanks, applied to the ext4 tree.

				- Ted

Re: [f2fs-dev] [PATCH 1/2] ext4: only set project inherit bit for directory

[Theodore Ts'o]

On Thu, Jun 06, 2019 at 01:32:24PM +0900, Wang Shilong wrote:
> From: Wang Shilong <wshilong@ddn.com>
> 
> It doesn't make any sense to have project inherit bits
> for regular files, even though this won't cause any
> problem, but it is better fix this.
> 
> Cc: Andreas Dilger <adilger@dilger.ca>
> Signed-off-by: Wang Shilong <wshilong@ddn.com>

Thanks, applied to the ext4 tree.

				- Ted


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel