Re: [Jfs-discussion] [PATCH 01/28] ext4: xattr-in-inode support

2017-05-31 Thread Tahsin Erdogan
>> allowing the ext4/e2fsck to verify the correct inode is accessed.
>
> Can we store the checksum of the xattr value somewhere?  We already
> checksum the values if they're stored in the ibody or a single external
> block, and I'd hate to lose that protection.
>
> We could probably reuse one of the inode fields (i_version?) for this.
>
The crc32c value of the xattr data is currently stored in the xattr inode:

struct ext4_xattr_ea_info {
__le64 ref_count; /* number of xattr entry references */
__le32 hash; /* crc32c hash of xattr data */
__le32 reserved; /* reserved, must be 0 */
};

We could also save that value in the ext4_xattr_entry->e_value_offs
for stronger binding between parent and xattr inodes. That field is
currently set to 0 for xattr inode references.

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Jfs-discussion mailing list
Jfs-discussion@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/jfs-discussion


Re: [Jfs-discussion] [PATCH 01/28] ext4: xattr-in-inode support

2017-05-31 Thread Darrick J. Wong
On Wed, May 31, 2017 at 01:14:50AM -0700, Tahsin Erdogan wrote:
> From: Andreas Dilger 
> 
> Large xattr support is implemented for EXT4_FEATURE_INCOMPAT_EA_INODE.
> 
> If the size of an xattr value is larger than will fit in a single
> external block, then the xattr value will be saved into the body
> of an external xattr inode.
> 
> The also helps support a larger number of xattr, since only the headers
> will be stored in the in-inode space or the single external block.
> 
> The inode is referenced from the xattr header via "e_value_inum",
> which was formerly "e_value_block", but that field was never used.
> The e_value_size still contains the xattr size so that listing
> xattrs does not need to look up the inode if the data is not accessed.
> 
> struct ext4_xattr_entry {
> __u8e_name_len; /* length of name */
> __u8e_name_index;   /* attribute name index */
> __le16  e_value_offs;   /* offset in disk block of value */
> __le32  e_value_inum;   /* inode in which value is stored */
> __le32  e_value_size;   /* size of attribute value */
> __le32  e_hash; /* hash value of name and value */
> chare_name[0];  /* attribute name */
> };
> 
> The xattr inode is marked with the EXT4_EA_INODE_FL flag and also
> holds a back-reference to the owning inode in its i_mtime field,
> allowing the ext4/e2fsck to verify the correct inode is accessed.

Can we store the checksum of the xattr value somewhere?  We already
checksum the values if they're stored in the ibody or a single external
block, and I'd hate to lose that protection.

We could probably reuse one of the inode fields (i_version?) for this.

--D 

> Lustre-Jira: https://jira.hpdd.intel.com/browse/LU-80
> Lustre-bugzilla: https://bugzilla.lustre.org/show_bug.cgi?id=4424
> Signed-off-by: Kalpak Shah 
> Signed-off-by: James Simmons 
> Signed-off-by: Andreas Dilger 
> Signed-off-by: Tahsin Erdogan 
> ---
>  fs/ext4/ext4.h   |  12 ++
>  fs/ext4/ialloc.c |   1 -
>  fs/ext4/inline.c |   2 +-
>  fs/ext4/inode.c  |  49 -
>  fs/ext4/xattr.c  | 565 
> ++-
>  fs/ext4/xattr.h  |  33 +++-
>  6 files changed, 606 insertions(+), 56 deletions(-)
> 
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 32191548abed..24ef56b4572f 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -1797,6 +1797,7 @@ EXT4_FEATURE_INCOMPAT_FUNCS(encrypt,ENCRYPT)
>EXT4_FEATURE_INCOMPAT_EXTENTS| \
>EXT4_FEATURE_INCOMPAT_64BIT| \
>EXT4_FEATURE_INCOMPAT_FLEX_BG| \
> +  EXT4_FEATURE_INCOMPAT_EA_INODE| \
>EXT4_FEATURE_INCOMPAT_MMP | \
>EXT4_FEATURE_INCOMPAT_INLINE_DATA | \
>EXT4_FEATURE_INCOMPAT_ENCRYPT | \
> @@ -2220,6 +2221,12 @@ 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
>   */
>  
> @@ -2231,6 +2238,10 @@ struct mmpd_data {
>  # define ATTRIB_NORET__attribute__((noreturn))
>  # define NORET_AND   noreturn,
>  
> +struct ext4_xattr_ino_array {
> + unsigned int xia_count; /* # of used item in the array */
> + unsigned int xia_inodes[0];
> +};
>  /* bitmap.c */
>  extern unsigned int ext4_count_free(char *bitmap, unsigned numchars);
>  void ext4_inode_bitmap_csum_set(struct super_block *sb, ext4_group_t group,
> @@ -2478,6 +2489,7 @@ extern int ext4_truncate_restart_trans(handle_t *, 
> struct inode *, int nblocks);
>  extern void ext4_set_inode_flags(struct inode *);
>  extern int ext4_alloc_da_blocks(struct inode *inode);
>  extern void ext4_set_aops(struct inode *inode);
> +extern int ext4_meta_trans_blocks(struct inode *, int nrblocks, int chunk);
>  extern int ext4_writepage_trans_blocks(struct inode *);
>  extern int ext4_chunk_trans_blocks(struct inode *, int nrblocks);
>  extern int ext4_zero_partial_blocks(handle_t *handle, struct inode *inode,
> diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
> index 98ac2f1f23b3..e2eb3cc06820 100644
> --- a/fs/ext4/ialloc.c
> +++ b/fs/ext4/ialloc.c
> @@ -294,7 +294,6 @@ void ext4_free_inode(handle_t *handle, struct inode 
> *inode)
>* as writing the quota to disk may need the lock as well.
>*/
>   dquot_initialize(inode);
> - ext4_xattr_delete_inode(handle, inode);
>   dquot_free_inode(inode);
>   dquot_drop(inode);
>  
> diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
> index 

[Jfs-discussion] [PATCH 01/28] ext4: xattr-in-inode support

2017-05-31 Thread Tahsin Erdogan
From: Andreas Dilger 

Large xattr support is implemented for EXT4_FEATURE_INCOMPAT_EA_INODE.

If the size of an xattr value is larger than will fit in a single
external block, then the xattr value will be saved into the body
of an external xattr inode.

The also helps support a larger number of xattr, since only the headers
will be stored in the in-inode space or the single external block.

The inode is referenced from the xattr header via "e_value_inum",
which was formerly "e_value_block", but that field was never used.
The e_value_size still contains the xattr size so that listing
xattrs does not need to look up the inode if the data is not accessed.

struct ext4_xattr_entry {
__u8e_name_len; /* length of name */
__u8e_name_index;   /* attribute name index */
__le16  e_value_offs;   /* offset in disk block of value */
__le32  e_value_inum;   /* inode in which value is stored */
__le32  e_value_size;   /* size of attribute value */
__le32  e_hash; /* hash value of name and value */
chare_name[0];  /* attribute name */
};

The xattr inode is marked with the EXT4_EA_INODE_FL flag and also
holds a back-reference to the owning inode in its i_mtime field,
allowing the ext4/e2fsck to verify the correct inode is accessed.

Lustre-Jira: https://jira.hpdd.intel.com/browse/LU-80
Lustre-bugzilla: https://bugzilla.lustre.org/show_bug.cgi?id=4424
Signed-off-by: Kalpak Shah 
Signed-off-by: James Simmons 
Signed-off-by: Andreas Dilger 
Signed-off-by: Tahsin Erdogan 
---
 fs/ext4/ext4.h   |  12 ++
 fs/ext4/ialloc.c |   1 -
 fs/ext4/inline.c |   2 +-
 fs/ext4/inode.c  |  49 -
 fs/ext4/xattr.c  | 565 ++-
 fs/ext4/xattr.h  |  33 +++-
 6 files changed, 606 insertions(+), 56 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 32191548abed..24ef56b4572f 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1797,6 +1797,7 @@ EXT4_FEATURE_INCOMPAT_FUNCS(encrypt,  ENCRYPT)
 EXT4_FEATURE_INCOMPAT_EXTENTS| \
 EXT4_FEATURE_INCOMPAT_64BIT| \
 EXT4_FEATURE_INCOMPAT_FLEX_BG| \
+EXT4_FEATURE_INCOMPAT_EA_INODE| \
 EXT4_FEATURE_INCOMPAT_MMP | \
 EXT4_FEATURE_INCOMPAT_INLINE_DATA | \
 EXT4_FEATURE_INCOMPAT_ENCRYPT | \
@@ -2220,6 +2221,12 @@ struct mmpd_data {
 #define EXT4_MMP_MAX_CHECK_INTERVAL300UL
 
 /*
+ * 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
  */
 
@@ -2231,6 +2238,10 @@ struct mmpd_data {
 # define ATTRIB_NORET  __attribute__((noreturn))
 # define NORET_AND noreturn,
 
+struct ext4_xattr_ino_array {
+   unsigned int xia_count; /* # of used item in the array */
+   unsigned int xia_inodes[0];
+};
 /* bitmap.c */
 extern unsigned int ext4_count_free(char *bitmap, unsigned numchars);
 void ext4_inode_bitmap_csum_set(struct super_block *sb, ext4_group_t group,
@@ -2478,6 +2489,7 @@ extern int ext4_truncate_restart_trans(handle_t *, struct 
inode *, int nblocks);
 extern void ext4_set_inode_flags(struct inode *);
 extern int ext4_alloc_da_blocks(struct inode *inode);
 extern void ext4_set_aops(struct inode *inode);
+extern int ext4_meta_trans_blocks(struct inode *, int nrblocks, int chunk);
 extern int ext4_writepage_trans_blocks(struct inode *);
 extern int ext4_chunk_trans_blocks(struct inode *, int nrblocks);
 extern int ext4_zero_partial_blocks(handle_t *handle, struct inode *inode,
diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
index 98ac2f1f23b3..e2eb3cc06820 100644
--- a/fs/ext4/ialloc.c
+++ b/fs/ext4/ialloc.c
@@ -294,7 +294,6 @@ void ext4_free_inode(handle_t *handle, struct inode *inode)
 * as writing the quota to disk may need the lock as well.
 */
dquot_initialize(inode);
-   ext4_xattr_delete_inode(handle, inode);
dquot_free_inode(inode);
dquot_drop(inode);
 
diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
index 8d141c0c8ff9..28c5c3abddb3 100644
--- a/fs/ext4/inline.c
+++ b/fs/ext4/inline.c
@@ -61,7 +61,7 @@ static int get_max_inline_xattr_value_size(struct inode 
*inode,
 
/* Compute min_offs. */
for (; !IS_LAST_ENTRY(entry); entry = EXT4_XATTR_NEXT(entry)) {
-   if (!entry->e_value_block && entry->e_value_size) {
+   if (!entry->e_value_inum && entry->e_value_size) {
size_t offs = le16_to_cpu(entry->e_value_offs);
if (offs < min_offs)