* extended attributes limitation of xattr_size_max @ 2020-01-02 22:28 Olga Kornievskaia 2020-01-02 23:31 ` Frank van der Linden 0 siblings, 1 reply; 4+ messages in thread From: Olga Kornievskaia @ 2020-01-02 22:28 UTC (permalink / raw) To: Andreas Gruenbacher, J. Bruce Fields; +Cc: linux-nfs Hi Andreas and Bruce, I thought of you folks as somebody who might have a strong opinion on the topic. Right now an NFS client is limited to setting and getting ACLs that can't be larger than 64K (XATTR_SIZE_MAX) (where some NFS server don't have such limit on the ACL size). There are limits in fs/xattr.c during getting and setting xattrs. I believe that's because linux local xattr system is limited to that particular constraint. However, an NFS acl that uses the xattr interface can be larger than that. Is it at all possible to suggest to the larger FS community to remove those limits or would that be a non-starter? diff --git a/fs/xattr.c b/fs/xattr.c index 90dd78f..52a3f91 100644 --- a/fs/xattr.c +++ b/fs/xattr.c @@ -428,8 +428,6 @@ int __vfs_setxattr_noperm(struct dentry *dentry, const char *name, return error; if (size) { - if (size > XATTR_SIZE_MAX) - return -E2BIG; kvalue = kvmalloc(size, GFP_KERNEL); if (!kvalue) return -ENOMEM; @@ -528,8 +526,6 @@ static int path_setxattr(const char __user *pathname, return error; if (size) { - if (size > XATTR_SIZE_MAX) - size = XATTR_SIZE_MAX; kvalue = kvzalloc(size, GFP_KERNEL); if (!kvalue) return -ENOMEM; -- ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: extended attributes limitation of xattr_size_max 2020-01-02 22:28 extended attributes limitation of xattr_size_max Olga Kornievskaia @ 2020-01-02 23:31 ` Frank van der Linden 2020-01-03 16:13 ` Olga Kornievskaia 0 siblings, 1 reply; 4+ messages in thread From: Frank van der Linden @ 2020-01-02 23:31 UTC (permalink / raw) To: Olga Kornievskaia; +Cc: Andreas Gruenbacher, J. Bruce Fields, linux-nfs On Thu, Jan 02, 2020 at 05:28:44PM -0500, Olga Kornievskaia wrote: > Hi Andreas and Bruce, > > I thought of you folks as somebody who might have a strong opinion on > the topic. Right now an NFS client is limited to setting and getting > ACLs that can't be larger than 64K (XATTR_SIZE_MAX) (where some NFS > server don't have such limit on the ACL size). There are limits in > fs/xattr.c during getting and setting xattrs. I believe that's because > linux local xattr system is limited to that particular constraint. > However, an NFS acl that uses the xattr interface can be larger than > that. Is it at all possible to suggest to the larger FS community to > remove those limits or would that be a non-starter? > > diff --git a/fs/xattr.c b/fs/xattr.c > index 90dd78f..52a3f91 100644 > --- a/fs/xattr.c > +++ b/fs/xattr.c > @@ -428,8 +428,6 @@ int __vfs_setxattr_noperm(struct dentry *dentry, > const char *name, > return error; > > if (size) { > - if (size > XATTR_SIZE_MAX) > - return -E2BIG; > kvalue = kvmalloc(size, GFP_KERNEL); > if (!kvalue) > return -ENOMEM; > @@ -528,8 +526,6 @@ static int path_setxattr(const char __user *pathname, > return error; > > if (size) { > - if (size > XATTR_SIZE_MAX) > - size = XATTR_SIZE_MAX; > kvalue = kvzalloc(size, GFP_KERNEL); > if (!kvalue) > return -ENOMEM; Aside from not wanting to allocate a raw amount of kernel memory based on a system call parameter without any checks, I support the idea :-) The internal xattr interface can be a little awkward to deal with, the static upper limit being one of the issues that caused me some problems when implementing user xattrs for NFS. In general, I would love to see paged-based xattr kernel interfaces, treating extended attributes as a secondary data stream, which would make caching fit in a lot more naturally, and means all FS-specific caching implementations could be removed in favor of a generic one. One issue right now is that, an xattr not being a 'stream', a lot of FS code caches the entire value in kmalloc-ed space, which becomes unwieldy if the XATTR_SIZE_MAX limit is removed. In other words, I think removing the limit won't work that well with the current implementation, but I hope that the implementation can be changed so that the limit can be removed. - Frank ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: extended attributes limitation of xattr_size_max 2020-01-02 23:31 ` Frank van der Linden @ 2020-01-03 16:13 ` Olga Kornievskaia 2020-01-03 17:18 ` Olga Kornievskaia 0 siblings, 1 reply; 4+ messages in thread From: Olga Kornievskaia @ 2020-01-03 16:13 UTC (permalink / raw) To: Frank van der Linden; +Cc: Andreas Gruenbacher, J. Bruce Fields, linux-nfs On Thu, Jan 2, 2020 at 6:31 PM Frank van der Linden <fllinden@amazon.com> wrote: > > On Thu, Jan 02, 2020 at 05:28:44PM -0500, Olga Kornievskaia wrote: > > Hi Andreas and Bruce, > > > > I thought of you folks as somebody who might have a strong opinion on > > the topic. Right now an NFS client is limited to setting and getting > > ACLs that can't be larger than 64K (XATTR_SIZE_MAX) (where some NFS > > server don't have such limit on the ACL size). There are limits in > > fs/xattr.c during getting and setting xattrs. I believe that's because > > linux local xattr system is limited to that particular constraint. > > However, an NFS acl that uses the xattr interface can be larger than > > that. Is it at all possible to suggest to the larger FS community to > > remove those limits or would that be a non-starter? > > > > diff --git a/fs/xattr.c b/fs/xattr.c > > index 90dd78f..52a3f91 100644 > > --- a/fs/xattr.c > > +++ b/fs/xattr.c > > @@ -428,8 +428,6 @@ int __vfs_setxattr_noperm(struct dentry *dentry, > > const char *name, > > return error; > > > > if (size) { > > - if (size > XATTR_SIZE_MAX) > > - return -E2BIG; > > kvalue = kvmalloc(size, GFP_KERNEL); > > if (!kvalue) > > return -ENOMEM; > > @@ -528,8 +526,6 @@ static int path_setxattr(const char __user *pathname, > > return error; > > > > if (size) { > > - if (size > XATTR_SIZE_MAX) > > - size = XATTR_SIZE_MAX; > > kvalue = kvzalloc(size, GFP_KERNEL); > > if (!kvalue) > > return -ENOMEM; > > Aside from not wanting to allocate a raw amount of kernel memory based > on a system call parameter without any checks, I support the idea :-) > > The internal xattr interface can be a little awkward to deal with, > the static upper limit being one of the issues that caused me some > problems when implementing user xattrs for NFS. > > In general, I would love to see paged-based xattr kernel interfaces, > treating extended attributes as a secondary data stream, which would > make caching fit in a lot more naturally, and means all FS-specific > caching implementations could be removed in favor of a generic one. > > One issue right now is that, an xattr not being a 'stream', a lot > of FS code caches the entire value in kmalloc-ed space, which becomes > unwieldy if the XATTR_SIZE_MAX limit is removed. > > In other words, I think removing the limit won't work that well with > the current implementation, but I hope that the implementation can > be changed so that the limit can be removed. Hi Frank, Thank you for your feedback. You are right, unchecked memory allocation in the kernel would not be a good idea. Your suggested approach of page-based xattr seems like a good idea but not something I feel like I can take on right now. I wonder if we can lobby for the xattr limit to be increased to 1M. That would serve NFS needs as right now rpc calls (and thus getattr) are no larger than 1M. Thoughts on that? I'm not familiar with generic xattr usage and I wonder if even local usage could benefit from having a larger limit. > > - Frank ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: extended attributes limitation of xattr_size_max 2020-01-03 16:13 ` Olga Kornievskaia @ 2020-01-03 17:18 ` Olga Kornievskaia 0 siblings, 0 replies; 4+ messages in thread From: Olga Kornievskaia @ 2020-01-03 17:18 UTC (permalink / raw) To: Frank van der Linden; +Cc: Andreas Gruenbacher, J. Bruce Fields, linux-nfs On Fri, Jan 3, 2020 at 11:13 AM Olga Kornievskaia <aglo@umich.edu> wrote: > > On Thu, Jan 2, 2020 at 6:31 PM Frank van der Linden <fllinden@amazon.com> wrote: > > > > On Thu, Jan 02, 2020 at 05:28:44PM -0500, Olga Kornievskaia wrote: > > > Hi Andreas and Bruce, > > > > > > I thought of you folks as somebody who might have a strong opinion on > > > the topic. Right now an NFS client is limited to setting and getting > > > ACLs that can't be larger than 64K (XATTR_SIZE_MAX) (where some NFS > > > server don't have such limit on the ACL size). There are limits in > > > fs/xattr.c during getting and setting xattrs. I believe that's because > > > linux local xattr system is limited to that particular constraint. > > > However, an NFS acl that uses the xattr interface can be larger than > > > that. Is it at all possible to suggest to the larger FS community to > > > remove those limits or would that be a non-starter? > > > > > > diff --git a/fs/xattr.c b/fs/xattr.c > > > index 90dd78f..52a3f91 100644 > > > --- a/fs/xattr.c > > > +++ b/fs/xattr.c > > > @@ -428,8 +428,6 @@ int __vfs_setxattr_noperm(struct dentry *dentry, > > > const char *name, > > > return error; > > > > > > if (size) { > > > - if (size > XATTR_SIZE_MAX) > > > - return -E2BIG; > > > kvalue = kvmalloc(size, GFP_KERNEL); > > > if (!kvalue) > > > return -ENOMEM; > > > @@ -528,8 +526,6 @@ static int path_setxattr(const char __user *pathname, > > > return error; > > > > > > if (size) { > > > - if (size > XATTR_SIZE_MAX) > > > - size = XATTR_SIZE_MAX; > > > kvalue = kvzalloc(size, GFP_KERNEL); > > > if (!kvalue) > > > return -ENOMEM; > > > > Aside from not wanting to allocate a raw amount of kernel memory based > > on a system call parameter without any checks, I support the idea :-) > > > > The internal xattr interface can be a little awkward to deal with, > > the static upper limit being one of the issues that caused me some > > problems when implementing user xattrs for NFS. > > > > In general, I would love to see paged-based xattr kernel interfaces, > > treating extended attributes as a secondary data stream, which would > > make caching fit in a lot more naturally, and means all FS-specific > > caching implementations could be removed in favor of a generic one. > > > > One issue right now is that, an xattr not being a 'stream', a lot > > of FS code caches the entire value in kmalloc-ed space, which becomes > > unwieldy if the XATTR_SIZE_MAX limit is removed. > > > > In other words, I think removing the limit won't work that well with > > the current implementation, but I hope that the implementation can > > be changed so that the limit can be removed. > > Hi Frank, > > Thank you for your feedback. You are right, unchecked memory > allocation in the kernel would not be a good idea. Your suggested > approach of page-based xattr seems like a good idea but not something > I feel like I can take on right now. I wonder if we can lobby for the > xattr limit to be increased to 1M. That would serve NFS needs as right > now rpc calls (and thus getattr) are no larger than 1M. Thoughts on > that? I'm not familiar with generic xattr usage and I wonder if even > local usage could benefit from having a larger limit. I guess I found an answer to my own question in https://www.spinics.net/lists/linux-fsdevel/msg85580.html "> Can we get rid of the 64k size limit for EAs? The API on AIX is the same as on > Linux. But there is a huge size limit - which would help us already a lot. No - the maximum xattr size of 64k is encoded into the on-disk format of many filesystems and that's not a simple thing to change." > > > > > - Frank ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-01-03 17:18 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-01-02 22:28 extended attributes limitation of xattr_size_max Olga Kornievskaia 2020-01-02 23:31 ` Frank van der Linden 2020-01-03 16:13 ` Olga Kornievskaia 2020-01-03 17:18 ` Olga Kornievskaia
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).