Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1672050 > unrolled thread

[PATCH 09/32] ext4: extended attribute value size limit is enforced by vfs

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 09/32] ext4: extended attribute value size limit is enforced by vfs Tahsin Erdogan <tahsin@google.com> - 2017-06-21 23:30 +0200
    Re: [PATCH 09/32] ext4: extended attribute value size limit is  enforced by vfs Theodore Ts'o <tytso@mit.edu> - 2017-06-22 03:50 +0200
    Re: [PATCH 09/32] ext4: extended attribute value size limit is  enforced by vfs "Darrick J. Wong" <darrick.wong@oracle.com> - 2017-06-22 20:10 +0200
      Re: [PATCH 09/32] ext4: extended attribute value size limit is  enforced by vfs Andreas Dilger <adilger@dilger.ca> - 2017-06-22 21:50 +0200
        Re: [PATCH 09/32] ext4: extended attribute value size limit is  enforced by vfs Theodore Ts'o <tytso@mit.edu> - 2017-06-23 00:30 +0200
          Re: [PATCH 09/32] ext4: extended attribute value size limit is  enforced by vfs "Darrick J. Wong" <darrick.wong@oracle.com> - 2017-06-23 01:30 +0200

#1672050 — [PATCH 09/32] ext4: extended attribute value size limit is enforced by vfs

EXT4_XATTR_MAX_LARGE_EA_SIZE definition in ext4 is currently unused.
Besides, vfs enforces its own 64k limit which makes the 1MB limit in
ext4 redundant. Remove it.

Signed-off-by: Tahsin Erdogan <tahsin@google.com>
---
 fs/ext4/ext4.h | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 5d5fc0d0e2bc..2cdd6070e348 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -2220,12 +2220,6 @@ struct mmpd_data {
  */
 #define EXT4_MMP_MAX_CHECK_INTERVAL	300UL
 
-/*
- * Maximum size of xattr attributes for FEATURE_INCOMPAT_EA_INODE 1Mb
- * This limit is arbitrary, but is reasonable for the xattr API.
- */
-#define EXT4_XATTR_MAX_LARGE_EA_SIZE    (1024 * 1024)
-
 /*
  * Function prototypes
  */
-- 
2.13.1.611.g7e3b11ae1-goog

[toc] | [next] | [standalone]

#1672232 — Re: [PATCH 09/32] ext4: extended attribute value size limit is enforced by vfs

On Wed, Jun 21, 2017 at 02:21:19PM -0700, Tahsin Erdogan wrote:
> EXT4_XATTR_MAX_LARGE_EA_SIZE definition in ext4 is currently unused.
> Besides, vfs enforces its own 64k limit which makes the 1MB limit in
> ext4 redundant. Remove it.
> 
> Signed-off-by: Tahsin Erdogan <tahsin@google.com>

Thanks, added to the ext4 patch queue.

	      	     	  	- Ted

[toc] | [prev] | [next] | [standalone]

#1672934 — Re: [PATCH 09/32] ext4: extended attribute value size limit is enforced by vfs

On Wed, Jun 21, 2017 at 02:21:19PM -0700, Tahsin Erdogan wrote:
> EXT4_XATTR_MAX_LARGE_EA_SIZE definition in ext4 is currently unused.
> Besides, vfs enforces its own 64k limit which makes the 1MB limit in
> ext4 redundant. Remove it.

Just FYI I believe the 64k VFS limit exists because XFS is the only fs
to allow large xattr values, and its maximum is 64k (on-disk field size
limitation).

I don't know if anyone actually wants larger values?  Now could be a
time to have such a conversation, if anyone is so interested.

--D

> 
> Signed-off-by: Tahsin Erdogan <tahsin@google.com>
> ---
>  fs/ext4/ext4.h | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 5d5fc0d0e2bc..2cdd6070e348 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -2220,12 +2220,6 @@ struct mmpd_data {
>   */
>  #define EXT4_MMP_MAX_CHECK_INTERVAL	300UL
>  
> -/*
> - * Maximum size of xattr attributes for FEATURE_INCOMPAT_EA_INODE 1Mb
> - * This limit is arbitrary, but is reasonable for the xattr API.
> - */
> -#define EXT4_XATTR_MAX_LARGE_EA_SIZE    (1024 * 1024)
> -
>  /*
>   * Function prototypes
>   */
> -- 
> 2.13.1.611.g7e3b11ae1-goog
> 

[toc] | [prev] | [next] | [standalone]

#1673008 — Re: [PATCH 09/32] ext4: extended attribute value size limit is enforced by vfs

[Multipart message — attachments visible in raw view] — view raw

On Jun 22, 2017, at 12:02 PM, Darrick J. Wong <darrick.wong@oracle.com> wrote:
> 
> On Wed, Jun 21, 2017 at 02:21:19PM -0700, Tahsin Erdogan wrote:
>> EXT4_XATTR_MAX_LARGE_EA_SIZE definition in ext4 is currently unused.
>> Besides, vfs enforces its own 64k limit which makes the 1MB limit in
>> ext4 redundant. Remove it.
> 
> Just FYI I believe the 64k VFS limit exists because XFS is the only fs
> to allow large xattr values, and its maximum is 64k (on-disk field size
> limitation).
> 
> I don't know if anyone actually wants larger values?  Now could be a
> time to have such a conversation, if anyone is so interested.

The EXT4_XATTR_MAX_LARGE_EA_SIZE limit of 1MB was also totally arbitrary,
but a reasonable upper limit for the atomic get/set interface used by
xattrs.  The underlying disk format could actually store xattrs of any size.

I'd think if we want to get huge xattrs that they should be handled by
having separate streams (e.g. open file descriptor, ioctl/syscall to select
a different stream number on that file) so that the data doesn't have to be
completely rewritten any time it is modified, but streams are frowned upon
by many Linux developers for security reasons so will probably be a no-go.

Cheers, Andreas

> --D
> 
>> 
>> Signed-off-by: Tahsin Erdogan <tahsin@google.com>
>> ---
>> fs/ext4/ext4.h | 6 ------
>> 1 file changed, 6 deletions(-)
>> 
>> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
>> index 5d5fc0d0e2bc..2cdd6070e348 100644
>> --- a/fs/ext4/ext4.h
>> +++ b/fs/ext4/ext4.h
>> @@ -2220,12 +2220,6 @@ struct mmpd_data {
>>  */
>> #define EXT4_MMP_MAX_CHECK_INTERVAL	300UL
>> 
>> -/*
>> - * Maximum size of xattr attributes for FEATURE_INCOMPAT_EA_INODE 1Mb
>> - * This limit is arbitrary, but is reasonable for the xattr API.
>> - */
>> -#define EXT4_XATTR_MAX_LARGE_EA_SIZE    (1024 * 1024)
>> -
>> /*
>>  * Function prototypes
>>  */
>> --
>> 2.13.1.611.g7e3b11ae1-goog
>> 


Cheers, Andreas

[toc] | [prev] | [next] | [standalone]

#1673093 — Re: [PATCH 09/32] ext4: extended attribute value size limit is enforced by vfs

On Thu, Jun 22, 2017 at 01:40:25PM -0600, Andreas Dilger wrote:
> 
> The EXT4_XATTR_MAX_LARGE_EA_SIZE limit of 1MB was also totally arbitrary,
> but a reasonable upper limit for the atomic get/set interface used by
> xattrs.  The underlying disk format could actually store xattrs of any size.
> 
> I'd think if we want to get huge xattrs that they should be handled by
> having separate streams (e.g. open file descriptor, ioctl/syscall to select
> a different stream number on that file) so that the data doesn't have to be
> completely rewritten any time it is modified, but streams are frowned upon
> by many Linux developers for security reasons so will probably be a no-go.

This has been discussed before.  Most of the arguments (both pro and
con) can be found on this thread (2016):

	https://www.spinics.net/lists/linux-nfs/msg57055.html

Jeremy Allison used to pester me for alternate data stream support
many years ago, but he has since completely changed his opinion on the
matter:

	http://www.spinics.net/lists/linux-fsdevel/msg96083.html

So if jra@ doesn't add support for this interface to Samba, the thing
I'd ask is who is actually going to used such an interface; because if
there aren't any user space applications lining up to use it, it's
probably not worth adding it until we can find a flagship user of the
proposed new API.

There are plenty of other things we could add that Jeremy has asked
for, that *would* improve Samba performance.  (For example, adding a
mount option which provides case-insensitive lookup; the one thing
that makes this hard is that while Android doesn't seem to care about
I18N --- they are currently using an ascii-only tolower() in their
sdcardfs hack --- Samba/Windows does care about I18N done right, so we
wouldn't have to get a Unicode case folding implementation into the
kernel.)  So I'd argue that those are probably more interesting
projects than O_XATTR support.

	       	  	       	    - Ted

[toc] | [prev] | [next] | [standalone]

#1673129 — Re: [PATCH 09/32] ext4: extended attribute value size limit is enforced by vfs

On Thu, Jun 22, 2017 at 06:27:08PM -0400, Theodore Ts'o wrote:
> On Thu, Jun 22, 2017 at 01:40:25PM -0600, Andreas Dilger wrote:
> > 
> > The EXT4_XATTR_MAX_LARGE_EA_SIZE limit of 1MB was also totally arbitrary,
> > but a reasonable upper limit for the atomic get/set interface used by
> > xattrs.  The underlying disk format could actually store xattrs of any size.
> > 
> > I'd think if we want to get huge xattrs that they should be handled by
> > having separate streams (e.g. open file descriptor, ioctl/syscall to select
> > a different stream number on that file) so that the data doesn't have to be
> > completely rewritten any time it is modified, but streams are frowned upon
> > by many Linux developers for security reasons so will probably be a no-go.
> 
> This has been discussed before.  Most of the arguments (both pro and
> con) can be found on this thread (2016):
> 
> 	https://www.spinics.net/lists/linux-nfs/msg57055.html
> 
> Jeremy Allison used to pester me for alternate data stream support
> many years ago, but he has since completely changed his opinion on the
> matter:
> 
> 	http://www.spinics.net/lists/linux-fsdevel/msg96083.html
> 
> So if jra@ doesn't add support for this interface to Samba, the thing
> I'd ask is who is actually going to used such an interface; because if
> there aren't any user space applications lining up to use it, it's
> probably not worth adding it until we can find a flagship user of the
> proposed new API.

Heh, ok, that's about what I expected to hear. :)

> There are plenty of other things we could add that Jeremy has asked
> for, that *would* improve Samba performance.  (For example, adding a
> mount option which provides case-insensitive lookup; the one thing
> that makes this hard is that while Android doesn't seem to care about
> I18N --- they are currently using an ascii-only tolower() in their
> sdcardfs hack --- Samba/Windows does care about I18N done right, so we
> wouldn't have to get a Unicode case folding implementation into the
> kernel.)  So I'd argue that those are probably more interesting
> projects than O_XATTR support.

--D

> 
> 	       	  	       	    - Ted

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web