* Re: [PATCH v2 2/2] staging: erofs: complete POSIX ACL support
@ 2019-01-26 2:48 ` Chao Yu
0 siblings, 0 replies; 32+ messages in thread
From: Chao Yu @ 2019-01-26 2:48 UTC (permalink / raw)
To: Gao Xiang, Greg Kroah-Hartman, devel
Cc: LKML, linux-erofs, Chao Yu, Miao Xie, weidu.du, Fang Wei
On 2019/1/26 0:10, Gao Xiang wrote:
> Let's add .get_acl() to read the file's acl from its xattrs
> to make POSIX ACL usable.
>
> Here is the on-disk detail,
> fullname: system.posix_acl_access
> struct erofs_xattr_entry:
> .e_name_len = 0
> .e_name_index = EROFS_XATTR_INDEX_POSIX_ACL_ACCESS (2)
>
> fullname: system.posix_acl_default
> struct erofs_xattr_entry:
> .e_name_len = 0
> .e_name_index = EROFS_XATTR_INDEX_POSIX_ACL_DEFAULT (3)
>
> Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
> ---
> change log v2:
> - add the missing SB_POSIXACL flag;
> - fix the on-disk detail, use .e_name_len == 0 and proper prefix value;
> - tested ok with xattr enabled erofs_mkfs;
>
> .../erofs/Documentation/filesystems/erofs.txt | 2 ++
> drivers/staging/erofs/inode.c | 3 ++
> drivers/staging/erofs/namei.c | 1 +
> drivers/staging/erofs/super.c | 8 +++++
> drivers/staging/erofs/xattr.c | 37 ++++++++++++++++++++++
> drivers/staging/erofs/xattr.h | 6 ++++
> 6 files changed, 57 insertions(+)
>
> diff --git a/drivers/staging/erofs/Documentation/filesystems/erofs.txt b/drivers/staging/erofs/Documentation/filesystems/erofs.txt
> index 803988d74c21..961ec4da7705 100644
> --- a/drivers/staging/erofs/Documentation/filesystems/erofs.txt
> +++ b/drivers/staging/erofs/Documentation/filesystems/erofs.txt
> @@ -36,6 +36,8 @@ Here is the main features of EROFS:
>
> - Support xattr inline and tail-end data inline for all files;
>
> + - Support POSIX.1e ACLs by using xattrs;
> +
> - Support transparent file compression as an option:
> LZ4 algorithm with 4 KB fixed-output compression for high performance;
>
> diff --git a/drivers/staging/erofs/inode.c b/drivers/staging/erofs/inode.c
> index 4f04f7c38cf2..924b8dfc7a8f 100644
> --- a/drivers/staging/erofs/inode.c
> +++ b/drivers/staging/erofs/inode.c
> @@ -287,6 +287,7 @@ const struct inode_operations erofs_generic_iops = {
> #ifdef CONFIG_EROFS_FS_XATTR
> .listxattr = erofs_listxattr,
> #endif
> + .get_acl = erofs_get_acl,
> };
>
> const struct inode_operations erofs_symlink_iops = {
> @@ -294,6 +295,7 @@ const struct inode_operations erofs_symlink_iops = {
> #ifdef CONFIG_EROFS_FS_XATTR
> .listxattr = erofs_listxattr,
> #endif
> + .get_acl = erofs_get_acl,
> };
>
> const struct inode_operations erofs_fast_symlink_iops = {
> @@ -301,5 +303,6 @@ const struct inode_operations erofs_fast_symlink_iops = {
> #ifdef CONFIG_EROFS_FS_XATTR
> .listxattr = erofs_listxattr,
> #endif
> + .get_acl = erofs_get_acl,
> };
>
> diff --git a/drivers/staging/erofs/namei.c b/drivers/staging/erofs/namei.c
> index 7fed1f996ab0..b1752adc5934 100644
> --- a/drivers/staging/erofs/namei.c
> +++ b/drivers/staging/erofs/namei.c
> @@ -238,5 +238,6 @@ const struct inode_operations erofs_dir_iops = {
> #ifdef CONFIG_EROFS_FS_XATTR
> .listxattr = erofs_listxattr,
> #endif
> + .get_acl = erofs_get_acl,
> };
>
> diff --git a/drivers/staging/erofs/super.c b/drivers/staging/erofs/super.c
> index 176fca2af379..54cd7dac0a1f 100644
> --- a/drivers/staging/erofs/super.c
> +++ b/drivers/staging/erofs/super.c
> @@ -398,6 +398,14 @@ static int erofs_read_super(struct super_block *sb,
> if (!silent)
> infoln("root inode @ nid %llu", ROOT_NID(sbi));
>
> +#ifdef CONFIG_EROFS_FS_POSIX_ACL
> + /* Update the POSIXACL Flag */
> + if (test_opt(sbi, POSIX_ACL))
> + sb->s_flags |= SB_POSIXACL;
> + else
> + sb->s_flags &= ~SB_POSIXACL;
> +#endif
> +
> #ifdef CONFIG_EROFS_FS_ZIP
> INIT_RADIX_TREE(&sbi->workstn_tree, GFP_ATOMIC);
> #endif
> diff --git a/drivers/staging/erofs/xattr.c b/drivers/staging/erofs/xattr.c
> index 7de46690d972..6759485ae862 100644
> --- a/drivers/staging/erofs/xattr.c
> +++ b/drivers/staging/erofs/xattr.c
> @@ -643,3 +643,40 @@ ssize_t erofs_listxattr(struct dentry *dentry,
> return shared_listxattr(&it);
> }
>
> +#ifdef CONFIG_EROFS_FS_POSIX_ACL
> +struct posix_acl *erofs_get_acl(struct inode *inode, int type)
> +{
> + struct posix_acl *acl;
> + int ea_prefix, rc;
> + char *value = NULL;
> +
> + switch(type) {
> + case ACL_TYPE_ACCESS:
> + ea_prefix = EROFS_XATTR_INDEX_POSIX_ACL_ACCESS;
> + break;
> + case ACL_TYPE_DEFAULT:
> + ea_prefix = EROFS_XATTR_INDEX_POSIX_ACL_DEFAULT;
> + break;
> + default:
> + return ERR_PTR(-EINVAL);
> + }
> +
> + rc = erofs_getxattr(inode, ea_prefix, "", NULL, 0);
> + if (rc > 0) {
> + value = kvmalloc(rc, GFP_KERNEL);
erofs_kmalloc() is enough?
Thanks,
> + if (!value)
> + return ERR_PTR(-ENOMEM);
> + rc = erofs_getxattr(inode, ea_prefix, "", value, rc);
> + }
> +
> + if (rc == -ENOATTR)
> + acl = NULL;
> + else if (rc < 0)
> + acl = ERR_PTR(rc);
> + else
> + acl = posix_acl_from_xattr(&init_user_ns, value, rc);
> + kvfree(value);
> + return acl;
> +}
> +#endif
> +
> diff --git a/drivers/staging/erofs/xattr.h b/drivers/staging/erofs/xattr.h
> index 634dae9aaa0b..35ba5ac2139a 100644
> --- a/drivers/staging/erofs/xattr.h
> +++ b/drivers/staging/erofs/xattr.h
> @@ -87,5 +87,11 @@ static ssize_t __maybe_unused erofs_listxattr(struct dentry *dentry,
> }
> #endif
>
> +#ifdef CONFIG_EROFS_FS_POSIX_ACL
> +struct posix_acl *erofs_get_acl(struct inode *inode, int type);
> +#else
> +#define erofs_get_acl (NULL)
> +#endif
> +
> #endif
>
>
^ permalink raw reply [flat|nested] 32+ messages in thread* [PATCH v2 2/2] staging: erofs: complete POSIX ACL support
2019-01-26 2:48 ` Chao Yu
@ 2019-01-26 3:06 ` Gao Xiang
-1 siblings, 0 replies; 32+ messages in thread
From: Gao Xiang @ 2019-01-26 3:06 UTC (permalink / raw)
Hi Chao,
On 2019/1/26 10:48, Chao Yu wrote:
> On 2019/1/26 0:10, Gao Xiang wrote:
>> Let's add .get_acl() to read the file's acl from its xattrs
>> to make POSIX ACL usable.
>>
>> Here is the on-disk detail,
>> fullname: system.posix_acl_access
>> struct erofs_xattr_entry:
>> .e_name_len = 0
>> .e_name_index = EROFS_XATTR_INDEX_POSIX_ACL_ACCESS (2)
>>
>> fullname: system.posix_acl_default
>> struct erofs_xattr_entry:
>> .e_name_len = 0
>> .e_name_index = EROFS_XATTR_INDEX_POSIX_ACL_DEFAULT (3)
>>
>> Signed-off-by: Gao Xiang <gaoxiang25 at huawei.com>
>> ---
>> change log v2:
>> - add the missing SB_POSIXACL flag;
>> - fix the on-disk detail, use .e_name_len == 0 and proper prefix value;
>> - tested ok with xattr enabled erofs_mkfs;
>>
>> .../erofs/Documentation/filesystems/erofs.txt | 2 ++
>> drivers/staging/erofs/inode.c | 3 ++
>> drivers/staging/erofs/namei.c | 1 +
>> drivers/staging/erofs/super.c | 8 +++++
>> drivers/staging/erofs/xattr.c | 37 ++++++++++++++++++++++
>> drivers/staging/erofs/xattr.h | 6 ++++
>> 6 files changed, 57 insertions(+)
>>
>> diff --git a/drivers/staging/erofs/Documentation/filesystems/erofs.txt b/drivers/staging/erofs/Documentation/filesystems/erofs.txt
>> index 803988d74c21..961ec4da7705 100644
>> --- a/drivers/staging/erofs/Documentation/filesystems/erofs.txt
>> +++ b/drivers/staging/erofs/Documentation/filesystems/erofs.txt
>> @@ -36,6 +36,8 @@ Here is the main features of EROFS:
>>
>> - Support xattr inline and tail-end data inline for all files;
>>
>> + - Support POSIX.1e ACLs by using xattrs;
>> +
>> - Support transparent file compression as an option:
>> LZ4 algorithm with 4 KB fixed-output compression for high performance;
>>
>> diff --git a/drivers/staging/erofs/inode.c b/drivers/staging/erofs/inode.c
>> index 4f04f7c38cf2..924b8dfc7a8f 100644
>> --- a/drivers/staging/erofs/inode.c
>> +++ b/drivers/staging/erofs/inode.c
>> @@ -287,6 +287,7 @@ const struct inode_operations erofs_generic_iops = {
>> #ifdef CONFIG_EROFS_FS_XATTR
>> .listxattr = erofs_listxattr,
>> #endif
>> + .get_acl = erofs_get_acl,
>> };
>>
>> const struct inode_operations erofs_symlink_iops = {
>> @@ -294,6 +295,7 @@ const struct inode_operations erofs_symlink_iops = {
>> #ifdef CONFIG_EROFS_FS_XATTR
>> .listxattr = erofs_listxattr,
>> #endif
>> + .get_acl = erofs_get_acl,
>> };
>>
>> const struct inode_operations erofs_fast_symlink_iops = {
>> @@ -301,5 +303,6 @@ const struct inode_operations erofs_fast_symlink_iops = {
>> #ifdef CONFIG_EROFS_FS_XATTR
>> .listxattr = erofs_listxattr,
>> #endif
>> + .get_acl = erofs_get_acl,
>> };
>>
>> diff --git a/drivers/staging/erofs/namei.c b/drivers/staging/erofs/namei.c
>> index 7fed1f996ab0..b1752adc5934 100644
>> --- a/drivers/staging/erofs/namei.c
>> +++ b/drivers/staging/erofs/namei.c
>> @@ -238,5 +238,6 @@ const struct inode_operations erofs_dir_iops = {
>> #ifdef CONFIG_EROFS_FS_XATTR
>> .listxattr = erofs_listxattr,
>> #endif
>> + .get_acl = erofs_get_acl,
>> };
>>
>> diff --git a/drivers/staging/erofs/super.c b/drivers/staging/erofs/super.c
>> index 176fca2af379..54cd7dac0a1f 100644
>> --- a/drivers/staging/erofs/super.c
>> +++ b/drivers/staging/erofs/super.c
>> @@ -398,6 +398,14 @@ static int erofs_read_super(struct super_block *sb,
>> if (!silent)
>> infoln("root inode @ nid %llu", ROOT_NID(sbi));
>>
>> +#ifdef CONFIG_EROFS_FS_POSIX_ACL
>> + /* Update the POSIXACL Flag */
>> + if (test_opt(sbi, POSIX_ACL))
>> + sb->s_flags |= SB_POSIXACL;
>> + else
>> + sb->s_flags &= ~SB_POSIXACL;
>> +#endif
>> +
>> #ifdef CONFIG_EROFS_FS_ZIP
>> INIT_RADIX_TREE(&sbi->workstn_tree, GFP_ATOMIC);
>> #endif
>> diff --git a/drivers/staging/erofs/xattr.c b/drivers/staging/erofs/xattr.c
>> index 7de46690d972..6759485ae862 100644
>> --- a/drivers/staging/erofs/xattr.c
>> +++ b/drivers/staging/erofs/xattr.c
>> @@ -643,3 +643,40 @@ ssize_t erofs_listxattr(struct dentry *dentry,
>> return shared_listxattr(&it);
>> }
>>
>> +#ifdef CONFIG_EROFS_FS_POSIX_ACL
>> +struct posix_acl *erofs_get_acl(struct inode *inode, int type)
>> +{
>> + struct posix_acl *acl;
>> + int ea_prefix, rc;
>> + char *value = NULL;
>> +
>> + switch(type) {
>> + case ACL_TYPE_ACCESS:
>> + ea_prefix = EROFS_XATTR_INDEX_POSIX_ACL_ACCESS;
>> + break;
>> + case ACL_TYPE_DEFAULT:
>> + ea_prefix = EROFS_XATTR_INDEX_POSIX_ACL_DEFAULT;
>> + break;
>> + default:
>> + return ERR_PTR(-EINVAL);
>> + }
>> +
>> + rc = erofs_getxattr(inode, ea_prefix, "", NULL, 0);
>> + if (rc > 0) {
>> + value = kvmalloc(rc, GFP_KERNEL);
>
> erofs_kmalloc() is enough?
I think kmalloc is enough for this case, but I think kvmalloc is not
bad as well.
Actually I want to introduce erofs_kvmalloc() in the later patch and
use it in general except for the context which is unsuitable for vmalloc..
Let me send v3 if it is unnecessary...
Thanks,
Gao Xiang
>
> Thanks,
>
>> + if (!value)
>> + return ERR_PTR(-ENOMEM);
>> + rc = erofs_getxattr(inode, ea_prefix, "", value, rc);
>> + }
>> +
>> + if (rc == -ENOATTR)
>> + acl = NULL;
>> + else if (rc < 0)
>> + acl = ERR_PTR(rc);
>> + else
>> + acl = posix_acl_from_xattr(&init_user_ns, value, rc);
>> + kvfree(value);
>> + return acl;
>> +}
>> +#endif
>> +
>> diff --git a/drivers/staging/erofs/xattr.h b/drivers/staging/erofs/xattr.h
>> index 634dae9aaa0b..35ba5ac2139a 100644
>> --- a/drivers/staging/erofs/xattr.h
>> +++ b/drivers/staging/erofs/xattr.h
>> @@ -87,5 +87,11 @@ static ssize_t __maybe_unused erofs_listxattr(struct dentry *dentry,
>> }
>> #endif
>>
>> +#ifdef CONFIG_EROFS_FS_POSIX_ACL
>> +struct posix_acl *erofs_get_acl(struct inode *inode, int type);
>> +#else
>> +#define erofs_get_acl (NULL)
>> +#endif
>> +
>> #endif
>>
>>
>
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v2 2/2] staging: erofs: complete POSIX ACL support
@ 2019-01-26 3:06 ` Gao Xiang
0 siblings, 0 replies; 32+ messages in thread
From: Gao Xiang @ 2019-01-26 3:06 UTC (permalink / raw)
To: Chao Yu
Cc: Gao Xiang, Greg Kroah-Hartman, devel, linux-erofs, LKML, weidu.du,
Miao Xie
Hi Chao,
On 2019/1/26 10:48, Chao Yu wrote:
> On 2019/1/26 0:10, Gao Xiang wrote:
>> Let's add .get_acl() to read the file's acl from its xattrs
>> to make POSIX ACL usable.
>>
>> Here is the on-disk detail,
>> fullname: system.posix_acl_access
>> struct erofs_xattr_entry:
>> .e_name_len = 0
>> .e_name_index = EROFS_XATTR_INDEX_POSIX_ACL_ACCESS (2)
>>
>> fullname: system.posix_acl_default
>> struct erofs_xattr_entry:
>> .e_name_len = 0
>> .e_name_index = EROFS_XATTR_INDEX_POSIX_ACL_DEFAULT (3)
>>
>> Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
>> ---
>> change log v2:
>> - add the missing SB_POSIXACL flag;
>> - fix the on-disk detail, use .e_name_len == 0 and proper prefix value;
>> - tested ok with xattr enabled erofs_mkfs;
>>
>> .../erofs/Documentation/filesystems/erofs.txt | 2 ++
>> drivers/staging/erofs/inode.c | 3 ++
>> drivers/staging/erofs/namei.c | 1 +
>> drivers/staging/erofs/super.c | 8 +++++
>> drivers/staging/erofs/xattr.c | 37 ++++++++++++++++++++++
>> drivers/staging/erofs/xattr.h | 6 ++++
>> 6 files changed, 57 insertions(+)
>>
>> diff --git a/drivers/staging/erofs/Documentation/filesystems/erofs.txt b/drivers/staging/erofs/Documentation/filesystems/erofs.txt
>> index 803988d74c21..961ec4da7705 100644
>> --- a/drivers/staging/erofs/Documentation/filesystems/erofs.txt
>> +++ b/drivers/staging/erofs/Documentation/filesystems/erofs.txt
>> @@ -36,6 +36,8 @@ Here is the main features of EROFS:
>>
>> - Support xattr inline and tail-end data inline for all files;
>>
>> + - Support POSIX.1e ACLs by using xattrs;
>> +
>> - Support transparent file compression as an option:
>> LZ4 algorithm with 4 KB fixed-output compression for high performance;
>>
>> diff --git a/drivers/staging/erofs/inode.c b/drivers/staging/erofs/inode.c
>> index 4f04f7c38cf2..924b8dfc7a8f 100644
>> --- a/drivers/staging/erofs/inode.c
>> +++ b/drivers/staging/erofs/inode.c
>> @@ -287,6 +287,7 @@ const struct inode_operations erofs_generic_iops = {
>> #ifdef CONFIG_EROFS_FS_XATTR
>> .listxattr = erofs_listxattr,
>> #endif
>> + .get_acl = erofs_get_acl,
>> };
>>
>> const struct inode_operations erofs_symlink_iops = {
>> @@ -294,6 +295,7 @@ const struct inode_operations erofs_symlink_iops = {
>> #ifdef CONFIG_EROFS_FS_XATTR
>> .listxattr = erofs_listxattr,
>> #endif
>> + .get_acl = erofs_get_acl,
>> };
>>
>> const struct inode_operations erofs_fast_symlink_iops = {
>> @@ -301,5 +303,6 @@ const struct inode_operations erofs_fast_symlink_iops = {
>> #ifdef CONFIG_EROFS_FS_XATTR
>> .listxattr = erofs_listxattr,
>> #endif
>> + .get_acl = erofs_get_acl,
>> };
>>
>> diff --git a/drivers/staging/erofs/namei.c b/drivers/staging/erofs/namei.c
>> index 7fed1f996ab0..b1752adc5934 100644
>> --- a/drivers/staging/erofs/namei.c
>> +++ b/drivers/staging/erofs/namei.c
>> @@ -238,5 +238,6 @@ const struct inode_operations erofs_dir_iops = {
>> #ifdef CONFIG_EROFS_FS_XATTR
>> .listxattr = erofs_listxattr,
>> #endif
>> + .get_acl = erofs_get_acl,
>> };
>>
>> diff --git a/drivers/staging/erofs/super.c b/drivers/staging/erofs/super.c
>> index 176fca2af379..54cd7dac0a1f 100644
>> --- a/drivers/staging/erofs/super.c
>> +++ b/drivers/staging/erofs/super.c
>> @@ -398,6 +398,14 @@ static int erofs_read_super(struct super_block *sb,
>> if (!silent)
>> infoln("root inode @ nid %llu", ROOT_NID(sbi));
>>
>> +#ifdef CONFIG_EROFS_FS_POSIX_ACL
>> + /* Update the POSIXACL Flag */
>> + if (test_opt(sbi, POSIX_ACL))
>> + sb->s_flags |= SB_POSIXACL;
>> + else
>> + sb->s_flags &= ~SB_POSIXACL;
>> +#endif
>> +
>> #ifdef CONFIG_EROFS_FS_ZIP
>> INIT_RADIX_TREE(&sbi->workstn_tree, GFP_ATOMIC);
>> #endif
>> diff --git a/drivers/staging/erofs/xattr.c b/drivers/staging/erofs/xattr.c
>> index 7de46690d972..6759485ae862 100644
>> --- a/drivers/staging/erofs/xattr.c
>> +++ b/drivers/staging/erofs/xattr.c
>> @@ -643,3 +643,40 @@ ssize_t erofs_listxattr(struct dentry *dentry,
>> return shared_listxattr(&it);
>> }
>>
>> +#ifdef CONFIG_EROFS_FS_POSIX_ACL
>> +struct posix_acl *erofs_get_acl(struct inode *inode, int type)
>> +{
>> + struct posix_acl *acl;
>> + int ea_prefix, rc;
>> + char *value = NULL;
>> +
>> + switch(type) {
>> + case ACL_TYPE_ACCESS:
>> + ea_prefix = EROFS_XATTR_INDEX_POSIX_ACL_ACCESS;
>> + break;
>> + case ACL_TYPE_DEFAULT:
>> + ea_prefix = EROFS_XATTR_INDEX_POSIX_ACL_DEFAULT;
>> + break;
>> + default:
>> + return ERR_PTR(-EINVAL);
>> + }
>> +
>> + rc = erofs_getxattr(inode, ea_prefix, "", NULL, 0);
>> + if (rc > 0) {
>> + value = kvmalloc(rc, GFP_KERNEL);
>
> erofs_kmalloc() is enough?
I think kmalloc is enough for this case, but I think kvmalloc is not
bad as well.
Actually I want to introduce erofs_kvmalloc() in the later patch and
use it in general except for the context which is unsuitable for vmalloc..
Let me send v3 if it is unnecessary...
Thanks,
Gao Xiang
>
> Thanks,
>
>> + if (!value)
>> + return ERR_PTR(-ENOMEM);
>> + rc = erofs_getxattr(inode, ea_prefix, "", value, rc);
>> + }
>> +
>> + if (rc == -ENOATTR)
>> + acl = NULL;
>> + else if (rc < 0)
>> + acl = ERR_PTR(rc);
>> + else
>> + acl = posix_acl_from_xattr(&init_user_ns, value, rc);
>> + kvfree(value);
>> + return acl;
>> +}
>> +#endif
>> +
>> diff --git a/drivers/staging/erofs/xattr.h b/drivers/staging/erofs/xattr.h
>> index 634dae9aaa0b..35ba5ac2139a 100644
>> --- a/drivers/staging/erofs/xattr.h
>> +++ b/drivers/staging/erofs/xattr.h
>> @@ -87,5 +87,11 @@ static ssize_t __maybe_unused erofs_listxattr(struct dentry *dentry,
>> }
>> #endif
>>
>> +#ifdef CONFIG_EROFS_FS_POSIX_ACL
>> +struct posix_acl *erofs_get_acl(struct inode *inode, int type);
>> +#else
>> +#define erofs_get_acl (NULL)
>> +#endif
>> +
>> #endif
>>
>>
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 2/2] staging: erofs: complete POSIX ACL support
2019-01-26 2:48 ` Chao Yu
@ 2019-01-28 13:33 ` Dan Carpenter
-1 siblings, 0 replies; 32+ messages in thread
From: Dan Carpenter @ 2019-01-28 13:33 UTC (permalink / raw)
On Sat, Jan 26, 2019@10:48:53AM +0800, Chao Yu wrote:
> On 2019/1/26 0:10, Gao Xiang wrote:
> > Let's add .get_acl() to read the file's acl from its xattrs
> > to make POSIX ACL usable.
> >
> > Here is the on-disk detail,
> > fullname: system.posix_acl_access
> > struct erofs_xattr_entry:
> > .e_name_len = 0
> > .e_name_index = EROFS_XATTR_INDEX_POSIX_ACL_ACCESS (2)
> >
> > fullname: system.posix_acl_default
> > struct erofs_xattr_entry:
> > .e_name_len = 0
> > .e_name_index = EROFS_XATTR_INDEX_POSIX_ACL_DEFAULT (3)
> >
> > Signed-off-by: Gao Xiang <gaoxiang25 at huawei.com>
> > ---
> > change log v2:
> > - add the missing SB_POSIXACL flag;
> > - fix the on-disk detail, use .e_name_len == 0 and proper prefix value;
> > - tested ok with xattr enabled erofs_mkfs;
>
> > .../erofs/Documentation/filesystems/erofs.txt | 2 ++
> > drivers/staging/erofs/inode.c | 3 ++
> > drivers/staging/erofs/namei.c | 1 +
> > drivers/staging/erofs/super.c | 8 +++++
> > drivers/staging/erofs/xattr.c | 37 ++++++++++++++++++++++
> > drivers/staging/erofs/xattr.h | 6 ++++
> > 6 files changed, 57 insertions(+)
> >
> > diff --git a/drivers/staging/erofs/Documentation/filesystems/erofs.txt b/drivers/staging/erofs/Documentation/filesystems/erofs.txt
> > index 803988d74c21..961ec4da7705 100644
> > --- a/drivers/staging/erofs/Documentation/filesystems/erofs.txt
> > +++ b/drivers/staging/erofs/Documentation/filesystems/erofs.txt
> > @@ -36,6 +36,8 @@ Here is the main features of EROFS:
> >
> > - Support xattr inline and tail-end data inline for all files;
> >
> > + - Support POSIX.1e ACLs by using xattrs;
> > +
> > - Support transparent file compression as an option:
> > LZ4 algorithm with 4 KB fixed-output compression for high performance;
> >
> > diff --git a/drivers/staging/erofs/inode.c b/drivers/staging/erofs/inode.c
> > index 4f04f7c38cf2..924b8dfc7a8f 100644
> > --- a/drivers/staging/erofs/inode.c
> > +++ b/drivers/staging/erofs/inode.c
> > @@ -287,6 +287,7 @@ const struct inode_operations erofs_generic_iops = {
> > #ifdef CONFIG_EROFS_FS_XATTR
> > .listxattr = erofs_listxattr,
> > #endif
> > + .get_acl = erofs_get_acl,
> > };
> >
> > const struct inode_operations erofs_symlink_iops = {
> > @@ -294,6 +295,7 @@ const struct inode_operations erofs_symlink_iops = {
> > #ifdef CONFIG_EROFS_FS_XATTR
> > .listxattr = erofs_listxattr,
> > #endif
> > + .get_acl = erofs_get_acl,
> > };
> >
> > const struct inode_operations erofs_fast_symlink_iops = {
> > @@ -301,5 +303,6 @@ const struct inode_operations erofs_fast_symlink_iops = {
> > #ifdef CONFIG_EROFS_FS_XATTR
> > .listxattr = erofs_listxattr,
> > #endif
> > + .get_acl = erofs_get_acl,
> > };
> >
> > diff --git a/drivers/staging/erofs/namei.c b/drivers/staging/erofs/namei.c
> > index 7fed1f996ab0..b1752adc5934 100644
> > --- a/drivers/staging/erofs/namei.c
> > +++ b/drivers/staging/erofs/namei.c
> > @@ -238,5 +238,6 @@ const struct inode_operations erofs_dir_iops = {
> > #ifdef CONFIG_EROFS_FS_XATTR
> > .listxattr = erofs_listxattr,
> > #endif
> > + .get_acl = erofs_get_acl,
> > };
> >
> > diff --git a/drivers/staging/erofs/super.c b/drivers/staging/erofs/super.c
> > index 176fca2af379..54cd7dac0a1f 100644
> > --- a/drivers/staging/erofs/super.c
> > +++ b/drivers/staging/erofs/super.c
> > @@ -398,6 +398,14 @@ static int erofs_read_super(struct super_block *sb,
> > if (!silent)
> > infoln("root inode @ nid %llu", ROOT_NID(sbi));
> >
> > +#ifdef CONFIG_EROFS_FS_POSIX_ACL
> > + /* Update the POSIXACL Flag */
> > + if (test_opt(sbi, POSIX_ACL))
> > + sb->s_flags |= SB_POSIXACL;
> > + else
> > + sb->s_flags &= ~SB_POSIXACL;
> > +#endif
> > +
> > #ifdef CONFIG_EROFS_FS_ZIP
> > INIT_RADIX_TREE(&sbi->workstn_tree, GFP_ATOMIC);
> > #endif
> > diff --git a/drivers/staging/erofs/xattr.c b/drivers/staging/erofs/xattr.c
> > index 7de46690d972..6759485ae862 100644
> > --- a/drivers/staging/erofs/xattr.c
> > +++ b/drivers/staging/erofs/xattr.c
> > @@ -643,3 +643,40 @@ ssize_t erofs_listxattr(struct dentry *dentry,
> > return shared_listxattr(&it);
> > }
> >
> > +#ifdef CONFIG_EROFS_FS_POSIX_ACL
> > +struct posix_acl *erofs_get_acl(struct inode *inode, int type)
> > +{
> > + struct posix_acl *acl;
> > + int ea_prefix, rc;
> > + char *value = NULL;
> > +
> > + switch(type) {
> > + case ACL_TYPE_ACCESS:
> > + ea_prefix = EROFS_XATTR_INDEX_POSIX_ACL_ACCESS;
> > + break;
> > + case ACL_TYPE_DEFAULT:
> > + ea_prefix = EROFS_XATTR_INDEX_POSIX_ACL_DEFAULT;
> > + break;
> > + default:
> > + return ERR_PTR(-EINVAL);
> > + }
> > +
> > + rc = erofs_getxattr(inode, ea_prefix, "", NULL, 0);
> > + if (rc > 0) {
> > + value = kvmalloc(rc, GFP_KERNEL);
>
> erofs_kmalloc() is enough?
>
> Thanks,
>
Hopefully, regular kmalloc() is enough.
Do really need the erofs_kmalloc() function? Regular kmalloc() has
fault injection already. Have you tried to use it?
regards,
dan carpenter
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v2 2/2] staging: erofs: complete POSIX ACL support
@ 2019-01-28 13:33 ` Dan Carpenter
0 siblings, 0 replies; 32+ messages in thread
From: Dan Carpenter @ 2019-01-28 13:33 UTC (permalink / raw)
To: Chao Yu
Cc: Gao Xiang, Greg Kroah-Hartman, devel, linux-erofs, Chao Yu, LKML,
weidu.du, Fang Wei, Miao Xie
On Sat, Jan 26, 2019 at 10:48:53AM +0800, Chao Yu wrote:
> On 2019/1/26 0:10, Gao Xiang wrote:
> > Let's add .get_acl() to read the file's acl from its xattrs
> > to make POSIX ACL usable.
> >
> > Here is the on-disk detail,
> > fullname: system.posix_acl_access
> > struct erofs_xattr_entry:
> > .e_name_len = 0
> > .e_name_index = EROFS_XATTR_INDEX_POSIX_ACL_ACCESS (2)
> >
> > fullname: system.posix_acl_default
> > struct erofs_xattr_entry:
> > .e_name_len = 0
> > .e_name_index = EROFS_XATTR_INDEX_POSIX_ACL_DEFAULT (3)
> >
> > Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
> > ---
> > change log v2:
> > - add the missing SB_POSIXACL flag;
> > - fix the on-disk detail, use .e_name_len == 0 and proper prefix value;
> > - tested ok with xattr enabled erofs_mkfs;
>
> > .../erofs/Documentation/filesystems/erofs.txt | 2 ++
> > drivers/staging/erofs/inode.c | 3 ++
> > drivers/staging/erofs/namei.c | 1 +
> > drivers/staging/erofs/super.c | 8 +++++
> > drivers/staging/erofs/xattr.c | 37 ++++++++++++++++++++++
> > drivers/staging/erofs/xattr.h | 6 ++++
> > 6 files changed, 57 insertions(+)
> >
> > diff --git a/drivers/staging/erofs/Documentation/filesystems/erofs.txt b/drivers/staging/erofs/Documentation/filesystems/erofs.txt
> > index 803988d74c21..961ec4da7705 100644
> > --- a/drivers/staging/erofs/Documentation/filesystems/erofs.txt
> > +++ b/drivers/staging/erofs/Documentation/filesystems/erofs.txt
> > @@ -36,6 +36,8 @@ Here is the main features of EROFS:
> >
> > - Support xattr inline and tail-end data inline for all files;
> >
> > + - Support POSIX.1e ACLs by using xattrs;
> > +
> > - Support transparent file compression as an option:
> > LZ4 algorithm with 4 KB fixed-output compression for high performance;
> >
> > diff --git a/drivers/staging/erofs/inode.c b/drivers/staging/erofs/inode.c
> > index 4f04f7c38cf2..924b8dfc7a8f 100644
> > --- a/drivers/staging/erofs/inode.c
> > +++ b/drivers/staging/erofs/inode.c
> > @@ -287,6 +287,7 @@ const struct inode_operations erofs_generic_iops = {
> > #ifdef CONFIG_EROFS_FS_XATTR
> > .listxattr = erofs_listxattr,
> > #endif
> > + .get_acl = erofs_get_acl,
> > };
> >
> > const struct inode_operations erofs_symlink_iops = {
> > @@ -294,6 +295,7 @@ const struct inode_operations erofs_symlink_iops = {
> > #ifdef CONFIG_EROFS_FS_XATTR
> > .listxattr = erofs_listxattr,
> > #endif
> > + .get_acl = erofs_get_acl,
> > };
> >
> > const struct inode_operations erofs_fast_symlink_iops = {
> > @@ -301,5 +303,6 @@ const struct inode_operations erofs_fast_symlink_iops = {
> > #ifdef CONFIG_EROFS_FS_XATTR
> > .listxattr = erofs_listxattr,
> > #endif
> > + .get_acl = erofs_get_acl,
> > };
> >
> > diff --git a/drivers/staging/erofs/namei.c b/drivers/staging/erofs/namei.c
> > index 7fed1f996ab0..b1752adc5934 100644
> > --- a/drivers/staging/erofs/namei.c
> > +++ b/drivers/staging/erofs/namei.c
> > @@ -238,5 +238,6 @@ const struct inode_operations erofs_dir_iops = {
> > #ifdef CONFIG_EROFS_FS_XATTR
> > .listxattr = erofs_listxattr,
> > #endif
> > + .get_acl = erofs_get_acl,
> > };
> >
> > diff --git a/drivers/staging/erofs/super.c b/drivers/staging/erofs/super.c
> > index 176fca2af379..54cd7dac0a1f 100644
> > --- a/drivers/staging/erofs/super.c
> > +++ b/drivers/staging/erofs/super.c
> > @@ -398,6 +398,14 @@ static int erofs_read_super(struct super_block *sb,
> > if (!silent)
> > infoln("root inode @ nid %llu", ROOT_NID(sbi));
> >
> > +#ifdef CONFIG_EROFS_FS_POSIX_ACL
> > + /* Update the POSIXACL Flag */
> > + if (test_opt(sbi, POSIX_ACL))
> > + sb->s_flags |= SB_POSIXACL;
> > + else
> > + sb->s_flags &= ~SB_POSIXACL;
> > +#endif
> > +
> > #ifdef CONFIG_EROFS_FS_ZIP
> > INIT_RADIX_TREE(&sbi->workstn_tree, GFP_ATOMIC);
> > #endif
> > diff --git a/drivers/staging/erofs/xattr.c b/drivers/staging/erofs/xattr.c
> > index 7de46690d972..6759485ae862 100644
> > --- a/drivers/staging/erofs/xattr.c
> > +++ b/drivers/staging/erofs/xattr.c
> > @@ -643,3 +643,40 @@ ssize_t erofs_listxattr(struct dentry *dentry,
> > return shared_listxattr(&it);
> > }
> >
> > +#ifdef CONFIG_EROFS_FS_POSIX_ACL
> > +struct posix_acl *erofs_get_acl(struct inode *inode, int type)
> > +{
> > + struct posix_acl *acl;
> > + int ea_prefix, rc;
> > + char *value = NULL;
> > +
> > + switch(type) {
> > + case ACL_TYPE_ACCESS:
> > + ea_prefix = EROFS_XATTR_INDEX_POSIX_ACL_ACCESS;
> > + break;
> > + case ACL_TYPE_DEFAULT:
> > + ea_prefix = EROFS_XATTR_INDEX_POSIX_ACL_DEFAULT;
> > + break;
> > + default:
> > + return ERR_PTR(-EINVAL);
> > + }
> > +
> > + rc = erofs_getxattr(inode, ea_prefix, "", NULL, 0);
> > + if (rc > 0) {
> > + value = kvmalloc(rc, GFP_KERNEL);
>
> erofs_kmalloc() is enough?
>
> Thanks,
>
Hopefully, regular kmalloc() is enough.
Do really need the erofs_kmalloc() function? Regular kmalloc() has
fault injection already. Have you tried to use it?
regards,
dan carpenter
^ permalink raw reply [flat|nested] 32+ messages in thread* [PATCH v2 2/2] staging: erofs: complete POSIX ACL support
2019-01-28 13:33 ` Dan Carpenter
@ 2019-01-28 13:48 ` Gao Xiang
-1 siblings, 0 replies; 32+ messages in thread
From: Gao Xiang @ 2019-01-28 13:48 UTC (permalink / raw)
Hi Dan,
On 2019/1/28 21:33, Dan Carpenter wrote:
> Hopefully, regular kmalloc() is enough.
>
> Do really need the erofs_kmalloc() function? Regular kmalloc() has
> fault injection already. Have you tried to use it?
The fault injection subsystem was introduced in the initial upstreamed
EROFS version, which is taken from f2fs by Chao Yu.
Fault injection is important to test fs.. However, I have no more idea
about the future plan of this feature...Follow f2fs or use the general
fault injection...
There already exists erofs_kmalloc, thus I think it's both ok to use
erofs_kmalloc or kmalloc for this patch at least.
Chao, any idea about this?
Thanks,
Gao Xiang
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 2/2] staging: erofs: complete POSIX ACL support
@ 2019-01-28 13:48 ` Gao Xiang
0 siblings, 0 replies; 32+ messages in thread
From: Gao Xiang @ 2019-01-28 13:48 UTC (permalink / raw)
To: Dan Carpenter, Chao Yu, Chao Yu
Cc: Greg Kroah-Hartman, devel, linux-erofs, LKML, weidu.du, Fang Wei,
Miao Xie
Hi Dan,
On 2019/1/28 21:33, Dan Carpenter wrote:
> Hopefully, regular kmalloc() is enough.
>
> Do really need the erofs_kmalloc() function? Regular kmalloc() has
> fault injection already. Have you tried to use it?
The fault injection subsystem was introduced in the initial upstreamed
EROFS version, which is taken from f2fs by Chao Yu.
Fault injection is important to test fs.. However, I have no more idea
about the future plan of this feature...Follow f2fs or use the general
fault injection...
There already exists erofs_kmalloc, thus I think it's both ok to use
erofs_kmalloc or kmalloc for this patch at least.
Chao, any idea about this?
Thanks,
Gao Xiang
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 2/2] staging: erofs: complete POSIX ACL support
2019-01-28 13:48 ` Gao Xiang
@ 2019-01-28 14:28 ` Dan Carpenter
-1 siblings, 0 replies; 32+ messages in thread
From: Dan Carpenter @ 2019-01-28 14:28 UTC (permalink / raw)
The point is, that people shouldn't recreate core code that already
exists. At least try it out and have an explanation why the other code
doesn't work.
In other projects, the default is to keep code around once it has been
written but in staging the default choice is to delete the code unless
there is an explanation.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 2/2] staging: erofs: complete POSIX ACL support
@ 2019-01-28 14:28 ` Dan Carpenter
0 siblings, 0 replies; 32+ messages in thread
From: Dan Carpenter @ 2019-01-28 14:28 UTC (permalink / raw)
To: Gao Xiang
Cc: Chao Yu, Chao Yu, devel, Greg Kroah-Hartman, Miao Xie, LKML,
weidu.du, Fang Wei, linux-erofs
The point is, that people shouldn't recreate core code that already
exists. At least try it out and have an explanation why the other code
doesn't work.
In other projects, the default is to keep code around once it has been
written but in staging the default choice is to delete the code unless
there is an explanation.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 2/2] staging: erofs: complete POSIX ACL support
2019-01-28 14:28 ` Dan Carpenter
@ 2019-01-28 15:04 ` Gao Xiang
-1 siblings, 0 replies; 32+ messages in thread
From: Gao Xiang @ 2019-01-28 15:04 UTC (permalink / raw)
On 2019/1/28 22:28, Dan Carpenter wrote:
> The point is, that people shouldn't recreate core code that already
> exists. At least try it out and have an explanation why the other code
> doesn't work.
>
> In other projects, the default is to keep code around once it has been
> written but in staging the default choice is to delete the code unless
> there is an explanation.
That is why I need Chao's idea about the future of EROFS fault injection.
As far as I know, the fault injection of f2fs also introduces after
the general fault injection, and the original reason is to test separated
partitions with different fault injection rate/type simultaneously, as follows:
commit 1ecc0c5c50ce8834f7e35b63be7480bf1aaa4155
Author: Chao Yu <yuchao0 at huawei.com>
Date: Fri Sep 23 21:30:09 2016 +0800
f2fs: support configuring fault injection per superblock
Previously, we only support global fault injection configuration, so that
when we configure type/rate of fault injection through sysfs, mount
option, it will influence all f2fs partition which is being used.
It is not make sence, since it will be not convenient if developer want
to test separated partitions with different fault injection rate/type
simultaneously, also it's not possible to enable fault injection in one
partition and disable fault injection in other one.
>From now on, we move global configuration of fault injection in module
into per-superblock, hence injection testing can be more flexible.
Signed-off-by: Chao Yu <yuchao0 at huawei.com>
Signed-off-by: Jaegeuk Kim <jaegeuk at kernel.org>
For this patch, I could resend v4 to change erofs_kmalloc->kmalloc
if you like, it isn't an important stuff for this patch. However,
I'd like to get more ideas about the fault injection.
Thanks,
Gao Xiang
>
> regards,
> dan carpenter
>
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v2 2/2] staging: erofs: complete POSIX ACL support
@ 2019-01-28 15:04 ` Gao Xiang
0 siblings, 0 replies; 32+ messages in thread
From: Gao Xiang @ 2019-01-28 15:04 UTC (permalink / raw)
To: Dan Carpenter, Gao Xiang, Chao Yu, Chao Yu
Cc: devel, Greg Kroah-Hartman, linux-erofs, LKML, weidu.du, Miao Xie
On 2019/1/28 22:28, Dan Carpenter wrote:
> The point is, that people shouldn't recreate core code that already
> exists. At least try it out and have an explanation why the other code
> doesn't work.
>
> In other projects, the default is to keep code around once it has been
> written but in staging the default choice is to delete the code unless
> there is an explanation.
That is why I need Chao's idea about the future of EROFS fault injection.
As far as I know, the fault injection of f2fs also introduces after
the general fault injection, and the original reason is to test separated
partitions with different fault injection rate/type simultaneously, as follows:
commit 1ecc0c5c50ce8834f7e35b63be7480bf1aaa4155
Author: Chao Yu <yuchao0@huawei.com>
Date: Fri Sep 23 21:30:09 2016 +0800
f2fs: support configuring fault injection per superblock
Previously, we only support global fault injection configuration, so that
when we configure type/rate of fault injection through sysfs, mount
option, it will influence all f2fs partition which is being used.
It is not make sence, since it will be not convenient if developer want
to test separated partitions with different fault injection rate/type
simultaneously, also it's not possible to enable fault injection in one
partition and disable fault injection in other one.
>From now on, we move global configuration of fault injection in module
into per-superblock, hence injection testing can be more flexible.
Signed-off-by: Chao Yu <yuchao0@huawei.com>
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
For this patch, I could resend v4 to change erofs_kmalloc->kmalloc
if you like, it isn't an important stuff for this patch. However,
I'd like to get more ideas about the fault injection.
Thanks,
Gao Xiang
>
> regards,
> dan carpenter
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 2/2] staging: erofs: complete POSIX ACL support
2019-01-28 13:48 ` Gao Xiang
@ 2019-01-28 16:41 ` Chao Yu
-1 siblings, 0 replies; 32+ messages in thread
From: Chao Yu @ 2019-01-28 16:41 UTC (permalink / raw)
Hi Dan and Xiang,
On 2019-1-28 21:48, Gao Xiang wrote:
> Hi Dan,
>
> On 2019/1/28 21:33, Dan Carpenter wrote:
>> Hopefully, regular kmalloc() is enough.
>>
>> Do really need the erofs_kmalloc() function? Regular kmalloc() has
>> fault injection already. Have you tried to use it?
Yes, I think we'd better to use erofs_kmalloc(). :)
Actually, fault injection in erofs_kmalloc only affect erofs module, we can
expect that the range of fault can be limited in erofs code, rather than whole
kernel, so the test point can be aimed at more accurately.
>
> The fault injection subsystem was introduced in the initial upstreamed
> EROFS version, which is taken from f2fs by Chao Yu.
>
> Fault injection is important to test fs.. However, I have no more idea
> about the future plan of this feature...Follow f2fs or use the general
> fault injection...
I guess following f2fs is not bad.
Thanks,
>
> There already exists erofs_kmalloc, thus I think it's both ok to use
> erofs_kmalloc or kmalloc for this patch at least.
>
> Chao, any idea about this?
>
> Thanks,
> Gao Xiang
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 2/2] staging: erofs: complete POSIX ACL support
@ 2019-01-28 16:41 ` Chao Yu
0 siblings, 0 replies; 32+ messages in thread
From: Chao Yu @ 2019-01-28 16:41 UTC (permalink / raw)
To: Gao Xiang, Dan Carpenter, Chao Yu
Cc: Greg Kroah-Hartman, devel, linux-erofs, LKML, weidu.du, Fang Wei,
Miao Xie
Hi Dan and Xiang,
On 2019-1-28 21:48, Gao Xiang wrote:
> Hi Dan,
>
> On 2019/1/28 21:33, Dan Carpenter wrote:
>> Hopefully, regular kmalloc() is enough.
>>
>> Do really need the erofs_kmalloc() function? Regular kmalloc() has
>> fault injection already. Have you tried to use it?
Yes, I think we'd better to use erofs_kmalloc(). :)
Actually, fault injection in erofs_kmalloc only affect erofs module, we can
expect that the range of fault can be limited in erofs code, rather than whole
kernel, so the test point can be aimed at more accurately.
>
> The fault injection subsystem was introduced in the initial upstreamed
> EROFS version, which is taken from f2fs by Chao Yu.
>
> Fault injection is important to test fs.. However, I have no more idea
> about the future plan of this feature...Follow f2fs or use the general
> fault injection...
I guess following f2fs is not bad.
Thanks,
>
> There already exists erofs_kmalloc, thus I think it's both ok to use
> erofs_kmalloc or kmalloc for this patch at least.
>
> Chao, any idea about this?
>
> Thanks,
> Gao Xiang
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 2/2] staging: erofs: complete POSIX ACL support
2019-01-28 16:41 ` Chao Yu
@ 2019-01-28 18:30 ` Dan Carpenter
-1 siblings, 0 replies; 32+ messages in thread
From: Dan Carpenter @ 2019-01-28 18:30 UTC (permalink / raw)
On Tue, Jan 29, 2019@12:41:55AM +0800, Chao Yu wrote:
> Hi Dan and Xiang,
>
> On 2019-1-28 21:48, Gao Xiang wrote:
> > Hi Dan,
> >
> > On 2019/1/28 21:33, Dan Carpenter wrote:
> >> Hopefully, regular kmalloc() is enough.
> >>
> >> Do really need the erofs_kmalloc() function? Regular kmalloc() has
> >> fault injection already. Have you tried to use it?
>
> Yes, I think we'd better to use erofs_kmalloc(). :)
>
> Actually, fault injection in erofs_kmalloc only affect erofs module, we can
> expect that the range of fault can be limited in erofs code, rather than whole
> kernel, so the test point can be aimed at more accurately.
>
Are you serious? The standard fault injection doesn't do that???
Please fix it instead of creating a duplicate better implementation
which only your filesystem can use. I would have thought that obviously
any fault injection framework could at least be configured to test
specific code...
regards,
dan carpenter
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 2/2] staging: erofs: complete POSIX ACL support
@ 2019-01-28 18:30 ` Dan Carpenter
0 siblings, 0 replies; 32+ messages in thread
From: Dan Carpenter @ 2019-01-28 18:30 UTC (permalink / raw)
To: Chao Yu
Cc: Gao Xiang, Chao Yu, devel, Greg Kroah-Hartman, Miao Xie, LKML,
weidu.du, Fang Wei, linux-erofs
On Tue, Jan 29, 2019 at 12:41:55AM +0800, Chao Yu wrote:
> Hi Dan and Xiang,
>
> On 2019-1-28 21:48, Gao Xiang wrote:
> > Hi Dan,
> >
> > On 2019/1/28 21:33, Dan Carpenter wrote:
> >> Hopefully, regular kmalloc() is enough.
> >>
> >> Do really need the erofs_kmalloc() function? Regular kmalloc() has
> >> fault injection already. Have you tried to use it?
>
> Yes, I think we'd better to use erofs_kmalloc(). :)
>
> Actually, fault injection in erofs_kmalloc only affect erofs module, we can
> expect that the range of fault can be limited in erofs code, rather than whole
> kernel, so the test point can be aimed at more accurately.
>
Are you serious? The standard fault injection doesn't do that???
Please fix it instead of creating a duplicate better implementation
which only your filesystem can use. I would have thought that obviously
any fault injection framework could at least be configured to test
specific code...
regards,
dan carpenter
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 2/2] staging: erofs: complete POSIX ACL support
2019-01-28 18:30 ` Dan Carpenter
@ 2019-01-29 8:03 ` Gao Xiang
-1 siblings, 0 replies; 32+ messages in thread
From: Gao Xiang @ 2019-01-29 8:03 UTC (permalink / raw)
Hi,
On 2019/1/29 2:30, Dan Carpenter wrote:
> Are you serious? The standard fault injection doesn't do that???
>
> Please fix it instead of creating a duplicate better implementation
> which only your filesystem can use. I would have thought that obviously
> any fault injection framework could at least be configured to test
> specific code...
>
> regards,
> dan carpenter
Since Chao is now on the short business trip, I will resend v4 and change
erofs_kmalloc into kmalloc temporarily to make sure ACL is usable first
in the staging tree.
As for the EROFS fault injection, let's wait Chao's reply...
Thanks,
Gao Xiang
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 2/2] staging: erofs: complete POSIX ACL support
@ 2019-01-29 8:03 ` Gao Xiang
0 siblings, 0 replies; 32+ messages in thread
From: Gao Xiang @ 2019-01-29 8:03 UTC (permalink / raw)
To: Dan Carpenter, Chao Yu
Cc: Chao Yu, devel, Greg Kroah-Hartman, Miao Xie, LKML, weidu.du,
Fang Wei, linux-erofs
Hi,
On 2019/1/29 2:30, Dan Carpenter wrote:
> Are you serious? The standard fault injection doesn't do that???
>
> Please fix it instead of creating a duplicate better implementation
> which only your filesystem can use. I would have thought that obviously
> any fault injection framework could at least be configured to test
> specific code...
>
> regards,
> dan carpenter
Since Chao is now on the short business trip, I will resend v4 and change
erofs_kmalloc into kmalloc temporarily to make sure ACL is usable first
in the staging tree.
As for the EROFS fault injection, let's wait Chao's reply...
Thanks,
Gao Xiang
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 2/2] staging: erofs: complete POSIX ACL support
2019-01-28 18:30 ` Dan Carpenter
@ 2019-02-03 2:52 ` Chao Yu
-1 siblings, 0 replies; 32+ messages in thread
From: Chao Yu @ 2019-02-03 2:52 UTC (permalink / raw)
Sorry for the delay due to business travel.
On 2019/1/29 2:30, Dan Carpenter wrote:
> On Tue, Jan 29, 2019@12:41:55AM +0800, Chao Yu wrote:
>> Hi Dan and Xiang,
>>
>> On 2019-1-28 21:48, Gao Xiang wrote:
>>> Hi Dan,
>>>
>>> On 2019/1/28 21:33, Dan Carpenter wrote:
>>>> Hopefully, regular kmalloc() is enough.
>>>>
>>>> Do really need the erofs_kmalloc() function? Regular kmalloc() has
>>>> fault injection already. Have you tried to use it?
>>
>> Yes, I think we'd better to use erofs_kmalloc(). :)
>>
>> Actually, fault injection in erofs_kmalloc only affect erofs module, we can
>> expect that the range of fault can be limited in erofs code, rather than whole
>> kernel, so the test point can be aimed at more accurately.
>>
>
> Are you serious? The standard fault injection doesn't do that???
Oh, I just realized the common fault injection can inject into specified
module with function granularity, sorry.
>
> Please fix it instead of creating a duplicate better implementation
> which only your filesystem can use. I would have thought that obviously
I agreed that it will be good to make common fault injection better,
covering more cases, so that it can benefit all modules which need fault
injection functionality. But rather than injecting kmalloc, there will be
other injection demands from erofs/f2fs, like injecting in the middle of
their specified function, how could we do that? Could you give us advice?
Thanks,
> any fault injection framework could at least be configured to test
> specific code...
>
> regards,
> dan carpenter
>
>
> .
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 2/2] staging: erofs: complete POSIX ACL support
@ 2019-02-03 2:52 ` Chao Yu
0 siblings, 0 replies; 32+ messages in thread
From: Chao Yu @ 2019-02-03 2:52 UTC (permalink / raw)
To: Dan Carpenter, Chao Yu
Cc: Gao Xiang, devel, Greg Kroah-Hartman, Miao Xie, LKML, weidu.du,
Fang Wei, linux-erofs
Sorry for the delay due to business travel.
On 2019/1/29 2:30, Dan Carpenter wrote:
> On Tue, Jan 29, 2019 at 12:41:55AM +0800, Chao Yu wrote:
>> Hi Dan and Xiang,
>>
>> On 2019-1-28 21:48, Gao Xiang wrote:
>>> Hi Dan,
>>>
>>> On 2019/1/28 21:33, Dan Carpenter wrote:
>>>> Hopefully, regular kmalloc() is enough.
>>>>
>>>> Do really need the erofs_kmalloc() function? Regular kmalloc() has
>>>> fault injection already. Have you tried to use it?
>>
>> Yes, I think we'd better to use erofs_kmalloc(). :)
>>
>> Actually, fault injection in erofs_kmalloc only affect erofs module, we can
>> expect that the range of fault can be limited in erofs code, rather than whole
>> kernel, so the test point can be aimed at more accurately.
>>
>
> Are you serious? The standard fault injection doesn't do that???
Oh, I just realized the common fault injection can inject into specified
module with function granularity, sorry.
>
> Please fix it instead of creating a duplicate better implementation
> which only your filesystem can use. I would have thought that obviously
I agreed that it will be good to make common fault injection better,
covering more cases, so that it can benefit all modules which need fault
injection functionality. But rather than injecting kmalloc, there will be
other injection demands from erofs/f2fs, like injecting in the middle of
their specified function, how could we do that? Could you give us advice?
Thanks,
> any fault injection framework could at least be configured to test
> specific code...
>
> regards,
> dan carpenter
>
>
> .
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 2/2] staging: erofs: complete POSIX ACL support
2019-02-03 2:52 ` Chao Yu
@ 2019-02-15 2:10 ` Chao Yu
-1 siblings, 0 replies; 32+ messages in thread
From: Chao Yu @ 2019-02-15 2:10 UTC (permalink / raw)
Hi Dan,
Any suggestion?
On 2019/2/3 10:52, Chao Yu wrote:
> Sorry for the delay due to business travel.
>
> On 2019/1/29 2:30, Dan Carpenter wrote:
>> On Tue, Jan 29, 2019@12:41:55AM +0800, Chao Yu wrote:
>>> Hi Dan and Xiang,
>>>
>>> On 2019-1-28 21:48, Gao Xiang wrote:
>>>> Hi Dan,
>>>>
>>>> On 2019/1/28 21:33, Dan Carpenter wrote:
>>>>> Hopefully, regular kmalloc() is enough.
>>>>>
>>>>> Do really need the erofs_kmalloc() function? Regular kmalloc() has
>>>>> fault injection already. Have you tried to use it?
>>>
>>> Yes, I think we'd better to use erofs_kmalloc(). :)
>>>
>>> Actually, fault injection in erofs_kmalloc only affect erofs module, we can
>>> expect that the range of fault can be limited in erofs code, rather than whole
>>> kernel, so the test point can be aimed at more accurately.
>>>
>>
>> Are you serious? The standard fault injection doesn't do that???
>
> Oh, I just realized the common fault injection can inject into specified
> module with function granularity, sorry.
>
>>
>> Please fix it instead of creating a duplicate better implementation
>> which only your filesystem can use. I would have thought that obviously
>
> I agreed that it will be good to make common fault injection better,
> covering more cases, so that it can benefit all modules which need fault
> injection functionality. But rather than injecting kmalloc, there will be
> other injection demands from erofs/f2fs, like injecting in the middle of
> their specified function, how could we do that? Could you give us advice?
>
> Thanks,
>
>> any fault injection framework could at least be configured to test
>> specific code...
>>
>> regards,
>> dan carpenter
>>
>>
>> .
>>
>
>
> .
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 2/2] staging: erofs: complete POSIX ACL support
@ 2019-02-15 2:10 ` Chao Yu
0 siblings, 0 replies; 32+ messages in thread
From: Chao Yu @ 2019-02-15 2:10 UTC (permalink / raw)
To: Dan Carpenter, Chao Yu
Cc: devel, Greg Kroah-Hartman, linux-erofs, LKML, weidu.du, Miao Xie
Hi Dan,
Any suggestion?
On 2019/2/3 10:52, Chao Yu wrote:
> Sorry for the delay due to business travel.
>
> On 2019/1/29 2:30, Dan Carpenter wrote:
>> On Tue, Jan 29, 2019 at 12:41:55AM +0800, Chao Yu wrote:
>>> Hi Dan and Xiang,
>>>
>>> On 2019-1-28 21:48, Gao Xiang wrote:
>>>> Hi Dan,
>>>>
>>>> On 2019/1/28 21:33, Dan Carpenter wrote:
>>>>> Hopefully, regular kmalloc() is enough.
>>>>>
>>>>> Do really need the erofs_kmalloc() function? Regular kmalloc() has
>>>>> fault injection already. Have you tried to use it?
>>>
>>> Yes, I think we'd better to use erofs_kmalloc(). :)
>>>
>>> Actually, fault injection in erofs_kmalloc only affect erofs module, we can
>>> expect that the range of fault can be limited in erofs code, rather than whole
>>> kernel, so the test point can be aimed at more accurately.
>>>
>>
>> Are you serious? The standard fault injection doesn't do that???
>
> Oh, I just realized the common fault injection can inject into specified
> module with function granularity, sorry.
>
>>
>> Please fix it instead of creating a duplicate better implementation
>> which only your filesystem can use. I would have thought that obviously
>
> I agreed that it will be good to make common fault injection better,
> covering more cases, so that it can benefit all modules which need fault
> injection functionality. But rather than injecting kmalloc, there will be
> other injection demands from erofs/f2fs, like injecting in the middle of
> their specified function, how could we do that? Could you give us advice?
>
> Thanks,
>
>> any fault injection framework could at least be configured to test
>> specific code...
>>
>> regards,
>> dan carpenter
>>
>>
>> .
>>
>
>
> .
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 2/2] staging: erofs: complete POSIX ACL support
2019-02-15 2:10 ` Chao Yu
@ 2019-02-15 7:36 ` Dan Carpenter
-1 siblings, 0 replies; 32+ messages in thread
From: Dan Carpenter @ 2019-02-15 7:36 UTC (permalink / raw)
On Fri, Feb 15, 2019@10:10:34AM +0800, Chao Yu wrote:
> Hi Dan,
>
> Any suggestion?
>
I won't NAK whatever you decide. But my opinion is that you should
just use normal kernel memory allocators even though it means you have
to use two different fault injection frameworks.
Over time it would be good to improve and expand the standard kernel
error injection frameworks to cover more types.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 2/2] staging: erofs: complete POSIX ACL support
@ 2019-02-15 7:36 ` Dan Carpenter
0 siblings, 0 replies; 32+ messages in thread
From: Dan Carpenter @ 2019-02-15 7:36 UTC (permalink / raw)
To: Chao Yu
Cc: Chao Yu, devel, Greg Kroah-Hartman, linux-erofs, LKML, weidu.du,
Miao Xie
On Fri, Feb 15, 2019 at 10:10:34AM +0800, Chao Yu wrote:
> Hi Dan,
>
> Any suggestion?
>
I won't NAK whatever you decide. But my opinion is that you should
just use normal kernel memory allocators even though it means you have
to use two different fault injection frameworks.
Over time it would be good to improve and expand the standard kernel
error injection frameworks to cover more types.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 2/2] staging: erofs: complete POSIX ACL support
2019-02-15 7:36 ` Dan Carpenter
@ 2019-02-15 9:31 ` Chao Yu
-1 siblings, 0 replies; 32+ messages in thread
From: Chao Yu @ 2019-02-15 9:31 UTC (permalink / raw)
On 2019/2/15 15:36, Dan Carpenter wrote:
> On Fri, Feb 15, 2019@10:10:34AM +0800, Chao Yu wrote:
>> Hi Dan,
>>
>> Any suggestion?
>>
>
> I won't NAK whatever you decide. But my opinion is that you should
> just use normal kernel memory allocators even though it means you have
> to use two different fault injection frameworks.
Thanks for the suggestion.
There is only one injection type in erofs injection framework, I don't want
to remove all framework codes with kmalloc type, so I think we can do this
step by step:
a. use erofs_kamloc to keep all codes consistent.
b. add at least one erofs private injection type.
c. remove erofs_kmalloc and related type.
>
> Over time it would be good to improve and expand the standard kernel
> error injection frameworks to cover more types.
Agreed.
Thanks,
>
> regards,
> dan carpenter
>
>
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 2/2] staging: erofs: complete POSIX ACL support
@ 2019-02-15 9:31 ` Chao Yu
0 siblings, 0 replies; 32+ messages in thread
From: Chao Yu @ 2019-02-15 9:31 UTC (permalink / raw)
To: Dan Carpenter
Cc: Chao Yu, devel, Greg Kroah-Hartman, linux-erofs, LKML, weidu.du,
Miao Xie
On 2019/2/15 15:36, Dan Carpenter wrote:
> On Fri, Feb 15, 2019 at 10:10:34AM +0800, Chao Yu wrote:
>> Hi Dan,
>>
>> Any suggestion?
>>
>
> I won't NAK whatever you decide. But my opinion is that you should
> just use normal kernel memory allocators even though it means you have
> to use two different fault injection frameworks.
Thanks for the suggestion.
There is only one injection type in erofs injection framework, I don't want
to remove all framework codes with kmalloc type, so I think we can do this
step by step:
a. use erofs_kamloc to keep all codes consistent.
b. add at least one erofs private injection type.
c. remove erofs_kmalloc and related type.
>
> Over time it would be good to improve and expand the standard kernel
> error injection frameworks to cover more types.
Agreed.
Thanks,
>
> regards,
> dan carpenter
>
>
>
^ permalink raw reply [flat|nested] 32+ messages in thread