Pass GFP_KERNEL for transaction allocation for ext4 routines if caller
can handler failures
Signed-off-by: Manish Katiyar <[email protected]>
---
fs/ext4/acl.c | 5 +++--
fs/ext4/ext4_jbd2.h | 9 +++++----
fs/ext4/extents.c | 8 ++++----
fs/ext4/ialloc.c | 2 +-
fs/ext4/inode.c | 33 ++++++++++++++++++---------------
fs/ext4/ioctl.c | 4 ++--
fs/ext4/migrate.c | 4 ++--
fs/ext4/move_extent.c | 2 +-
fs/ext4/namei.c | 24 +++++++++++++++---------
fs/ext4/resize.c | 8 ++++----
fs/ext4/super.c | 13 +++++++------
fs/ext4/xattr.c | 3 ++-
12 files changed, 64 insertions(+), 51 deletions(-)
diff --git a/fs/ext4/acl.c b/fs/ext4/acl.c
index e0270d1..1a4a944 100644
--- a/fs/ext4/acl.c
+++ b/fs/ext4/acl.c
@@ -351,7 +351,7 @@ ext4_acl_chmod(struct inode *inode)
retry:
handle = ext4_journal_start(inode,
- EXT4_DATA_TRANS_BLOCKS(inode->i_sb));
+ EXT4_DATA_TRANS_BLOCKS(inode->i_sb), true);
if (IS_ERR(handle)) {
error = PTR_ERR(handle);
ext4_std_error(inode->i_sb, error);
@@ -449,7 +449,8 @@ ext4_xattr_set_acl(struct dentry *dentry, const
char *name, const void *value,
acl = NULL;
retry:
- handle = ext4_journal_start(inode, EXT4_DATA_TRANS_BLOCKS(inode->i_sb));
+ handle = ext4_journal_start(inode,
+ EXT4_DATA_TRANS_BLOCKS(inode->i_sb), true);
if (IS_ERR(handle))
return PTR_ERR(handle);
error = ext4_set_acl(handle, inode, type, acl);
diff --git a/fs/ext4/ext4_jbd2.h b/fs/ext4/ext4_jbd2.h
index d8b992e..38f128e 100644
--- a/fs/ext4/ext4_jbd2.h
+++ b/fs/ext4/ext4_jbd2.h
@@ -161,7 +161,7 @@ int __ext4_handle_dirty_super(const char *where,
unsigned int line,
#define ext4_handle_dirty_super(handle, sb) \
__ext4_handle_dirty_super(__func__, __LINE__, (handle), (sb))
-handle_t *ext4_journal_start_sb(struct super_block *sb, int nblocks);
+handle_t *ext4_journal_start_sb(struct super_block *sb, int nblocks,
bool errok);
int __ext4_journal_stop(const char *where, unsigned int line,
handle_t *handle);
#define EXT4_NOJOURNAL_MAX_REF_COUNT ((unsigned long) 4096)
@@ -209,9 +209,10 @@ static inline void
ext4_journal_release_buffer(handle_t *handle,
jbd2_journal_release_buffer(handle, bh);
}
-static inline handle_t *ext4_journal_start(struct inode *inode, int nblocks)
+static inline handle_t *ext4_journal_start(struct inode *inode,
+ int nblocks, bool errok)
{
- return ext4_journal_start_sb(inode->i_sb, nblocks);
+ return ext4_journal_start_sb(inode->i_sb, nblocks, errok);
}
#define ext4_journal_stop(handle) \
@@ -232,7 +233,7 @@ static inline int ext4_journal_extend(handle_t
*handle, int nblocks)
static inline int ext4_journal_restart(handle_t *handle, int nblocks)
{
if (ext4_handle_valid(handle))
- return jbd2_journal_restart(handle, nblocks);
+ return jbd2_journal_restart(handle, nblocks, false);
return 0;
}
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 63a7581..5944b1c 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -2333,7 +2333,7 @@ static int ext4_ext_remove_space(struct inode
*inode, ext4_lblk_t start)
ext_debug("truncate since %u\n", start);
/* probably first extent we're gonna free will be last in block */
- handle = ext4_journal_start(inode, depth + 1);
+ handle = ext4_journal_start(inode, depth + 1, false);
if (IS_ERR(handle))
return PTR_ERR(handle);
@@ -3544,7 +3544,7 @@ void ext4_ext_truncate(struct inode *inode)
* probably first extent we're gonna free will be last in block
*/
err = ext4_writepage_trans_blocks(inode);
- handle = ext4_journal_start(inode, err);
+ handle = ext4_journal_start(inode, err, false);
if (IS_ERR(handle))
return;
@@ -3677,7 +3677,7 @@ retry:
while (ret >= 0 && ret < max_blocks) {
map.m_lblk = map.m_lblk + ret;
map.m_len = max_blocks = max_blocks - ret;
- handle = ext4_journal_start(inode, credits);
+ handle = ext4_journal_start(inode, credits, true);
if (IS_ERR(handle)) {
ret = PTR_ERR(handle);
break;
@@ -3752,7 +3752,7 @@ int ext4_convert_unwritten_extents(struct inode
*inode, loff_t offset,
while (ret >= 0 && ret < max_blocks) {
map.m_lblk += ret;
map.m_len = (max_blocks -= ret);
- handle = ext4_journal_start(inode, credits);
+ handle = ext4_journal_start(inode, credits, false);
if (IS_ERR(handle)) {
ret = PTR_ERR(handle);
break;
diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
index eb9097a..e0e27a3 100644
--- a/fs/ext4/ialloc.c
+++ b/fs/ext4/ialloc.c
@@ -1257,7 +1257,7 @@ extern int ext4_init_inode_table(struct
super_block *sb, ext4_group_t group,
if (gdp->bg_flags & cpu_to_le16(EXT4_BG_INODE_ZEROED))
goto out;
- handle = ext4_journal_start_sb(sb, 1);
+ handle = ext4_journal_start_sb(sb, 1, true);
if (IS_ERR(handle)) {
ret = PTR_ERR(handle);
goto out;
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 9f7f9e4..76c20b8 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -129,7 +129,7 @@ static handle_t *start_transaction(struct inode *inode)
{
handle_t *result;
- result = ext4_journal_start(inode, blocks_for_truncate(inode));
+ result = ext4_journal_start(inode, blocks_for_truncate(inode), false);
if (!IS_ERR(result))
return result;
@@ -204,7 +204,7 @@ void ext4_evict_inode(struct inode *inode)
if (is_bad_inode(inode))
goto no_delete;
- handle = ext4_journal_start(inode, blocks_for_truncate(inode)+3);
+ handle = ext4_journal_start(inode, blocks_for_truncate(inode)+3, false);
if (IS_ERR(handle)) {
ext4_std_error(inode->i_sb, PTR_ERR(handle));
/*
@@ -1398,7 +1398,7 @@ static int _ext4_get_block(struct inode *inode,
sector_t iblock,
if (map.m_len > DIO_MAX_BLOCKS)
map.m_len = DIO_MAX_BLOCKS;
dio_credits = ext4_chunk_trans_blocks(inode, map.m_len);
- handle = ext4_journal_start(inode, dio_credits);
+ handle = ext4_journal_start(inode, dio_credits, false);
if (IS_ERR(handle)) {
ret = PTR_ERR(handle);
return ret;
@@ -1621,7 +1621,7 @@ static int ext4_write_begin(struct file *file,
struct address_space *mapping,
to = from + len;
retry:
- handle = ext4_journal_start(inode, needed_blocks);
+ handle = ext4_journal_start(inode, needed_blocks, false);
if (IS_ERR(handle)) {
ret = PTR_ERR(handle);
goto out;
@@ -2653,7 +2653,8 @@ static int __ext4_journalled_writepage(struct page *page,
* references to buffers so we are safe */
unlock_page(page);
- handle = ext4_journal_start(inode, ext4_writepage_trans_blocks(inode));
+ handle = ext4_journal_start(inode, ext4_writepage_trans_blocks(inode),
+ false);
if (IS_ERR(handle)) {
ret = PTR_ERR(handle);
goto out;
@@ -3049,7 +3050,7 @@ retry:
needed_blocks = ext4_da_writepages_trans_blocks(inode);
/* start a new transaction*/
- handle = ext4_journal_start(inode, needed_blocks);
+ handle = ext4_journal_start(inode, needed_blocks, false);
if (IS_ERR(handle)) {
ret = PTR_ERR(handle);
ext4_msg(inode->i_sb, KERN_CRIT, "%s: jbd2_start: "
@@ -3204,7 +3205,7 @@ retry:
* to journalling the i_disksize update if writes to the end
* of file which has an already mapped buffer.
*/
- handle = ext4_journal_start(inode, 1);
+ handle = ext4_journal_start(inode, 1, false);
if (IS_ERR(handle)) {
ret = PTR_ERR(handle);
goto out;
@@ -3553,7 +3554,7 @@ static ssize_t ext4_ind_direct_IO(int rw, struct
kiocb *iocb,
if (final_size > inode->i_size) {
/* Credits for sb + inode write */
- handle = ext4_journal_start(inode, 2);
+ handle = ext4_journal_start(inode, 2, false);
if (IS_ERR(handle)) {
ret = PTR_ERR(handle);
goto out;
@@ -3596,7 +3597,7 @@ retry:
int err;
/* Credits for sb + inode write */
- handle = ext4_journal_start(inode, 2);
+ handle = ext4_journal_start(inode, 2, false);
if (IS_ERR(handle)) {
/* This is really bad luck. We've written the data
* but cannot extend i_size. Bail out and pretend
@@ -5329,8 +5330,10 @@ int ext4_setattr(struct dentry *dentry, struct
iattr *attr)
/* (user+group)*(old+new) structure, inode write (sb,
* inode block, ? - but truncate inode update has it) */
- handle = ext4_journal_start(inode, (EXT4_MAXQUOTAS_INIT_BLOCKS(inode->i_sb)+
- EXT4_MAXQUOTAS_DEL_BLOCKS(inode->i_sb))+3);
+ handle = ext4_journal_start(inode,
+ (EXT4_MAXQUOTAS_INIT_BLOCKS(inode->i_sb)+
+ EXT4_MAXQUOTAS_DEL_BLOCKS(inode->i_sb))+3,
+ true);
if (IS_ERR(handle)) {
error = PTR_ERR(handle);
goto err_out;
@@ -5365,7 +5368,7 @@ int ext4_setattr(struct dentry *dentry, struct
iattr *attr)
(ext4_test_inode_flag(inode, EXT4_INODE_EOFBLOCKS)))) {
handle_t *handle;
- handle = ext4_journal_start(inode, 3);
+ handle = ext4_journal_start(inode, 3, true);
if (IS_ERR(handle)) {
error = PTR_ERR(handle);
goto err_out;
@@ -5385,7 +5388,7 @@ int ext4_setattr(struct dentry *dentry, struct
iattr *attr)
attr->ia_size);
if (error) {
/* Do as much error cleanup as possible */
- handle = ext4_journal_start(inode, 3);
+ handle = ext4_journal_start(inode, 3, false);
if (IS_ERR(handle)) {
ext4_orphan_del(NULL, inode);
goto err_out;
@@ -5738,7 +5741,7 @@ void ext4_dirty_inode(struct inode *inode)
{
handle_t *handle;
- handle = ext4_journal_start(inode, 2);
+ handle = ext4_journal_start(inode, 2, false);
if (IS_ERR(handle))
goto out;
@@ -5822,7 +5825,7 @@ int ext4_change_inode_journal_flag(struct inode
*inode, int val)
/* Finally we can mark the inode as dirty. */
- handle = ext4_journal_start(inode, 1);
+ handle = ext4_journal_start(inode, 1, true);
if (IS_ERR(handle))
return PTR_ERR(handle);
diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
index eb3bc2f..3e7977b 100644
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@ -101,7 +101,7 @@ long ext4_ioctl(struct file *filp, unsigned int
cmd, unsigned long arg)
} else if (oldflags & EXT4_EOFBLOCKS_FL)
ext4_truncate(inode);
- handle = ext4_journal_start(inode, 1);
+ handle = ext4_journal_start(inode, 1, false);
if (IS_ERR(handle)) {
err = PTR_ERR(handle);
goto flags_out;
@@ -157,7 +157,7 @@ flags_out:
goto setversion_out;
}
- handle = ext4_journal_start(inode, 1);
+ handle = ext4_journal_start(inode, 1, true);
if (IS_ERR(handle)) {
err = PTR_ERR(handle);
goto setversion_out;
diff --git a/fs/ext4/migrate.c b/fs/ext4/migrate.c
index b0a126f..729dcbc 100644
--- a/fs/ext4/migrate.c
+++ b/fs/ext4/migrate.c
@@ -488,7 +488,7 @@ int ext4_ext_migrate(struct inode *inode)
EXT4_DATA_TRANS_BLOCKS(inode->i_sb) +
EXT4_INDEX_EXTRA_TRANS_BLOCKS + 3 +
EXT4_MAXQUOTAS_INIT_BLOCKS(inode->i_sb)
- + 1);
+ + 1, true);
if (IS_ERR(handle)) {
retval = PTR_ERR(handle);
return retval;
@@ -533,7 +533,7 @@ int ext4_ext_migrate(struct inode *inode)
ext4_set_inode_state(inode, EXT4_STATE_EXT_MIGRATE);
up_read((&EXT4_I(inode)->i_data_sem));
- handle = ext4_journal_start(inode, 1);
+ handle = ext4_journal_start(inode, 1, true);
if (IS_ERR(handle)) {
/*
* It is impossible to update on-disk structures without
diff --git a/fs/ext4/move_extent.c b/fs/ext4/move_extent.c
index b9f3e78..d5aad4d 100644
--- a/fs/ext4/move_extent.c
+++ b/fs/ext4/move_extent.c
@@ -813,7 +813,7 @@ move_extent_per_page(struct file *o_filp, struct
inode *donor_inode,
* inode and donor_inode may change each different metadata blocks.
*/
jblocks = ext4_writepage_trans_blocks(orig_inode) * 2;
- handle = ext4_journal_start(orig_inode, jblocks);
+ handle = ext4_journal_start(orig_inode, jblocks, true);
if (IS_ERR(handle)) {
*err = PTR_ERR(handle);
return 0;
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index 5485390..a3ad11f 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -1739,7 +1739,8 @@ static int ext4_create(struct inode *dir, struct
dentry *dentry, int mode,
retry:
handle = ext4_journal_start(dir, EXT4_DATA_TRANS_BLOCKS(dir->i_sb) +
EXT4_INDEX_EXTRA_TRANS_BLOCKS + 3 +
- EXT4_MAXQUOTAS_INIT_BLOCKS(dir->i_sb));
+ EXT4_MAXQUOTAS_INIT_BLOCKS(dir->i_sb),
+ true);
if (IS_ERR(handle))
return PTR_ERR(handle);
@@ -1775,7 +1776,8 @@ static int ext4_mknod(struct inode *dir, struct
dentry *dentry,
retry:
handle = ext4_journal_start(dir, EXT4_DATA_TRANS_BLOCKS(dir->i_sb) +
EXT4_INDEX_EXTRA_TRANS_BLOCKS + 3 +
- EXT4_MAXQUOTAS_INIT_BLOCKS(dir->i_sb));
+ EXT4_MAXQUOTAS_INIT_BLOCKS(dir->i_sb),
+ true);
if (IS_ERR(handle))
return PTR_ERR(handle);
@@ -1814,7 +1816,8 @@ static int ext4_mkdir(struct inode *dir, struct
dentry *dentry, int mode)
retry:
handle = ext4_journal_start(dir, EXT4_DATA_TRANS_BLOCKS(dir->i_sb) +
EXT4_INDEX_EXTRA_TRANS_BLOCKS + 3 +
- EXT4_MAXQUOTAS_INIT_BLOCKS(dir->i_sb));
+ EXT4_MAXQUOTAS_INIT_BLOCKS(dir->i_sb),
+ true);
if (IS_ERR(handle))
return PTR_ERR(handle);
@@ -2127,7 +2130,8 @@ static int ext4_rmdir(struct inode *dir, struct
dentry *dentry)
dquot_initialize(dir);
dquot_initialize(dentry->d_inode);
- handle = ext4_journal_start(dir, EXT4_DELETE_TRANS_BLOCKS(dir->i_sb));
+ handle = ext4_journal_start(dir,
+ EXT4_DELETE_TRANS_BLOCKS(dir->i_sb), true);
if (IS_ERR(handle))
return PTR_ERR(handle);
@@ -2188,7 +2192,8 @@ static int ext4_unlink(struct inode *dir, struct
dentry *dentry)
dquot_initialize(dir);
dquot_initialize(dentry->d_inode);
- handle = ext4_journal_start(dir, EXT4_DELETE_TRANS_BLOCKS(dir->i_sb));
+ handle = ext4_journal_start(dir,
+ EXT4_DELETE_TRANS_BLOCKS(dir->i_sb), true);
if (IS_ERR(handle))
return PTR_ERR(handle);
@@ -2246,8 +2251,9 @@ static int ext4_symlink(struct inode *dir,
retry:
handle = ext4_journal_start(dir, EXT4_DATA_TRANS_BLOCKS(dir->i_sb) +
- EXT4_INDEX_EXTRA_TRANS_BLOCKS + 5 +
- EXT4_MAXQUOTAS_INIT_BLOCKS(dir->i_sb));
+ EXT4_INDEX_EXTRA_TRANS_BLOCKS + 5 +
+ EXT4_MAXQUOTAS_INIT_BLOCKS(dir->i_sb),
+ true);
if (IS_ERR(handle))
return PTR_ERR(handle);
@@ -2313,7 +2319,7 @@ static int ext4_link(struct dentry *old_dentry,
retry:
handle = ext4_journal_start(dir, EXT4_DATA_TRANS_BLOCKS(dir->i_sb) +
- EXT4_INDEX_EXTRA_TRANS_BLOCKS);
+ EXT4_INDEX_EXTRA_TRANS_BLOCKS, true);
if (IS_ERR(handle))
return PTR_ERR(handle);
@@ -2365,7 +2371,7 @@ static int ext4_rename(struct inode *old_dir,
struct dentry *old_dentry,
dquot_initialize(new_dentry->d_inode);
handle = ext4_journal_start(old_dir, 2 *
EXT4_DATA_TRANS_BLOCKS(old_dir->i_sb) +
- EXT4_INDEX_EXTRA_TRANS_BLOCKS + 2);
+ EXT4_INDEX_EXTRA_TRANS_BLOCKS + 2, true);
if (IS_ERR(handle))
return PTR_ERR(handle);
diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
index 3ecc6e4..e50d083 100644
--- a/fs/ext4/resize.c
+++ b/fs/ext4/resize.c
@@ -176,7 +176,7 @@ static int setup_new_group_blocks(struct super_block *sb,
int err = 0, err2;
/* This transaction may be extended/restarted along the way */
- handle = ext4_journal_start_sb(sb, EXT4_MAX_TRANS_DATA);
+ handle = ext4_journal_start_sb(sb, EXT4_MAX_TRANS_DATA, true);
if (IS_ERR(handle))
return PTR_ERR(handle);
@@ -655,7 +655,7 @@ static void update_backups(struct super_block *sb,
handle_t *handle;
int err = 0, err2;
- handle = ext4_journal_start_sb(sb, EXT4_MAX_TRANS_DATA);
+ handle = ext4_journal_start_sb(sb, EXT4_MAX_TRANS_DATA, true);
if (IS_ERR(handle)) {
group = 1;
err = PTR_ERR(handle);
@@ -793,7 +793,7 @@ int ext4_group_add(struct super_block *sb, struct
ext4_new_group_data *input)
*/
handle = ext4_journal_start_sb(sb,
ext4_bg_has_super(sb, input->group) ?
- 3 + reserved_gdb : 4);
+ 3 + reserved_gdb : 4, true);
if (IS_ERR(handle)) {
err = PTR_ERR(handle);
goto exit_put;
@@ -1031,7 +1031,7 @@ int ext4_group_extend(struct super_block *sb,
struct ext4_super_block *es,
/* We will update the superblock, one block bitmap, and
* one group descriptor via ext4_free_blocks().
*/
- handle = ext4_journal_start_sb(sb, 3);
+ handle = ext4_journal_start_sb(sb, 3, true);
if (IS_ERR(handle)) {
err = PTR_ERR(handle);
ext4_warning(sb, "error %d on journal start", err);
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index cb10a06..7714a15 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -241,7 +241,7 @@ static void ext4_put_nojournal(handle_t *handle)
* that sync() will call the filesystem's write_super callback if
* appropriate.
*/
-handle_t *ext4_journal_start_sb(struct super_block *sb, int nblocks)
+handle_t *ext4_journal_start_sb(struct super_block *sb, int nblocks,
bool errok)
{
journal_t *journal;
@@ -258,7 +258,7 @@ handle_t *ext4_journal_start_sb(struct super_block
*sb, int nblocks)
ext4_abort(sb, "Detected aborted journal");
return ERR_PTR(-EROFS);
}
- return jbd2_journal_start(journal, nblocks);
+ return jbd2_journal_start(journal, nblocks, errok);
}
return ext4_get_nojournal();
}
@@ -4471,7 +4471,8 @@ static int ext4_write_dquot(struct dquot *dquot)
inode = dquot_to_inode(dquot);
handle = ext4_journal_start(inode,
- EXT4_QUOTA_TRANS_BLOCKS(dquot->dq_sb));
+ EXT4_QUOTA_TRANS_BLOCKS(dquot->dq_sb),
+ false);
if (IS_ERR(handle))
return PTR_ERR(handle);
ret = dquot_commit(dquot);
@@ -4487,7 +4488,7 @@ static int ext4_acquire_dquot(struct dquot *dquot)
handle_t *handle;
handle = ext4_journal_start(dquot_to_inode(dquot),
- EXT4_QUOTA_INIT_BLOCKS(dquot->dq_sb));
+ EXT4_QUOTA_INIT_BLOCKS(dquot->dq_sb), true);
if (IS_ERR(handle))
return PTR_ERR(handle);
ret = dquot_acquire(dquot);
@@ -4503,7 +4504,7 @@ static int ext4_release_dquot(struct dquot *dquot)
handle_t *handle;
handle = ext4_journal_start(dquot_to_inode(dquot),
- EXT4_QUOTA_DEL_BLOCKS(dquot->dq_sb));
+ EXT4_QUOTA_DEL_BLOCKS(dquot->dq_sb), true);
if (IS_ERR(handle)) {
/* Release dquot anyway to avoid endless cycle in dqput() */
dquot_release(dquot);
@@ -4534,7 +4535,7 @@ static int ext4_write_info(struct super_block
*sb, int type)
handle_t *handle;
/* Data block + inode block */
- handle = ext4_journal_start(sb->s_root->d_inode, 2);
+ handle = ext4_journal_start(sb->s_root->d_inode, 2, false);
if (IS_ERR(handle))
return PTR_ERR(handle);
ret = dquot_commit_info(sb, type);
diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index fc32176..0e39f57 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -1084,7 +1084,8 @@ ext4_xattr_set(struct inode *inode, int
name_index, const char *name,
int error, retries = 0;
retry:
- handle = ext4_journal_start(inode, EXT4_DATA_TRANS_BLOCKS(inode->i_sb));
+ handle = ext4_journal_start(inode,
+ EXT4_DATA_TRANS_BLOCKS(inode->i_sb), true);
if (IS_ERR(handle)) {
error = PTR_ERR(handle);
} else {
--
1.6.0.4
--
Thanks -
Manish
==================================
[$\*.^ -- I miss being one of them
==================================
Hi,
On Sat 22-01-11 19:34:55, Manish Katiyar wrote:
> Pass GFP_KERNEL for transaction allocation for ext4 routines if caller
> can handler failures
Some error recovery paths will need cleaning up before you actually start
using them - see below:
> diff --git a/fs/ext4/acl.c b/fs/ext4/acl.c
> index e0270d1..1a4a944 100644
> --- a/fs/ext4/acl.c
> +++ b/fs/ext4/acl.c
> @@ -351,7 +351,7 @@ ext4_acl_chmod(struct inode *inode)
>
> retry:
> handle = ext4_journal_start(inode,
> - EXT4_DATA_TRANS_BLOCKS(inode->i_sb));
> + EXT4_DATA_TRANS_BLOCKS(inode->i_sb), true);
> if (IS_ERR(handle)) {
> error = PTR_ERR(handle);
> ext4_std_error(inode->i_sb, error);
We shouldn't call ext4_std_error() (that possibly logs the message in
the kernel log and remounts the fs read-only, panics the kernel or so) in
case of ENOMEM...
> @@ -449,7 +449,8 @@ ext4_xattr_set_acl(struct dentry *dentry, const
> char *name, const void *value,
> acl = NULL;
>
> retry:
> - handle = ext4_journal_start(inode, EXT4_DATA_TRANS_BLOCKS(inode->i_sb));
> + handle = ext4_journal_start(inode,
> + EXT4_DATA_TRANS_BLOCKS(inode->i_sb), true);
> if (IS_ERR(handle))
> return PTR_ERR(handle);
It's actually not your bug, but the above should be:
error = PTR_ERR(handle);
goto release_and_out;
> error = ext4_set_acl(handle, inode, type, acl);
> diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
> index eb9097a..e0e27a3 100644
> --- a/fs/ext4/ialloc.c
> +++ b/fs/ext4/ialloc.c
> @@ -1257,7 +1257,7 @@ extern int ext4_init_inode_table(struct
> super_block *sb, ext4_group_t group,
> if (gdp->bg_flags & cpu_to_le16(EXT4_BG_INODE_ZEROED))
> goto out;
>
> - handle = ext4_journal_start_sb(sb, 1);
> + handle = ext4_journal_start_sb(sb, 1, true);
> if (IS_ERR(handle)) {
> ret = PTR_ERR(handle);
> goto out;
Well, this might be disputable. This function is used to lazily
initialize inode table. If the initialization fails, thread removes the
request for initialization from the queue. But in case of ENOMEM, it might
be more suitable to just postpone the initialization work to a more
suitable time...
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 9f7f9e4..76c20b8 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
...
> @@ -1398,7 +1398,7 @@ static int _ext4_get_block(struct inode *inode,
> sector_t iblock,
> if (map.m_len > DIO_MAX_BLOCKS)
> map.m_len = DIO_MAX_BLOCKS;
> dio_credits = ext4_chunk_trans_blocks(inode, map.m_len);
> - handle = ext4_journal_start(inode, dio_credits);
> + handle = ext4_journal_start(inode, dio_credits, false);
> if (IS_ERR(handle)) {
> ret = PTR_ERR(handle);
> return ret;
Hmm, this would be actually another useful prerequisite cleanup of this
series. _ext4_get_block() should need to start a transaction only when
called from direct IO path (otherwise transaction should be already started
when creating blocks). But this is only implicit so it would be good to
create ext4_get_block_directIO() which would start a transaction, use it
as a callback of __blockdev_direct_IO(), and remove the code from standard
_ext4_get_block() function. Then you can also make ext4_journal_start()
possibly fail and still it will be clear you do not introduce any potential
problems.
> @@ -1621,7 +1621,7 @@ static int ext4_write_begin(struct file *file,
> struct address_space *mapping,
> to = from + len;
>
> retry:
> - handle = ext4_journal_start(inode, needed_blocks);
> + handle = ext4_journal_start(inode, needed_blocks, false);
> if (IS_ERR(handle)) {
> ret = PTR_ERR(handle);
> goto out;
Failing this with ENOMEM is OK. Note that grab_cache_page_write_begin()
called just below can fail with ENOMEM as well.
> @@ -3553,7 +3554,7 @@ static ssize_t ext4_ind_direct_IO(int rw, struct
> kiocb *iocb,
>
> if (final_size > inode->i_size) {
> /* Credits for sb + inode write */
> - handle = ext4_journal_start(inode, 2);
> + handle = ext4_journal_start(inode, 2, false);
> if (IS_ERR(handle)) {
> ret = PTR_ERR(handle);
> goto out;
This can fail without introducing problems. It's standard directIO write
path.
> @@ -3596,7 +3597,7 @@ retry:
> int err;
>
> /* Credits for sb + inode write */
> - handle = ext4_journal_start(inode, 2);
> + handle = ext4_journal_start(inode, 2, false);
> if (IS_ERR(handle)) {
> /* This is really bad luck. We've written the data
> * but cannot extend i_size. Bail out and pretend
This one can fail just fine as well.
> @@ -5329,8 +5330,10 @@ int ext4_setattr(struct dentry *dentry, struct
> iattr *attr)
>
> /* (user+group)*(old+new) structure, inode write (sb,
> * inode block, ? - but truncate inode update has it) */
> - handle = ext4_journal_start(inode, (EXT4_MAXQUOTAS_INIT_BLOCKS(inode->i_sb)+
> - EXT4_MAXQUOTAS_DEL_BLOCKS(inode->i_sb))+3);
> + handle = ext4_journal_start(inode,
> + (EXT4_MAXQUOTAS_INIT_BLOCKS(inode->i_sb)+
> + EXT4_MAXQUOTAS_DEL_BLOCKS(inode->i_sb))+3,
> + true);
> if (IS_ERR(handle)) {
> error = PTR_ERR(handle);
> goto err_out;
> @@ -5365,7 +5368,7 @@ int ext4_setattr(struct dentry *dentry, struct
> iattr *attr)
> (ext4_test_inode_flag(inode, EXT4_INODE_EOFBLOCKS)))) {
> handle_t *handle;
>
> - handle = ext4_journal_start(inode, 3);
> + handle = ext4_journal_start(inode, 3, true);
> if (IS_ERR(handle)) {
> error = PTR_ERR(handle);
> goto err_out;
The above two sites are fine but note that err_out calls ext4_std_error()
which we don't want to happen in case of ENOMEM.
> @@ -5822,7 +5825,7 @@ int ext4_change_inode_journal_flag(struct inode
> *inode, int val)
>
> /* Finally we can mark the inode as dirty. */
>
> - handle = ext4_journal_start(inode, 1);
> + handle = ext4_journal_start(inode, 1, true);
> if (IS_ERR(handle))
> return PTR_ERR(handle);
This can fail OK, but you should undo inode flag and aops change before
returning error (that would be probably better as a separate preparatory
patch because it won't be completely trivial - you need to lock the updates
again etc. possibly create a helper function for that so that you don't
duplicate the code).
> diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
> index eb3bc2f..3e7977b 100644
> --- a/fs/ext4/ioctl.c
> +++ b/fs/ext4/ioctl.c
> @@ -101,7 +101,7 @@ long ext4_ioctl(struct file *filp, unsigned int
> cmd, unsigned long arg)
> } else if (oldflags & EXT4_EOFBLOCKS_FL)
> ext4_truncate(inode);
>
> - handle = ext4_journal_start(inode, 1);
> + handle = ext4_journal_start(inode, 1, false);
> if (IS_ERR(handle)) {
> err = PTR_ERR(handle);
> goto flags_out;
This can handle failure just fine...
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index cb10a06..7714a15 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -4487,7 +4488,7 @@ static int ext4_acquire_dquot(struct dquot *dquot)
> handle_t *handle;
>
> handle = ext4_journal_start(dquot_to_inode(dquot),
> - EXT4_QUOTA_INIT_BLOCKS(dquot->dq_sb));
> + EXT4_QUOTA_INIT_BLOCKS(dquot->dq_sb), true);
> if (IS_ERR(handle))
> return PTR_ERR(handle);
> ret = dquot_acquire(dquot);
> @@ -4503,7 +4504,7 @@ static int ext4_release_dquot(struct dquot *dquot)
> handle_t *handle;
>
> handle = ext4_journal_start(dquot_to_inode(dquot),
> - EXT4_QUOTA_DEL_BLOCKS(dquot->dq_sb));
> + EXT4_QUOTA_DEL_BLOCKS(dquot->dq_sb), true);
> if (IS_ERR(handle)) {
> /* Release dquot anyway to avoid endless cycle in dqput() */
> dquot_release(dquot);
For now put 'false' in these quota functions. Because failure here
results in a failure of dquot_initialize() which is not tested in most
places and thus results in quota miscomputations... Properly handling this
would require another set of cleanups.
Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR
Thanks a lot Jan, I will have a look at the functions you mentioned
and send an updated version.
On Tue, Jan 25, 2011 at 8:17 AM, Jan Kara <[email protected]> wrote:
> ?Hi,
>
> On Sat 22-01-11 19:34:55, Manish Katiyar wrote:
>> Pass GFP_KERNEL for transaction allocation for ext4 routines if caller
>> can handler failures
> ?Some error recovery paths will need cleaning up before you actually start
> using them - see below:
>
>> diff --git a/fs/ext4/acl.c b/fs/ext4/acl.c
>> index e0270d1..1a4a944 100644
>> --- a/fs/ext4/acl.c
>> +++ b/fs/ext4/acl.c
>> @@ -351,7 +351,7 @@ ext4_acl_chmod(struct inode *inode)
>>
>> ? ? ? retry:
>> ? ? ? ? ? ? ? handle = ext4_journal_start(inode,
>> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? EXT4_DATA_TRANS_BLOCKS(inode->i_sb));
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? EXT4_DATA_TRANS_BLOCKS(inode->i_sb), true);
>> ? ? ? ? ? ? ? if (IS_ERR(handle)) {
>> ? ? ? ? ? ? ? ? ? ? ? error = PTR_ERR(handle);
>> ? ? ? ? ? ? ? ? ? ? ? ext4_std_error(inode->i_sb, error);
> ?We shouldn't call ext4_std_error() (that possibly logs the message in
> the kernel log and remounts the fs read-only, panics the kernel or so) in
> case of ENOMEM...
>
>> @@ -449,7 +449,8 @@ ext4_xattr_set_acl(struct dentry *dentry, const
>> char *name, const void *value,
>> ? ? ? ? ? ? ? acl = NULL;
>>
>> ?retry:
>> - ? ? handle = ext4_journal_start(inode, EXT4_DATA_TRANS_BLOCKS(inode->i_sb));
>> + ? ? handle = ext4_journal_start(inode,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? EXT4_DATA_TRANS_BLOCKS(inode->i_sb), true);
>> ? ? ? if (IS_ERR(handle))
>> ? ? ? ? ? ? ? return PTR_ERR(handle);
> ?It's actually not your bug, but the above should be:
> ?error = PTR_ERR(handle);
> ?goto release_and_out;
>
>> ? ? ? error = ext4_set_acl(handle, inode, type, acl);
>> diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
>> index eb9097a..e0e27a3 100644
>> --- a/fs/ext4/ialloc.c
>> +++ b/fs/ext4/ialloc.c
>> @@ -1257,7 +1257,7 @@ extern int ext4_init_inode_table(struct
>> super_block *sb, ext4_group_t group,
>> ? ? ? if (gdp->bg_flags & cpu_to_le16(EXT4_BG_INODE_ZEROED))
>> ? ? ? ? ? ? ? goto out;
>>
>> - ? ? handle = ext4_journal_start_sb(sb, 1);
>> + ? ? handle = ext4_journal_start_sb(sb, 1, true);
>> ? ? ? if (IS_ERR(handle)) {
>> ? ? ? ? ? ? ? ret = PTR_ERR(handle);
>> ? ? ? ? ? ? ? goto out;
> ?Well, this might be disputable. This function is used to lazily
> initialize inode table. ?If the initialization fails, thread removes the
> request for initialization from the queue. But in case of ENOMEM, it might
> be more suitable to just postpone the initialization work to a more
> suitable time...
>
>> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
>> index 9f7f9e4..76c20b8 100644
>> --- a/fs/ext4/inode.c
>> +++ b/fs/ext4/inode.c
> ...
>> @@ -1398,7 +1398,7 @@ static int _ext4_get_block(struct inode *inode,
>> sector_t iblock,
>> ? ? ? ? ? ? ? if (map.m_len > DIO_MAX_BLOCKS)
>> ? ? ? ? ? ? ? ? ? ? ? map.m_len = DIO_MAX_BLOCKS;
>> ? ? ? ? ? ? ? dio_credits = ext4_chunk_trans_blocks(inode, map.m_len);
>> - ? ? ? ? ? ? handle = ext4_journal_start(inode, dio_credits);
>> + ? ? ? ? ? ? handle = ext4_journal_start(inode, dio_credits, false);
>> ? ? ? ? ? ? ? if (IS_ERR(handle)) {
>> ? ? ? ? ? ? ? ? ? ? ? ret = PTR_ERR(handle);
>> ? ? ? ? ? ? ? ? ? ? ? return ret;
> ?Hmm, this would be actually another useful prerequisite cleanup of this
> series. _ext4_get_block() should need to start a transaction only when
> called from direct IO path (otherwise transaction should be already started
> when creating blocks). But this is only implicit so it would be good to
> create ext4_get_block_directIO() which would start a transaction, use it
> as a callback of __blockdev_direct_IO(), and remove the code from standard
> _ext4_get_block() function. Then you can also make ext4_journal_start()
> possibly fail and still it will be clear you do not introduce any potential
> problems.
>
>> @@ -1621,7 +1621,7 @@ static int ext4_write_begin(struct file *file,
>> struct address_space *mapping,
>> ? ? ? to = from + len;
>>
>> ?retry:
>> - ? ? handle = ext4_journal_start(inode, needed_blocks);
>> + ? ? handle = ext4_journal_start(inode, needed_blocks, false);
>> ? ? ? if (IS_ERR(handle)) {
>> ? ? ? ? ? ? ? ret = PTR_ERR(handle);
>> ? ? ? ? ? ? ? goto out;
> ?Failing this with ENOMEM is OK. Note that grab_cache_page_write_begin()
> called just below can fail with ENOMEM as well.
>
>> @@ -3553,7 +3554,7 @@ static ssize_t ext4_ind_direct_IO(int rw, struct
>> kiocb *iocb,
>>
>> ? ? ? ? ? ? ? if (final_size > inode->i_size) {
>> ? ? ? ? ? ? ? ? ? ? ? /* Credits for sb + inode write */
>> - ? ? ? ? ? ? ? ? ? ? handle = ext4_journal_start(inode, 2);
>> + ? ? ? ? ? ? ? ? ? ? handle = ext4_journal_start(inode, 2, false);
>> ? ? ? ? ? ? ? ? ? ? ? if (IS_ERR(handle)) {
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ret = PTR_ERR(handle);
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? goto out;
> ?This can fail without introducing problems. It's standard directIO write
> path.
>
>> @@ -3596,7 +3597,7 @@ retry:
>> ? ? ? ? ? ? ? int err;
>>
>> ? ? ? ? ? ? ? /* Credits for sb + inode write */
>> - ? ? ? ? ? ? handle = ext4_journal_start(inode, 2);
>> + ? ? ? ? ? ? handle = ext4_journal_start(inode, 2, false);
>> ? ? ? ? ? ? ? if (IS_ERR(handle)) {
>> ? ? ? ? ? ? ? ? ? ? ? /* This is really bad luck. We've written the data
>> ? ? ? ? ? ? ? ? ? ? ? ?* but cannot extend i_size. Bail out and pretend
> ?This one can fail just fine as well.
>
>> @@ -5329,8 +5330,10 @@ int ext4_setattr(struct dentry *dentry, struct
>> iattr *attr)
>>
>> ? ? ? ? ? ? ? /* (user+group)*(old+new) structure, inode write (sb,
>> ? ? ? ? ? ? ? ?* inode block, ? - but truncate inode update has it) */
>> - ? ? ? ? ? ? handle = ext4_journal_start(inode, (EXT4_MAXQUOTAS_INIT_BLOCKS(inode->i_sb)+
>> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? EXT4_MAXQUOTAS_DEL_BLOCKS(inode->i_sb))+3);
>> + ? ? ? ? ? ? handle = ext4_journal_start(inode,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? (EXT4_MAXQUOTAS_INIT_BLOCKS(inode->i_sb)+
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? EXT4_MAXQUOTAS_DEL_BLOCKS(inode->i_sb))+3,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? true);
>> ? ? ? ? ? ? ? if (IS_ERR(handle)) {
>> ? ? ? ? ? ? ? ? ? ? ? error = PTR_ERR(handle);
>> ? ? ? ? ? ? ? ? ? ? ? goto err_out;
>> @@ -5365,7 +5368,7 @@ int ext4_setattr(struct dentry *dentry, struct
>> iattr *attr)
>> ? ? ? ? ? ?(ext4_test_inode_flag(inode, EXT4_INODE_EOFBLOCKS)))) {
>> ? ? ? ? ? ? ? handle_t *handle;
>>
>> - ? ? ? ? ? ? handle = ext4_journal_start(inode, 3);
>> + ? ? ? ? ? ? handle = ext4_journal_start(inode, 3, true);
>> ? ? ? ? ? ? ? if (IS_ERR(handle)) {
>> ? ? ? ? ? ? ? ? ? ? ? error = PTR_ERR(handle);
>> ? ? ? ? ? ? ? ? ? ? ? goto err_out;
> ?The above two sites are fine but note that err_out calls ext4_std_error()
> which we don't want to happen in case of ENOMEM.
>
>> @@ -5822,7 +5825,7 @@ int ext4_change_inode_journal_flag(struct inode
>> *inode, int val)
>>
>> ? ? ? /* Finally we can mark the inode as dirty. */
>>
>> - ? ? handle = ext4_journal_start(inode, 1);
>> + ? ? handle = ext4_journal_start(inode, 1, true);
>> ? ? ? if (IS_ERR(handle))
>> ? ? ? ? ? ? ? return PTR_ERR(handle);
> ?This can fail OK, but you should undo inode flag and aops change before
> returning error (that would be probably better as a separate preparatory
> patch because it won't be completely trivial - you need to lock the updates
> again etc. possibly create a helper function for that so that you don't
> duplicate the code).
>
>> diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
>> index eb3bc2f..3e7977b 100644
>> --- a/fs/ext4/ioctl.c
>> +++ b/fs/ext4/ioctl.c
>> @@ -101,7 +101,7 @@ long ext4_ioctl(struct file *filp, unsigned int
>> cmd, unsigned long arg)
>> ? ? ? ? ? ? ? } else if (oldflags & EXT4_EOFBLOCKS_FL)
>> ? ? ? ? ? ? ? ? ? ? ? ext4_truncate(inode);
>>
>> - ? ? ? ? ? ? handle = ext4_journal_start(inode, 1);
>> + ? ? ? ? ? ? handle = ext4_journal_start(inode, 1, false);
>> ? ? ? ? ? ? ? if (IS_ERR(handle)) {
>> ? ? ? ? ? ? ? ? ? ? ? err = PTR_ERR(handle);
>> ? ? ? ? ? ? ? ? ? ? ? goto flags_out;
> ?This can handle failure just fine...
I wasn't sure about this since this was calling ext4_truncate if the
old_flags had EXT4_EOFBLOCKS_FL. And then in ext4_truncate() had
start_transaction which was passing false.
>
>> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
>> index cb10a06..7714a15 100644
>> --- a/fs/ext4/super.c
>> +++ b/fs/ext4/super.c
>> @@ -4487,7 +4488,7 @@ static int ext4_acquire_dquot(struct dquot *dquot)
>> ? ? ? handle_t *handle;
>>
>> ? ? ? handle = ext4_journal_start(dquot_to_inode(dquot),
>> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? EXT4_QUOTA_INIT_BLOCKS(dquot->dq_sb));
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? EXT4_QUOTA_INIT_BLOCKS(dquot->dq_sb), true);
>> ? ? ? if (IS_ERR(handle))
>> ? ? ? ? ? ? ? return PTR_ERR(handle);
>> ? ? ? ret = dquot_acquire(dquot);
>> @@ -4503,7 +4504,7 @@ static int ext4_release_dquot(struct dquot *dquot)
>> ? ? ? handle_t *handle;
>>
>> ? ? ? handle = ext4_journal_start(dquot_to_inode(dquot),
>> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? EXT4_QUOTA_DEL_BLOCKS(dquot->dq_sb));
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? EXT4_QUOTA_DEL_BLOCKS(dquot->dq_sb), true);
>> ? ? ? if (IS_ERR(handle)) {
>> ? ? ? ? ? ? ? /* Release dquot anyway to avoid endless cycle in dqput() */
>> ? ? ? ? ? ? ? dquot_release(dquot);
> ?For now put 'false' in these quota functions. Because failure here
> results in a failure of dquot_initialize() which is not tested in most
> places and thus results in quota miscomputations... Properly handling this
> would require another set of cleanups.
>
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?Honza
> --
> Jan Kara <[email protected]>
> SUSE Labs, CR
>
--
Thanks -
Manish
Hi Manish,
I might be very helpful to also improve commit description, because as
it is it's very confusing and shallow, at least for me.
Thanks!
-Lukas
On Tue, 25 Jan 2011, Manish Katiyar wrote:
> Thanks a lot Jan, I will have a look at the functions you mentioned
> and send an updated version.
>
> On Tue, Jan 25, 2011 at 8:17 AM, Jan Kara <[email protected]> wrote:
> > ?Hi,
> >
> > On Sat 22-01-11 19:34:55, Manish Katiyar wrote:
> >> Pass GFP_KERNEL for transaction allocation for ext4 routines if caller
> >> can handler failures
> > ?Some error recovery paths will need cleaning up before you actually start
> > using them - see below:
> >
> >> diff --git a/fs/ext4/acl.c b/fs/ext4/acl.c
> >> index e0270d1..1a4a944 100644
> >> --- a/fs/ext4/acl.c
> >> +++ b/fs/ext4/acl.c
> >> @@ -351,7 +351,7 @@ ext4_acl_chmod(struct inode *inode)
> >>
> >> ? ? ? retry:
> >> ? ? ? ? ? ? ? handle = ext4_journal_start(inode,
> >> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? EXT4_DATA_TRANS_BLOCKS(inode->i_sb));
> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? EXT4_DATA_TRANS_BLOCKS(inode->i_sb), true);
> >> ? ? ? ? ? ? ? if (IS_ERR(handle)) {
> >> ? ? ? ? ? ? ? ? ? ? ? error = PTR_ERR(handle);
> >> ? ? ? ? ? ? ? ? ? ? ? ext4_std_error(inode->i_sb, error);
> > ?We shouldn't call ext4_std_error() (that possibly logs the message in
> > the kernel log and remounts the fs read-only, panics the kernel or so) in
> > case of ENOMEM...
> >
> >> @@ -449,7 +449,8 @@ ext4_xattr_set_acl(struct dentry *dentry, const
> >> char *name, const void *value,
> >> ? ? ? ? ? ? ? acl = NULL;
> >>
> >> ?retry:
> >> - ? ? handle = ext4_journal_start(inode, EXT4_DATA_TRANS_BLOCKS(inode->i_sb));
> >> + ? ? handle = ext4_journal_start(inode,
> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? EXT4_DATA_TRANS_BLOCKS(inode->i_sb), true);
> >> ? ? ? if (IS_ERR(handle))
> >> ? ? ? ? ? ? ? return PTR_ERR(handle);
> > ?It's actually not your bug, but the above should be:
> > ?error = PTR_ERR(handle);
> > ?goto release_and_out;
> >
> >> ? ? ? error = ext4_set_acl(handle, inode, type, acl);
> >> diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
> >> index eb9097a..e0e27a3 100644
> >> --- a/fs/ext4/ialloc.c
> >> +++ b/fs/ext4/ialloc.c
> >> @@ -1257,7 +1257,7 @@ extern int ext4_init_inode_table(struct
> >> super_block *sb, ext4_group_t group,
> >> ? ? ? if (gdp->bg_flags & cpu_to_le16(EXT4_BG_INODE_ZEROED))
> >> ? ? ? ? ? ? ? goto out;
> >>
> >> - ? ? handle = ext4_journal_start_sb(sb, 1);
> >> + ? ? handle = ext4_journal_start_sb(sb, 1, true);
> >> ? ? ? if (IS_ERR(handle)) {
> >> ? ? ? ? ? ? ? ret = PTR_ERR(handle);
> >> ? ? ? ? ? ? ? goto out;
> > ?Well, this might be disputable. This function is used to lazily
> > initialize inode table. ?If the initialization fails, thread removes the
> > request for initialization from the queue. But in case of ENOMEM, it might
> > be more suitable to just postpone the initialization work to a more
> > suitable time...
> >
> >> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> >> index 9f7f9e4..76c20b8 100644
> >> --- a/fs/ext4/inode.c
> >> +++ b/fs/ext4/inode.c
> > ...
> >> @@ -1398,7 +1398,7 @@ static int _ext4_get_block(struct inode *inode,
> >> sector_t iblock,
> >> ? ? ? ? ? ? ? if (map.m_len > DIO_MAX_BLOCKS)
> >> ? ? ? ? ? ? ? ? ? ? ? map.m_len = DIO_MAX_BLOCKS;
> >> ? ? ? ? ? ? ? dio_credits = ext4_chunk_trans_blocks(inode, map.m_len);
> >> - ? ? ? ? ? ? handle = ext4_journal_start(inode, dio_credits);
> >> + ? ? ? ? ? ? handle = ext4_journal_start(inode, dio_credits, false);
> >> ? ? ? ? ? ? ? if (IS_ERR(handle)) {
> >> ? ? ? ? ? ? ? ? ? ? ? ret = PTR_ERR(handle);
> >> ? ? ? ? ? ? ? ? ? ? ? return ret;
> > ?Hmm, this would be actually another useful prerequisite cleanup of this
> > series. _ext4_get_block() should need to start a transaction only when
> > called from direct IO path (otherwise transaction should be already started
> > when creating blocks). But this is only implicit so it would be good to
> > create ext4_get_block_directIO() which would start a transaction, use it
> > as a callback of __blockdev_direct_IO(), and remove the code from standard
> > _ext4_get_block() function. Then you can also make ext4_journal_start()
> > possibly fail and still it will be clear you do not introduce any potential
> > problems.
> >
> >> @@ -1621,7 +1621,7 @@ static int ext4_write_begin(struct file *file,
> >> struct address_space *mapping,
> >> ? ? ? to = from + len;
> >>
> >> ?retry:
> >> - ? ? handle = ext4_journal_start(inode, needed_blocks);
> >> + ? ? handle = ext4_journal_start(inode, needed_blocks, false);
> >> ? ? ? if (IS_ERR(handle)) {
> >> ? ? ? ? ? ? ? ret = PTR_ERR(handle);
> >> ? ? ? ? ? ? ? goto out;
> > ?Failing this with ENOMEM is OK. Note that grab_cache_page_write_begin()
> > called just below can fail with ENOMEM as well.
> >
> >> @@ -3553,7 +3554,7 @@ static ssize_t ext4_ind_direct_IO(int rw, struct
> >> kiocb *iocb,
> >>
> >> ? ? ? ? ? ? ? if (final_size > inode->i_size) {
> >> ? ? ? ? ? ? ? ? ? ? ? /* Credits for sb + inode write */
> >> - ? ? ? ? ? ? ? ? ? ? handle = ext4_journal_start(inode, 2);
> >> + ? ? ? ? ? ? ? ? ? ? handle = ext4_journal_start(inode, 2, false);
> >> ? ? ? ? ? ? ? ? ? ? ? if (IS_ERR(handle)) {
> >> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ret = PTR_ERR(handle);
> >> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? goto out;
> > ?This can fail without introducing problems. It's standard directIO write
> > path.
> >
> >> @@ -3596,7 +3597,7 @@ retry:
> >> ? ? ? ? ? ? ? int err;
> >>
> >> ? ? ? ? ? ? ? /* Credits for sb + inode write */
> >> - ? ? ? ? ? ? handle = ext4_journal_start(inode, 2);
> >> + ? ? ? ? ? ? handle = ext4_journal_start(inode, 2, false);
> >> ? ? ? ? ? ? ? if (IS_ERR(handle)) {
> >> ? ? ? ? ? ? ? ? ? ? ? /* This is really bad luck. We've written the data
> >> ? ? ? ? ? ? ? ? ? ? ? ?* but cannot extend i_size. Bail out and pretend
> > ?This one can fail just fine as well.
> >
> >> @@ -5329,8 +5330,10 @@ int ext4_setattr(struct dentry *dentry, struct
> >> iattr *attr)
> >>
> >> ? ? ? ? ? ? ? /* (user+group)*(old+new) structure, inode write (sb,
> >> ? ? ? ? ? ? ? ?* inode block, ? - but truncate inode update has it) */
> >> - ? ? ? ? ? ? handle = ext4_journal_start(inode, (EXT4_MAXQUOTAS_INIT_BLOCKS(inode->i_sb)+
> >> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? EXT4_MAXQUOTAS_DEL_BLOCKS(inode->i_sb))+3);
> >> + ? ? ? ? ? ? handle = ext4_journal_start(inode,
> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? (EXT4_MAXQUOTAS_INIT_BLOCKS(inode->i_sb)+
> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? EXT4_MAXQUOTAS_DEL_BLOCKS(inode->i_sb))+3,
> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? true);
> >> ? ? ? ? ? ? ? if (IS_ERR(handle)) {
> >> ? ? ? ? ? ? ? ? ? ? ? error = PTR_ERR(handle);
> >> ? ? ? ? ? ? ? ? ? ? ? goto err_out;
> >> @@ -5365,7 +5368,7 @@ int ext4_setattr(struct dentry *dentry, struct
> >> iattr *attr)
> >> ? ? ? ? ? ?(ext4_test_inode_flag(inode, EXT4_INODE_EOFBLOCKS)))) {
> >> ? ? ? ? ? ? ? handle_t *handle;
> >>
> >> - ? ? ? ? ? ? handle = ext4_journal_start(inode, 3);
> >> + ? ? ? ? ? ? handle = ext4_journal_start(inode, 3, true);
> >> ? ? ? ? ? ? ? if (IS_ERR(handle)) {
> >> ? ? ? ? ? ? ? ? ? ? ? error = PTR_ERR(handle);
> >> ? ? ? ? ? ? ? ? ? ? ? goto err_out;
> > ?The above two sites are fine but note that err_out calls ext4_std_error()
> > which we don't want to happen in case of ENOMEM.
> >
> >> @@ -5822,7 +5825,7 @@ int ext4_change_inode_journal_flag(struct inode
> >> *inode, int val)
> >>
> >> ? ? ? /* Finally we can mark the inode as dirty. */
> >>
> >> - ? ? handle = ext4_journal_start(inode, 1);
> >> + ? ? handle = ext4_journal_start(inode, 1, true);
> >> ? ? ? if (IS_ERR(handle))
> >> ? ? ? ? ? ? ? return PTR_ERR(handle);
> > ?This can fail OK, but you should undo inode flag and aops change before
> > returning error (that would be probably better as a separate preparatory
> > patch because it won't be completely trivial - you need to lock the updates
> > again etc. possibly create a helper function for that so that you don't
> > duplicate the code).
> >
> >> diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
> >> index eb3bc2f..3e7977b 100644
> >> --- a/fs/ext4/ioctl.c
> >> +++ b/fs/ext4/ioctl.c
> >> @@ -101,7 +101,7 @@ long ext4_ioctl(struct file *filp, unsigned int
> >> cmd, unsigned long arg)
> >> ? ? ? ? ? ? ? } else if (oldflags & EXT4_EOFBLOCKS_FL)
> >> ? ? ? ? ? ? ? ? ? ? ? ext4_truncate(inode);
> >>
> >> - ? ? ? ? ? ? handle = ext4_journal_start(inode, 1);
> >> + ? ? ? ? ? ? handle = ext4_journal_start(inode, 1, false);
> >> ? ? ? ? ? ? ? if (IS_ERR(handle)) {
> >> ? ? ? ? ? ? ? ? ? ? ? err = PTR_ERR(handle);
> >> ? ? ? ? ? ? ? ? ? ? ? goto flags_out;
> > ?This can handle failure just fine...
>
> I wasn't sure about this since this was calling ext4_truncate if the
> old_flags had EXT4_EOFBLOCKS_FL. And then in ext4_truncate() had
> start_transaction which was passing false.
>
>
> >
> >> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> >> index cb10a06..7714a15 100644
> >> --- a/fs/ext4/super.c
> >> +++ b/fs/ext4/super.c
> >> @@ -4487,7 +4488,7 @@ static int ext4_acquire_dquot(struct dquot *dquot)
> >> ? ? ? handle_t *handle;
> >>
> >> ? ? ? handle = ext4_journal_start(dquot_to_inode(dquot),
> >> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? EXT4_QUOTA_INIT_BLOCKS(dquot->dq_sb));
> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? EXT4_QUOTA_INIT_BLOCKS(dquot->dq_sb), true);
> >> ? ? ? if (IS_ERR(handle))
> >> ? ? ? ? ? ? ? return PTR_ERR(handle);
> >> ? ? ? ret = dquot_acquire(dquot);
> >> @@ -4503,7 +4504,7 @@ static int ext4_release_dquot(struct dquot *dquot)
> >> ? ? ? handle_t *handle;
> >>
> >> ? ? ? handle = ext4_journal_start(dquot_to_inode(dquot),
> >> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? EXT4_QUOTA_DEL_BLOCKS(dquot->dq_sb));
> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? EXT4_QUOTA_DEL_BLOCKS(dquot->dq_sb), true);
> >> ? ? ? if (IS_ERR(handle)) {
> >> ? ? ? ? ? ? ? /* Release dquot anyway to avoid endless cycle in dqput() */
> >> ? ? ? ? ? ? ? dquot_release(dquot);
> > ?For now put 'false' in these quota functions. Because failure here
> > results in a failure of dquot_initialize() which is not tested in most
> > places and thus results in quota miscomputations... Properly handling this
> > would require another set of cleanups.
> >
> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?Honza
> > --
> > Jan Kara <[email protected]>
> > SUSE Labs, CR
> >
>
>
>
>
--
On Tue, Jan 25, 2011 at 11:55 PM, Lukas Czerner <[email protected]> wrote:
> Hi Manish,
>
> I might be very helpful to also improve commit description, because as
> it is it's very confusing and shallow, at least for me.
Sure, will update that too when I send the updated version.
--
Thanks -
Manish
Hi Jan,
Below is the updated patch based on your feedback.
On Tue, Jan 25, 2011 at 8:17 AM, Jan Kara <[email protected]> wrote:
> ?Hi,
>
> On Sat 22-01-11 19:34:55, Manish Katiyar wrote:
>> Pass GFP_KERNEL for transaction allocation for ext4 routines if caller
>> can handler failures
> ?Some error recovery paths will need cleaning up before you actually start
> using them - see below:
>
>> diff --git a/fs/ext4/acl.c b/fs/ext4/acl.c
>> index e0270d1..1a4a944 100644
>> --- a/fs/ext4/acl.c
>> +++ b/fs/ext4/acl.c
>> @@ -351,7 +351,7 @@ ext4_acl_chmod(struct inode *inode)
>>
>> ? ? ? retry:
>> ? ? ? ? ? ? ? handle = ext4_journal_start(inode,
>> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? EXT4_DATA_TRANS_BLOCKS(inode->i_sb));
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? EXT4_DATA_TRANS_BLOCKS(inode->i_sb), true);
>> ? ? ? ? ? ? ? if (IS_ERR(handle)) {
>> ? ? ? ? ? ? ? ? ? ? ? error = PTR_ERR(handle);
>> ? ? ? ? ? ? ? ? ? ? ? ext4_std_error(inode->i_sb, error);
> ?We shouldn't call ext4_std_error() (that possibly logs the message in
> the kernel log and remounts the fs read-only, panics the kernel or so) in
> case of ENOMEM...
>
>> @@ -449,7 +449,8 @@ ext4_xattr_set_acl(struct dentry *dentry, const
>> char *name, const void *value,
>> ? ? ? ? ? ? ? acl = NULL;
>>
>> ?retry:
>> - ? ? handle = ext4_journal_start(inode, EXT4_DATA_TRANS_BLOCKS(inode->i_sb));
>> + ? ? handle = ext4_journal_start(inode,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? EXT4_DATA_TRANS_BLOCKS(inode->i_sb), true);
>> ? ? ? if (IS_ERR(handle))
>> ? ? ? ? ? ? ? return PTR_ERR(handle);
> ?It's actually not your bug, but the above should be:
> ?error = PTR_ERR(handle);
> ?goto release_and_out;
>
>> ? ? ? error = ext4_set_acl(handle, inode, type, acl);
>> diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
>> index eb9097a..e0e27a3 100644
>> --- a/fs/ext4/ialloc.c
>> +++ b/fs/ext4/ialloc.c
>> @@ -1257,7 +1257,7 @@ extern int ext4_init_inode_table(struct
>> super_block *sb, ext4_group_t group,
>> ? ? ? if (gdp->bg_flags & cpu_to_le16(EXT4_BG_INODE_ZEROED))
>> ? ? ? ? ? ? ? goto out;
>>
>> - ? ? handle = ext4_journal_start_sb(sb, 1);
>> + ? ? handle = ext4_journal_start_sb(sb, 1, true);
>> ? ? ? if (IS_ERR(handle)) {
>> ? ? ? ? ? ? ? ret = PTR_ERR(handle);
>> ? ? ? ? ? ? ? goto out;
> ?Well, this might be disputable. This function is used to lazily
> initialize inode table. ?If the initialization fails, thread removes the
> request for initialization from the queue. But in case of ENOMEM, it might
> be more suitable to just postpone the initialization work to a more
> suitable time...
I have converted it back to pass false. Will send a separate patch later.
>
>> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
>> index 9f7f9e4..76c20b8 100644
>> --- a/fs/ext4/inode.c
>> +++ b/fs/ext4/inode.c
> ...
>> @@ -1398,7 +1398,7 @@ static int _ext4_get_block(struct inode *inode,
>> sector_t iblock,
>> ? ? ? ? ? ? ? if (map.m_len > DIO_MAX_BLOCKS)
>> ? ? ? ? ? ? ? ? ? ? ? map.m_len = DIO_MAX_BLOCKS;
>> ? ? ? ? ? ? ? dio_credits = ext4_chunk_trans_blocks(inode, map.m_len);
>> - ? ? ? ? ? ? handle = ext4_journal_start(inode, dio_credits);
>> + ? ? ? ? ? ? handle = ext4_journal_start(inode, dio_credits, false);
>> ? ? ? ? ? ? ? if (IS_ERR(handle)) {
>> ? ? ? ? ? ? ? ? ? ? ? ret = PTR_ERR(handle);
>> ? ? ? ? ? ? ? ? ? ? ? return ret;
> ?Hmm, this would be actually another useful prerequisite cleanup of this
> series. _ext4_get_block() should need to start a transaction only when
> called from direct IO path (otherwise transaction should be already started
> when creating blocks). But this is only implicit so it would be good to
> create ext4_get_block_directIO() which would start a transaction, use it
> as a callback of __blockdev_direct_IO(), and remove the code from standard
> _ext4_get_block() function. Then you can also make ext4_journal_start()
> possibly fail and still it will be clear you do not introduce any potential
> problems.
>
>> @@ -1621,7 +1621,7 @@ static int ext4_write_begin(struct file *file,
>> struct address_space *mapping,
>> ? ? ? to = from + len;
>>
>> ?retry:
>> - ? ? handle = ext4_journal_start(inode, needed_blocks);
>> + ? ? handle = ext4_journal_start(inode, needed_blocks, false);
>> ? ? ? if (IS_ERR(handle)) {
>> ? ? ? ? ? ? ? ret = PTR_ERR(handle);
>> ? ? ? ? ? ? ? goto out;
> ?Failing this with ENOMEM is OK. Note that grab_cache_page_write_begin()
> called just below can fail with ENOMEM as well.
Using the same reasoning, I also changed ext4_da_write_begin().
>
>> @@ -3553,7 +3554,7 @@ static ssize_t ext4_ind_direct_IO(int rw, struct
>> kiocb *iocb,
>>
>> ? ? ? ? ? ? ? if (final_size > inode->i_size) {
>> ? ? ? ? ? ? ? ? ? ? ? /* Credits for sb + inode write */
>> - ? ? ? ? ? ? ? ? ? ? handle = ext4_journal_start(inode, 2);
>> + ? ? ? ? ? ? ? ? ? ? handle = ext4_journal_start(inode, 2, false);
>> ? ? ? ? ? ? ? ? ? ? ? if (IS_ERR(handle)) {
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ret = PTR_ERR(handle);
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? goto out;
> ?This can fail without introducing problems. It's standard directIO write
> path.
>
>> @@ -3596,7 +3597,7 @@ retry:
>> ? ? ? ? ? ? ? int err;
>>
>> ? ? ? ? ? ? ? /* Credits for sb + inode write */
>> - ? ? ? ? ? ? handle = ext4_journal_start(inode, 2);
>> + ? ? ? ? ? ? handle = ext4_journal_start(inode, 2, false);
>> ? ? ? ? ? ? ? if (IS_ERR(handle)) {
>> ? ? ? ? ? ? ? ? ? ? ? /* This is really bad luck. We've written the data
>> ? ? ? ? ? ? ? ? ? ? ? ?* but cannot extend i_size. Bail out and pretend
> ?This one can fail just fine as well.
>
>> @@ -5329,8 +5330,10 @@ int ext4_setattr(struct dentry *dentry, struct
>> iattr *attr)
>>
>> ? ? ? ? ? ? ? /* (user+group)*(old+new) structure, inode write (sb,
>> ? ? ? ? ? ? ? ?* inode block, ? - but truncate inode update has it) */
>> - ? ? ? ? ? ? handle = ext4_journal_start(inode, (EXT4_MAXQUOTAS_INIT_BLOCKS(inode->i_sb)+
>> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? EXT4_MAXQUOTAS_DEL_BLOCKS(inode->i_sb))+3);
>> + ? ? ? ? ? ? handle = ext4_journal_start(inode,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? (EXT4_MAXQUOTAS_INIT_BLOCKS(inode->i_sb)+
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? EXT4_MAXQUOTAS_DEL_BLOCKS(inode->i_sb))+3,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? true);
>> ? ? ? ? ? ? ? if (IS_ERR(handle)) {
>> ? ? ? ? ? ? ? ? ? ? ? error = PTR_ERR(handle);
>> ? ? ? ? ? ? ? ? ? ? ? goto err_out;
>> @@ -5365,7 +5368,7 @@ int ext4_setattr(struct dentry *dentry, struct
>> iattr *attr)
>> ? ? ? ? ? ?(ext4_test_inode_flag(inode, EXT4_INODE_EOFBLOCKS)))) {
>> ? ? ? ? ? ? ? handle_t *handle;
>>
>> - ? ? ? ? ? ? handle = ext4_journal_start(inode, 3);
>> + ? ? ? ? ? ? handle = ext4_journal_start(inode, 3, true);
>> ? ? ? ? ? ? ? if (IS_ERR(handle)) {
>> ? ? ? ? ? ? ? ? ? ? ? error = PTR_ERR(handle);
>> ? ? ? ? ? ? ? ? ? ? ? goto err_out;
> ?The above two sites are fine but note that err_out calls ext4_std_error()
> which we don't want to happen in case of ENOMEM.
>
>> @@ -5822,7 +5825,7 @@ int ext4_change_inode_journal_flag(struct inode
>> *inode, int val)
>>
>> ? ? ? /* Finally we can mark the inode as dirty. */
>>
>> - ? ? handle = ext4_journal_start(inode, 1);
>> + ? ? handle = ext4_journal_start(inode, 1, true);
>> ? ? ? if (IS_ERR(handle))
>> ? ? ? ? ? ? ? return PTR_ERR(handle);
> ?This can fail OK, but you should undo inode flag and aops change before
> returning error (that would be probably better as a separate preparatory
> patch because it won't be completely trivial - you need to lock the updates
> again etc. possibly create a helper function for that so that you don't
> duplicate the code).
Changed back to pass false. Will do the cleanup and error handling as
part of separate patch once I start understanding the code better.
For ext4 functions which can handle transaction allocation failures,
pass GFP_KERNEL flag for transaction allocation dring journal start.
Otherwise GFP_NOFS is passed which retries in a loop till allocation
succeeds.
Signed-off-by: Manish Katiyar <[email protected]>
---
fs/ext4/acl.c | 12 +++++++-----
fs/ext4/ext4_jbd2.h | 9 +++++----
fs/ext4/extents.c | 8 ++++----
fs/ext4/ialloc.c | 2 +-
fs/ext4/inode.c | 37 +++++++++++++++++++++----------------
fs/ext4/ioctl.c | 4 ++--
fs/ext4/migrate.c | 4 ++--
fs/ext4/move_extent.c | 2 +-
fs/ext4/namei.c | 24 +++++++++++++++---------
fs/ext4/resize.c | 8 ++++----
fs/ext4/super.c | 13 +++++++------
fs/ext4/xattr.c | 3 ++-
12 files changed, 71 insertions(+), 55 deletions(-)
diff --git a/fs/ext4/acl.c b/fs/ext4/acl.c
index e0270d1..0f7aac2 100644
--- a/fs/ext4/acl.c
+++ b/fs/ext4/acl.c
@@ -351,10 +351,9 @@ ext4_acl_chmod(struct inode *inode)
retry:
handle = ext4_journal_start(inode,
- EXT4_DATA_TRANS_BLOCKS(inode->i_sb));
+ EXT4_DATA_TRANS_BLOCKS(inode->i_sb), true);
if (IS_ERR(handle)) {
error = PTR_ERR(handle);
- ext4_std_error(inode->i_sb, error);
goto out;
}
error = ext4_set_acl(handle, inode, ACL_TYPE_ACCESS, clone);
@@ -449,9 +448,12 @@ ext4_xattr_set_acl(struct dentry *dentry, const
char *name, const void *value,
acl = NULL;
retry:
- handle = ext4_journal_start(inode, EXT4_DATA_TRANS_BLOCKS(inode->i_sb));
- if (IS_ERR(handle))
- return PTR_ERR(handle);
+ handle = ext4_journal_start(inode,
+ EXT4_DATA_TRANS_BLOCKS(inode->i_sb), true);
+ if (IS_ERR(handle)) {
+ error = PTR_ERR(handle);
+ goto release_and_out;
+ }
error = ext4_set_acl(handle, inode, type, acl);
ext4_journal_stop(handle);
if (error == -ENOSPC && ext4_should_retry_alloc(inode->i_sb, &retries))
diff --git a/fs/ext4/ext4_jbd2.h b/fs/ext4/ext4_jbd2.h
index d8b992e..38f128e 100644
--- a/fs/ext4/ext4_jbd2.h
+++ b/fs/ext4/ext4_jbd2.h
@@ -161,7 +161,7 @@ int __ext4_handle_dirty_super(const char *where,
unsigned int line,
#define ext4_handle_dirty_super(handle, sb) \
__ext4_handle_dirty_super(__func__, __LINE__, (handle), (sb))
-handle_t *ext4_journal_start_sb(struct super_block *sb, int nblocks);
+handle_t *ext4_journal_start_sb(struct super_block *sb, int nblocks,
bool errok);
int __ext4_journal_stop(const char *where, unsigned int line,
handle_t *handle);
#define EXT4_NOJOURNAL_MAX_REF_COUNT ((unsigned long) 4096)
@@ -209,9 +209,10 @@ static inline void
ext4_journal_release_buffer(handle_t *handle,
jbd2_journal_release_buffer(handle, bh);
}
-static inline handle_t *ext4_journal_start(struct inode *inode, int nblocks)
+static inline handle_t *ext4_journal_start(struct inode *inode,
+ int nblocks, bool errok)
{
- return ext4_journal_start_sb(inode->i_sb, nblocks);
+ return ext4_journal_start_sb(inode->i_sb, nblocks, errok);
}
#define ext4_journal_stop(handle) \
@@ -232,7 +233,7 @@ static inline int ext4_journal_extend(handle_t
*handle, int nblocks)
static inline int ext4_journal_restart(handle_t *handle, int nblocks)
{
if (ext4_handle_valid(handle))
- return jbd2_journal_restart(handle, nblocks);
+ return jbd2_journal_restart(handle, nblocks, false);
return 0;
}
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 63a7581..5944b1c 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -2333,7 +2333,7 @@ static int ext4_ext_remove_space(struct inode
*inode, ext4_lblk_t start)
ext_debug("truncate since %u\n", start);
/* probably first extent we're gonna free will be last in block */
- handle = ext4_journal_start(inode, depth + 1);
+ handle = ext4_journal_start(inode, depth + 1, false);
if (IS_ERR(handle))
return PTR_ERR(handle);
@@ -3544,7 +3544,7 @@ void ext4_ext_truncate(struct inode *inode)
* probably first extent we're gonna free will be last in block
*/
err = ext4_writepage_trans_blocks(inode);
- handle = ext4_journal_start(inode, err);
+ handle = ext4_journal_start(inode, err, false);
if (IS_ERR(handle))
return;
@@ -3677,7 +3677,7 @@ retry:
while (ret >= 0 && ret < max_blocks) {
map.m_lblk = map.m_lblk + ret;
map.m_len = max_blocks = max_blocks - ret;
- handle = ext4_journal_start(inode, credits);
+ handle = ext4_journal_start(inode, credits, true);
if (IS_ERR(handle)) {
ret = PTR_ERR(handle);
break;
@@ -3752,7 +3752,7 @@ int ext4_convert_unwritten_extents(struct inode
*inode, loff_t offset,
while (ret >= 0 && ret < max_blocks) {
map.m_lblk += ret;
map.m_len = (max_blocks -= ret);
- handle = ext4_journal_start(inode, credits);
+ handle = ext4_journal_start(inode, credits, false);
if (IS_ERR(handle)) {
ret = PTR_ERR(handle);
break;
diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
index eb9097a..4499dcd 100644
--- a/fs/ext4/ialloc.c
+++ b/fs/ext4/ialloc.c
@@ -1257,7 +1257,7 @@ extern int ext4_init_inode_table(struct
super_block *sb, ext4_group_t group,
if (gdp->bg_flags & cpu_to_le16(EXT4_BG_INODE_ZEROED))
goto out;
- handle = ext4_journal_start_sb(sb, 1);
+ handle = ext4_journal_start_sb(sb, 1, false);
if (IS_ERR(handle)) {
ret = PTR_ERR(handle);
goto out;
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 9f7f9e4..12a9b74 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -129,7 +129,7 @@ static handle_t *start_transaction(struct inode *inode)
{
handle_t *result;
- result = ext4_journal_start(inode, blocks_for_truncate(inode));
+ result = ext4_journal_start(inode, blocks_for_truncate(inode), false);
if (!IS_ERR(result))
return result;
@@ -204,7 +204,7 @@ void ext4_evict_inode(struct inode *inode)
if (is_bad_inode(inode))
goto no_delete;
- handle = ext4_journal_start(inode, blocks_for_truncate(inode)+3);
+ handle = ext4_journal_start(inode, blocks_for_truncate(inode)+3, false);
if (IS_ERR(handle)) {
ext4_std_error(inode->i_sb, PTR_ERR(handle));
/*
@@ -1398,7 +1398,7 @@ static int _ext4_get_block(struct inode *inode,
sector_t iblock,
if (map.m_len > DIO_MAX_BLOCKS)
map.m_len = DIO_MAX_BLOCKS;
dio_credits = ext4_chunk_trans_blocks(inode, map.m_len);
- handle = ext4_journal_start(inode, dio_credits);
+ handle = ext4_journal_start(inode, dio_credits, false);
if (IS_ERR(handle)) {
ret = PTR_ERR(handle);
return ret;
@@ -1621,7 +1621,7 @@ static int ext4_write_begin(struct file *file,
struct address_space *mapping,
to = from + len;
retry:
- handle = ext4_journal_start(inode, needed_blocks);
+ handle = ext4_journal_start(inode, needed_blocks, true);
if (IS_ERR(handle)) {
ret = PTR_ERR(handle);
goto out;
@@ -2653,7 +2653,8 @@ static int __ext4_journalled_writepage(struct page *page,
* references to buffers so we are safe */
unlock_page(page);
- handle = ext4_journal_start(inode, ext4_writepage_trans_blocks(inode));
+ handle = ext4_journal_start(inode, ext4_writepage_trans_blocks(inode),
+ false);
if (IS_ERR(handle)) {
ret = PTR_ERR(handle);
goto out;
@@ -3049,7 +3050,7 @@ retry:
needed_blocks = ext4_da_writepages_trans_blocks(inode);
/* start a new transaction*/
- handle = ext4_journal_start(inode, needed_blocks);
+ handle = ext4_journal_start(inode, needed_blocks, false);
if (IS_ERR(handle)) {
ret = PTR_ERR(handle);
ext4_msg(inode->i_sb, KERN_CRIT, "%s: jbd2_start: "
@@ -3204,7 +3205,7 @@ retry:
* to journalling the i_disksize update if writes to the end
* of file which has an already mapped buffer.
*/
- handle = ext4_journal_start(inode, 1);
+ handle = ext4_journal_start(inode, 1, true);
if (IS_ERR(handle)) {
ret = PTR_ERR(handle);
goto out;
@@ -3553,7 +3554,7 @@ static ssize_t ext4_ind_direct_IO(int rw, struct
kiocb *iocb,
if (final_size > inode->i_size) {
/* Credits for sb + inode write */
- handle = ext4_journal_start(inode, 2);
+ handle = ext4_journal_start(inode, 2, true);
if (IS_ERR(handle)) {
ret = PTR_ERR(handle);
goto out;
@@ -3596,7 +3597,7 @@ retry:
int err;
/* Credits for sb + inode write */
- handle = ext4_journal_start(inode, 2);
+ handle = ext4_journal_start(inode, 2, true);
if (IS_ERR(handle)) {
/* This is really bad luck. We've written the data
* but cannot extend i_size. Bail out and pretend
@@ -5329,8 +5330,10 @@ int ext4_setattr(struct dentry *dentry, struct
iattr *attr)
/* (user+group)*(old+new) structure, inode write (sb,
* inode block, ? - but truncate inode update has it) */
- handle = ext4_journal_start(inode, (EXT4_MAXQUOTAS_INIT_BLOCKS(inode->i_sb)+
- EXT4_MAXQUOTAS_DEL_BLOCKS(inode->i_sb))+3);
+ handle = ext4_journal_start(inode,
+ (EXT4_MAXQUOTAS_INIT_BLOCKS(inode->i_sb)+
+ EXT4_MAXQUOTAS_DEL_BLOCKS(inode->i_sb))+3,
+ true);
if (IS_ERR(handle)) {
error = PTR_ERR(handle);
goto err_out;
@@ -5365,7 +5368,7 @@ int ext4_setattr(struct dentry *dentry, struct
iattr *attr)
(ext4_test_inode_flag(inode, EXT4_INODE_EOFBLOCKS)))) {
handle_t *handle;
- handle = ext4_journal_start(inode, 3);
+ handle = ext4_journal_start(inode, 3, true);
if (IS_ERR(handle)) {
error = PTR_ERR(handle);
goto err_out;
@@ -5385,7 +5388,7 @@ int ext4_setattr(struct dentry *dentry, struct
iattr *attr)
attr->ia_size);
if (error) {
/* Do as much error cleanup as possible */
- handle = ext4_journal_start(inode, 3);
+ handle = ext4_journal_start(inode, 3, false);
if (IS_ERR(handle)) {
ext4_orphan_del(NULL, inode);
goto err_out;
@@ -5421,7 +5424,9 @@ int ext4_setattr(struct dentry *dentry, struct
iattr *attr)
rc = ext4_acl_chmod(inode);
err_out:
- ext4_std_error(inode->i_sb, error);
+ if (error != -ENOMEM) {
+ ext4_std_error(inode->i_sb, error);
+ }
if (!error)
error = rc;
return error;
@@ -5738,7 +5743,7 @@ void ext4_dirty_inode(struct inode *inode)
{
handle_t *handle;
- handle = ext4_journal_start(inode, 2);
+ handle = ext4_journal_start(inode, 2, false);
if (IS_ERR(handle))
goto out;
@@ -5822,7 +5827,7 @@ int ext4_change_inode_journal_flag(struct inode
*inode, int val)
/* Finally we can mark the inode as dirty. */
- handle = ext4_journal_start(inode, 1);
+ handle = ext4_journal_start(inode, 1, false);
if (IS_ERR(handle))
return PTR_ERR(handle);
diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
index eb3bc2f..a357b9a 100644
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@ -101,7 +101,7 @@ long ext4_ioctl(struct file *filp, unsigned int
cmd, unsigned long arg)
} else if (oldflags & EXT4_EOFBLOCKS_FL)
ext4_truncate(inode);
- handle = ext4_journal_start(inode, 1);
+ handle = ext4_journal_start(inode, 1, true);
if (IS_ERR(handle)) {
err = PTR_ERR(handle);
goto flags_out;
@@ -157,7 +157,7 @@ flags_out:
goto setversion_out;
}
- handle = ext4_journal_start(inode, 1);
+ handle = ext4_journal_start(inode, 1, true);
if (IS_ERR(handle)) {
err = PTR_ERR(handle);
goto setversion_out;
diff --git a/fs/ext4/migrate.c b/fs/ext4/migrate.c
index b0a126f..729dcbc 100644
--- a/fs/ext4/migrate.c
+++ b/fs/ext4/migrate.c
@@ -488,7 +488,7 @@ int ext4_ext_migrate(struct inode *inode)
EXT4_DATA_TRANS_BLOCKS(inode->i_sb) +
EXT4_INDEX_EXTRA_TRANS_BLOCKS + 3 +
EXT4_MAXQUOTAS_INIT_BLOCKS(inode->i_sb)
- + 1);
+ + 1, true);
if (IS_ERR(handle)) {
retval = PTR_ERR(handle);
return retval;
@@ -533,7 +533,7 @@ int ext4_ext_migrate(struct inode *inode)
ext4_set_inode_state(inode, EXT4_STATE_EXT_MIGRATE);
up_read((&EXT4_I(inode)->i_data_sem));
- handle = ext4_journal_start(inode, 1);
+ handle = ext4_journal_start(inode, 1, true);
if (IS_ERR(handle)) {
/*
* It is impossible to update on-disk structures without
diff --git a/fs/ext4/move_extent.c b/fs/ext4/move_extent.c
index b9f3e78..d5aad4d 100644
--- a/fs/ext4/move_extent.c
+++ b/fs/ext4/move_extent.c
@@ -813,7 +813,7 @@ move_extent_per_page(struct file *o_filp, struct
inode *donor_inode,
* inode and donor_inode may change each different metadata blocks.
*/
jblocks = ext4_writepage_trans_blocks(orig_inode) * 2;
- handle = ext4_journal_start(orig_inode, jblocks);
+ handle = ext4_journal_start(orig_inode, jblocks, true);
if (IS_ERR(handle)) {
*err = PTR_ERR(handle);
return 0;
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index 5485390..a3ad11f 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -1739,7 +1739,8 @@ static int ext4_create(struct inode *dir, struct
dentry *dentry, int mode,
retry:
handle = ext4_journal_start(dir, EXT4_DATA_TRANS_BLOCKS(dir->i_sb) +
EXT4_INDEX_EXTRA_TRANS_BLOCKS + 3 +
- EXT4_MAXQUOTAS_INIT_BLOCKS(dir->i_sb));
+ EXT4_MAXQUOTAS_INIT_BLOCKS(dir->i_sb),
+ true);
if (IS_ERR(handle))
return PTR_ERR(handle);
@@ -1775,7 +1776,8 @@ static int ext4_mknod(struct inode *dir, struct
dentry *dentry,
retry:
handle = ext4_journal_start(dir, EXT4_DATA_TRANS_BLOCKS(dir->i_sb) +
EXT4_INDEX_EXTRA_TRANS_BLOCKS + 3 +
- EXT4_MAXQUOTAS_INIT_BLOCKS(dir->i_sb));
+ EXT4_MAXQUOTAS_INIT_BLOCKS(dir->i_sb),
+ true);
if (IS_ERR(handle))
return PTR_ERR(handle);
@@ -1814,7 +1816,8 @@ static int ext4_mkdir(struct inode *dir, struct
dentry *dentry, int mode)
retry:
handle = ext4_journal_start(dir, EXT4_DATA_TRANS_BLOCKS(dir->i_sb) +
EXT4_INDEX_EXTRA_TRANS_BLOCKS + 3 +
- EXT4_MAXQUOTAS_INIT_BLOCKS(dir->i_sb));
+ EXT4_MAXQUOTAS_INIT_BLOCKS(dir->i_sb),
+ true);
if (IS_ERR(handle))
return PTR_ERR(handle);
@@ -2127,7 +2130,8 @@ static int ext4_rmdir(struct inode *dir, struct
dentry *dentry)
dquot_initialize(dir);
dquot_initialize(dentry->d_inode);
- handle = ext4_journal_start(dir, EXT4_DELETE_TRANS_BLOCKS(dir->i_sb));
+ handle = ext4_journal_start(dir,
+ EXT4_DELETE_TRANS_BLOCKS(dir->i_sb), true);
if (IS_ERR(handle))
return PTR_ERR(handle);
@@ -2188,7 +2192,8 @@ static int ext4_unlink(struct inode *dir, struct
dentry *dentry)
dquot_initialize(dir);
dquot_initialize(dentry->d_inode);
- handle = ext4_journal_start(dir, EXT4_DELETE_TRANS_BLOCKS(dir->i_sb));
+ handle = ext4_journal_start(dir,
+ EXT4_DELETE_TRANS_BLOCKS(dir->i_sb), true);
if (IS_ERR(handle))
return PTR_ERR(handle);
@@ -2246,8 +2251,9 @@ static int ext4_symlink(struct inode *dir,
retry:
handle = ext4_journal_start(dir, EXT4_DATA_TRANS_BLOCKS(dir->i_sb) +
- EXT4_INDEX_EXTRA_TRANS_BLOCKS + 5 +
- EXT4_MAXQUOTAS_INIT_BLOCKS(dir->i_sb));
+ EXT4_INDEX_EXTRA_TRANS_BLOCKS + 5 +
+ EXT4_MAXQUOTAS_INIT_BLOCKS(dir->i_sb),
+ true);
if (IS_ERR(handle))
return PTR_ERR(handle);
@@ -2313,7 +2319,7 @@ static int ext4_link(struct dentry *old_dentry,
retry:
handle = ext4_journal_start(dir, EXT4_DATA_TRANS_BLOCKS(dir->i_sb) +
- EXT4_INDEX_EXTRA_TRANS_BLOCKS);
+ EXT4_INDEX_EXTRA_TRANS_BLOCKS, true);
if (IS_ERR(handle))
return PTR_ERR(handle);
@@ -2365,7 +2371,7 @@ static int ext4_rename(struct inode *old_dir,
struct dentry *old_dentry,
dquot_initialize(new_dentry->d_inode);
handle = ext4_journal_start(old_dir, 2 *
EXT4_DATA_TRANS_BLOCKS(old_dir->i_sb) +
- EXT4_INDEX_EXTRA_TRANS_BLOCKS + 2);
+ EXT4_INDEX_EXTRA_TRANS_BLOCKS + 2, true);
if (IS_ERR(handle))
return PTR_ERR(handle);
diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
index 3ecc6e4..e50d083 100644
--- a/fs/ext4/resize.c
+++ b/fs/ext4/resize.c
@@ -176,7 +176,7 @@ static int setup_new_group_blocks(struct super_block *sb,
int err = 0, err2;
/* This transaction may be extended/restarted along the way */
- handle = ext4_journal_start_sb(sb, EXT4_MAX_TRANS_DATA);
+ handle = ext4_journal_start_sb(sb, EXT4_MAX_TRANS_DATA, true);
if (IS_ERR(handle))
return PTR_ERR(handle);
@@ -655,7 +655,7 @@ static void update_backups(struct super_block *sb,
handle_t *handle;
int err = 0, err2;
- handle = ext4_journal_start_sb(sb, EXT4_MAX_TRANS_DATA);
+ handle = ext4_journal_start_sb(sb, EXT4_MAX_TRANS_DATA, true);
if (IS_ERR(handle)) {
group = 1;
err = PTR_ERR(handle);
@@ -793,7 +793,7 @@ int ext4_group_add(struct super_block *sb, struct
ext4_new_group_data *input)
*/
handle = ext4_journal_start_sb(sb,
ext4_bg_has_super(sb, input->group) ?
- 3 + reserved_gdb : 4);
+ 3 + reserved_gdb : 4, true);
if (IS_ERR(handle)) {
err = PTR_ERR(handle);
goto exit_put;
@@ -1031,7 +1031,7 @@ int ext4_group_extend(struct super_block *sb,
struct ext4_super_block *es,
/* We will update the superblock, one block bitmap, and
* one group descriptor via ext4_free_blocks().
*/
- handle = ext4_journal_start_sb(sb, 3);
+ handle = ext4_journal_start_sb(sb, 3, true);
if (IS_ERR(handle)) {
err = PTR_ERR(handle);
ext4_warning(sb, "error %d on journal start", err);
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index cb10a06..eacfea7 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -241,7 +241,7 @@ static void ext4_put_nojournal(handle_t *handle)
* that sync() will call the filesystem's write_super callback if
* appropriate.
*/
-handle_t *ext4_journal_start_sb(struct super_block *sb, int nblocks)
+handle_t *ext4_journal_start_sb(struct super_block *sb, int nblocks,
bool errok)
{
journal_t *journal;
@@ -258,7 +258,7 @@ handle_t *ext4_journal_start_sb(struct super_block
*sb, int nblocks)
ext4_abort(sb, "Detected aborted journal");
return ERR_PTR(-EROFS);
}
- return jbd2_journal_start(journal, nblocks);
+ return jbd2_journal_start(journal, nblocks, errok);
}
return ext4_get_nojournal();
}
@@ -4471,7 +4471,8 @@ static int ext4_write_dquot(struct dquot *dquot)
inode = dquot_to_inode(dquot);
handle = ext4_journal_start(inode,
- EXT4_QUOTA_TRANS_BLOCKS(dquot->dq_sb));
+ EXT4_QUOTA_TRANS_BLOCKS(dquot->dq_sb),
+ false);
if (IS_ERR(handle))
return PTR_ERR(handle);
ret = dquot_commit(dquot);
@@ -4487,7 +4488,7 @@ static int ext4_acquire_dquot(struct dquot *dquot)
handle_t *handle;
handle = ext4_journal_start(dquot_to_inode(dquot),
- EXT4_QUOTA_INIT_BLOCKS(dquot->dq_sb));
+ EXT4_QUOTA_INIT_BLOCKS(dquot->dq_sb), false);
if (IS_ERR(handle))
return PTR_ERR(handle);
ret = dquot_acquire(dquot);
@@ -4503,7 +4504,7 @@ static int ext4_release_dquot(struct dquot *dquot)
handle_t *handle;
handle = ext4_journal_start(dquot_to_inode(dquot),
- EXT4_QUOTA_DEL_BLOCKS(dquot->dq_sb));
+ EXT4_QUOTA_DEL_BLOCKS(dquot->dq_sb), false);
if (IS_ERR(handle)) {
/* Release dquot anyway to avoid endless cycle in dqput() */
dquot_release(dquot);
@@ -4534,7 +4535,7 @@ static int ext4_write_info(struct super_block
*sb, int type)
handle_t *handle;
/* Data block + inode block */
- handle = ext4_journal_start(sb->s_root->d_inode, 2);
+ handle = ext4_journal_start(sb->s_root->d_inode, 2, false);
if (IS_ERR(handle))
return PTR_ERR(handle);
ret = dquot_commit_info(sb, type);
diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index fc32176..0e39f57 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -1084,7 +1084,8 @@ ext4_xattr_set(struct inode *inode, int
name_index, const char *name,
int error, retries = 0;
retry:
- handle = ext4_journal_start(inode, EXT4_DATA_TRANS_BLOCKS(inode->i_sb));
+ handle = ext4_journal_start(inode,
+ EXT4_DATA_TRANS_BLOCKS(inode->i_sb), true);
if (IS_ERR(handle)) {
error = PTR_ERR(handle);
} else {
--
1.6.0.4
Thanks -
Manish
>
>> diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
>> index eb3bc2f..3e7977b 100644
>> --- a/fs/ext4/ioctl.c
>> +++ b/fs/ext4/ioctl.c
>> @@ -101,7 +101,7 @@ long ext4_ioctl(struct file *filp, unsigned int
>> cmd, unsigned long arg)
>> ? ? ? ? ? ? ? } else if (oldflags & EXT4_EOFBLOCKS_FL)
>> ? ? ? ? ? ? ? ? ? ? ? ext4_truncate(inode);
>>
>> - ? ? ? ? ? ? handle = ext4_journal_start(inode, 1);
>> + ? ? ? ? ? ? handle = ext4_journal_start(inode, 1, false);
>> ? ? ? ? ? ? ? if (IS_ERR(handle)) {
>> ? ? ? ? ? ? ? ? ? ? ? err = PTR_ERR(handle);
>> ? ? ? ? ? ? ? ? ? ? ? goto flags_out;
> ?This can handle failure just fine...
>
>> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
>> index cb10a06..7714a15 100644
>> --- a/fs/ext4/super.c
>> +++ b/fs/ext4/super.c
>> @@ -4487,7 +4488,7 @@ static int ext4_acquire_dquot(struct dquot *dquot)
>> ? ? ? handle_t *handle;
>>
>> ? ? ? handle = ext4_journal_start(dquot_to_inode(dquot),
>> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? EXT4_QUOTA_INIT_BLOCKS(dquot->dq_sb));
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? EXT4_QUOTA_INIT_BLOCKS(dquot->dq_sb), true);
>> ? ? ? if (IS_ERR(handle))
>> ? ? ? ? ? ? ? return PTR_ERR(handle);
>> ? ? ? ret = dquot_acquire(dquot);
>> @@ -4503,7 +4504,7 @@ static int ext4_release_dquot(struct dquot *dquot)
>> ? ? ? handle_t *handle;
>>
>> ? ? ? handle = ext4_journal_start(dquot_to_inode(dquot),
>> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? EXT4_QUOTA_DEL_BLOCKS(dquot->dq_sb));
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? EXT4_QUOTA_DEL_BLOCKS(dquot->dq_sb), true);
>> ? ? ? if (IS_ERR(handle)) {
>> ? ? ? ? ? ? ? /* Release dquot anyway to avoid endless cycle in dqput() */
>> ? ? ? ? ? ? ? dquot_release(dquot);
> ?For now put 'false' in these quota functions. Because failure here
> results in a failure of dquot_initialize() which is not tested in most
> places and thus results in quota miscomputations... Properly handling this
> would require another set of cleanups.
>
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?Honza
> --
> Jan Kara <[email protected]>
> SUSE Labs, CR
>
On Tue, Jan 25, 2011 at 12:05:07PM -0800, Manish Katiyar wrote:
> Thanks a lot Jan, I will have a look at the functions you mentioned
> and send an updated version.
Please do the cleanups as separate patches, each with a separate
commit description.
Thanks!!
- Ted
On Sun, Jan 30, 2011 at 11:42 AM, Ted Ts'o <[email protected]> wrote:
> On Tue, Jan 25, 2011 at 12:05:07PM -0800, Manish Katiyar wrote:
>> Thanks a lot Jan, I will have a look at the functions you mentioned
>> and send an updated version.
>
> Please do the cleanups as separate patches, each with a separate
> commit description.
Hi Ted,
Do you mean separate patch for each of the changed functions ?
--
Thanks -
Manish
On Sun, Jan 30, 2011 at 11:44:25AM -0800, Manish Katiyar wrote:
>
> Do you mean separate patch for each of the changed functions ?
Unless the cleanups for a set of functions are all the same issue,
yes. The idea is that commit description should apply to all of the
patch hunks in the commit.
That way if there is a problem, it becomes easier to bisect and then
determine what's going on by reading the commit description. It also
becomes easier to review the changes, as well.
Does that make sense?
Thanks,
- Ted
On Sun, Jan 30, 2011 at 12:07 PM, Ted Ts'o <[email protected]> wrote:
> On Sun, Jan 30, 2011 at 11:44:25AM -0800, Manish Katiyar wrote:
>>
>> Do you mean separate patch for each of the changed functions ?
>
> Unless the cleanups for a set of functions are all the same issue,
> yes. ?The idea is that commit description should apply to all of the
> patch hunks in the commit.
>
> That way if there is a problem, it becomes easier to bisect and then
> determine what's going on by reading the commit description. ?It also
> becomes easier to review the changes, as well.
>
> Does that make sense?
Hi Ted,
Resending the above patch broken in following series of patch.
--
Thanks -
Manish