Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1653968 > unrolled thread
| Started by | Tahsin Erdogan <tahsin@google.com> |
|---|---|
| First post | 2017-05-31 10:20 +0200 |
| Last post | 2017-06-01 18:00 +0200 |
| Articles | 20 on this page of 28 — 3 participants |
Back to article view | Back to linux.kernel
[PATCH 01/28] ext4: xattr-in-inode support Tahsin Erdogan <tahsin@google.com> - 2017-05-31 10:20 +0200
[PATCH 05/28] ext4: attach jinode after creation of xattr inode Tahsin Erdogan <tahsin@google.com> - 2017-05-31 10:20 +0200
[PATCH 08/28] ext4: fix ref counting for ea_inode Tahsin Erdogan <tahsin@google.com> - 2017-05-31 10:20 +0200
[PATCH 25/28] ext4: add ext4_is_quota_file() Tahsin Erdogan <tahsin@google.com> - 2017-05-31 10:30 +0200
[PATCH 15/28] ext4: fix ext4_xattr_move_to_block() Tahsin Erdogan <tahsin@google.com> - 2017-05-31 10:30 +0200
[PATCH 24/28] ext4: rename mb block cache functions Tahsin Erdogan <tahsin@google.com> - 2017-05-31 10:30 +0200
[PATCH 21/28] ext4: modify ext4_xattr_ino_array to hold struct inode * Tahsin Erdogan <tahsin@google.com> - 2017-05-31 10:30 +0200
[PATCH 23/28] mbcache: make mbcache more generic Tahsin Erdogan <tahsin@google.com> - 2017-05-31 10:30 +0200
[PATCH 13/28] ext4: ext4_xattr_value_same() should return false for external data Tahsin Erdogan <tahsin@google.com> - 2017-05-31 10:30 +0200
[PATCH 11/28] ext4: clean up ext4_xattr_inode_get() Tahsin Erdogan <tahsin@google.com> - 2017-05-31 10:30 +0200
[PATCH 12/28] ext4: add missing le32_to_cpu(e_value_inum) conversions Tahsin Erdogan <tahsin@google.com> - 2017-05-31 10:30 +0200
[PATCH 20/28] ext4: improve journal credit handling in set xattr paths Tahsin Erdogan <tahsin@google.com> - 2017-05-31 10:30 +0200
[PATCH 19/28] ext4: ext4_xattr_delete_inode() should return accurate errors Tahsin Erdogan <tahsin@google.com> - 2017-05-31 10:30 +0200
[PATCH 14/28] ext4: fix ext4_xattr_make_inode_space() value size calculation Tahsin Erdogan <tahsin@google.com> - 2017-05-31 10:30 +0200
[PATCH 16/28] ext4: fix ext4_xattr_cmp() Tahsin Erdogan <tahsin@google.com> - 2017-05-31 10:30 +0200
[PATCH 10/28] ext4: change ext4_xattr_inode_iget() signature Tahsin Erdogan <tahsin@google.com> - 2017-05-31 10:40 +0200
[PATCH 04/28] ext4: do not set posix acls on xattr inodes Tahsin Erdogan <tahsin@google.com> - 2017-05-31 10:40 +0200
[PATCH 07/28] ext4: call journal revoke when freeing ea_inode blocks Tahsin Erdogan <tahsin@google.com> - 2017-05-31 10:40 +0200
Re: [PATCH 07/28] ext4: call journal revoke when freeing ea_inode blocks "Darrick J. Wong" <darrick.wong@oracle.com> - 2017-05-31 18:20 +0200
Re: [PATCH 07/28] ext4: call journal revoke when freeing ea_inode blocks Tahsin Erdogan <tahsin@google.com> - 2017-05-31 23:10 +0200
Re: [PATCH 07/28] ext4: call journal revoke when freeing ea_inode blocks Andreas Dilger <adilger@dilger.ca> - 2017-06-06 00:10 +0200
[PATCH 09/28] ext4: extended attribute value size limit is enforced by vfs Tahsin Erdogan <tahsin@google.com> - 2017-05-31 10:40 +0200
Re: [PATCH 09/28] ext4: extended attribute value size limit is enforced by vfs "Darrick J. Wong" <darrick.wong@oracle.com> - 2017-05-31 18:10 +0200
Re: [PATCH 09/28] ext4: extended attribute value size limit is enforced by vfs Tahsin Erdogan <tahsin@google.com> - 2017-05-31 18:20 +0200
[PATCH 06/28] ext4: ea_inode owner should be the same as the inode owner Tahsin Erdogan <tahsin@google.com> - 2017-05-31 10:40 +0200
Re: [PATCH 01/28] ext4: xattr-in-inode support "Darrick J. Wong" <darrick.wong@oracle.com> - 2017-05-31 18:50 +0200
Re: [PATCH 01/28] ext4: xattr-in-inode support Tahsin Erdogan <tahsin@google.com> - 2017-05-31 22:10 +0200
[PATCH v2 01/28] ext4: xattr-in-inode support Tahsin Erdogan <tahsin@google.com> - 2017-06-01 18:00 +0200
Page 1 of 2 [1] 2 Next page →
| From | Tahsin Erdogan <tahsin@google.com> |
|---|---|
| Date | 2017-05-31 10:20 +0200 |
| Subject | [PATCH 01/28] ext4: xattr-in-inode support |
| Message-ID | <tN71f-2f2-7@gated-at.bofh.it> |
From: Andreas Dilger <andreas.dilger@intel.com>
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 {
__u8 e_name_len; /* length of name */
__u8 e_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 */
char e_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 <kalpak.shah@sun.com>
Signed-off-by: James Simmons <uja.ornl@gmail.com>
Signed-off-by: Andreas Dilger <andreas.dilger@intel.com>
Signed-off-by: Tahsin Erdogan <tahsin@google.com>
---
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 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)
min_offs = offs;
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 5cf82d03968c..e5535e5b3dc5 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -139,8 +139,6 @@ static void ext4_invalidatepage(struct page *page, unsigned int offset,
unsigned int length);
static int __ext4_journalled_writepage(struct page *page, unsigned int len);
static int ext4_bh_delay_or_unwritten(handle_t *handle, struct buffer_head *bh);
-static int ext4_meta_trans_blocks(struct inode *inode, int lblocks,
- int pextents);
/*
* Test whether an inode is a fast symlink.
@@ -189,6 +187,8 @@ void ext4_evict_inode(struct inode *inode)
{
handle_t *handle;
int err;
+ int extra_credits = 3;
+ struct ext4_xattr_ino_array *lea_ino_array = NULL;
trace_ext4_evict_inode(inode);
@@ -238,8 +238,8 @@ void ext4_evict_inode(struct inode *inode)
* protection against it
*/
sb_start_intwrite(inode->i_sb);
- handle = ext4_journal_start(inode, EXT4_HT_TRUNCATE,
- ext4_blocks_for_truncate(inode)+3);
+
+ handle = ext4_journal_start(inode, EXT4_HT_TRUNCATE, extra_credits);
if (IS_ERR(handle)) {
ext4_std_error(inode->i_sb, PTR_ERR(handle));
/*
@@ -251,9 +251,36 @@ void ext4_evict_inode(struct inode *inode)
sb_end_intwrite(inode->i_sb);
goto no_delete;
}
-
if (IS_SYNC(inode))
ext4_handle_sync(handle);
+
+ /*
+ * Delete xattr inode before deleting the main inode.
+ */
+ err = ext4_xattr_delete_inode(handle, inode, &lea_ino_array);
+ if (err) {
+ ext4_warning(inode->i_sb,
+ "couldn't delete inode's xattr (err %d)", err);
+ goto stop_handle;
+ }
+
+ if (!IS_NOQUOTA(inode))
+ extra_credits += 2 * EXT4_QUOTA_DEL_BLOCKS(inode->i_sb);
+
+ if (!ext4_handle_has_enough_credits(handle,
+ ext4_blocks_for_truncate(inode) + extra_credits)) {
+ err = ext4_journal_extend(handle,
+ ext4_blocks_for_truncate(inode) + extra_credits);
+ if (err > 0)
+ err = ext4_journal_restart(handle,
+ ext4_blocks_for_truncate(inode) + extra_credits);
+ if (err != 0) {
+ ext4_warning(inode->i_sb,
+ "couldn't extend journal (err %d)", err);
+ goto stop_handle;
+ }
+ }
+
inode->i_size = 0;
err = ext4_mark_inode_dirty(handle, inode);
if (err) {
@@ -277,10 +304,10 @@ void ext4_evict_inode(struct inode *inode)
* enough credits left in the handle to remove the inode from
* the orphan list and set the dtime field.
*/
- if (!ext4_handle_has_enough_credits(handle, 3)) {
- err = ext4_journal_extend(handle, 3);
+ if (!ext4_handle_has_enough_credits(handle, extra_credits)) {
+ err = ext4_journal_extend(handle, extra_credits);
if (err > 0)
- err = ext4_journal_restart(handle, 3);
+ err = ext4_journal_restart(handle, extra_credits);
if (err != 0) {
ext4_warning(inode->i_sb,
"couldn't extend journal (err %d)", err);
@@ -315,8 +342,12 @@ void ext4_evict_inode(struct inode *inode)
ext4_clear_inode(inode);
else
ext4_free_inode(handle, inode);
+
ext4_journal_stop(handle);
sb_end_intwrite(inode->i_sb);
+
+ if (lea_ino_array != NULL)
+ ext4_xattr_inode_array_free(inode, lea_ino_array);
return;
no_delete:
ext4_clear_inode(inode); /* We must guarantee clearing of inode... */
@@ -5504,7 +5535,7 @@ static int ext4_index_trans_blocks(struct inode *inode, int lblocks,
*
* Also account for superblock, inode, quota and xattr blocks
*/
-static int ext4_meta_trans_blocks(struct inode *inode, int lblocks,
+int ext4_meta_trans_blocks(struct inode *inode, int lblocks,
int pextents)
{
ext4_group_t groups, ngroups = ext4_get_groups_count(inode->i_sb);
diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index 5d3c2536641c..444be5c7a1d5 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -177,9 +177,8 @@ ext4_xattr_check_entries(struct ext4_xattr_entry *entry, void *end,
/* Check the values */
while (!IS_LAST_ENTRY(entry)) {
- if (entry->e_value_block != 0)
- return -EFSCORRUPTED;
- if (entry->e_value_size != 0) {
+ if (entry->e_value_size != 0 &&
+ entry->e_value_inum == 0) {
u16 offs = le16_to_cpu(entry->e_value_offs);
u32 size = le32_to_cpu(entry->e_value_size);
void *value;
@@ -269,6 +268,99 @@ ext4_xattr_find_entry(struct ext4_xattr_entry **pentry, int name_index,
return cmp ? -ENODATA : 0;
}
+/*
+ * Read the EA value from an inode.
+ */
+static int
+ext4_xattr_inode_read(struct inode *ea_inode, void *buf, size_t *size)
+{
+ unsigned long block = 0;
+ struct buffer_head *bh = NULL;
+ int blocksize;
+ size_t csize, ret_size = 0;
+
+ if (*size == 0)
+ return 0;
+
+ blocksize = ea_inode->i_sb->s_blocksize;
+
+ while (ret_size < *size) {
+ csize = (*size - ret_size) > blocksize ? blocksize :
+ *size - ret_size;
+ bh = ext4_bread(NULL, ea_inode, block, 0);
+ if (IS_ERR(bh)) {
+ *size = ret_size;
+ return PTR_ERR(bh);
+ }
+ memcpy(buf, bh->b_data, csize);
+ brelse(bh);
+
+ buf += csize;
+ block += 1;
+ ret_size += csize;
+ }
+
+ *size = ret_size;
+
+ return 0;
+}
+
+struct inode *ext4_xattr_inode_iget(struct inode *parent, unsigned long ea_ino, int *err)
+{
+ struct inode *ea_inode = NULL;
+
+ ea_inode = ext4_iget(parent->i_sb, ea_ino);
+ if (IS_ERR(ea_inode) || is_bad_inode(ea_inode)) {
+ int rc = IS_ERR(ea_inode) ? PTR_ERR(ea_inode) : 0;
+ ext4_error(parent->i_sb, "error while reading EA inode %lu "
+ "/ %d %d", ea_ino, rc, is_bad_inode(ea_inode));
+ *err = rc != 0 ? rc : -EIO;
+ return NULL;
+ }
+
+ if (EXT4_XATTR_INODE_GET_PARENT(ea_inode) != parent->i_ino ||
+ ea_inode->i_generation != parent->i_generation) {
+ ext4_error(parent->i_sb, "Backpointer from EA inode %lu "
+ "to parent invalid.", ea_ino);
+ *err = -EINVAL;
+ goto error;
+ }
+
+ if (!(EXT4_I(ea_inode)->i_flags & EXT4_EA_INODE_FL)) {
+ ext4_error(parent->i_sb, "EA inode %lu does not have "
+ "EXT4_EA_INODE_FL flag set.\n", ea_ino);
+ *err = -EINVAL;
+ goto error;
+ }
+
+ *err = 0;
+ return ea_inode;
+
+error:
+ iput(ea_inode);
+ return NULL;
+}
+
+/*
+ * Read the value from the EA inode.
+ */
+static int
+ext4_xattr_inode_get(struct inode *inode, unsigned long ea_ino, void *buffer,
+ size_t *size)
+{
+ struct inode *ea_inode = NULL;
+ int err;
+
+ ea_inode = ext4_xattr_inode_iget(inode, ea_ino, &err);
+ if (err)
+ return err;
+
+ err = ext4_xattr_inode_read(ea_inode, buffer, size);
+ iput(ea_inode);
+
+ return err;
+}
+
static int
ext4_xattr_block_get(struct inode *inode, int name_index, const char *name,
void *buffer, size_t buffer_size)
@@ -308,8 +400,16 @@ ext4_xattr_block_get(struct inode *inode, int name_index, const char *name,
error = -ERANGE;
if (size > buffer_size)
goto cleanup;
- memcpy(buffer, bh->b_data + le16_to_cpu(entry->e_value_offs),
- size);
+ if (entry->e_value_inum) {
+ error = ext4_xattr_inode_get(inode,
+ le32_to_cpu(entry->e_value_inum),
+ buffer, &size);
+ if (error)
+ goto cleanup;
+ } else {
+ memcpy(buffer, bh->b_data +
+ le16_to_cpu(entry->e_value_offs), size);
+ }
}
error = size;
@@ -350,8 +450,16 @@ ext4_xattr_ibody_get(struct inode *inode, int name_index, const char *name,
error = -ERANGE;
if (size > buffer_size)
goto cleanup;
- memcpy(buffer, (void *)IFIRST(header) +
- le16_to_cpu(entry->e_value_offs), size);
+ if (entry->e_value_inum) {
+ error = ext4_xattr_inode_get(inode,
+ le32_to_cpu(entry->e_value_inum),
+ buffer, &size);
+ if (error)
+ goto cleanup;
+ } else {
+ memcpy(buffer, (void *)IFIRST(header) +
+ le16_to_cpu(entry->e_value_offs), size);
+ }
}
error = size;
@@ -620,7 +728,7 @@ static size_t ext4_xattr_free_space(struct ext4_xattr_entry *last,
size_t *min_offs, void *base, int *total)
{
for (; !IS_LAST_ENTRY(last); last = EXT4_XATTR_NEXT(last)) {
- if (last->e_value_size) {
+ if (!last->e_value_inum && last->e_value_size) {
size_t offs = le16_to_cpu(last->e_value_offs);
if (offs < *min_offs)
*min_offs = offs;
@@ -631,16 +739,173 @@ static size_t ext4_xattr_free_space(struct ext4_xattr_entry *last,
return (*min_offs - ((void *)last - base) - sizeof(__u32));
}
-static int
-ext4_xattr_set_entry(struct ext4_xattr_info *i, struct ext4_xattr_search *s)
+/*
+ * Write the value of the EA in an inode.
+ */
+static int ext4_xattr_inode_write(handle_t *handle, struct inode *ea_inode,
+ const void *buf, int bufsize)
+{
+ struct buffer_head *bh = NULL;
+ unsigned long block = 0;
+ unsigned blocksize = ea_inode->i_sb->s_blocksize;
+ unsigned max_blocks = (bufsize + blocksize - 1) >> ea_inode->i_blkbits;
+ int csize, wsize = 0;
+ int ret = 0;
+ int retries = 0;
+
+retry:
+ while (ret >= 0 && ret < max_blocks) {
+ struct ext4_map_blocks map;
+ map.m_lblk = block += ret;
+ map.m_len = max_blocks -= ret;
+
+ ret = ext4_map_blocks(handle, ea_inode, &map,
+ EXT4_GET_BLOCKS_CREATE);
+ if (ret <= 0) {
+ ext4_mark_inode_dirty(handle, ea_inode);
+ if (ret == -ENOSPC &&
+ ext4_should_retry_alloc(ea_inode->i_sb, &retries)) {
+ ret = 0;
+ goto retry;
+ }
+ break;
+ }
+ }
+
+ if (ret < 0)
+ return ret;
+
+ block = 0;
+ while (wsize < bufsize) {
+ if (bh != NULL)
+ brelse(bh);
+ csize = (bufsize - wsize) > blocksize ? blocksize :
+ bufsize - wsize;
+ bh = ext4_getblk(handle, ea_inode, block, 0);
+ if (IS_ERR(bh)) {
+ ret = PTR_ERR(bh);
+ goto out;
+ }
+ ret = ext4_journal_get_write_access(handle, bh);
+ if (ret)
+ goto out;
+
+ memcpy(bh->b_data, buf, csize);
+ set_buffer_uptodate(bh);
+ ext4_handle_dirty_metadata(handle, ea_inode, bh);
+
+ buf += csize;
+ wsize += csize;
+ block += 1;
+ }
+
+ inode_lock(ea_inode);
+ i_size_write(ea_inode, wsize);
+ ext4_update_i_disksize(ea_inode, wsize);
+ inode_unlock(ea_inode);
+
+ ext4_mark_inode_dirty(handle, ea_inode);
+
+out:
+ brelse(bh);
+
+ return ret;
+}
+
+/*
+ * Create an inode to store the value of a large EA.
+ */
+static struct inode *ext4_xattr_inode_create(handle_t *handle,
+ struct inode *inode)
+{
+ struct inode *ea_inode = NULL;
+
+ /*
+ * Let the next inode be the goal, so we try and allocate the EA inode
+ * in the same group, or nearby one.
+ */
+ ea_inode = ext4_new_inode(handle, inode->i_sb->s_root->d_inode,
+ S_IFREG | 0600, NULL, inode->i_ino + 1, NULL);
+ if (!IS_ERR(ea_inode)) {
+ ea_inode->i_op = &ext4_file_inode_operations;
+ ea_inode->i_fop = &ext4_file_operations;
+ ext4_set_aops(ea_inode);
+ ea_inode->i_generation = inode->i_generation;
+ EXT4_I(ea_inode)->i_flags |= EXT4_EA_INODE_FL;
+
+ /*
+ * A back-pointer from EA inode to parent inode will be useful
+ * for e2fsck.
+ */
+ EXT4_XATTR_INODE_SET_PARENT(ea_inode, inode->i_ino);
+ unlock_new_inode(ea_inode);
+ }
+
+ return ea_inode;
+}
+
+/*
+ * Unlink the inode storing the value of the EA.
+ */
+int ext4_xattr_inode_unlink(struct inode *inode, unsigned long ea_ino)
+{
+ struct inode *ea_inode = NULL;
+ int err;
+
+ ea_inode = ext4_xattr_inode_iget(inode, ea_ino, &err);
+ if (err)
+ return err;
+
+ clear_nlink(ea_inode);
+ iput(ea_inode);
+
+ return 0;
+}
+
+/*
+ * Add value of the EA in an inode.
+ */
+static int ext4_xattr_inode_set(handle_t *handle, struct inode *inode,
+ unsigned long *ea_ino, const void *value,
+ size_t value_len)
+{
+ struct inode *ea_inode;
+ int err;
+
+ /* Create an inode for the EA value */
+ ea_inode = ext4_xattr_inode_create(handle, inode);
+ if (IS_ERR(ea_inode))
+ return PTR_ERR(ea_inode);
+
+ err = ext4_xattr_inode_write(handle, ea_inode, value, value_len);
+ if (err)
+ clear_nlink(ea_inode);
+ else
+ *ea_ino = ea_inode->i_ino;
+
+ iput(ea_inode);
+
+ return err;
+}
+
+static int ext4_xattr_set_entry(struct ext4_xattr_info *i,
+ struct ext4_xattr_search *s,
+ handle_t *handle, struct inode *inode)
{
struct ext4_xattr_entry *last;
size_t free, min_offs = s->end - s->base, name_len = strlen(i->name);
+ int in_inode = i->in_inode;
+ int rc;
+
+ if (ext4_has_feature_ea_inode(inode->i_sb) &&
+ (EXT4_XATTR_SIZE(i->value_len) >
+ EXT4_XATTR_MIN_LARGE_EA_SIZE(inode->i_sb->s_blocksize)))
+ in_inode = 1;
/* Compute min_offs and last. */
last = s->first;
for (; !IS_LAST_ENTRY(last); last = EXT4_XATTR_NEXT(last)) {
- if (last->e_value_size) {
+ if (!last->e_value_inum && last->e_value_size) {
size_t offs = le16_to_cpu(last->e_value_offs);
if (offs < min_offs)
min_offs = offs;
@@ -648,15 +913,20 @@ ext4_xattr_set_entry(struct ext4_xattr_info *i, struct ext4_xattr_search *s)
}
free = min_offs - ((void *)last - s->base) - sizeof(__u32);
if (!s->not_found) {
- if (s->here->e_value_size) {
+ if (!in_inode &&
+ !s->here->e_value_inum && s->here->e_value_size) {
size_t size = le32_to_cpu(s->here->e_value_size);
free += EXT4_XATTR_SIZE(size);
}
free += EXT4_XATTR_LEN(name_len);
}
if (i->value) {
- if (free < EXT4_XATTR_LEN(name_len) +
- EXT4_XATTR_SIZE(i->value_len))
+ size_t value_len = EXT4_XATTR_SIZE(i->value_len);
+
+ if (in_inode)
+ value_len = 0;
+
+ if (free < EXT4_XATTR_LEN(name_len) + value_len)
return -ENOSPC;
}
@@ -670,7 +940,8 @@ ext4_xattr_set_entry(struct ext4_xattr_info *i, struct ext4_xattr_search *s)
s->here->e_name_len = name_len;
memcpy(s->here->e_name, i->name, name_len);
} else {
- if (s->here->e_value_size) {
+ if (!s->here->e_value_inum && s->here->e_value_size &&
+ s->here->e_value_offs > 0) {
void *first_val = s->base + min_offs;
size_t offs = le16_to_cpu(s->here->e_value_offs);
void *val = s->base + offs;
@@ -704,12 +975,18 @@ ext4_xattr_set_entry(struct ext4_xattr_info *i, struct ext4_xattr_search *s)
last = s->first;
while (!IS_LAST_ENTRY(last)) {
size_t o = le16_to_cpu(last->e_value_offs);
- if (last->e_value_size && o < offs)
+ if (!last->e_value_inum &&
+ last->e_value_size && o < offs)
last->e_value_offs =
cpu_to_le16(o + size);
last = EXT4_XATTR_NEXT(last);
}
}
+ if (s->here->e_value_inum) {
+ ext4_xattr_inode_unlink(inode,
+ le32_to_cpu(s->here->e_value_inum));
+ s->here->e_value_inum = 0;
+ }
if (!i->value) {
/* Remove the old name. */
size_t size = EXT4_XATTR_LEN(name_len);
@@ -722,11 +999,20 @@ ext4_xattr_set_entry(struct ext4_xattr_info *i, struct ext4_xattr_search *s)
if (i->value) {
/* Insert the new value. */
- s->here->e_value_size = cpu_to_le32(i->value_len);
- if (i->value_len) {
+ if (in_inode) {
+ unsigned long ea_ino =
+ le32_to_cpu(s->here->e_value_inum);
+ rc = ext4_xattr_inode_set(handle, inode, &ea_ino,
+ i->value, i->value_len);
+ if (rc)
+ goto out;
+ s->here->e_value_inum = cpu_to_le32(ea_ino);
+ s->here->e_value_offs = 0;
+ } else if (i->value_len) {
size_t size = EXT4_XATTR_SIZE(i->value_len);
void *val = s->base + min_offs - size;
s->here->e_value_offs = cpu_to_le16(min_offs - size);
+ s->here->e_value_inum = 0;
if (i->value == EXT4_ZERO_XATTR_VALUE) {
memset(val, 0, size);
} else {
@@ -736,8 +1022,11 @@ ext4_xattr_set_entry(struct ext4_xattr_info *i, struct ext4_xattr_search *s)
memcpy(val, i->value, i->value_len);
}
}
+ s->here->e_value_size = cpu_to_le32(i->value_len);
}
- return 0;
+
+out:
+ return rc;
}
struct ext4_xattr_block_find {
@@ -801,8 +1090,6 @@ ext4_xattr_block_set(handle_t *handle, struct inode *inode,
#define header(x) ((struct ext4_xattr_header *)(x))
- if (i->value && i->value_len > sb->s_blocksize)
- return -ENOSPC;
if (s->base) {
BUFFER_TRACE(bs->bh, "get_write_access");
error = ext4_journal_get_write_access(handle, bs->bh);
@@ -821,7 +1108,7 @@ ext4_xattr_block_set(handle_t *handle, struct inode *inode,
mb_cache_entry_delete_block(ext4_mb_cache, hash,
bs->bh->b_blocknr);
ea_bdebug(bs->bh, "modifying in-place");
- error = ext4_xattr_set_entry(i, s);
+ error = ext4_xattr_set_entry(i, s, handle, inode);
if (!error) {
if (!IS_LAST_ENTRY(s->first))
ext4_xattr_rehash(header(s->base),
@@ -870,7 +1157,7 @@ ext4_xattr_block_set(handle_t *handle, struct inode *inode,
s->end = s->base + sb->s_blocksize;
}
- error = ext4_xattr_set_entry(i, s);
+ error = ext4_xattr_set_entry(i, s, handle, inode);
if (error == -EFSCORRUPTED)
goto bad_block;
if (error)
@@ -1070,7 +1357,7 @@ int ext4_xattr_ibody_inline_set(handle_t *handle, struct inode *inode,
if (EXT4_I(inode)->i_extra_isize == 0)
return -ENOSPC;
- error = ext4_xattr_set_entry(i, s);
+ error = ext4_xattr_set_entry(i, s, handle, inode);
if (error) {
if (error == -ENOSPC &&
ext4_has_inline_data(inode)) {
@@ -1082,7 +1369,7 @@ int ext4_xattr_ibody_inline_set(handle_t *handle, struct inode *inode,
error = ext4_xattr_ibody_find(inode, i, is);
if (error)
return error;
- error = ext4_xattr_set_entry(i, s);
+ error = ext4_xattr_set_entry(i, s, handle, inode);
}
if (error)
return error;
@@ -1098,7 +1385,7 @@ int ext4_xattr_ibody_inline_set(handle_t *handle, struct inode *inode,
return 0;
}
-static int ext4_xattr_ibody_set(struct inode *inode,
+static int ext4_xattr_ibody_set(handle_t *handle, struct inode *inode,
struct ext4_xattr_info *i,
struct ext4_xattr_ibody_find *is)
{
@@ -1108,7 +1395,7 @@ static int ext4_xattr_ibody_set(struct inode *inode,
if (EXT4_I(inode)->i_extra_isize == 0)
return -ENOSPC;
- error = ext4_xattr_set_entry(i, s);
+ error = ext4_xattr_set_entry(i, s, handle, inode);
if (error)
return error;
header = IHDR(inode, ext4_raw_inode(&is->iloc));
@@ -1155,7 +1442,7 @@ ext4_xattr_set_handle(handle_t *handle, struct inode *inode, int name_index,
.name = name,
.value = value,
.value_len = value_len,
-
+ .in_inode = 0,
};
struct ext4_xattr_ibody_find is = {
.s = { .not_found = -ENODATA, },
@@ -1204,7 +1491,7 @@ ext4_xattr_set_handle(handle_t *handle, struct inode *inode, int name_index,
}
if (!value) {
if (!is.s.not_found)
- error = ext4_xattr_ibody_set(inode, &i, &is);
+ error = ext4_xattr_ibody_set(handle, inode, &i, &is);
else if (!bs.s.not_found)
error = ext4_xattr_block_set(handle, inode, &i, &bs);
} else {
@@ -1215,7 +1502,7 @@ ext4_xattr_set_handle(handle_t *handle, struct inode *inode, int name_index,
if (!bs.s.not_found && ext4_xattr_value_same(&bs.s, &i))
goto cleanup;
- error = ext4_xattr_ibody_set(inode, &i, &is);
+ error = ext4_xattr_ibody_set(handle, inode, &i, &is);
if (!error && !bs.s.not_found) {
i.value = NULL;
error = ext4_xattr_block_set(handle, inode, &i, &bs);
@@ -1226,11 +1513,20 @@ ext4_xattr_set_handle(handle_t *handle, struct inode *inode, int name_index,
goto cleanup;
}
error = ext4_xattr_block_set(handle, inode, &i, &bs);
+ if (ext4_has_feature_ea_inode(inode->i_sb) &&
+ error == -ENOSPC) {
+ /* xattr not fit to block, store at external
+ * inode */
+ i.in_inode = 1;
+ error = ext4_xattr_ibody_set(handle, inode,
+ &i, &is);
+ }
if (error)
goto cleanup;
if (!is.s.not_found) {
i.value = NULL;
- error = ext4_xattr_ibody_set(inode, &i, &is);
+ error = ext4_xattr_ibody_set(handle, inode, &i,
+ &is);
}
}
}
@@ -1269,12 +1565,26 @@ ext4_xattr_set(struct inode *inode, int name_index, const char *name,
const void *value, size_t value_len, int flags)
{
handle_t *handle;
+ struct super_block *sb = inode->i_sb;
int error, retries = 0;
int credits = ext4_jbd2_credits_xattr(inode);
error = dquot_initialize(inode);
if (error)
return error;
+
+ if ((value_len >= EXT4_XATTR_MIN_LARGE_EA_SIZE(sb->s_blocksize)) &&
+ ext4_has_feature_ea_inode(sb)) {
+ int nrblocks = (value_len + sb->s_blocksize - 1) >>
+ sb->s_blocksize_bits;
+
+ /* For new inode */
+ credits += EXT4_SINGLEDATA_TRANS_BLOCKS(sb) + 3;
+
+ /* For data blocks of EA inode */
+ credits += ext4_meta_trans_blocks(inode, nrblocks, 0);
+ }
+
retry:
handle = ext4_journal_start(inode, EXT4_HT_XATTR, credits);
if (IS_ERR(handle)) {
@@ -1286,7 +1596,7 @@ ext4_xattr_set(struct inode *inode, int name_index, const char *name,
value, value_len, flags);
error2 = ext4_journal_stop(handle);
if (error == -ENOSPC &&
- ext4_should_retry_alloc(inode->i_sb, &retries))
+ ext4_should_retry_alloc(sb, &retries))
goto retry;
if (error == 0)
error = error2;
@@ -1311,7 +1621,7 @@ static void ext4_xattr_shift_entries(struct ext4_xattr_entry *entry,
/* Adjust the value offsets of the entries */
for (; !IS_LAST_ENTRY(last); last = EXT4_XATTR_NEXT(last)) {
- if (last->e_value_size) {
+ if (!last->e_value_inum && last->e_value_size) {
new_offs = le16_to_cpu(last->e_value_offs) +
value_offs_shift;
last->e_value_offs = cpu_to_le16(new_offs);
@@ -1372,7 +1682,7 @@ static int ext4_xattr_move_to_block(handle_t *handle, struct inode *inode,
goto out;
/* Remove the chosen entry from the inode */
- error = ext4_xattr_ibody_set(inode, &i, is);
+ error = ext4_xattr_ibody_set(handle, inode, &i, is);
if (error)
goto out;
@@ -1572,21 +1882,135 @@ int ext4_expand_extra_isize_ea(struct inode *inode, int new_extra_isize,
}
+#define EIA_INCR 16 /* must be 2^n */
+#define EIA_MASK (EIA_INCR - 1)
+/* Add the large xattr @ino into @lea_ino_array for later deletion.
+ * If @lea_ino_array is new or full it will be grown and the old
+ * contents copied over.
+ */
+static int
+ext4_expand_ino_array(struct ext4_xattr_ino_array **lea_ino_array, __u32 ino)
+{
+ if (*lea_ino_array == NULL) {
+ /*
+ * Start with 15 inodes, so it fits into a power-of-two size.
+ * If *lea_ino_array is NULL, this is essentially offsetof()
+ */
+ (*lea_ino_array) =
+ kmalloc(offsetof(struct ext4_xattr_ino_array,
+ xia_inodes[EIA_MASK]),
+ GFP_NOFS);
+ if (*lea_ino_array == NULL)
+ return -ENOMEM;
+ (*lea_ino_array)->xia_count = 0;
+ } else if (((*lea_ino_array)->xia_count & EIA_MASK) == EIA_MASK) {
+ /* expand the array once all 15 + n * 16 slots are full */
+ struct ext4_xattr_ino_array *new_array = NULL;
+ int count = (*lea_ino_array)->xia_count;
+
+ /* if new_array is NULL, this is essentially offsetof() */
+ new_array = kmalloc(
+ offsetof(struct ext4_xattr_ino_array,
+ xia_inodes[count + EIA_INCR]),
+ GFP_NOFS);
+ if (new_array == NULL)
+ return -ENOMEM;
+ memcpy(new_array, *lea_ino_array,
+ offsetof(struct ext4_xattr_ino_array,
+ xia_inodes[count]));
+ kfree(*lea_ino_array);
+ *lea_ino_array = new_array;
+ }
+ (*lea_ino_array)->xia_inodes[(*lea_ino_array)->xia_count++] = ino;
+ return 0;
+}
+
+/**
+ * Add xattr inode to orphan list
+ */
+static int
+ext4_xattr_inode_orphan_add(handle_t *handle, struct inode *inode,
+ int credits, struct ext4_xattr_ino_array *lea_ino_array)
+{
+ struct inode *ea_inode = NULL;
+ int idx = 0, error = 0;
+
+ if (lea_ino_array == NULL)
+ return 0;
+
+ for (; idx < lea_ino_array->xia_count; ++idx) {
+ if (!ext4_handle_has_enough_credits(handle, credits)) {
+ error = ext4_journal_extend(handle, credits);
+ if (error > 0)
+ error = ext4_journal_restart(handle, credits);
+
+ if (error != 0) {
+ ext4_warning(inode->i_sb,
+ "couldn't extend journal "
+ "(err %d)", error);
+ return error;
+ }
+ }
+ ea_inode = ext4_xattr_inode_iget(inode,
+ lea_ino_array->xia_inodes[idx], &error);
+ if (error)
+ continue;
+ ext4_orphan_add(handle, ea_inode);
+ /* the inode's i_count will be released by caller */
+ }
+
+ return 0;
+}
/*
* ext4_xattr_delete_inode()
*
- * Free extended attribute resources associated with this inode. This
+ * Free extended attribute resources associated with this inode. Traverse
+ * all entries and unlink any xattr inodes associated with this inode. This
* is called immediately before an inode is freed. We have exclusive
- * access to the inode.
+ * access to the inode. If an orphan inode is deleted it will also delete any
+ * xattr block and all xattr inodes. They are checked by ext4_xattr_inode_iget()
+ * to ensure they belong to the parent inode and were not deleted already.
*/
-void
-ext4_xattr_delete_inode(handle_t *handle, struct inode *inode)
+int
+ext4_xattr_delete_inode(handle_t *handle, struct inode *inode,
+ struct ext4_xattr_ino_array **lea_ino_array)
{
struct buffer_head *bh = NULL;
+ struct ext4_xattr_ibody_header *header;
+ struct ext4_inode *raw_inode;
+ struct ext4_iloc iloc;
+ struct ext4_xattr_entry *entry;
+ int credits = 3, error = 0;
- if (!EXT4_I(inode)->i_file_acl)
+ if (!ext4_test_inode_state(inode, EXT4_STATE_XATTR))
+ goto delete_external_ea;
+
+ error = ext4_get_inode_loc(inode, &iloc);
+ if (error)
+ goto cleanup;
+ raw_inode = ext4_raw_inode(&iloc);
+ header = IHDR(inode, raw_inode);
+ for (entry = IFIRST(header); !IS_LAST_ENTRY(entry);
+ entry = EXT4_XATTR_NEXT(entry)) {
+ if (!entry->e_value_inum)
+ continue;
+ if (ext4_expand_ino_array(lea_ino_array,
+ entry->e_value_inum) != 0) {
+ brelse(iloc.bh);
+ goto cleanup;
+ }
+ entry->e_value_inum = 0;
+ }
+ brelse(iloc.bh);
+
+delete_external_ea:
+ if (!EXT4_I(inode)->i_file_acl) {
+ /* add xattr inode to orphan list */
+ ext4_xattr_inode_orphan_add(handle, inode, credits,
+ *lea_ino_array);
goto cleanup;
+ }
bh = sb_bread(inode->i_sb, EXT4_I(inode)->i_file_acl);
if (!bh) {
EXT4_ERROR_INODE(inode, "block %llu read error",
@@ -1599,11 +2023,69 @@ ext4_xattr_delete_inode(handle_t *handle, struct inode *inode)
EXT4_I(inode)->i_file_acl);
goto cleanup;
}
+
+ for (entry = BFIRST(bh); !IS_LAST_ENTRY(entry);
+ entry = EXT4_XATTR_NEXT(entry)) {
+ if (!entry->e_value_inum)
+ continue;
+ if (ext4_expand_ino_array(lea_ino_array,
+ entry->e_value_inum) != 0)
+ goto cleanup;
+ entry->e_value_inum = 0;
+ }
+
+ /* add xattr inode to orphan list */
+ error = ext4_xattr_inode_orphan_add(handle, inode, credits,
+ *lea_ino_array);
+ if (error != 0)
+ goto cleanup;
+
+ if (!IS_NOQUOTA(inode))
+ credits += 2 * EXT4_QUOTA_DEL_BLOCKS(inode->i_sb);
+
+ if (!ext4_handle_has_enough_credits(handle, credits)) {
+ error = ext4_journal_extend(handle, credits);
+ if (error > 0)
+ error = ext4_journal_restart(handle, credits);
+ if (error != 0) {
+ ext4_warning(inode->i_sb,
+ "couldn't extend journal (err %d)", error);
+ goto cleanup;
+ }
+ }
+
ext4_xattr_release_block(handle, inode, bh);
EXT4_I(inode)->i_file_acl = 0;
cleanup:
brelse(bh);
+
+ return error;
+}
+
+void
+ext4_xattr_inode_array_free(struct inode *inode,
+ struct ext4_xattr_ino_array *lea_ino_array)
+{
+ struct inode *ea_inode = NULL;
+ int idx = 0;
+ int err;
+
+ if (lea_ino_array == NULL)
+ return;
+
+ for (; idx < lea_ino_array->xia_count; ++idx) {
+ ea_inode = ext4_xattr_inode_iget(inode,
+ lea_ino_array->xia_inodes[idx], &err);
+ if (err)
+ continue;
+ /* for inode's i_count get from ext4_xattr_delete_inode */
+ if (!list_empty(&EXT4_I(ea_inode)->i_orphan))
+ iput(ea_inode);
+ clear_nlink(ea_inode);
+ iput(ea_inode);
+ }
+ kfree(lea_ino_array);
}
/*
@@ -1655,10 +2137,9 @@ ext4_xattr_cmp(struct ext4_xattr_header *header1,
entry1->e_name_index != entry2->e_name_index ||
entry1->e_name_len != entry2->e_name_len ||
entry1->e_value_size != entry2->e_value_size ||
+ entry1->e_value_inum != entry2->e_value_inum ||
memcmp(entry1->e_name, entry2->e_name, entry1->e_name_len))
return 1;
- if (entry1->e_value_block != 0 || entry2->e_value_block != 0)
- return -EFSCORRUPTED;
if (memcmp((char *)header1 + le16_to_cpu(entry1->e_value_offs),
(char *)header2 + le16_to_cpu(entry2->e_value_offs),
le32_to_cpu(entry1->e_value_size)))
@@ -1730,7 +2211,7 @@ static inline void ext4_xattr_hash_entry(struct ext4_xattr_header *header,
*name++;
}
- if (entry->e_value_size != 0) {
+ if (!entry->e_value_inum && entry->e_value_size) {
__le32 *value = (__le32 *)((char *)header +
le16_to_cpu(entry->e_value_offs));
for (n = (le32_to_cpu(entry->e_value_size) +
diff --git a/fs/ext4/xattr.h b/fs/ext4/xattr.h
index 099c8b670ef5..6e10ff9393d4 100644
--- a/fs/ext4/xattr.h
+++ b/fs/ext4/xattr.h
@@ -44,7 +44,7 @@ struct ext4_xattr_entry {
__u8 e_name_len; /* length of name */
__u8 e_name_index; /* attribute name index */
__le16 e_value_offs; /* offset in disk block of value */
- __le32 e_value_block; /* disk block attribute is stored on (n/i) */
+ __le32 e_value_inum; /* inode in which the value is stored */
__le32 e_value_size; /* size of attribute value */
__le32 e_hash; /* hash value of name and value */
char e_name[0]; /* attribute name */
@@ -69,6 +69,26 @@ struct ext4_xattr_entry {
EXT4_I(inode)->i_extra_isize))
#define IFIRST(hdr) ((struct ext4_xattr_entry *)((hdr)+1))
+/*
+ * Link EA inode back to parent one using i_mtime field.
+ * Extra integer type conversion added to ignore higher
+ * bits in i_mtime.tv_sec which might be set by ext4_get()
+ */
+#define EXT4_XATTR_INODE_SET_PARENT(inode, inum) \
+do { \
+ (inode)->i_mtime.tv_sec = inum; \
+} while(0)
+
+#define EXT4_XATTR_INODE_GET_PARENT(inode) \
+((__u32)(inode)->i_mtime.tv_sec)
+
+/*
+ * The minimum size of EA value when you start storing it in an external inode
+ * size of block - size of header - size of 1 entry - 4 null bytes
+*/
+#define EXT4_XATTR_MIN_LARGE_EA_SIZE(b) \
+ ((b) - EXT4_XATTR_LEN(3) - sizeof(struct ext4_xattr_header) - 4)
+
#define BHDR(bh) ((struct ext4_xattr_header *)((bh)->b_data))
#define ENTRY(ptr) ((struct ext4_xattr_entry *)(ptr))
#define BFIRST(bh) ENTRY(BHDR(bh)+1)
@@ -77,10 +97,11 @@ struct ext4_xattr_entry {
#define EXT4_ZERO_XATTR_VALUE ((void *)-1)
struct ext4_xattr_info {
- int name_index;
const char *name;
const void *value;
size_t value_len;
+ int name_index;
+ int in_inode;
};
struct ext4_xattr_search {
@@ -140,7 +161,13 @@ extern int ext4_xattr_get(struct inode *, int, const char *, void *, size_t);
extern int ext4_xattr_set(struct inode *, int, const char *, const void *, size_t, int);
extern int ext4_xattr_set_handle(handle_t *, struct inode *, int, const char *, const void *, size_t, int);
-extern void ext4_xattr_delete_inode(handle_t *, struct inode *);
+extern struct inode *ext4_xattr_inode_iget(struct inode *parent, unsigned long ea_ino,
+ int *err);
+extern int ext4_xattr_inode_unlink(struct inode *inode, unsigned long ea_ino);
+extern int ext4_xattr_delete_inode(handle_t *handle, struct inode *inode,
+ struct ext4_xattr_ino_array **array);
+extern void ext4_xattr_inode_array_free(struct inode *inode,
+ struct ext4_xattr_ino_array *array);
extern int ext4_expand_extra_isize_ea(struct inode *inode, int new_extra_isize,
struct ext4_inode *raw_inode, handle_t *handle);
--
2.13.0.219.gdb65acc882-goog
[toc] | [next] | [standalone]
| From | Tahsin Erdogan <tahsin@google.com> |
|---|---|
| Date | 2017-05-31 10:20 +0200 |
| Subject | [PATCH 05/28] ext4: attach jinode after creation of xattr inode |
| Message-ID | <tN71g-2f2-37@gated-at.bofh.it> |
| In reply to | #1653968 |
In data=ordered mode jinode needs to be attached to the xattr inode when
writing data to it. Attachment normally occurs during file open for regular
files. Since we are not using file interface to write to the xattr inode,
the jinode attach needs to be done manually.
Otherwise the following crash occurs in data=ordered mode.
BUG: unable to handle kernel NULL pointer dereference at (null)
IP: jbd2_journal_file_inode+0x37/0x110
PGD 13b3c0067
P4D 13b3c0067
PUD 137660067
PMD 0
Oops: 0000 [#1] SMP
CPU: 3 PID: 1877 Comm: python Not tainted 4.12.0-rc1+ #749
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
task: ffff88010e368980 task.stack: ffffc90000374000
RIP: 0010:jbd2_journal_file_inode+0x37/0x110
RSP: 0018:ffffc90000377980 EFLAGS: 00010246
RAX: 0000000000000000 RBX: ffff880123b06230 RCX: 0000000000280000
RDX: 0000000000000006 RSI: 0000000000000000 RDI: ffff88012c8585d0
RBP: ffffc900003779b0 R08: 0000000000000202 R09: 0000000000000001
R10: 0000000000000000 R11: 0000000000000400 R12: ffff8801111f81c0
R13: ffff88013b2b6800 R14: ffffc90000377ab0 R15: 0000000000000001
FS: 00007f0c99b77740(0000) GS:ffff88013fd80000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000000 CR3: 0000000136d91000 CR4: 00000000000006e0
Call Trace:
jbd2_journal_inode_add_write+0xe/0x10
ext4_map_blocks+0x59e/0x620
ext4_xattr_set_entry+0x501/0x7d0
ext4_xattr_block_set+0x1b2/0x9b0
ext4_xattr_set_handle+0x322/0x4f0
ext4_xattr_set+0x144/0x1a0
ext4_xattr_user_set+0x34/0x40
__vfs_setxattr+0x66/0x80
__vfs_setxattr_noperm+0x69/0x1c0
vfs_setxattr+0xa2/0xb0
setxattr+0x12e/0x150
path_setxattr+0x87/0xb0
SyS_setxattr+0xf/0x20
entry_SYSCALL_64_fastpath+0x18/0xad
Signed-off-by: Tahsin Erdogan <tahsin@google.com>
---
fs/ext4/xattr.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index 12210fe87ea3..8e123533315f 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -826,6 +826,7 @@ static struct inode *ext4_xattr_inode_create(handle_t *handle,
struct inode *inode)
{
struct inode *ea_inode = NULL;
+ int err;
/*
* Let the next inode be the goal, so we try and allocate the EA inode
@@ -848,6 +849,11 @@ static struct inode *ext4_xattr_inode_create(handle_t *handle,
*/
EXT4_XATTR_INODE_SET_PARENT(ea_inode, inode->i_ino);
unlock_new_inode(ea_inode);
+ err = ext4_inode_attach_jinode(ea_inode);
+ if (err) {
+ iput(ea_inode);
+ return ERR_PTR(err);
+ }
}
return ea_inode;
--
2.13.0.219.gdb65acc882-goog
[toc] | [prev] | [next] | [standalone]
| From | Tahsin Erdogan <tahsin@google.com> |
|---|---|
| Date | 2017-05-31 10:20 +0200 |
| Subject | [PATCH 08/28] ext4: fix ref counting for ea_inode |
| Message-ID | <tN71g-2f2-43@gated-at.bofh.it> |
| In reply to | #1653968 |
The ref count on ea_inode is incremented by ext4_xattr_inode_orphan_add() which is supposed to be decremented by ext4_xattr_inode_array_free(). The decrement is conditioned on whether the ea_inode is currently on the orphan list. However, the orphan list addition only happens when journaling is enabled. In non-journaled case,r we fail to release the ref count causing an error message like below. "VFS: Busy inodes after unmount of sdb. Self-destruct in 5 seconds. Have a nice day..." Signed-off-by: Tahsin Erdogan <tahsin@google.com> --- fs/ext4/xattr.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c index 32ad2f2870e9..13daf634244b 100644 --- a/fs/ext4/xattr.c +++ b/fs/ext4/xattr.c @@ -2098,8 +2098,7 @@ ext4_xattr_inode_array_free(struct inode *inode, if (err) continue; /* for inode's i_count get from ext4_xattr_delete_inode */ - if (!list_empty(&EXT4_I(ea_inode)->i_orphan)) - iput(ea_inode); + iput(ea_inode); clear_nlink(ea_inode); iput(ea_inode); } -- 2.13.0.219.gdb65acc882-goog
[toc] | [prev] | [next] | [standalone]
| From | Tahsin Erdogan <tahsin@google.com> |
|---|---|
| Date | 2017-05-31 10:30 +0200 |
| Subject | [PATCH 25/28] ext4: add ext4_is_quota_file() |
| Message-ID | <tN7aV-2iQ-3@gated-at.bofh.it> |
| In reply to | #1653968 |
IS_NOQUOTA() indicates whether quota is disabled for an inode. Ext4
also uses it to check whether an inode is for a quota file. The
distinction currently doesn't matter because quota is disabled only
for the quota files. When we start disabling quota for other inodes
in the future, we will want to make the distinction clear.
Replace IS_NOQUOTA() call with ext4_is_quota_file() at places where
we are checking for quota files.
Signed-off-by: Tahsin Erdogan <tahsin@google.com>
---
fs/ext4/ext4.h | 2 ++
fs/ext4/inode.c | 2 +-
fs/ext4/ioctl.c | 4 ++--
fs/ext4/mballoc.c | 2 +-
fs/ext4/move_extent.c | 2 +-
5 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 580fdb753f29..d79d8d7bee88 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -2099,6 +2099,8 @@ static inline struct ext4_inode *ext4_raw_inode(struct ext4_iloc *iloc)
return (struct ext4_inode *) (iloc->bh->b_data + iloc->offset);
}
+#define ext4_is_quota_file(inode) IS_NOQUOTA(inode)
+
/*
* This structure is stuffed into the struct file's private_data field
* for directories. It is where we put information so that we can do
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 8ee20b586567..cf91532765a4 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -739,7 +739,7 @@ int ext4_map_blocks(handle_t *handle, struct inode *inode,
if (map->m_flags & EXT4_MAP_NEW &&
!(map->m_flags & EXT4_MAP_UNWRITTEN) &&
!(flags & EXT4_GET_BLOCKS_ZERO) &&
- !IS_NOQUOTA(inode) &&
+ !ext4_is_quota_file(inode) &&
ext4_should_order_data(inode)) {
if (flags & EXT4_GET_BLOCKS_IO_SUBMIT)
ret = ext4_jbd2_inode_add_wait(handle, inode);
diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
index 0c21e22acd74..dde8deb11e59 100644
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@ -218,7 +218,7 @@ static int ext4_ioctl_setflags(struct inode *inode,
unsigned int jflag;
/* Is it quota file? Do not allow user to mess with it */
- if (IS_NOQUOTA(inode))
+ if (ext4_is_quota_file(inode))
goto flags_out;
oldflags = ei->i_flags;
@@ -342,7 +342,7 @@ static int ext4_ioctl_setproject(struct file *filp, __u32 projid)
err = -EPERM;
inode_lock(inode);
/* Is it quota file? Do not allow user to mess with it */
- if (IS_NOQUOTA(inode))
+ if (ext4_is_quota_file(inode))
goto out_unlock;
err = ext4_get_inode_loc(inode, &iloc);
diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index b7928cddd539..d109a2a2fea0 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -4464,7 +4464,7 @@ ext4_fsblk_t ext4_mb_new_blocks(handle_t *handle,
trace_ext4_request_blocks(ar);
/* Allow to use superuser reservation for quota file */
- if (IS_NOQUOTA(ar->inode))
+ if (ext4_is_quota_file(ar->inode))
ar->flags |= EXT4_MB_USE_ROOT_BLOCKS;
if ((ar->flags & EXT4_MB_DELALLOC_RESERVED) == 0) {
diff --git a/fs/ext4/move_extent.c b/fs/ext4/move_extent.c
index c992ef2c2f94..9bb36909ec92 100644
--- a/fs/ext4/move_extent.c
+++ b/fs/ext4/move_extent.c
@@ -484,7 +484,7 @@ mext_check_arguments(struct inode *orig_inode,
return -EBUSY;
}
- if (IS_NOQUOTA(orig_inode) || IS_NOQUOTA(donor_inode)) {
+ if (ext4_is_quota_file(orig_inode) && ext4_is_quota_file(donor_inode)) {
ext4_debug("ext4 move extent: The argument files should "
"not be quota files [ino:orig %lu, donor %lu]\n",
orig_inode->i_ino, donor_inode->i_ino);
--
2.13.0.219.gdb65acc882-goog
[toc] | [prev] | [next] | [standalone]
| From | Tahsin Erdogan <tahsin@google.com> |
|---|---|
| Date | 2017-05-31 10:30 +0200 |
| Subject | [PATCH 15/28] ext4: fix ext4_xattr_move_to_block() |
| Message-ID | <tN7aV-2iQ-5@gated-at.bofh.it> |
| In reply to | #1653968 |
When moving xattr entries from inline area to a xattr block, entries
that refer to external xattr inodes need special handling because
value data is not available in the inline area but rather should be
read from its external inode.
Signed-off-by: Tahsin Erdogan <tahsin@google.com>
---
fs/ext4/xattr.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index 6a6bee246873..9c243b3510b7 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -1658,18 +1658,16 @@ static int ext4_xattr_move_to_block(handle_t *handle, struct inode *inode,
struct ext4_xattr_ibody_find *is = NULL;
struct ext4_xattr_block_find *bs = NULL;
char *buffer = NULL, *b_entry_name = NULL;
- size_t value_offs, value_size;
+ size_t value_size = le32_to_cpu(entry->e_value_size);
struct ext4_xattr_info i = {
.value = NULL,
.value_len = 0,
.name_index = entry->e_name_index,
+ .in_inode = !!entry->e_value_inum,
};
struct ext4_xattr_ibody_header *header = IHDR(inode, raw_inode);
int error;
- value_offs = le16_to_cpu(entry->e_value_offs);
- value_size = le32_to_cpu(entry->e_value_size);
-
is = kzalloc(sizeof(struct ext4_xattr_ibody_find), GFP_NOFS);
bs = kzalloc(sizeof(struct ext4_xattr_block_find), GFP_NOFS);
buffer = kmalloc(value_size, GFP_NOFS);
@@ -1685,7 +1683,17 @@ static int ext4_xattr_move_to_block(handle_t *handle, struct inode *inode,
bs->bh = NULL;
/* Save the entry name and the entry value */
- memcpy(buffer, (void *)IFIRST(header) + value_offs, value_size);
+ if (entry->e_value_inum) {
+ error = ext4_xattr_inode_get(inode,
+ le32_to_cpu(entry->e_value_inum),
+ buffer, value_size);
+ if (error)
+ goto out;
+ } else {
+ size_t value_offs = le16_to_cpu(entry->e_value_offs);
+ memcpy(buffer, (void *)IFIRST(header) + value_offs, value_size);
+ }
+
memcpy(b_entry_name, entry->e_name, entry->e_name_len);
b_entry_name[entry->e_name_len] = '\0';
i.name = b_entry_name;
@@ -1703,7 +1711,6 @@ static int ext4_xattr_move_to_block(handle_t *handle, struct inode *inode,
if (error)
goto out;
- i.name = b_entry_name;
i.value = buffer;
i.value_len = value_size;
error = ext4_xattr_block_find(inode, &i, bs);
--
2.13.0.219.gdb65acc882-goog
[toc] | [prev] | [next] | [standalone]
| From | Tahsin Erdogan <tahsin@google.com> |
|---|---|
| Date | 2017-05-31 10:30 +0200 |
| Subject | [PATCH 24/28] ext4: rename mb block cache functions |
| Message-ID | <tN7aV-2iQ-7@gated-at.bofh.it> |
| In reply to | #1653968 |
There will be a second cache instance that tracks ea_inodes. Make
existing names more explicit so that it is clear that they refer to
xattr block cache.
Signed-off-by: Tahsin Erdogan <tahsin@google.com>
---
fs/ext4/xattr.c | 36 ++++++++++++++++++++----------------
1 file changed, 20 insertions(+), 16 deletions(-)
diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index 772948f168c3..3ee7e2f68476 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -72,10 +72,11 @@
# define ea_bdebug(bh, fmt, ...) no_printk(fmt, ##__VA_ARGS__)
#endif
-static void ext4_xattr_cache_insert(struct mb_cache *, struct buffer_head *);
-static struct buffer_head *ext4_xattr_cache_find(struct inode *,
- struct ext4_xattr_header *,
- struct mb_cache_entry **);
+static void ext4_xattr_block_cache_insert(struct mb_cache *,
+ struct buffer_head *);
+static struct buffer_head *
+ext4_xattr_block_cache_find(struct inode *, struct ext4_xattr_header *,
+ struct mb_cache_entry **);
static void ext4_xattr_rehash(struct ext4_xattr_header *,
struct ext4_xattr_entry *);
@@ -395,7 +396,7 @@ ext4_xattr_block_get(struct inode *inode, int name_index, const char *name,
error = -EFSCORRUPTED;
goto cleanup;
}
- ext4_xattr_cache_insert(ext4_mb_cache, bh);
+ ext4_xattr_block_cache_insert(ext4_mb_cache, bh);
entry = BFIRST(bh);
error = ext4_xattr_find_entry(&entry, name_index, name, 1);
if (error)
@@ -563,7 +564,7 @@ ext4_xattr_block_list(struct dentry *dentry, char *buffer, size_t buffer_size)
error = -EFSCORRUPTED;
goto cleanup;
}
- ext4_xattr_cache_insert(ext4_mb_cache, bh);
+ ext4_xattr_block_cache_insert(ext4_mb_cache, bh);
error = ext4_xattr_list_entries(dentry, BFIRST(bh), buffer, buffer_size);
cleanup:
@@ -1123,8 +1124,8 @@ ext4_xattr_block_set(handle_t *handle, struct inode *inode,
if (!IS_LAST_ENTRY(s->first))
ext4_xattr_rehash(header(s->base),
s->here);
- ext4_xattr_cache_insert(ext4_mb_cache,
- bs->bh);
+ ext4_xattr_block_cache_insert(ext4_mb_cache,
+ bs->bh);
}
ext4_xattr_block_csum_set(inode, bs->bh);
unlock_buffer(bs->bh);
@@ -1177,7 +1178,8 @@ ext4_xattr_block_set(handle_t *handle, struct inode *inode,
inserted:
if (!IS_LAST_ENTRY(s->first)) {
- new_bh = ext4_xattr_cache_find(inode, header(s->base), &ce);
+ new_bh = ext4_xattr_block_cache_find(inode, header(s->base),
+ &ce);
if (new_bh) {
/* We found an identical block in the cache. */
if (new_bh == bs->bh)
@@ -1292,7 +1294,7 @@ ext4_xattr_block_set(handle_t *handle, struct inode *inode,
ext4_xattr_block_csum_set(inode, new_bh);
set_buffer_uptodate(new_bh);
unlock_buffer(new_bh);
- ext4_xattr_cache_insert(ext4_mb_cache, new_bh);
+ ext4_xattr_block_cache_insert(ext4_mb_cache, new_bh);
error = ext4_handle_dirty_metadata(handle, inode,
new_bh);
if (error)
@@ -2150,15 +2152,16 @@ void ext4_xattr_inode_array_free(struct ext4_xattr_inode_array *ea_inode_array)
}
/*
- * ext4_xattr_cache_insert()
+ * ext4_xattr_block_cache_insert()
*
- * Create a new entry in the extended attribute cache, and insert
+ * Create a new entry in the extended attribute block cache, and insert
* it unless such an entry is already in the cache.
*
* Returns 0, or a negative error number on failure.
*/
static void
-ext4_xattr_cache_insert(struct mb_cache *ext4_mb_cache, struct buffer_head *bh)
+ext4_xattr_block_cache_insert(struct mb_cache *ext4_mb_cache,
+ struct buffer_head *bh)
{
struct ext4_xattr_header *header = BHDR(bh);
__u32 hash = le32_to_cpu(header->h_hash);
@@ -2216,7 +2219,7 @@ ext4_xattr_cmp(struct ext4_xattr_header *header1,
}
/*
- * ext4_xattr_cache_find()
+ * ext4_xattr_block_cache_find()
*
* Find an identical extended attribute block.
*
@@ -2224,8 +2227,9 @@ ext4_xattr_cmp(struct ext4_xattr_header *header1,
* not found or an error occurred.
*/
static struct buffer_head *
-ext4_xattr_cache_find(struct inode *inode, struct ext4_xattr_header *header,
- struct mb_cache_entry **pce)
+ext4_xattr_block_cache_find(struct inode *inode,
+ struct ext4_xattr_header *header,
+ struct mb_cache_entry **pce)
{
__u32 hash = le32_to_cpu(header->h_hash);
struct mb_cache_entry *ce;
--
2.13.0.219.gdb65acc882-goog
[toc] | [prev] | [next] | [standalone]
| From | Tahsin Erdogan <tahsin@google.com> |
|---|---|
| Date | 2017-05-31 10:30 +0200 |
| Subject | [PATCH 21/28] ext4: modify ext4_xattr_ino_array to hold struct inode * |
| Message-ID | <tN7aW-2iQ-21@gated-at.bofh.it> |
| In reply to | #1653968 |
Tracking struct inode * rather than the inode number eliminates the
repeated ext4_xattr_inode_iget() call later. The second call cannot
fail in practice but still requires explanation when it wants to ignore
the return value. Avoid the trouble and make things simple.
Signed-off-by: Tahsin Erdogan <tahsin@google.com>
---
fs/ext4/ext4.h | 6 ++--
fs/ext4/inode.c | 8 ++---
fs/ext4/xattr.c | 93 ++++++++++++++++++++++++++++-----------------------------
fs/ext4/xattr.h | 5 ++--
4 files changed, 53 insertions(+), 59 deletions(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 2cdd6070e348..603edb5ff304 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -2232,9 +2232,9 @@ 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];
+struct ext4_xattr_inode_array {
+ unsigned int count; /* # of used items in the array */
+ struct inode *inodes[0];
};
/* bitmap.c */
extern unsigned int ext4_count_free(char *bitmap, unsigned numchars);
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index d095bf7ad390..8ee20b586567 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -188,7 +188,7 @@ void ext4_evict_inode(struct inode *inode)
handle_t *handle;
int err;
int extra_credits = 3;
- struct ext4_xattr_ino_array *lea_ino_array = NULL;
+ struct ext4_xattr_inode_array *ea_inode_array = NULL;
trace_ext4_evict_inode(inode);
@@ -257,7 +257,7 @@ void ext4_evict_inode(struct inode *inode)
/*
* Delete xattr inode before deleting the main inode.
*/
- err = ext4_xattr_delete_inode(handle, inode, &lea_ino_array);
+ err = ext4_xattr_delete_inode(handle, inode, &ea_inode_array);
if (err) {
ext4_warning(inode->i_sb,
"couldn't delete inode's xattr (err %d)", err);
@@ -345,9 +345,7 @@ void ext4_evict_inode(struct inode *inode)
ext4_journal_stop(handle);
sb_end_intwrite(inode->i_sb);
-
- if (lea_ino_array != NULL)
- ext4_xattr_inode_array_free(inode, lea_ino_array);
+ ext4_xattr_inode_array_free(ea_inode_array);
return;
no_delete:
ext4_clear_inode(inode); /* We must guarantee clearing of inode... */
diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index 5bce73b43b2a..886d06e409b6 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -1942,44 +1942,44 @@ int ext4_expand_extra_isize_ea(struct inode *inode, int new_extra_isize,
#define EIA_INCR 16 /* must be 2^n */
#define EIA_MASK (EIA_INCR - 1)
-/* Add the large xattr @ino into @lea_ino_array for later deletion.
- * If @lea_ino_array is new or full it will be grown and the old
+/* Add the large xattr @inode into @ea_inode_array for later deletion.
+ * If @ea_inode_array is new or full it will be grown and the old
* contents copied over.
*/
static int
-ext4_expand_ino_array(struct ext4_xattr_ino_array **lea_ino_array, __u32 ino)
+ext4_expand_inode_array(struct ext4_xattr_inode_array **ea_inode_array,
+ struct inode *inode)
{
- if (*lea_ino_array == NULL) {
+ if (*ea_inode_array == NULL) {
/*
* Start with 15 inodes, so it fits into a power-of-two size.
- * If *lea_ino_array is NULL, this is essentially offsetof()
+ * If *ea_inode_array is NULL, this is essentially offsetof()
*/
- (*lea_ino_array) =
- kmalloc(offsetof(struct ext4_xattr_ino_array,
- xia_inodes[EIA_MASK]),
+ (*ea_inode_array) =
+ kmalloc(offsetof(struct ext4_xattr_inode_array,
+ inodes[EIA_MASK]),
GFP_NOFS);
- if (*lea_ino_array == NULL)
+ if (*ea_inode_array == NULL)
return -ENOMEM;
- (*lea_ino_array)->xia_count = 0;
- } else if (((*lea_ino_array)->xia_count & EIA_MASK) == EIA_MASK) {
+ (*ea_inode_array)->count = 0;
+ } else if (((*ea_inode_array)->count & EIA_MASK) == EIA_MASK) {
/* expand the array once all 15 + n * 16 slots are full */
- struct ext4_xattr_ino_array *new_array = NULL;
- int count = (*lea_ino_array)->xia_count;
+ struct ext4_xattr_inode_array *new_array = NULL;
+ int count = (*ea_inode_array)->count;
/* if new_array is NULL, this is essentially offsetof() */
new_array = kmalloc(
- offsetof(struct ext4_xattr_ino_array,
- xia_inodes[count + EIA_INCR]),
+ offsetof(struct ext4_xattr_inode_array,
+ inodes[count + EIA_INCR]),
GFP_NOFS);
if (new_array == NULL)
return -ENOMEM;
- memcpy(new_array, *lea_ino_array,
- offsetof(struct ext4_xattr_ino_array,
- xia_inodes[count]));
- kfree(*lea_ino_array);
- *lea_ino_array = new_array;
+ memcpy(new_array, *ea_inode_array,
+ offsetof(struct ext4_xattr_inode_array, inodes[count]));
+ kfree(*ea_inode_array);
+ *ea_inode_array = new_array;
}
- (*lea_ino_array)->xia_inodes[(*lea_ino_array)->xia_count++] = ino;
+ (*ea_inode_array)->inodes[(*ea_inode_array)->count++] = inode;
return 0;
}
@@ -1987,16 +1987,16 @@ ext4_expand_ino_array(struct ext4_xattr_ino_array **lea_ino_array, __u32 ino)
* Add xattr inode to orphan list
*/
static int
-ext4_xattr_inode_orphan_add(handle_t *handle, struct inode *inode,
- int credits, struct ext4_xattr_ino_array *lea_ino_array)
+ext4_xattr_inode_orphan_add(handle_t *handle, struct inode *inode, int credits,
+ struct ext4_xattr_inode_array *ea_inode_array)
{
- struct inode *ea_inode;
int idx = 0, error = 0;
+ struct inode *ea_inode;
- if (lea_ino_array == NULL)
+ if (ea_inode_array == NULL)
return 0;
- for (; idx < lea_ino_array->xia_count; ++idx) {
+ for (; idx < ea_inode_array->count; ++idx) {
if (!ext4_handle_has_enough_credits(handle, credits)) {
error = ext4_journal_extend(handle, credits);
if (error > 0)
@@ -2009,10 +2009,7 @@ ext4_xattr_inode_orphan_add(handle_t *handle, struct inode *inode,
return error;
}
}
- error = ext4_xattr_inode_iget(inode,
- lea_ino_array->xia_inodes[idx], &ea_inode);
- if (error)
- continue;
+ ea_inode = ea_inode_array->inodes[idx];
inode_lock(ea_inode);
ext4_orphan_add(handle, ea_inode);
inode_unlock(ea_inode);
@@ -2034,13 +2031,14 @@ ext4_xattr_inode_orphan_add(handle_t *handle, struct inode *inode,
*/
int
ext4_xattr_delete_inode(handle_t *handle, struct inode *inode,
- struct ext4_xattr_ino_array **lea_ino_array)
+ struct ext4_xattr_inode_array **ea_inode_array)
{
struct buffer_head *bh = NULL;
struct ext4_xattr_ibody_header *header;
struct ext4_inode *raw_inode;
struct ext4_iloc iloc;
struct ext4_xattr_entry *entry;
+ struct inode *ea_inode;
unsigned int ea_ino;
int credits = 3, error = 0;
@@ -2057,8 +2055,12 @@ ext4_xattr_delete_inode(handle_t *handle, struct inode *inode,
if (!entry->e_value_inum)
continue;
ea_ino = le32_to_cpu(entry->e_value_inum);
- error = ext4_expand_ino_array(lea_ino_array, ea_ino);
+ error = ext4_xattr_inode_iget(inode, ea_ino, &ea_inode);
+ if (error)
+ continue;
+ error = ext4_expand_inode_array(ea_inode_array, ea_inode);
if (error) {
+ iput(ea_inode);
brelse(iloc.bh);
goto cleanup;
}
@@ -2070,7 +2072,7 @@ ext4_xattr_delete_inode(handle_t *handle, struct inode *inode,
if (!EXT4_I(inode)->i_file_acl) {
/* add xattr inode to orphan list */
error = ext4_xattr_inode_orphan_add(handle, inode, credits,
- *lea_ino_array);
+ *ea_inode_array);
goto cleanup;
}
bh = sb_bread(inode->i_sb, EXT4_I(inode)->i_file_acl);
@@ -2093,7 +2095,10 @@ ext4_xattr_delete_inode(handle_t *handle, struct inode *inode,
if (!entry->e_value_inum)
continue;
ea_ino = le32_to_cpu(entry->e_value_inum);
- error = ext4_expand_ino_array(lea_ino_array, ea_ino);
+ error = ext4_xattr_inode_iget(inode, ea_ino, &ea_inode);
+ if (error)
+ continue;
+ error = ext4_expand_inode_array(ea_inode_array, ea_inode);
if (error)
goto cleanup;
entry->e_value_inum = 0;
@@ -2101,7 +2106,7 @@ ext4_xattr_delete_inode(handle_t *handle, struct inode *inode,
/* add xattr inode to orphan list */
error = ext4_xattr_inode_orphan_add(handle, inode, credits,
- *lea_ino_array);
+ *ea_inode_array);
if (error)
goto cleanup;
@@ -2128,28 +2133,20 @@ ext4_xattr_delete_inode(handle_t *handle, struct inode *inode,
return error;
}
-void
-ext4_xattr_inode_array_free(struct inode *inode,
- struct ext4_xattr_ino_array *lea_ino_array)
+void ext4_xattr_inode_array_free(struct ext4_xattr_inode_array *ea_inode_array)
{
struct inode *ea_inode;
int idx = 0;
- int err;
- if (lea_ino_array == NULL)
+ if (ea_inode_array == NULL)
return;
- for (; idx < lea_ino_array->xia_count; ++idx) {
- err = ext4_xattr_inode_iget(inode,
- lea_ino_array->xia_inodes[idx], &ea_inode);
- if (err)
- continue;
- /* for inode's i_count get from ext4_xattr_delete_inode */
- iput(ea_inode);
+ for (; idx < ea_inode_array->count; ++idx) {
+ ea_inode = ea_inode_array->inodes[idx];
clear_nlink(ea_inode);
iput(ea_inode);
}
- kfree(lea_ino_array);
+ kfree(ea_inode_array);
}
/*
diff --git a/fs/ext4/xattr.h b/fs/ext4/xattr.h
index e82c5fe36a26..323eba54f72f 100644
--- a/fs/ext4/xattr.h
+++ b/fs/ext4/xattr.h
@@ -164,9 +164,8 @@ extern int ext4_xattr_set_credits(struct inode *inode, size_t value_len);
extern int ext4_xattr_inode_unlink(struct inode *inode, unsigned long ea_ino);
extern int ext4_xattr_delete_inode(handle_t *handle, struct inode *inode,
- struct ext4_xattr_ino_array **array);
-extern void ext4_xattr_inode_array_free(struct inode *inode,
- struct ext4_xattr_ino_array *array);
+ struct ext4_xattr_inode_array **array);
+extern void ext4_xattr_inode_array_free(struct ext4_xattr_inode_array *array);
extern int ext4_expand_extra_isize_ea(struct inode *inode, int new_extra_isize,
struct ext4_inode *raw_inode, handle_t *handle);
--
2.13.0.219.gdb65acc882-goog
[toc] | [prev] | [next] | [standalone]
| From | Tahsin Erdogan <tahsin@google.com> |
|---|---|
| Date | 2017-05-31 10:30 +0200 |
| Subject | [PATCH 23/28] mbcache: make mbcache more generic |
| Message-ID | <tN7aW-2iQ-17@gated-at.bofh.it> |
| In reply to | #1653968 |
Large xattr feature would like to use the mbcache for xattr value
deduplication. Current implementation is geared towards xattr block
deduplication. Make it more generic so that it can be used by both.
Signed-off-by: Tahsin Erdogan <tahsin@google.com>
---
fs/ext2/xattr.c | 18 +++++++++---------
fs/ext4/xattr.c | 10 +++++-----
fs/mbcache.c | 43 +++++++++++++++++++++----------------------
include/linux/mbcache.h | 14 ++++++++------
4 files changed, 43 insertions(+), 42 deletions(-)
diff --git a/fs/ext2/xattr.c b/fs/ext2/xattr.c
index fbdb8f171893..1e5f76070580 100644
--- a/fs/ext2/xattr.c
+++ b/fs/ext2/xattr.c
@@ -493,8 +493,8 @@ bad_block: ext2_error(sb, "ext2_xattr_set",
* This must happen under buffer lock for
* ext2_xattr_set2() to reliably detect modified block
*/
- mb_cache_entry_delete_block(EXT2_SB(sb)->s_mb_cache,
- hash, bh->b_blocknr);
+ mb_cache_entry_delete(EXT2_SB(sb)->s_mb_cache, hash,
+ bh->b_blocknr);
/* keep the buffer locked while modifying it. */
} else {
@@ -721,8 +721,8 @@ ext2_xattr_set2(struct inode *inode, struct buffer_head *old_bh,
* This must happen under buffer lock for
* ext2_xattr_set2() to reliably detect freed block
*/
- mb_cache_entry_delete_block(ext2_mb_cache,
- hash, old_bh->b_blocknr);
+ mb_cache_entry_delete(ext2_mb_cache, hash,
+ old_bh->b_blocknr);
/* Free the old block. */
ea_bdebug(old_bh, "freeing");
ext2_free_blocks(inode, old_bh->b_blocknr, 1);
@@ -795,8 +795,8 @@ ext2_xattr_delete_inode(struct inode *inode)
* This must happen under buffer lock for ext2_xattr_set2() to
* reliably detect freed block
*/
- mb_cache_entry_delete_block(EXT2_SB(inode->i_sb)->s_mb_cache,
- hash, bh->b_blocknr);
+ mb_cache_entry_delete(EXT2_SB(inode->i_sb)->s_mb_cache, hash,
+ bh->b_blocknr);
ext2_free_blocks(inode, EXT2_I(inode)->i_file_acl, 1);
get_bh(bh);
bforget(bh);
@@ -907,11 +907,11 @@ ext2_xattr_cache_find(struct inode *inode, struct ext2_xattr_header *header)
while (ce) {
struct buffer_head *bh;
- bh = sb_bread(inode->i_sb, ce->e_block);
+ bh = sb_bread(inode->i_sb, ce->e_value);
if (!bh) {
ext2_error(inode->i_sb, "ext2_xattr_cache_find",
"inode %ld: block %ld read error",
- inode->i_ino, (unsigned long) ce->e_block);
+ inode->i_ino, (unsigned long) ce->e_value);
} else {
lock_buffer(bh);
/*
@@ -931,7 +931,7 @@ ext2_xattr_cache_find(struct inode *inode, struct ext2_xattr_header *header)
} else if (le32_to_cpu(HDR(bh)->h_refcount) >
EXT2_XATTR_REFCOUNT_MAX) {
ea_idebug(inode, "block %ld refcount %d>%d",
- (unsigned long) ce->e_block,
+ (unsigned long) ce->e_value,
le32_to_cpu(HDR(bh)->h_refcount),
EXT2_XATTR_REFCOUNT_MAX);
} else if (!ext2_xattr_cmp(header, HDR(bh))) {
diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index 886d06e409b6..772948f168c3 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -678,7 +678,7 @@ ext4_xattr_release_block(handle_t *handle, struct inode *inode,
* This must happen under buffer lock for
* ext4_xattr_block_set() to reliably detect freed block
*/
- mb_cache_entry_delete_block(ext4_mb_cache, hash, bh->b_blocknr);
+ mb_cache_entry_delete(ext4_mb_cache, hash, bh->b_blocknr);
get_bh(bh);
unlock_buffer(bh);
ext4_free_blocks(handle, inode, bh, 0, 1,
@@ -1115,8 +1115,8 @@ ext4_xattr_block_set(handle_t *handle, struct inode *inode,
* ext4_xattr_block_set() to reliably detect modified
* block
*/
- mb_cache_entry_delete_block(ext4_mb_cache, hash,
- bs->bh->b_blocknr);
+ mb_cache_entry_delete(ext4_mb_cache, hash,
+ bs->bh->b_blocknr);
ea_bdebug(bs->bh, "modifying in-place");
error = ext4_xattr_set_entry(i, s, handle, inode);
if (!error) {
@@ -2238,10 +2238,10 @@ ext4_xattr_cache_find(struct inode *inode, struct ext4_xattr_header *header,
while (ce) {
struct buffer_head *bh;
- bh = sb_bread(inode->i_sb, ce->e_block);
+ bh = sb_bread(inode->i_sb, ce->e_value);
if (!bh) {
EXT4_ERROR_INODE(inode, "block %lu read error",
- (unsigned long) ce->e_block);
+ (unsigned long) ce->e_value);
} else if (ext4_xattr_cmp(header, BHDR(bh)) == 0) {
*pce = ce;
return bh;
diff --git a/fs/mbcache.c b/fs/mbcache.c
index b19be429d655..77a5b99d8f92 100644
--- a/fs/mbcache.c
+++ b/fs/mbcache.c
@@ -10,7 +10,7 @@
/*
* Mbcache is a simple key-value store. Keys need not be unique, however
* key-value pairs are expected to be unique (we use this fact in
- * mb_cache_entry_delete_block()).
+ * mb_cache_entry_delete()).
*
* Ext2 and ext4 use this cache for deduplication of extended attribute blocks.
* They use hash of a block contents as a key and block number as a value.
@@ -62,15 +62,15 @@ static inline struct hlist_bl_head *mb_cache_entry_head(struct mb_cache *cache,
* @cache - cache where the entry should be created
* @mask - gfp mask with which the entry should be allocated
* @key - key of the entry
- * @block - block that contains data
- * @reusable - is the block reusable by other inodes?
+ * @value - value of the entry
+ * @reusable - is the entry reusable by others?
*
- * Creates entry in @cache with key @key and records that data is stored in
- * block @block. The function returns -EBUSY if entry with the same key
- * and for the same block already exists in cache. Otherwise 0 is returned.
+ * Creates entry in @cache with key @key and value @value. The function returns
+ * -EBUSY if entry with the same key and value already exists in cache.
+ * Otherwise 0 is returned.
*/
int mb_cache_entry_create(struct mb_cache *cache, gfp_t mask, u32 key,
- sector_t block, bool reusable)
+ cache_value_t value, bool reusable)
{
struct mb_cache_entry *entry, *dup;
struct hlist_bl_node *dup_node;
@@ -91,12 +91,12 @@ int mb_cache_entry_create(struct mb_cache *cache, gfp_t mask, u32 key,
/* One ref for hash, one ref returned */
atomic_set(&entry->e_refcnt, 1);
entry->e_key = key;
- entry->e_block = block;
+ entry->e_value = value;
entry->e_reusable = reusable;
head = mb_cache_entry_head(cache, key);
hlist_bl_lock(head);
hlist_bl_for_each_entry(dup, dup_node, head, e_hash_list) {
- if (dup->e_key == key && dup->e_block == block) {
+ if (dup->e_key == key && dup->e_value == value) {
hlist_bl_unlock(head);
kmem_cache_free(mb_entry_cache, entry);
return -EBUSY;
@@ -187,13 +187,13 @@ struct mb_cache_entry *mb_cache_entry_find_next(struct mb_cache *cache,
EXPORT_SYMBOL(mb_cache_entry_find_next);
/*
- * mb_cache_entry_get - get a cache entry by block number (and key)
+ * mb_cache_entry_get - get a cache entry by value (and key)
* @cache - cache we work with
- * @key - key of block number @block
- * @block - block number
+ * @key - key
+ * @value - value
*/
struct mb_cache_entry *mb_cache_entry_get(struct mb_cache *cache, u32 key,
- sector_t block)
+ cache_value_t value)
{
struct hlist_bl_node *node;
struct hlist_bl_head *head;
@@ -202,7 +202,7 @@ struct mb_cache_entry *mb_cache_entry_get(struct mb_cache *cache, u32 key,
head = mb_cache_entry_head(cache, key);
hlist_bl_lock(head);
hlist_bl_for_each_entry(entry, node, head, e_hash_list) {
- if (entry->e_key == key && entry->e_block == block) {
+ if (entry->e_key == key && entry->e_value == value) {
atomic_inc(&entry->e_refcnt);
goto out;
}
@@ -214,15 +214,14 @@ struct mb_cache_entry *mb_cache_entry_get(struct mb_cache *cache, u32 key,
}
EXPORT_SYMBOL(mb_cache_entry_get);
-/* mb_cache_entry_delete_block - remove information about block from cache
+/* mb_cache_entry_delete - remove a cache entry
* @cache - cache we work with
- * @key - key of block @block
- * @block - block number
+ * @key - key
+ * @value - value
*
- * Remove entry from cache @cache with key @key with data stored in @block.
+ * Remove entry from cache @cache with key @key and value @value.
*/
-void mb_cache_entry_delete_block(struct mb_cache *cache, u32 key,
- sector_t block)
+void mb_cache_entry_delete(struct mb_cache *cache, u32 key, cache_value_t value)
{
struct hlist_bl_node *node;
struct hlist_bl_head *head;
@@ -231,7 +230,7 @@ void mb_cache_entry_delete_block(struct mb_cache *cache, u32 key,
head = mb_cache_entry_head(cache, key);
hlist_bl_lock(head);
hlist_bl_for_each_entry(entry, node, head, e_hash_list) {
- if (entry->e_key == key && entry->e_block == block) {
+ if (entry->e_key == key && entry->e_value == value) {
/* We keep hash list reference to keep entry alive */
hlist_bl_del_init(&entry->e_hash_list);
hlist_bl_unlock(head);
@@ -248,7 +247,7 @@ void mb_cache_entry_delete_block(struct mb_cache *cache, u32 key,
}
hlist_bl_unlock(head);
}
-EXPORT_SYMBOL(mb_cache_entry_delete_block);
+EXPORT_SYMBOL(mb_cache_entry_delete);
/* mb_cache_entry_touch - cache entry got used
* @cache - cache the entry belongs to
diff --git a/include/linux/mbcache.h b/include/linux/mbcache.h
index 86c9a8b480c5..e2d9f2f926a4 100644
--- a/include/linux/mbcache.h
+++ b/include/linux/mbcache.h
@@ -9,6 +9,8 @@
struct mb_cache;
+typedef sector_t cache_value_t;
+
struct mb_cache_entry {
/* List of entries in cache - protected by cache->c_list_lock */
struct list_head e_list;
@@ -19,15 +21,15 @@ struct mb_cache_entry {
u32 e_key;
u32 e_referenced:1;
u32 e_reusable:1;
- /* Block number of hashed block - stable during lifetime of the entry */
- sector_t e_block;
+ /* User provided value - stable during lifetime of the entry */
+ cache_value_t e_value;
};
struct mb_cache *mb_cache_create(int bucket_bits);
void mb_cache_destroy(struct mb_cache *cache);
int mb_cache_entry_create(struct mb_cache *cache, gfp_t mask, u32 key,
- sector_t block, bool reusable);
+ cache_value_t value, bool reusable);
void __mb_cache_entry_free(struct mb_cache_entry *entry);
static inline int mb_cache_entry_put(struct mb_cache *cache,
struct mb_cache_entry *entry)
@@ -38,10 +40,10 @@ static inline int mb_cache_entry_put(struct mb_cache *cache,
return 1;
}
-void mb_cache_entry_delete_block(struct mb_cache *cache, u32 key,
- sector_t block);
+void mb_cache_entry_delete(struct mb_cache *cache, u32 key,
+ cache_value_t value);
struct mb_cache_entry *mb_cache_entry_get(struct mb_cache *cache, u32 key,
- sector_t block);
+ cache_value_t value);
struct mb_cache_entry *mb_cache_entry_find_first(struct mb_cache *cache,
u32 key);
struct mb_cache_entry *mb_cache_entry_find_next(struct mb_cache *cache,
--
2.13.0.219.gdb65acc882-goog
[toc] | [prev] | [next] | [standalone]
| From | Tahsin Erdogan <tahsin@google.com> |
|---|---|
| Date | 2017-05-31 10:30 +0200 |
| Subject | [PATCH 13/28] ext4: ext4_xattr_value_same() should return false for external data |
| Message-ID | <tN7aW-2iQ-19@gated-at.bofh.it> |
| In reply to | #1653968 |
ext4_xattr_value_same() is used as a quick optimization in case the new
xattr value is identical to the previous value. When xattr value is
stored in a xattr inode the check becomes expensive so it is better to
just assume that they are not equal.
Signed-off-by: Tahsin Erdogan <tahsin@google.com>
---
fs/ext4/xattr.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index 4dd8be16d175..681a9b5eefd8 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -1428,6 +1428,9 @@ static int ext4_xattr_value_same(struct ext4_xattr_search *s,
{
void *value;
+ /* When e_value_inum is set the value is stored externally. */
+ if (s->here->e_value_inum)
+ return 0;
if (le32_to_cpu(s->here->e_value_size) != i->value_len)
return 0;
value = ((void *)s->base) + le16_to_cpu(s->here->e_value_offs);
--
2.13.0.219.gdb65acc882-goog
[toc] | [prev] | [next] | [standalone]
| From | Tahsin Erdogan <tahsin@google.com> |
|---|---|
| Date | 2017-05-31 10:30 +0200 |
| Subject | [PATCH 11/28] ext4: clean up ext4_xattr_inode_get() |
| Message-ID | <tN7aW-2iQ-35@gated-at.bofh.it> |
| In reply to | #1653968 |
The input and output values of *size parameter are equal on successful
return from ext4_xattr_inode_get(). On error return, the callers ignore
the output value so there is no need to update it.
Also check for NULL return from ext4_bread(). If the actual xattr inode
size happens to be smaller than the expected size, ext4_bread() may
return NULL which would indicate data corruption.
Signed-off-by: Tahsin Erdogan <tahsin@google.com>
---
fs/ext4/xattr.c | 35 +++++++++++++----------------------
1 file changed, 13 insertions(+), 22 deletions(-)
diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index d9477d01be9b..8e855fc2eb03 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -278,37 +278,28 @@ ext4_xattr_find_entry(struct ext4_xattr_entry **pentry, int name_index,
/*
* Read the EA value from an inode.
*/
-static int
-ext4_xattr_inode_read(struct inode *ea_inode, void *buf, size_t *size)
+static int ext4_xattr_inode_read(struct inode *ea_inode, void *buf, size_t size)
{
unsigned long block = 0;
struct buffer_head *bh = NULL;
- int blocksize;
- size_t csize, ret_size = 0;
-
- if (*size == 0)
- return 0;
+ int blocksize = ea_inode->i_sb->s_blocksize;
+ size_t csize, copied = 0;
- blocksize = ea_inode->i_sb->s_blocksize;
-
- while (ret_size < *size) {
- csize = (*size - ret_size) > blocksize ? blocksize :
- *size - ret_size;
+ while (copied < size) {
+ csize = (size - copied) > blocksize ? blocksize : size - copied;
bh = ext4_bread(NULL, ea_inode, block, 0);
- if (IS_ERR(bh)) {
- *size = ret_size;
+ if (IS_ERR(bh))
return PTR_ERR(bh);
- }
+ if (!bh)
+ return -EFSCORRUPTED;
+
memcpy(buf, bh->b_data, csize);
brelse(bh);
buf += csize;
block += 1;
- ret_size += csize;
+ copied += csize;
}
-
- *size = ret_size;
-
return 0;
}
@@ -360,7 +351,7 @@ static int ext4_xattr_inode_iget(struct inode *parent, unsigned long ea_ino,
*/
static int
ext4_xattr_inode_get(struct inode *inode, unsigned long ea_ino, void *buffer,
- size_t *size)
+ size_t size)
{
struct inode *ea_inode;
int ret;
@@ -417,7 +408,7 @@ ext4_xattr_block_get(struct inode *inode, int name_index, const char *name,
if (entry->e_value_inum) {
error = ext4_xattr_inode_get(inode,
le32_to_cpu(entry->e_value_inum),
- buffer, &size);
+ buffer, size);
if (error)
goto cleanup;
} else {
@@ -467,7 +458,7 @@ ext4_xattr_ibody_get(struct inode *inode, int name_index, const char *name,
if (entry->e_value_inum) {
error = ext4_xattr_inode_get(inode,
le32_to_cpu(entry->e_value_inum),
- buffer, &size);
+ buffer, size);
if (error)
goto cleanup;
} else {
--
2.13.0.219.gdb65acc882-goog
[toc] | [prev] | [next] | [standalone]
| From | Tahsin Erdogan <tahsin@google.com> |
|---|---|
| Date | 2017-05-31 10:30 +0200 |
| Subject | [PATCH 12/28] ext4: add missing le32_to_cpu(e_value_inum) conversions |
| Message-ID | <tN7aW-2iQ-31@gated-at.bofh.it> |
| In reply to | #1653968 |
Two places in code missed converting xattr inode number using
le32_to_cpu().
Signed-off-by: Tahsin Erdogan <tahsin@google.com>
---
fs/ext4/xattr.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index 8e855fc2eb03..4dd8be16d175 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -1997,6 +1997,7 @@ ext4_xattr_delete_inode(handle_t *handle, struct inode *inode,
struct ext4_inode *raw_inode;
struct ext4_iloc iloc;
struct ext4_xattr_entry *entry;
+ unsigned int ea_ino;
int credits = 3, error = 0;
if (!ext4_test_inode_state(inode, EXT4_STATE_XATTR))
@@ -2011,8 +2012,8 @@ ext4_xattr_delete_inode(handle_t *handle, struct inode *inode,
entry = EXT4_XATTR_NEXT(entry)) {
if (!entry->e_value_inum)
continue;
- if (ext4_expand_ino_array(lea_ino_array,
- entry->e_value_inum) != 0) {
+ ea_ino = le32_to_cpu(entry->e_value_inum);
+ if (ext4_expand_ino_array(lea_ino_array, ea_ino) != 0) {
brelse(iloc.bh);
goto cleanup;
}
@@ -2044,8 +2045,8 @@ ext4_xattr_delete_inode(handle_t *handle, struct inode *inode,
entry = EXT4_XATTR_NEXT(entry)) {
if (!entry->e_value_inum)
continue;
- if (ext4_expand_ino_array(lea_ino_array,
- entry->e_value_inum) != 0)
+ ea_ino = le32_to_cpu(entry->e_value_inum);
+ if (ext4_expand_ino_array(lea_ino_array, ea_ino) != 0)
goto cleanup;
entry->e_value_inum = 0;
}
--
2.13.0.219.gdb65acc882-goog
[toc] | [prev] | [next] | [standalone]
| From | Tahsin Erdogan <tahsin@google.com> |
|---|---|
| Date | 2017-05-31 10:30 +0200 |
| Subject | [PATCH 20/28] ext4: improve journal credit handling in set xattr paths |
| Message-ID | <tN7aW-2iQ-33@gated-at.bofh.it> |
| In reply to | #1653968 |
Both ext4_set_acl() and ext4_set_context() need to be made aware of
ea_inode feature when it comes to credits calculation.
Also add a sufficient credits check in ext4_xattr_set_handle() right
after xattr write lock is grabbed. Original credits calculation is done
outside the lock so there is a possiblity that the initially calculated
credits are not sufficient anymore.
Signed-off-by: Tahsin Erdogan <tahsin@google.com>
---
fs/ext4/acl.c | 7 ++++---
fs/ext4/ext4_jbd2.h | 14 --------------
fs/ext4/super.c | 6 +++---
fs/ext4/xattr.c | 55 +++++++++++++++++++++++++++++++++++++++++------------
fs/ext4/xattr.h | 1 +
5 files changed, 51 insertions(+), 32 deletions(-)
diff --git a/fs/ext4/acl.c b/fs/ext4/acl.c
index 3ec0e46de95f..74f7ac539e00 100644
--- a/fs/ext4/acl.c
+++ b/fs/ext4/acl.c
@@ -231,14 +231,15 @@ int
ext4_set_acl(struct inode *inode, struct posix_acl *acl, int type)
{
handle_t *handle;
- int error, retries = 0;
+ int error, credits, retries = 0;
+ size_t acl_size = acl ? ext4_acl_size(acl->a_count) : 0;
error = dquot_initialize(inode);
if (error)
return error;
retry:
- handle = ext4_journal_start(inode, EXT4_HT_XATTR,
- ext4_jbd2_credits_xattr(inode));
+ credits = ext4_xattr_set_credits(inode, acl_size);
+ handle = ext4_journal_start(inode, EXT4_HT_XATTR, credits);
if (IS_ERR(handle))
return PTR_ERR(handle);
diff --git a/fs/ext4/ext4_jbd2.h b/fs/ext4/ext4_jbd2.h
index f97611171023..a5bda70feed5 100644
--- a/fs/ext4/ext4_jbd2.h
+++ b/fs/ext4/ext4_jbd2.h
@@ -104,20 +104,6 @@
#define EXT4_MAXQUOTAS_INIT_BLOCKS(sb) (EXT4_MAXQUOTAS*EXT4_QUOTA_INIT_BLOCKS(sb))
#define EXT4_MAXQUOTAS_DEL_BLOCKS(sb) (EXT4_MAXQUOTAS*EXT4_QUOTA_DEL_BLOCKS(sb))
-static inline int ext4_jbd2_credits_xattr(struct inode *inode)
-{
- int credits = EXT4_DATA_TRANS_BLOCKS(inode->i_sb);
-
- /*
- * In case of inline data, we may push out the data to a block,
- * so we need to reserve credits for this eventuality
- */
- if (ext4_has_inline_data(inode))
- credits += ext4_writepage_trans_blocks(inode) + 1;
- return credits;
-}
-
-
/*
* Ext4 handle operation types -- for logging purposes
*/
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index d37c81f327e7..b02a23ec92ca 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1143,7 +1143,7 @@ static int ext4_set_context(struct inode *inode, const void *ctx, size_t len,
void *fs_data)
{
handle_t *handle = fs_data;
- int res, res2, retries = 0;
+ int res, res2, credits, retries = 0;
res = ext4_convert_inline_data(inode);
if (res)
@@ -1178,8 +1178,8 @@ static int ext4_set_context(struct inode *inode, const void *ctx, size_t len,
if (res)
return res;
retry:
- handle = ext4_journal_start(inode, EXT4_HT_MISC,
- ext4_jbd2_credits_xattr(inode));
+ credits = ext4_xattr_set_credits(inode, len);
+ handle = ext4_journal_start(inode, EXT4_HT_MISC, credits);
if (IS_ERR(handle))
return PTR_ERR(handle);
diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index 230e0aa76777..5bce73b43b2a 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -1473,6 +1473,17 @@ ext4_xattr_set_handle(handle_t *handle, struct inode *inode, int name_index,
ext4_write_lock_xattr(inode, &no_expand);
+ /* Check journal credits under write lock. */
+ if (ext4_handle_valid(handle)) {
+ int credits;
+
+ credits = ext4_xattr_set_credits(inode, value_len);
+ if (!ext4_handle_has_enough_credits(handle, credits)) {
+ error = -ENOSPC;
+ goto cleanup;
+ }
+ }
+
error = ext4_reserve_inode_write(handle, inode, &is.iloc);
if (error)
goto cleanup;
@@ -1570,6 +1581,36 @@ ext4_xattr_set_handle(handle_t *handle, struct inode *inode, int name_index,
return error;
}
+int ext4_xattr_set_credits(struct inode *inode, size_t value_len)
+{
+ struct super_block *sb = inode->i_sb;
+ int credits;
+
+ if (!EXT4_SB(sb)->s_journal)
+ return 0;
+
+ credits = EXT4_DATA_TRANS_BLOCKS(inode->i_sb);
+
+ /*
+ * In case of inline data, we may push out the data to a block,
+ * so we need to reserve credits for this eventuality
+ */
+ if (ext4_has_inline_data(inode))
+ credits += ext4_writepage_trans_blocks(inode) + 1;
+
+ if (ext4_has_feature_ea_inode(sb)) {
+ int nrblocks = (value_len + sb->s_blocksize - 1) >>
+ sb->s_blocksize_bits;
+
+ /* For new inode */
+ credits += EXT4_SINGLEDATA_TRANS_BLOCKS(sb) + 3;
+
+ /* For data blocks of EA inode */
+ credits += ext4_meta_trans_blocks(inode, nrblocks, 0);
+ }
+ return credits;
+}
+
/*
* ext4_xattr_set()
*
@@ -1585,24 +1626,14 @@ ext4_xattr_set(struct inode *inode, int name_index, const char *name,
handle_t *handle;
struct super_block *sb = inode->i_sb;
int error, retries = 0;
- int credits = ext4_jbd2_credits_xattr(inode);
+ int credits;
error = dquot_initialize(inode);
if (error)
return error;
- if (ext4_has_feature_ea_inode(sb)) {
- int nrblocks = (value_len + sb->s_blocksize - 1) >>
- sb->s_blocksize_bits;
-
- /* For new inode */
- credits += EXT4_SINGLEDATA_TRANS_BLOCKS(sb) + 3;
-
- /* For data blocks of EA inode */
- credits += ext4_meta_trans_blocks(inode, nrblocks, 0);
- }
-
retry:
+ credits = ext4_xattr_set_credits(inode, value_len);
handle = ext4_journal_start(inode, EXT4_HT_XATTR, credits);
if (IS_ERR(handle)) {
error = PTR_ERR(handle);
diff --git a/fs/ext4/xattr.h b/fs/ext4/xattr.h
index b6ef99d1a061..e82c5fe36a26 100644
--- a/fs/ext4/xattr.h
+++ b/fs/ext4/xattr.h
@@ -160,6 +160,7 @@ extern ssize_t ext4_listxattr(struct dentry *, char *, size_t);
extern int ext4_xattr_get(struct inode *, int, const char *, void *, size_t);
extern int ext4_xattr_set(struct inode *, int, const char *, const void *, size_t, int);
extern int ext4_xattr_set_handle(handle_t *, struct inode *, int, const char *, const void *, size_t, int);
+extern int ext4_xattr_set_credits(struct inode *inode, size_t value_len);
extern int ext4_xattr_inode_unlink(struct inode *inode, unsigned long ea_ino);
extern int ext4_xattr_delete_inode(handle_t *handle, struct inode *inode,
--
2.13.0.219.gdb65acc882-goog
[toc] | [prev] | [next] | [standalone]
| From | Tahsin Erdogan <tahsin@google.com> |
|---|---|
| Date | 2017-05-31 10:30 +0200 |
| Subject | [PATCH 19/28] ext4: ext4_xattr_delete_inode() should return accurate errors |
| Message-ID | <tN7aX-2iQ-45@gated-at.bofh.it> |
| In reply to | #1653968 |
In a few places the function returns without trying to pass the actual
error code to the caller. Fix those.
Signed-off-by: Tahsin Erdogan <tahsin@google.com>
---
fs/ext4/xattr.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index 1d354e447842..230e0aa76777 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -2026,7 +2026,8 @@ ext4_xattr_delete_inode(handle_t *handle, struct inode *inode,
if (!entry->e_value_inum)
continue;
ea_ino = le32_to_cpu(entry->e_value_inum);
- if (ext4_expand_ino_array(lea_ino_array, ea_ino) != 0) {
+ error = ext4_expand_ino_array(lea_ino_array, ea_ino);
+ if (error) {
brelse(iloc.bh);
goto cleanup;
}
@@ -2037,20 +2038,22 @@ ext4_xattr_delete_inode(handle_t *handle, struct inode *inode,
delete_external_ea:
if (!EXT4_I(inode)->i_file_acl) {
/* add xattr inode to orphan list */
- ext4_xattr_inode_orphan_add(handle, inode, credits,
- *lea_ino_array);
+ error = ext4_xattr_inode_orphan_add(handle, inode, credits,
+ *lea_ino_array);
goto cleanup;
}
bh = sb_bread(inode->i_sb, EXT4_I(inode)->i_file_acl);
if (!bh) {
EXT4_ERROR_INODE(inode, "block %llu read error",
EXT4_I(inode)->i_file_acl);
+ error = -EIO;
goto cleanup;
}
if (BHDR(bh)->h_magic != cpu_to_le32(EXT4_XATTR_MAGIC) ||
BHDR(bh)->h_blocks != cpu_to_le32(1)) {
EXT4_ERROR_INODE(inode, "bad block %llu",
EXT4_I(inode)->i_file_acl);
+ error = -EFSCORRUPTED;
goto cleanup;
}
@@ -2059,7 +2062,8 @@ ext4_xattr_delete_inode(handle_t *handle, struct inode *inode,
if (!entry->e_value_inum)
continue;
ea_ino = le32_to_cpu(entry->e_value_inum);
- if (ext4_expand_ino_array(lea_ino_array, ea_ino) != 0)
+ error = ext4_expand_ino_array(lea_ino_array, ea_ino);
+ if (error)
goto cleanup;
entry->e_value_inum = 0;
}
@@ -2067,7 +2071,7 @@ ext4_xattr_delete_inode(handle_t *handle, struct inode *inode,
/* add xattr inode to orphan list */
error = ext4_xattr_inode_orphan_add(handle, inode, credits,
*lea_ino_array);
- if (error != 0)
+ if (error)
goto cleanup;
if (!IS_NOQUOTA(inode))
@@ -2077,7 +2081,7 @@ ext4_xattr_delete_inode(handle_t *handle, struct inode *inode,
error = ext4_journal_extend(handle, credits);
if (error > 0)
error = ext4_journal_restart(handle, credits);
- if (error != 0) {
+ if (error) {
ext4_warning(inode->i_sb,
"couldn't extend journal (err %d)", error);
goto cleanup;
--
2.13.0.219.gdb65acc882-goog
[toc] | [prev] | [next] | [standalone]
| From | Tahsin Erdogan <tahsin@google.com> |
|---|---|
| Date | 2017-05-31 10:30 +0200 |
| Subject | [PATCH 14/28] ext4: fix ext4_xattr_make_inode_space() value size calculation |
| Message-ID | <tN7aX-2iQ-47@gated-at.bofh.it> |
| In reply to | #1653968 |
ext4_xattr_make_inode_space() is interested in calculating the inline
space used in an inode. When a xattr entry refers to an external inode
the value size indicates the external inode size, not the value size in
the inline area. Change the function to take this into account.
Signed-off-by: Tahsin Erdogan <tahsin@google.com>
---
fs/ext4/xattr.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index 681a9b5eefd8..6a6bee246873 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -1747,9 +1747,10 @@ static int ext4_xattr_make_inode_space(handle_t *handle, struct inode *inode,
last = IFIRST(header);
/* Find the entry best suited to be pushed into EA block */
for (; !IS_LAST_ENTRY(last); last = EXT4_XATTR_NEXT(last)) {
- total_size =
- EXT4_XATTR_SIZE(le32_to_cpu(last->e_value_size)) +
- EXT4_XATTR_LEN(last->e_name_len);
+ total_size = EXT4_XATTR_LEN(last->e_name_len);
+ if (!last->e_value_inum)
+ total_size += EXT4_XATTR_SIZE(
+ le32_to_cpu(last->e_value_size));
if (total_size <= bfree &&
total_size < min_total_size) {
if (total_size + ifree < isize_diff) {
@@ -1768,8 +1769,10 @@ static int ext4_xattr_make_inode_space(handle_t *handle, struct inode *inode,
}
entry_size = EXT4_XATTR_LEN(entry->e_name_len);
- total_size = entry_size +
- EXT4_XATTR_SIZE(le32_to_cpu(entry->e_value_size));
+ total_size = entry_size;
+ if (!entry->e_value_inum)
+ total_size += EXT4_XATTR_SIZE(
+ le32_to_cpu(entry->e_value_size));
error = ext4_xattr_move_to_block(handle, inode, raw_inode,
entry);
if (error)
--
2.13.0.219.gdb65acc882-goog
[toc] | [prev] | [next] | [standalone]
| From | Tahsin Erdogan <tahsin@google.com> |
|---|---|
| Date | 2017-05-31 10:30 +0200 |
| Subject | [PATCH 16/28] ext4: fix ext4_xattr_cmp() |
| Message-ID | <tN7aX-2iQ-51@gated-at.bofh.it> |
| In reply to | #1653968 |
When a xattr entry refers to an external inode, the value data is not available in the inline area so we should not attempt to read it using value offset. Signed-off-by: Tahsin Erdogan <tahsin@google.com> --- fs/ext4/xattr.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c index 9c243b3510b7..739f73a5a345 100644 --- a/fs/ext4/xattr.c +++ b/fs/ext4/xattr.c @@ -2169,7 +2169,8 @@ ext4_xattr_cmp(struct ext4_xattr_header *header1, entry1->e_value_inum != entry2->e_value_inum || memcmp(entry1->e_name, entry2->e_name, entry1->e_name_len)) return 1; - if (memcmp((char *)header1 + le16_to_cpu(entry1->e_value_offs), + if (!entry1->e_value_inum && + memcmp((char *)header1 + le16_to_cpu(entry1->e_value_offs), (char *)header2 + le16_to_cpu(entry2->e_value_offs), le32_to_cpu(entry1->e_value_size))) return 1; -- 2.13.0.219.gdb65acc882-goog
[toc] | [prev] | [next] | [standalone]
| From | Tahsin Erdogan <tahsin@google.com> |
|---|---|
| Date | 2017-05-31 10:40 +0200 |
| Subject | [PATCH 10/28] ext4: change ext4_xattr_inode_iget() signature |
| Message-ID | <tN7kB-2mN-1@gated-at.bofh.it> |
| In reply to | #1653968 |
In general, kernel functions indicate success/failure through their return
values. This function returns the status as an output parameter and reserves
the return value for the inode. Make it follow the general convention.
Signed-off-by: Tahsin Erdogan <tahsin@google.com>
---
fs/ext4/xattr.c | 73 +++++++++++++++++++++++++++++++--------------------------
fs/ext4/xattr.h | 2 --
2 files changed, 40 insertions(+), 35 deletions(-)
diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index 13daf634244b..d9477d01be9b 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -312,40 +312,47 @@ ext4_xattr_inode_read(struct inode *ea_inode, void *buf, size_t *size)
return 0;
}
-struct inode *ext4_xattr_inode_iget(struct inode *parent, unsigned long ea_ino, int *err)
+static int ext4_xattr_inode_iget(struct inode *parent, unsigned long ea_ino,
+ struct inode **ea_inode)
{
- struct inode *ea_inode = NULL;
+ struct inode *inode;
+ int err;
+
+ inode = ext4_iget(parent->i_sb, ea_ino);
+ if (IS_ERR(inode)) {
+ err = PTR_ERR(inode);
+ ext4_error(parent->i_sb, "error while reading EA inode %lu "
+ "err=%d", ea_ino, err);
+ return err;
+ }
- ea_inode = ext4_iget(parent->i_sb, ea_ino);
- if (IS_ERR(ea_inode) || is_bad_inode(ea_inode)) {
- int rc = IS_ERR(ea_inode) ? PTR_ERR(ea_inode) : 0;
+ if (is_bad_inode(inode)) {
ext4_error(parent->i_sb, "error while reading EA inode %lu "
- "/ %d %d", ea_ino, rc, is_bad_inode(ea_inode));
- *err = rc != 0 ? rc : -EIO;
- return NULL;
+ "is_bad_inode", ea_ino);
+ err = -EIO;
+ goto error;
}
- if (EXT4_XATTR_INODE_GET_PARENT(ea_inode) != parent->i_ino ||
- ea_inode->i_generation != parent->i_generation) {
+ if (EXT4_XATTR_INODE_GET_PARENT(inode) != parent->i_ino ||
+ inode->i_generation != parent->i_generation) {
ext4_error(parent->i_sb, "Backpointer from EA inode %lu "
- "to parent invalid.", ea_ino);
- *err = -EINVAL;
+ "to parent is invalid.", ea_ino);
+ err = -EINVAL;
goto error;
}
- if (!(EXT4_I(ea_inode)->i_flags & EXT4_EA_INODE_FL)) {
+ if (!(EXT4_I(inode)->i_flags & EXT4_EA_INODE_FL)) {
ext4_error(parent->i_sb, "EA inode %lu does not have "
"EXT4_EA_INODE_FL flag set.\n", ea_ino);
- *err = -EINVAL;
+ err = -EINVAL;
goto error;
}
- *err = 0;
- return ea_inode;
-
+ *ea_inode = inode;
+ return 0;
error:
- iput(ea_inode);
- return NULL;
+ iput(inode);
+ return err;
}
/*
@@ -355,17 +362,17 @@ static int
ext4_xattr_inode_get(struct inode *inode, unsigned long ea_ino, void *buffer,
size_t *size)
{
- struct inode *ea_inode = NULL;
- int err;
+ struct inode *ea_inode;
+ int ret;
- ea_inode = ext4_xattr_inode_iget(inode, ea_ino, &err);
- if (err)
- return err;
+ ret = ext4_xattr_inode_iget(inode, ea_ino, &ea_inode);
+ if (ret)
+ return ret;
- err = ext4_xattr_inode_read(ea_inode, buffer, size);
+ ret = ext4_xattr_inode_read(ea_inode, buffer, size);
iput(ea_inode);
- return err;
+ return ret;
}
static int
@@ -868,7 +875,7 @@ int ext4_xattr_inode_unlink(struct inode *inode, unsigned long ea_ino)
struct inode *ea_inode = NULL;
int err;
- ea_inode = ext4_xattr_inode_iget(inode, ea_ino, &err);
+ err = ext4_xattr_inode_iget(inode, ea_ino, &ea_inode);
if (err)
return err;
@@ -1948,7 +1955,7 @@ static int
ext4_xattr_inode_orphan_add(handle_t *handle, struct inode *inode,
int credits, struct ext4_xattr_ino_array *lea_ino_array)
{
- struct inode *ea_inode = NULL;
+ struct inode *ea_inode;
int idx = 0, error = 0;
if (lea_ino_array == NULL)
@@ -1967,8 +1974,8 @@ ext4_xattr_inode_orphan_add(handle_t *handle, struct inode *inode,
return error;
}
}
- ea_inode = ext4_xattr_inode_iget(inode,
- lea_ino_array->xia_inodes[idx], &error);
+ error = ext4_xattr_inode_iget(inode,
+ lea_ino_array->xia_inodes[idx], &ea_inode);
if (error)
continue;
inode_lock(ea_inode);
@@ -2085,7 +2092,7 @@ void
ext4_xattr_inode_array_free(struct inode *inode,
struct ext4_xattr_ino_array *lea_ino_array)
{
- struct inode *ea_inode = NULL;
+ struct inode *ea_inode;
int idx = 0;
int err;
@@ -2093,8 +2100,8 @@ ext4_xattr_inode_array_free(struct inode *inode,
return;
for (; idx < lea_ino_array->xia_count; ++idx) {
- ea_inode = ext4_xattr_inode_iget(inode,
- lea_ino_array->xia_inodes[idx], &err);
+ err = ext4_xattr_inode_iget(inode,
+ lea_ino_array->xia_inodes[idx], &ea_inode);
if (err)
continue;
/* for inode's i_count get from ext4_xattr_delete_inode */
diff --git a/fs/ext4/xattr.h b/fs/ext4/xattr.h
index e8bef79bdc38..b6ef99d1a061 100644
--- a/fs/ext4/xattr.h
+++ b/fs/ext4/xattr.h
@@ -161,8 +161,6 @@ extern int ext4_xattr_get(struct inode *, int, const char *, void *, size_t);
extern int ext4_xattr_set(struct inode *, int, const char *, const void *, size_t, int);
extern int ext4_xattr_set_handle(handle_t *, struct inode *, int, const char *, const void *, size_t, int);
-extern struct inode *ext4_xattr_inode_iget(struct inode *parent, unsigned long ea_ino,
- int *err);
extern int ext4_xattr_inode_unlink(struct inode *inode, unsigned long ea_ino);
extern int ext4_xattr_delete_inode(handle_t *handle, struct inode *inode,
struct ext4_xattr_ino_array **array);
--
2.13.0.219.gdb65acc882-goog
[toc] | [prev] | [next] | [standalone]
| From | Tahsin Erdogan <tahsin@google.com> |
|---|---|
| Date | 2017-05-31 10:40 +0200 |
| Subject | [PATCH 04/28] ext4: do not set posix acls on xattr inodes |
| Message-ID | <tN7kC-2mN-15@gated-at.bofh.it> |
| In reply to | #1653968 |
We don't need acls on xattr inodes because they are not directly
accessible from user mode.
Besides lockdep complains about recursive locking of xattr_sem as seen
below.
=============================================
[ INFO: possible recursive locking detected ]
4.11.0-rc8+ #402 Not tainted
---------------------------------------------
python/1894 is trying to acquire lock:
(&ei->xattr_sem){++++..}, at: [<ffffffff804878a6>] ext4_xattr_get+0x66/0x270
but task is already holding lock:
(&ei->xattr_sem){++++..}, at: [<ffffffff80489500>] ext4_xattr_set_handle+0xa0/0x5d0
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0
----
lock(&ei->xattr_sem);
lock(&ei->xattr_sem);
*** DEADLOCK ***
May be due to missing lock nesting notation
3 locks held by python/1894:
#0: (sb_writers#10){.+.+.+}, at: [<ffffffff803d829f>] mnt_want_write+0x1f/0x50
#1: (&sb->s_type->i_mutex_key#15){+.+...}, at: [<ffffffff803dda27>] vfs_setxattr+0x57/0xb0
#2: (&ei->xattr_sem){++++..}, at: [<ffffffff80489500>] ext4_xattr_set_handle+0xa0/0x5d0
stack backtrace:
CPU: 0 PID: 1894 Comm: python Not tainted 4.11.0-rc8+ #402
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
Call Trace:
dump_stack+0x67/0x99
__lock_acquire+0x5f3/0x1830
lock_acquire+0xb5/0x1d0
down_read+0x2f/0x60
ext4_xattr_get+0x66/0x270
ext4_get_acl+0x43/0x1e0
get_acl+0x72/0xf0
posix_acl_create+0x5e/0x170
ext4_init_acl+0x21/0xc0
__ext4_new_inode+0xffd/0x16b0
ext4_xattr_set_entry+0x5ea/0xb70
ext4_xattr_block_set+0x1b5/0x970
ext4_xattr_set_handle+0x351/0x5d0
ext4_xattr_set+0x124/0x180
ext4_xattr_user_set+0x34/0x40
__vfs_setxattr+0x66/0x80
__vfs_setxattr_noperm+0x69/0x1c0
vfs_setxattr+0xa2/0xb0
setxattr+0x129/0x160
path_setxattr+0x87/0xb0
SyS_setxattr+0xf/0x20
entry_SYSCALL_64_fastpath+0x18/0xad
Signed-off-by: Tahsin Erdogan <tahsin@google.com>
---
fs/ext4/ext4.h | 11 ++++++-----
fs/ext4/ialloc.c | 14 +++++++++-----
fs/ext4/migrate.c | 2 +-
fs/ext4/xattr.c | 3 ++-
4 files changed, 18 insertions(+), 12 deletions(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 24ef56b4572f..5d5fc0d0e2bc 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -2400,16 +2400,17 @@ extern int ext4fs_dirhash(const char *name, int len, struct
/* ialloc.c */
extern struct inode *__ext4_new_inode(handle_t *, struct inode *, umode_t,
const struct qstr *qstr, __u32 goal,
- uid_t *owner, int handle_type,
- unsigned int line_no, int nblocks);
+ uid_t *owner, __u32 i_flags,
+ int handle_type, unsigned int line_no,
+ int nblocks);
-#define ext4_new_inode(handle, dir, mode, qstr, goal, owner) \
+#define ext4_new_inode(handle, dir, mode, qstr, goal, owner, i_flags) \
__ext4_new_inode((handle), (dir), (mode), (qstr), (goal), (owner), \
- 0, 0, 0)
+ i_flags, 0, 0, 0)
#define ext4_new_inode_start_handle(dir, mode, qstr, goal, owner, \
type, nblocks) \
__ext4_new_inode(NULL, (dir), (mode), (qstr), (goal), (owner), \
- (type), __LINE__, (nblocks))
+ 0, (type), __LINE__, (nblocks))
extern void ext4_free_inode(handle_t *, struct inode *);
diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
index e2eb3cc06820..fb1b3df17f6e 100644
--- a/fs/ext4/ialloc.c
+++ b/fs/ext4/ialloc.c
@@ -742,8 +742,9 @@ static int recently_deleted(struct super_block *sb, ext4_group_t group, int ino)
*/
struct inode *__ext4_new_inode(handle_t *handle, struct inode *dir,
umode_t mode, const struct qstr *qstr,
- __u32 goal, uid_t *owner, int handle_type,
- unsigned int line_no, int nblocks)
+ __u32 goal, uid_t *owner, __u32 i_flags,
+ int handle_type, unsigned int line_no,
+ int nblocks)
{
struct super_block *sb;
struct buffer_head *inode_bitmap_bh = NULL;
@@ -1052,6 +1053,7 @@ struct inode *__ext4_new_inode(handle_t *handle, struct inode *dir,
/* Don't inherit extent flag from directory, amongst others. */
ei->i_flags =
ext4_mask_flags(mode, EXT4_I(dir)->i_flags & EXT4_FL_INHERITED);
+ ei->i_flags |= i_flags;
ei->i_file_acl = 0;
ei->i_dtime = 0;
ei->i_block_group = group;
@@ -1108,9 +1110,11 @@ struct inode *__ext4_new_inode(handle_t *handle, struct inode *dir,
goto fail_free_drop;
}
- err = ext4_init_acl(handle, inode, dir);
- if (err)
- goto fail_free_drop;
+ if (!(ei->i_flags & EXT4_EA_INODE_FL)) {
+ err = ext4_init_acl(handle, inode, dir);
+ if (err)
+ goto fail_free_drop;
+ }
err = ext4_init_security(handle, inode, dir, qstr);
if (err)
diff --git a/fs/ext4/migrate.c b/fs/ext4/migrate.c
index 364ea4d4a943..cf5181b62df1 100644
--- a/fs/ext4/migrate.c
+++ b/fs/ext4/migrate.c
@@ -475,7 +475,7 @@ int ext4_ext_migrate(struct inode *inode)
owner[0] = i_uid_read(inode);
owner[1] = i_gid_read(inode);
tmp_inode = ext4_new_inode(handle, d_inode(inode->i_sb->s_root),
- S_IFREG, NULL, goal, owner);
+ S_IFREG, NULL, goal, owner, 0);
if (IS_ERR(tmp_inode)) {
retval = PTR_ERR(tmp_inode);
ext4_journal_stop(handle);
diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index 09ba0137d529..12210fe87ea3 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -832,7 +832,8 @@ static struct inode *ext4_xattr_inode_create(handle_t *handle,
* in the same group, or nearby one.
*/
ea_inode = ext4_new_inode(handle, inode->i_sb->s_root->d_inode,
- S_IFREG | 0600, NULL, inode->i_ino + 1, NULL);
+ S_IFREG | 0600, NULL, inode->i_ino + 1, NULL,
+ EXT4_EA_INODE_FL);
if (!IS_ERR(ea_inode)) {
ea_inode->i_op = &ext4_file_inode_operations;
ea_inode->i_fop = &ext4_file_operations;
--
2.13.0.219.gdb65acc882-goog
[toc] | [prev] | [next] | [standalone]
| From | Tahsin Erdogan <tahsin@google.com> |
|---|---|
| Date | 2017-05-31 10:40 +0200 |
| Subject | [PATCH 07/28] ext4: call journal revoke when freeing ea_inode blocks |
| Message-ID | <tN7kC-2mN-21@gated-at.bofh.it> |
| In reply to | #1653968 |
ea_inode contents are treated as metadata, that's why it is journaled
during initial writes. Failing to call revoke during freeing could cause
user data to be overwritten with original ea_inode contents during journal
replay.
Signed-off-by: Tahsin Erdogan <tahsin@google.com>
---
fs/ext4/extents.c | 3 ++-
fs/ext4/indirect.c | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 3e36508610b7..e0a8425ff74d 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -2488,7 +2488,8 @@ int ext4_ext_index_trans_blocks(struct inode *inode, int extents)
static inline int get_default_free_blocks_flags(struct inode *inode)
{
- if (S_ISDIR(inode->i_mode) || S_ISLNK(inode->i_mode))
+ if (S_ISDIR(inode->i_mode) || S_ISLNK(inode->i_mode) ||
+ ext4_test_inode_flag(inode, EXT4_INODE_EA_INODE))
return EXT4_FREE_BLOCKS_METADATA | EXT4_FREE_BLOCKS_FORGET;
else if (ext4_should_journal_data(inode))
return EXT4_FREE_BLOCKS_FORGET;
diff --git a/fs/ext4/indirect.c b/fs/ext4/indirect.c
index bc15c2c17633..7ffa290cbb8e 100644
--- a/fs/ext4/indirect.c
+++ b/fs/ext4/indirect.c
@@ -829,7 +829,8 @@ static int ext4_clear_blocks(handle_t *handle, struct inode *inode,
int flags = EXT4_FREE_BLOCKS_VALIDATED;
int err;
- if (S_ISDIR(inode->i_mode) || S_ISLNK(inode->i_mode))
+ if (S_ISDIR(inode->i_mode) || S_ISLNK(inode->i_mode) ||
+ ext4_test_inode_flag(inode, EXT4_INODE_EA_INODE))
flags |= EXT4_FREE_BLOCKS_FORGET | EXT4_FREE_BLOCKS_METADATA;
else if (ext4_should_journal_data(inode))
flags |= EXT4_FREE_BLOCKS_FORGET;
--
2.13.0.219.gdb65acc882-goog
[toc] | [prev] | [next] | [standalone]
| From | "Darrick J. Wong" <darrick.wong@oracle.com> |
|---|---|
| Date | 2017-05-31 18:20 +0200 |
| Subject | Re: [PATCH 07/28] ext4: call journal revoke when freeing ea_inode blocks |
| Message-ID | <tNevM-77a-27@gated-at.bofh.it> |
| In reply to | #1654004 |
On Wed, May 31, 2017 at 01:14:56AM -0700, Tahsin Erdogan wrote:
> ea_inode contents are treated as metadata, that's why it is journaled
> during initial writes. Failing to call revoke during freeing could cause
> user data to be overwritten with original ea_inode contents during journal
> replay.
>
> Signed-off-by: Tahsin Erdogan <tahsin@google.com>
> ---
> fs/ext4/extents.c | 3 ++-
> fs/ext4/indirect.c | 3 ++-
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index 3e36508610b7..e0a8425ff74d 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -2488,7 +2488,8 @@ int ext4_ext_index_trans_blocks(struct inode *inode, int extents)
>
> static inline int get_default_free_blocks_flags(struct inode *inode)
> {
> - if (S_ISDIR(inode->i_mode) || S_ISLNK(inode->i_mode))
> + if (S_ISDIR(inode->i_mode) || S_ISLNK(inode->i_mode) ||
> + ext4_test_inode_flag(inode, EXT4_INODE_EA_INODE))
> return EXT4_FREE_BLOCKS_METADATA | EXT4_FREE_BLOCKS_FORGET;
> else if (ext4_should_journal_data(inode))
> return EXT4_FREE_BLOCKS_FORGET;
> diff --git a/fs/ext4/indirect.c b/fs/ext4/indirect.c
> index bc15c2c17633..7ffa290cbb8e 100644
> --- a/fs/ext4/indirect.c
> +++ b/fs/ext4/indirect.c
> @@ -829,7 +829,8 @@ static int ext4_clear_blocks(handle_t *handle, struct inode *inode,
> int flags = EXT4_FREE_BLOCKS_VALIDATED;
> int err;
>
> - if (S_ISDIR(inode->i_mode) || S_ISLNK(inode->i_mode))
> + if (S_ISDIR(inode->i_mode) || S_ISLNK(inode->i_mode) ||
> + ext4_test_inode_flag(inode, EXT4_INODE_EA_INODE))
I appreciate the thoroughness of doing this even for blockmapped
ea_inode files, and I'm not complaining about this hunk at all. :)
However, please consider requiring the extents feature + format as a
prerequisite for ea_inodes. ext4 has traditionally been very ...
permissive about supporting a diverse range of feature options, but the
cost of that diversity is that the feature support matrix that the
community has to support is already untestably large.
I think it would be wise not to support !extents && ea_inode,
particularly since blockmaps aren't protected by metadata_csum and so in
the long run it's probably best to minimize the introduction of new
blockmap files (on ext4 anyway).
--D
> flags |= EXT4_FREE_BLOCKS_FORGET | EXT4_FREE_BLOCKS_METADATA;
> else if (ext4_should_journal_data(inode))
> flags |= EXT4_FREE_BLOCKS_FORGET;
> --
> 2.13.0.219.gdb65acc882-goog
>
[toc] | [prev] | [next] | [standalone]
| From | Tahsin Erdogan <tahsin@google.com> |
|---|---|
| Date | 2017-05-31 23:10 +0200 |
| Subject | Re: [PATCH 07/28] ext4: call journal revoke when freeing ea_inode blocks |
| Message-ID | <tNj2q-1HY-27@gated-at.bofh.it> |
| In reply to | #1654411 |
> I think it would be wise not to support !extents && ea_inode, > particularly since blockmaps aren't protected by metadata_csum and so in > the long run it's probably best to minimize the introduction of new > blockmap files (on ext4 anyway). Sounds good. I will add that to e2fsprogs patches. thanks
[toc] | [prev] | [next] | [standalone]
Page 1 of 2 [1] 2 Next page →
Back to top | Article view | linux.kernel
csiph-web