* [PATCH 00/12] quota: Redesign IO error handling interface V2
@ 2010-05-19 6:01 Dmitry Monakhov
2010-05-19 6:01 ` [PATCH 01/12] quota: Add proper error handling on quota initialization Dmitry Monakhov
0 siblings, 1 reply; 20+ messages in thread
From: Dmitry Monakhov @ 2010-05-19 6:01 UTC (permalink / raw)
To: linux-fsdevel; +Cc: jack, hch, Dmitry Monakhov
This patchset is tended to provide interface for handling IO errors
from internal quota code.
Any error must being returned to fs-caller to signal about possible
quota inconsistency. I've done it in following way:
1) Handle low-level io errors from dqget() and it's callers
2) Handle errors from dquot_initialize
This path catch most of IO error, but no all.
3) Check what i_dquot was initialized in each low-level function.
There are two types of such functions
3A) Charging functions (alloc_{space,inode}): Caller of such
function may easy handle an error and abort an operation.
3B) nofail functions (claim_space,free_{space,inode})
In most cases caller can not abort an operation even if
inode's quotas was semi-initialized, so I just skip this
functions for now.
Notes: Ext{3,4} orphan_list_cleanup patch may be folded to corresponding
dquot_initialize() patch, but IMHO this makes review more complicate.
Changes from V2
- First patch (unify quota init condition in setattr) was accepted.
- Fix according to Jan's comments. Except error handling in charge methods
- Add missed error handling in add_dquot_ref
- Split fs-speciffic callers of dquot_initialize() in to per-fs parts.
This makes number of patches dangerously big, but this is the only
way to make things right(Thank Jan for an suggestion).
Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 01/12] quota: Add proper error handling on quota initialization.
2010-05-19 6:01 [PATCH 00/12] quota: Redesign IO error handling interface V2 Dmitry Monakhov
@ 2010-05-19 6:01 ` Dmitry Monakhov
2010-05-19 6:01 ` [PATCH 02/12] quota: Check what quota is properly initialized for inode before charge Dmitry Monakhov
2010-05-20 18:05 ` [PATCH 01/12] quota: Add proper error handling on quota initialization Jan Kara
0 siblings, 2 replies; 20+ messages in thread
From: Dmitry Monakhov @ 2010-05-19 6:01 UTC (permalink / raw)
To: linux-fsdevel; +Cc: jack, hch, Dmitry Monakhov
Currently dqget_initialize() is a black-box so IO errors are simply
ignored. In order to pass to the caller real error codes quota
interface must being redesigned in following way.
- return real error code from dqget()
- return real error code from dquot_initialize()
Now filesystem it is filesystem's responsibility to dquot_initialize()
return value.
Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
fs/ocfs2/file.c | 8 ++--
fs/ocfs2/quota_local.c | 10 ++--
fs/quota/dquot.c | 121 +++++++++++++++++++++++++++++++---------------
include/linux/quotaops.h | 5 +-
4 files changed, 93 insertions(+), 51 deletions(-)
diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
index 312b8f5..92d91a1 100644
--- a/fs/ocfs2/file.c
+++ b/fs/ocfs2/file.c
@@ -1031,8 +1031,8 @@ int ocfs2_setattr(struct dentry *dentry, struct iattr *attr)
OCFS2_FEATURE_RO_COMPAT_USRQUOTA)) {
transfer_to[USRQUOTA] = dqget(sb, attr->ia_uid,
USRQUOTA);
- if (!transfer_to[USRQUOTA]) {
- status = -ESRCH;
+ if (IS_ERR(transfer_to[USRQUOTA])) {
+ status = PTR_ERR(transfer_to[USRQUOTA]);
goto bail_unlock;
}
}
@@ -1041,8 +1041,8 @@ int ocfs2_setattr(struct dentry *dentry, struct iattr *attr)
OCFS2_FEATURE_RO_COMPAT_GRPQUOTA)) {
transfer_to[GRPQUOTA] = dqget(sb, attr->ia_gid,
GRPQUOTA);
- if (!transfer_to[GRPQUOTA]) {
- status = -ESRCH;
+ if (IS_ERR(transfer_to[GRPQUOTA])) {
+ status = PTR_ERR(transfer_to[GRPQUOTA]);
goto bail_unlock;
}
}
diff --git a/fs/ocfs2/quota_local.c b/fs/ocfs2/quota_local.c
index 1b55b1d..6cf42b5 100644
--- a/fs/ocfs2/quota_local.c
+++ b/fs/ocfs2/quota_local.c
@@ -504,13 +504,13 @@ static int ocfs2_recover_local_quota_file(struct inode *lqinode,
dqblk = (struct ocfs2_local_disk_dqblk *)(qbh->b_data +
ol_dqblk_block_off(sb, chunk, bit));
dquot = dqget(sb, le64_to_cpu(dqblk->dqb_id), type);
- if (!dquot) {
- status = -EIO;
+ if (IS_ERR(dquot)) {
+ status = PTR_ERR(dquot);
mlog(ML_ERROR, "Failed to get quota structure "
- "for id %u, type %d. Cannot finish quota "
- "file recovery.\n",
+ "for id %u, type %d err %d. Cannot finish"
+ " quota file recovery.\n",
(unsigned)le64_to_cpu(dqblk->dqb_id),
- type);
+ type, status);
goto out_put_bh;
}
status = ocfs2_lock_global_qf(oinfo, 1);
diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
index 7c624e1..95aa445 100644
--- a/fs/quota/dquot.c
+++ b/fs/quota/dquot.c
@@ -234,7 +234,7 @@ EXPORT_SYMBOL(dqstats_pcpu);
#endif
static qsize_t inode_get_rsv_space(struct inode *inode);
-static void __dquot_initialize(struct inode *inode, int type);
+static int __dquot_initialize(struct inode *inode, int type);
static inline unsigned int
hashfn(const struct super_block *sb, unsigned int id, int type)
@@ -720,7 +720,7 @@ void dqput(struct dquot *dquot)
{
int ret;
- if (!dquot)
+ if (!dquot || IS_ERR(dquot))
return;
#ifdef CONFIG_QUOTA_DEBUG
if (!atomic_read(&dquot->dq_count)) {
@@ -815,14 +815,16 @@ static struct dquot *get_empty_dquot(struct super_block *sb, int type)
* destroying our dquot by:
* a) checking for quota flags under dq_list_lock and
* b) getting a reference to dquot before we release dq_list_lock
+ * Return: valid pointer on success, ERR_PTR otherwise.
*/
struct dquot *dqget(struct super_block *sb, unsigned int id, int type)
{
unsigned int hashent = hashfn(sb, id, type);
- struct dquot *dquot = NULL, *empty = NULL;
+ struct dquot *dquot = ERR_PTR(-ESRCH);
+ struct dquot *empty = NULL;
if (!sb_has_quota_active(sb, type))
- return NULL;
+ goto out;
we_slept:
spin_lock(&dq_list_lock);
spin_lock(&dq_state_lock);
@@ -863,11 +865,13 @@ we_slept:
* already finished or it will be canceled due to dq_count > 1 test */
wait_on_dquot(dquot);
/* Read the dquot / allocate space in quota file */
- if (!test_bit(DQ_ACTIVE_B, &dquot->dq_flags) &&
- sb->dq_op->acquire_dquot(dquot) < 0) {
- dqput(dquot);
- dquot = NULL;
- goto out;
+ if (!test_bit(DQ_ACTIVE_B, &dquot->dq_flags)) {
+ int ret = sb->dq_op->acquire_dquot(dquot);
+ if (ret < 0) {
+ dqput(dquot);
+ dquot = ERR_PTR(ret);
+ goto out;
+ }
}
#ifdef CONFIG_QUOTA_DEBUG
BUG_ON(!dquot->dq_sb); /* Has somebody invalidated entry under us? */
@@ -895,9 +899,10 @@ static int dqinit_needed(struct inode *inode, int type)
}
/* This routine is guarded by dqonoff_mutex mutex */
-static void add_dquot_ref(struct super_block *sb, int type)
+static int add_dquot_ref(struct super_block *sb, int type)
{
struct inode *inode, *old_inode = NULL;
+ int err = 0;
#ifdef CONFIG_QUOTA_DEBUG
int reserved = 0;
#endif
@@ -919,7 +924,7 @@ static void add_dquot_ref(struct super_block *sb, int type)
spin_unlock(&inode_lock);
iput(old_inode);
- __dquot_initialize(inode, type);
+ err = __dquot_initialize(inode, type);
/* We hold a reference to 'inode' so it couldn't have been
* removed from s_inodes list while we dropped the inode_lock.
* We cannot iput the inode now as we can be holding the last
@@ -927,6 +932,8 @@ static void add_dquot_ref(struct super_block *sb, int type)
* keep the reference and iput it later. */
old_inode = inode;
spin_lock(&inode_lock);
+ if (unlikely(err))
+ break;
}
spin_unlock(&inode_lock);
iput(old_inode);
@@ -938,6 +945,7 @@ static void add_dquot_ref(struct super_block *sb, int type)
"inconsistent. Please run quotacheck(8).\n", sb->s_id);
}
#endif
+ return err;
}
/*
@@ -1332,18 +1340,19 @@ static int info_bdq_free(struct dquot *dquot, qsize_t space)
* It is better to call this function outside of any transaction as it
* might need a lot of space in journal for dquot structure allocation.
*/
-static void __dquot_initialize(struct inode *inode, int type)
+static int __dquot_initialize(struct inode *inode, int type)
{
unsigned int id = 0;
- int cnt;
+ int cnt, err = 0;
struct dquot *got[MAXQUOTAS];
struct super_block *sb = inode->i_sb;
qsize_t rsv;
+repeat:
/* First test before acquiring mutex - solves deadlocks when we
* re-enter the quota code and are already holding the mutex */
if (!sb_any_quota_active(inode->i_sb) || IS_NOQUOTA(inode))
- return;
+ return 0;
/* First get references to structures we might need. */
for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
@@ -1362,35 +1371,50 @@ static void __dquot_initialize(struct inode *inode, int type)
}
down_write(&sb_dqopt(sb)->dqptr_sem);
- if (IS_NOQUOTA(inode))
+ if (IS_NOQUOTA(inode)) {
+ err = 0;
goto out_err;
+ }
for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
if (type != -1 && cnt != type)
continue;
/* Avoid races with quotaoff() */
if (!sb_has_quota_active(sb, cnt))
continue;
- if (!inode->i_dquot[cnt]) {
- inode->i_dquot[cnt] = got[cnt];
- got[cnt] = NULL;
- /*
- * Make quota reservation system happy if someone
- * did a write before quota was turned on
- */
- rsv = inode_get_rsv_space(inode);
- if (unlikely(rsv))
- dquot_resv_space(inode->i_dquot[cnt], rsv);
+ if (inode->i_dquot[cnt])
+ continue;
+ if (unlikely(IS_ERR(got[cnt]))) {
+ err = PTR_ERR(got[cnt]);
+ /* If dqget() was raced with quotaon() then we have to
+ * repeat lookup. */
+ if (err == -ESRCH) {
+ err = 0;
+ up_write(&sb_dqopt(sb)->dqptr_sem);
+ dqput_all(got);
+ goto repeat;
+ }
+ goto out_err;
}
+ inode->i_dquot[cnt] = got[cnt];
+ got[cnt] = NULL;
+ /*
+ * Make quota reservation system happy if someone
+ * did a write before quota was turned on
+ */
+ rsv = inode_get_rsv_space(inode);
+ if (unlikely(rsv))
+ dquot_resv_space(inode->i_dquot[cnt], rsv);
}
out_err:
up_write(&sb_dqopt(sb)->dqptr_sem);
/* Drop unused references */
dqput_all(got);
+ return err;
}
-void dquot_initialize(struct inode *inode)
+int dquot_initialize(struct inode *inode)
{
- __dquot_initialize(inode, -1);
+ return __dquot_initialize(inode, -1);
}
EXPORT_SYMBOL(dquot_initialize);
@@ -1809,12 +1833,23 @@ int dquot_transfer(struct inode *inode, struct iattr *iattr)
if (!sb_any_quota_active(sb) || IS_NOQUOTA(inode))
return 0;
- if (iattr->ia_valid & ATTR_UID && iattr->ia_uid != inode->i_uid)
+ if (iattr->ia_valid & ATTR_UID && iattr->ia_uid != inode->i_uid) {
transfer_to[USRQUOTA] = dqget(sb, iattr->ia_uid, USRQUOTA);
- if (iattr->ia_valid & ATTR_GID && iattr->ia_gid != inode->i_gid)
+ if (IS_ERR(transfer_to[USRQUOTA])) {
+ ret = PTR_ERR(transfer_to[USRQUOTA]);
+ goto out;
+ }
+ }
+ if (iattr->ia_valid & ATTR_GID && iattr->ia_gid != inode->i_gid) {
transfer_to[GRPQUOTA] = dqget(sb, iattr->ia_uid, GRPQUOTA);
+ if (IS_ERR(transfer_to[GRPQUOTA])) {
+ ret = PTR_ERR(transfer_to[GRPQUOTA]);
+ goto out;
+ }
+ }
ret = __dquot_transfer(inode, transfer_to);
+out:
dqput_all(transfer_to);
return ret;
}
@@ -1857,7 +1892,7 @@ int dquot_file_open(struct inode *inode, struct file *file)
error = generic_file_open(inode, file);
if (!error && (file->f_mode & FMODE_WRITE))
- dquot_initialize(inode);
+ error = dquot_initialize(inode);
return error;
}
EXPORT_SYMBOL(dquot_file_open);
@@ -2018,7 +2053,7 @@ static int vfs_load_quota_inode(struct inode *inode, int type, int format_id,
struct super_block *sb = inode->i_sb;
struct quota_info *dqopt = sb_dqopt(sb);
int error;
- int oldflags = -1;
+ unsigned int dqflags, iflags = -1;
if (!fmt)
return -ESRCH;
@@ -2061,7 +2096,7 @@ static int vfs_load_quota_inode(struct inode *inode, int type, int format_id,
* possible) Also nobody should write to the file - we use
* special IO operations which ignore the immutable bit. */
mutex_lock_nested(&inode->i_mutex, I_MUTEX_QUOTA);
- oldflags = inode->i_flags & (S_NOATIME | S_IMMUTABLE |
+ iflags = inode->i_flags & (S_NOATIME | S_IMMUTABLE |
S_NOQUOTA);
inode->i_flags |= S_NOQUOTA | S_NOATIME | S_IMMUTABLE;
mutex_unlock(&inode->i_mutex);
@@ -2092,24 +2127,30 @@ static int vfs_load_quota_inode(struct inode *inode, int type, int format_id,
}
mutex_unlock(&dqopt->dqio_mutex);
spin_lock(&dq_state_lock);
+ dqflags = dqopt->flags;
dqopt->flags |= dquot_state_flag(flags, type);
spin_unlock(&dq_state_lock);
- add_dquot_ref(sb, type);
+ error = add_dquot_ref(sb, type);
+ if (error)
+ goto out_dquot_init;
mutex_unlock(&dqopt->dqonoff_mutex);
return 0;
-
+out_dquot_init:
+ spin_lock(&dq_state_lock);
+ dqopt->flags = dqflags;
+ spin_unlock(&dq_state_lock);
out_file_init:
dqopt->files[type] = NULL;
iput(inode);
out_lock:
- if (oldflags != -1) {
+ if (iflags != -1) {
mutex_lock_nested(&inode->i_mutex, I_MUTEX_QUOTA);
/* Set the flags back (in the case of accidental quotaon()
* on a wrong file we don't want to mess up the flags) */
inode->i_flags &= ~(S_NOATIME | S_NOQUOTA | S_IMMUTABLE);
- inode->i_flags |= oldflags;
+ inode->i_flags |= iflags;
mutex_unlock(&inode->i_mutex);
}
mutex_unlock(&dqopt->dqonoff_mutex);
@@ -2319,8 +2360,8 @@ int vfs_get_dqblk(struct super_block *sb, int type, qid_t id,
struct dquot *dquot;
dquot = dqget(sb, id, type);
- if (!dquot)
- return -ESRCH;
+ if (IS_ERR(dquot))
+ return PTR_ERR(dquot);
do_get_dqblk(dquot, di);
dqput(dquot);
@@ -2432,8 +2473,8 @@ int vfs_set_dqblk(struct super_block *sb, int type, qid_t id,
int rc;
dquot = dqget(sb, id, type);
- if (!dquot) {
- rc = -ESRCH;
+ if (IS_ERR(dquot)) {
+ rc = PTR_ERR(dquot);
goto out;
}
rc = do_set_dqblk(dquot, di);
diff --git a/include/linux/quotaops.h b/include/linux/quotaops.h
index 370abb1..4fc5a9c 100644
--- a/include/linux/quotaops.h
+++ b/include/linux/quotaops.h
@@ -31,7 +31,7 @@ void inode_add_rsv_space(struct inode *inode, qsize_t number);
void inode_claim_rsv_space(struct inode *inode, qsize_t number);
void inode_sub_rsv_space(struct inode *inode, qsize_t number);
-void dquot_initialize(struct inode *inode);
+int dquot_initialize(struct inode *inode);
void dquot_drop(struct inode *inode);
struct dquot *dqget(struct super_block *sb, unsigned int id, int type);
void dqput(struct dquot *dquot);
@@ -209,8 +209,9 @@ static inline int sb_any_quota_active(struct super_block *sb)
#define sb_dquot_ops (NULL)
#define sb_quotactl_ops (NULL)
-static inline void dquot_initialize(struct inode *inode)
+static inline int dquot_initialize(struct inode *inode)
{
+ return 0;
}
static inline void dquot_drop(struct inode *inode)
--
1.6.6.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 02/12] quota: Check what quota is properly initialized for inode before charge
2010-05-19 6:01 ` [PATCH 01/12] quota: Add proper error handling on quota initialization Dmitry Monakhov
@ 2010-05-19 6:01 ` Dmitry Monakhov
2010-05-19 6:01 ` [PATCH 03/12] ext3: handle errors in orphan_cleanup Dmitry Monakhov
2010-05-20 18:13 ` [PATCH 02/12] quota: Check what quota is properly initialized for inode before charge Jan Kara
2010-05-20 18:05 ` [PATCH 01/12] quota: Add proper error handling on quota initialization Jan Kara
1 sibling, 2 replies; 20+ messages in thread
From: Dmitry Monakhov @ 2010-05-19 6:01 UTC (permalink / raw)
To: linux-fsdevel; +Cc: jack, hch, Dmitry Monakhov
Due to previous IO error or other bugs an inode may not has quota
pointer. This definite sign of quota inconsistency/corruption.
In fact this condition must being checked in all charge/uncharge
functions. And if error was detected it is reasonable to fail whole
operation. But uncharge(free_space/claim_space/free_inode) functions
has no fail nature. This is because caller can't handle errors from
such functions. How can you handle error from following call-trace?
write_page()->quota_claim_space()
So alloc_space() and alloc_inode() are the only functions which we may
reliably abort in case of any errors.
This patch add corresponding checks only for charge functions.
Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
fs/quota/dquot.c | 41 +++++++++++++++++++++++++++++++++++------
1 files changed, 35 insertions(+), 6 deletions(-)
diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
index 95aa445..f0e68b3 100644
--- a/fs/quota/dquot.c
+++ b/fs/quota/dquot.c
@@ -1541,7 +1541,7 @@ static void inode_decr_space(struct inode *inode, qsize_t number, int reserve)
int __dquot_alloc_space(struct inode *inode, qsize_t number,
int warn, int reserve)
{
- int cnt, ret = 0;
+ int cnt, active, ret = 0;
char warntype[MAXQUOTAS];
/*
@@ -1554,13 +1554,28 @@ int __dquot_alloc_space(struct inode *inode, qsize_t number,
}
down_read(&sb_dqopt(inode->i_sb)->dqptr_sem);
+ /* Now recheck reliably when holding dqptr_sem */
+ active = sb_any_quota_active(inode->i_sb);
+ if (!active || IS_NOQUOTA(inode)) {
+ inode_incr_space(inode, number, reserve);
+ goto out;
+ }
+
for (cnt = 0; cnt < MAXQUOTAS; cnt++)
warntype[cnt] = QUOTA_NL_NOWARN;
spin_lock(&dq_data_lock);
for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
- if (!inode->i_dquot[cnt])
+ if (!(active & (1 << cnt)))
continue;
+ /*
+ * Given quota type is active, so quota must being
+ * initialized for this inode
+ */
+ if (!inode->i_dquot[cnt]) {
+ ret = -EIO;
+ goto out_flush_warn;
+ }
ret = check_bdq(inode->i_dquot[cnt], number, !warn,
warntype+cnt);
if (ret) {
@@ -1569,7 +1584,7 @@ int __dquot_alloc_space(struct inode *inode, qsize_t number,
}
}
for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
- if (!inode->i_dquot[cnt])
+ if (!(active & (1 << cnt)))
continue;
if (reserve)
dquot_resv_space(inode->i_dquot[cnt], number);
@@ -1595,7 +1610,7 @@ EXPORT_SYMBOL(__dquot_alloc_space);
*/
int dquot_alloc_inode(const struct inode *inode)
{
- int cnt, ret = 0;
+ int cnt, active, ret = 0;
char warntype[MAXQUOTAS];
/* First test before acquiring mutex - solves deadlocks when we
@@ -1605,17 +1620,31 @@ int dquot_alloc_inode(const struct inode *inode)
for (cnt = 0; cnt < MAXQUOTAS; cnt++)
warntype[cnt] = QUOTA_NL_NOWARN;
down_read(&sb_dqopt(inode->i_sb)->dqptr_sem);
+ /* Now recheck reliably when holding dqptr_sem */
+ active = sb_any_quota_active(inode->i_sb);
+ if (!active || IS_NOQUOTA(inode)) {
+ return 0;
+ }
+
spin_lock(&dq_data_lock);
for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
- if (!inode->i_dquot[cnt])
+ if (!(active & (1 < cnt)))
continue;
+ /*
+ * Given quota type is active, so quota must being
+ * initialized for this inode
+ */
+ if (!inode->i_dquot[cnt]) {
+ ret = -EIO;
+ goto warn_put_all;
+ }
ret = check_idq(inode->i_dquot[cnt], 1, warntype + cnt);
if (ret)
goto warn_put_all;
}
for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
- if (!inode->i_dquot[cnt])
+ if ((!active & (1 < cnt)))
continue;
dquot_incr_inodes(inode->i_dquot[cnt], 1);
}
--
1.6.6.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 03/12] ext3: handle errors in orphan_cleanup
2010-05-19 6:01 ` [PATCH 02/12] quota: Check what quota is properly initialized for inode before charge Dmitry Monakhov
@ 2010-05-19 6:01 ` Dmitry Monakhov
2010-05-19 6:02 ` [PATCH 04/12] ext4: " Dmitry Monakhov
2010-05-20 17:35 ` [PATCH 03/12] ext3: handle errors in orphan_cleanup Jan Kara
2010-05-20 18:13 ` [PATCH 02/12] quota: Check what quota is properly initialized for inode before charge Jan Kara
1 sibling, 2 replies; 20+ messages in thread
From: Dmitry Monakhov @ 2010-05-19 6:01 UTC (permalink / raw)
To: linux-fsdevel; +Cc: jack, hch, Dmitry Monakhov
Orphan cleanup procedure is complex task and may fail due to number
of reasons. Handle errors from orphan_cleanp according to
predefined per-sb errors behavior flags.
Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
fs/ext3/super.c | 27 +++++++++++++++++++--------
1 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/fs/ext3/super.c b/fs/ext3/super.c
index 0fc1293..5ca068f 100644
--- a/fs/ext3/super.c
+++ b/fs/ext3/super.c
@@ -1438,23 +1438,24 @@ static int ext3_check_descriptors(struct super_block *sb)
* e2fsck was run on this filesystem, and it must have already done the orphan
* inode cleanup for us, so we can safely abort without any further action.
*/
-static void ext3_orphan_cleanup (struct super_block * sb,
+static int ext3_orphan_cleanup (struct super_block * sb,
struct ext3_super_block * es)
{
unsigned int s_flags = sb->s_flags;
int nr_orphans = 0, nr_truncates = 0;
+ int ret = 0;
#ifdef CONFIG_QUOTA
int i;
#endif
if (!es->s_last_orphan) {
jbd_debug(4, "no orphan inodes to clean up\n");
- return;
+ return ret;
}
if (bdev_read_only(sb->s_bdev)) {
ext3_msg(sb, KERN_ERR, "error: write access "
"unavailable, skipping orphan cleanup.");
- return;
+ return ret;
}
if (EXT3_SB(sb)->s_mount_state & EXT3_ERROR_FS) {
@@ -1463,7 +1464,7 @@ static void ext3_orphan_cleanup (struct super_block * sb,
"clearing orphan list.\n");
es->s_last_orphan = 0;
jbd_debug(1, "Skipping orphan recovery on fs with errors.\n");
- return;
+ return ret;
}
if (s_flags & MS_RDONLY) {
@@ -1476,11 +1477,13 @@ static void ext3_orphan_cleanup (struct super_block * sb,
/* Turn on quotas so that they are updated correctly */
for (i = 0; i < MAXQUOTAS; i++) {
if (EXT3_SB(sb)->s_qf_names[i]) {
- int ret = ext3_quota_on_mount(sb, i);
- if (ret < 0)
+ ret = ext3_quota_on_mount(sb, i);
+ if (ret < 0) {
ext3_msg(sb, KERN_ERR,
"error: cannot turn on journaled "
"quota: %d", ret);
+ goto out;
+ }
}
}
#endif
@@ -1514,7 +1517,7 @@ static void ext3_orphan_cleanup (struct super_block * sb,
}
iput(inode); /* The delete magic happens here! */
}
-
+out:
#define PLURAL(x) (x), ((x)==1) ? "" : "s"
if (nr_orphans)
@@ -1523,6 +1526,9 @@ static void ext3_orphan_cleanup (struct super_block * sb,
if (nr_truncates)
ext3_msg(sb, KERN_INFO, "%d truncate%s cleaned up",
PLURAL(nr_truncates));
+ if (ret)
+ ext3_msg(sb, KERN_ERR, "Error %d wile orphan cleanup", ret);
+
#ifdef CONFIG_QUOTA
/* Turn quotas off */
for (i = 0; i < MAXQUOTAS; i++) {
@@ -1531,6 +1537,7 @@ static void ext3_orphan_cleanup (struct super_block * sb,
}
#endif
sb->s_flags = s_flags; /* Restore MS_RDONLY status */
+ return ret;
}
/*
@@ -2034,7 +2041,11 @@ static int ext3_fill_super (struct super_block *sb, void *data, int silent)
ext3_setup_super (sb, es, sb->s_flags & MS_RDONLY);
EXT3_SB(sb)->s_mount_state |= EXT3_ORPHAN_FS;
- ext3_orphan_cleanup(sb, es);
+ ret = ext3_orphan_cleanup(sb, es);
+
+ if (ret)
+ goto failed_mount3;
+
EXT3_SB(sb)->s_mount_state &= ~EXT3_ORPHAN_FS;
if (needs_recovery)
ext3_msg(sb, KERN_INFO, "recovery complete");
--
1.6.6.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 04/12] ext4: handle errors in orphan_cleanup
2010-05-19 6:01 ` [PATCH 03/12] ext3: handle errors in orphan_cleanup Dmitry Monakhov
@ 2010-05-19 6:02 ` Dmitry Monakhov
2010-05-19 6:02 ` [PATCH 05/12] ufs: add error handling for dquot_initialize Dmitry Monakhov
2010-05-20 17:35 ` [PATCH 03/12] ext3: handle errors in orphan_cleanup Jan Kara
1 sibling, 1 reply; 20+ messages in thread
From: Dmitry Monakhov @ 2010-05-19 6:02 UTC (permalink / raw)
To: linux-fsdevel; +Cc: jack, hch, Dmitry Monakhov
Orphan cleanup procedure is complex task and may fail due to number
of reasons. Handle errors from orphan_cleanp according to
predefined per-sb errors behavior flags.
Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
fs/ext4/super.c | 29 +++++++++++++++++++++--------
1 files changed, 21 insertions(+), 8 deletions(-)
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 03db4af..a794707 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1962,23 +1962,24 @@ static int ext4_check_descriptors(struct super_block *sb)
* e2fsck was run on this filesystem, and it must have already done the orphan
* inode cleanup for us, so we can safely abort without any further action.
*/
-static void ext4_orphan_cleanup(struct super_block *sb,
+static int ext4_orphan_cleanup(struct super_block *sb,
struct ext4_super_block *es)
{
unsigned int s_flags = sb->s_flags;
int nr_orphans = 0, nr_truncates = 0;
+ int ret = 0;
#ifdef CONFIG_QUOTA
int i;
#endif
if (!es->s_last_orphan) {
jbd_debug(4, "no orphan inodes to clean up\n");
- return;
+ return 0;
}
if (bdev_read_only(sb->s_bdev)) {
ext4_msg(sb, KERN_ERR, "write access "
"unavailable, skipping orphan cleanup");
- return;
+ return 0;
}
if (EXT4_SB(sb)->s_mount_state & EXT4_ERROR_FS) {
@@ -1987,7 +1988,7 @@ static void ext4_orphan_cleanup(struct super_block *sb,
"clearing orphan list.\n");
es->s_last_orphan = 0;
jbd_debug(1, "Skipping orphan recovery on fs with errors.\n");
- return;
+ return 0;
}
if (s_flags & MS_RDONLY) {
@@ -2000,11 +2001,13 @@ static void ext4_orphan_cleanup(struct super_block *sb,
/* Turn on quotas so that they are updated correctly */
for (i = 0; i < MAXQUOTAS; i++) {
if (EXT4_SB(sb)->s_qf_names[i]) {
- int ret = ext4_quota_on_mount(sb, i);
- if (ret < 0)
+ ret = ext4_quota_on_mount(sb, i);
+ if (ret < 0) {
ext4_msg(sb, KERN_ERR,
"Cannot turn on journaled "
"quota: error %d", ret);
+ goto out;
+ }
}
}
#endif
@@ -2040,13 +2043,16 @@ static void ext4_orphan_cleanup(struct super_block *sb,
}
#define PLURAL(x) (x), ((x) == 1) ? "" : "s"
-
+out:
if (nr_orphans)
ext4_msg(sb, KERN_INFO, "%d orphan inode%s deleted",
PLURAL(nr_orphans));
if (nr_truncates)
ext4_msg(sb, KERN_INFO, "%d truncate%s cleaned up",
PLURAL(nr_truncates));
+ if (ret)
+ ext4_msg(sb, KERN_ERR, "Error %d wile orphan cleanup", ret);
+
#ifdef CONFIG_QUOTA
/* Turn quotas off */
for (i = 0; i < MAXQUOTAS; i++) {
@@ -2055,6 +2061,7 @@ static void ext4_orphan_cleanup(struct super_block *sb,
}
#endif
sb->s_flags = s_flags; /* Restore MS_RDONLY status */
+ return ret;
}
/*
@@ -3020,7 +3027,13 @@ no_journal:
};
EXT4_SB(sb)->s_mount_state |= EXT4_ORPHAN_FS;
- ext4_orphan_cleanup(sb, es);
+ ret = ext4_orphan_cleanup(sb, es);
+ if (err) {
+ ext4_mb_release(sb);
+ ext4_ext_release(sb);
+ goto failed_mount4;
+ }
+
EXT4_SB(sb)->s_mount_state &= ~EXT4_ORPHAN_FS;
if (needs_recovery) {
ext4_msg(sb, KERN_INFO, "recovery complete");
--
1.6.6.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 05/12] ufs: add error handling for dquot_initialize
2010-05-19 6:02 ` [PATCH 04/12] ext4: " Dmitry Monakhov
@ 2010-05-19 6:02 ` Dmitry Monakhov
2010-05-19 6:02 ` [PATCH 06/12] udf: " Dmitry Monakhov
2010-05-20 17:33 ` [PATCH 05/12] ufs: " Jan Kara
0 siblings, 2 replies; 20+ messages in thread
From: Dmitry Monakhov @ 2010-05-19 6:02 UTC (permalink / raw)
To: linux-fsdevel; +Cc: jack, hch, Dmitry Monakhov
Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
fs/ufs/ialloc.c | 5 ++++-
fs/ufs/namei.c | 46 +++++++++++++++++++++++++++++++++-------------
fs/ufs/truncate.c | 6 ++++--
3 files changed, 41 insertions(+), 16 deletions(-)
diff --git a/fs/ufs/ialloc.c b/fs/ufs/ialloc.c
index 230ecf6..33e6fab 100644
--- a/fs/ufs/ialloc.c
+++ b/fs/ufs/ialloc.c
@@ -355,7 +355,10 @@ cg_found:
unlock_super (sb);
- dquot_initialize(inode);
+ err = dquot_initialize(inode);
+ if (err)
+ goto fail_without_unlock;
+
err = dquot_alloc_inode(inode);
if (err) {
dquot_drop(inode);
diff --git a/fs/ufs/namei.c b/fs/ufs/namei.c
index eabc02e..94edec7 100644
--- a/fs/ufs/namei.c
+++ b/fs/ufs/namei.c
@@ -86,7 +86,9 @@ static int ufs_create (struct inode * dir, struct dentry * dentry, int mode,
UFSD("BEGIN\n");
- dquot_initialize(dir);
+ err = dquot_initialize(dir);
+ if (err)
+ return err;
inode = ufs_new_inode(dir, mode);
err = PTR_ERR(inode);
@@ -112,7 +114,9 @@ static int ufs_mknod (struct inode * dir, struct dentry *dentry, int mode, dev_t
if (!old_valid_dev(rdev))
return -EINVAL;
- dquot_initialize(dir);
+ err = dquot_initialize(dir);
+ if (err)
+ return err;
inode = ufs_new_inode(dir, mode);
err = PTR_ERR(inode);
@@ -138,7 +142,9 @@ static int ufs_symlink (struct inode * dir, struct dentry * dentry,
if (l > sb->s_blocksize)
goto out_notlocked;
- dquot_initialize(dir);
+ err = dquot_initialize(dir);
+ if (err)
+ goto out;
lock_kernel();
inode = ufs_new_inode(dir, S_IFLNK | S_IRWXUGO);
@@ -181,17 +187,20 @@ static int ufs_link (struct dentry * old_dentry, struct inode * dir,
lock_kernel();
if (inode->i_nlink >= UFS_LINK_MAX) {
- unlock_kernel();
- return -EMLINK;
+ error = -EMLINK;
+ goto out_unlock;
}
- dquot_initialize(dir);
+ error = dquot_initialize(dir);
+ if (error)
+ goto out_unlock;
inode->i_ctime = CURRENT_TIME_SEC;
inode_inc_link_count(inode);
atomic_inc(&inode->i_count);
error = ufs_add_nondir(dentry, inode);
+out_unlock:
unlock_kernel();
return error;
}
@@ -204,7 +213,9 @@ static int ufs_mkdir(struct inode * dir, struct dentry * dentry, int mode)
if (dir->i_nlink >= UFS_LINK_MAX)
goto out;
- dquot_initialize(dir);
+ err = dquot_initialize(dir);
+ if (err)
+ goto out;
lock_kernel();
inode_inc_link_count(dir);
@@ -248,10 +259,13 @@ static int ufs_unlink(struct inode *dir, struct dentry *dentry)
struct inode * inode = dentry->d_inode;
struct ufs_dir_entry *de;
struct page *page;
- int err = -ENOENT;
+ int err;
- dquot_initialize(dir);
+ err = dquot_initialize(dir);
+ if (err)
+ goto out;
+ err = -ENOENT;
de = ufs_find_entry(dir, &dentry->d_name, &page);
if (!de)
goto out;
@@ -294,14 +308,20 @@ static int ufs_rename(struct inode *old_dir, struct dentry *old_dentry,
struct ufs_dir_entry * dir_de = NULL;
struct page *old_page;
struct ufs_dir_entry *old_de;
- int err = -ENOENT;
+ int err;
- dquot_initialize(old_dir);
- dquot_initialize(new_dir);
+ err = dquot_initialize(old_dir);
+ if (err)
+ goto out;
+ err = dquot_initialize(new_dir);
+ if (err)
+ goto out;
old_de = ufs_find_entry(old_dir, &old_dentry->d_name, &old_page);
- if (!old_de)
+ if (!old_de) {
+ err = -ENOENT;
goto out;
+ }
if (S_ISDIR(old_inode->i_mode)) {
err = -EIO;
diff --git a/fs/ufs/truncate.c b/fs/ufs/truncate.c
index f294c44..2de89ee 100644
--- a/fs/ufs/truncate.c
+++ b/fs/ufs/truncate.c
@@ -518,8 +518,10 @@ int ufs_setattr(struct dentry *dentry, struct iattr *attr)
if (error)
return error;
- if (is_quota_modification(inode, attr))
- dquot_initialize(inode);
+ if (is_quota_modification(inode, attr)) {
+ error = dquot_initialize(inode);
+ return error;
+ }
if ((ia_valid & ATTR_UID && attr->ia_uid != inode->i_uid) ||
(ia_valid & ATTR_GID && attr->ia_gid != inode->i_gid)) {
--
1.6.6.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 06/12] udf: add error handling for dquot_initialize
2010-05-19 6:02 ` [PATCH 05/12] ufs: add error handling for dquot_initialize Dmitry Monakhov
@ 2010-05-19 6:02 ` Dmitry Monakhov
2010-05-19 6:02 ` [PATCH 07/12] reiserfs: " Dmitry Monakhov
2010-05-20 17:34 ` [PATCH 06/12] udf: " Jan Kara
2010-05-20 17:33 ` [PATCH 05/12] ufs: " Jan Kara
1 sibling, 2 replies; 20+ messages in thread
From: Dmitry Monakhov @ 2010-05-19 6:02 UTC (permalink / raw)
To: linux-fsdevel; +Cc: jack, hch, Dmitry Monakhov
Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
fs/udf/file.c | 6 ++++--
fs/udf/ialloc.c | 18 +++++++++++-------
fs/udf/namei.c | 39 +++++++++++++++++++++++++++++----------
3 files changed, 44 insertions(+), 19 deletions(-)
diff --git a/fs/udf/file.c b/fs/udf/file.c
index 6ebc043..1ef70d6 100644
--- a/fs/udf/file.c
+++ b/fs/udf/file.c
@@ -227,8 +227,10 @@ int udf_setattr(struct dentry *dentry, struct iattr *iattr)
if (error)
return error;
- if (is_quota_modification(inode, iattr))
- dquot_initialize(inode);
+ if (is_quota_modification(inode, iattr)) {
+ error = dquot_initialize(inode);
+ return error;
+ }
if ((iattr->ia_valid & ATTR_UID && iattr->ia_uid != inode->i_uid) ||
(iattr->ia_valid & ATTR_GID && iattr->ia_gid != inode->i_gid)) {
diff --git a/fs/udf/ialloc.c b/fs/udf/ialloc.c
index fb68c9c..dc78998 100644
--- a/fs/udf/ialloc.c
+++ b/fs/udf/ialloc.c
@@ -153,17 +153,21 @@ struct inode *udf_new_inode(struct inode *dir, int mode, int *err)
insert_inode_hash(inode);
mark_inode_dirty(inode);
- dquot_initialize(inode);
+ ret = dquot_initialize(inode);
+ if (ret)
+ goto out_put;
ret = dquot_alloc_inode(inode);
if (ret) {
dquot_drop(inode);
- inode->i_flags |= S_NOQUOTA;
- inode->i_nlink = 0;
- iput(inode);
- *err = ret;
- return NULL;
+ goto out_put;
}
-
*err = 0;
return inode;
+out_put:
+ inode->i_flags |= S_NOQUOTA;
+ clear_nlink(inode);
+ iput(inode);
+ *err = ret;
+ return NULL;
+
}
diff --git a/fs/udf/namei.c b/fs/udf/namei.c
index 7581602..0cb8167 100644
--- a/fs/udf/namei.c
+++ b/fs/udf/namei.c
@@ -563,7 +563,9 @@ static int udf_create(struct inode *dir, struct dentry *dentry, int mode,
int err;
struct udf_inode_info *iinfo;
- dquot_initialize(dir);
+ err = dquot_initialize(dir);
+ if (err)
+ return err;
lock_kernel();
inode = udf_new_inode(dir, mode, &err);
@@ -618,7 +620,9 @@ static int udf_mknod(struct inode *dir, struct dentry *dentry, int mode,
if (!old_valid_dev(rdev))
return -EINVAL;
- dquot_initialize(dir);
+ err = dquot_initialize(dir);
+ if (err)
+ return err;
lock_kernel();
err = -EIO;
@@ -666,7 +670,9 @@ static int udf_mkdir(struct inode *dir, struct dentry *dentry, int mode)
struct udf_inode_info *dinfo = UDF_I(dir);
struct udf_inode_info *iinfo;
- dquot_initialize(dir);
+ err = dquot_initialize(dir);
+ if (err)
+ return err;
lock_kernel();
err = -EMLINK;
@@ -805,7 +811,9 @@ static int udf_rmdir(struct inode *dir, struct dentry *dentry)
struct fileIdentDesc *fi, cfi;
struct kernel_lb_addr tloc;
- dquot_initialize(dir);
+ retval = dquot_initialize(dir);
+ if (retval)
+ return retval;
retval = -ENOENT;
lock_kernel();
@@ -853,7 +861,9 @@ static int udf_unlink(struct inode *dir, struct dentry *dentry)
struct fileIdentDesc cfi;
struct kernel_lb_addr tloc;
- dquot_initialize(dir);
+ retval = dquot_initialize(dir);
+ if (retval)
+ return retval;
retval = -ENOENT;
lock_kernel();
@@ -909,7 +919,9 @@ static int udf_symlink(struct inode *dir, struct dentry *dentry,
struct buffer_head *bh;
struct udf_inode_info *iinfo;
- dquot_initialize(dir);
+ err = dquot_initialize(dir);
+ if (err)
+ return err;
lock_kernel();
inode = udf_new_inode(dir, S_IFLNK, &err);
@@ -1081,7 +1093,9 @@ static int udf_link(struct dentry *old_dentry, struct inode *dir,
int err;
struct buffer_head *bh;
- dquot_initialize(dir);
+ err = dquot_initialize(dir);
+ if (err)
+ return err;
lock_kernel();
if (inode->i_nlink >= (256 << sizeof(inode->i_nlink)) - 1) {
@@ -1141,13 +1155,18 @@ static int udf_rename(struct inode *old_dir, struct dentry *old_dentry,
struct fileIdentDesc *ofi = NULL, *nfi = NULL, *dir_fi = NULL;
struct fileIdentDesc ocfi, ncfi;
struct buffer_head *dir_bh = NULL;
- int retval = -ENOENT;
+ int retval;
struct kernel_lb_addr tloc;
struct udf_inode_info *old_iinfo = UDF_I(old_inode);
- dquot_initialize(old_dir);
- dquot_initialize(new_dir);
+ retval = dquot_initialize(old_dir);
+ if (retval)
+ return retval;
+ retval = dquot_initialize(new_dir);
+ if (retval)
+ return retval;
+ retval = -ENOENT;
lock_kernel();
ofi = udf_find_entry(old_dir, &old_dentry->d_name, &ofibh, &ocfi);
if (ofi) {
--
1.6.6.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 07/12] reiserfs: add error handling for dquot_initialize
2010-05-19 6:02 ` [PATCH 06/12] udf: " Dmitry Monakhov
@ 2010-05-19 6:02 ` Dmitry Monakhov
2010-05-19 6:02 ` [PATCH 08/12] ocfs2: " Dmitry Monakhov
2010-05-20 17:34 ` [PATCH 06/12] udf: " Jan Kara
1 sibling, 1 reply; 20+ messages in thread
From: Dmitry Monakhov @ 2010-05-19 6:02 UTC (permalink / raw)
To: linux-fsdevel; +Cc: jack, hch, Dmitry Monakhov
Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
fs/reiserfs/inode.c | 10 +++++--
fs/reiserfs/namei.c | 64 +++++++++++++++++++++++++++++++++++++++------------
fs/reiserfs/super.c | 3 +-
3 files changed, 58 insertions(+), 19 deletions(-)
diff --git a/fs/reiserfs/inode.c b/fs/reiserfs/inode.c
index 0f22fda..83f893b 100644
--- a/fs/reiserfs/inode.c
+++ b/fs/reiserfs/inode.c
@@ -1769,7 +1769,9 @@ int reiserfs_new_inode(struct reiserfs_transaction_handle *th,
BUG_ON(!th->t_trans_id);
- dquot_initialize(inode);
+ err = dquot_initialize(inode);
+ if (err)
+ goto out_end_trans;
err = dquot_alloc_inode(inode);
if (err)
goto out_end_trans;
@@ -3076,8 +3078,10 @@ int reiserfs_setattr(struct dentry *dentry, struct iattr *attr)
ia_valid = attr->ia_valid &= ~(ATTR_KILL_SUID|ATTR_KILL_SGID);
depth = reiserfs_write_lock_once(inode->i_sb);
- if (is_quota_modification(inode, attr))
- dquot_initialize(inode);
+ if (is_quota_modification(inode, attr)) {
+ error = dquot_initialize(inode);
+ goto out;
+ }
if (attr->ia_valid & ATTR_SIZE) {
/* version 2 items will be caught by the s_maxbytes check
diff --git a/fs/reiserfs/namei.c b/fs/reiserfs/namei.c
index d0c43cb..05c0616 100644
--- a/fs/reiserfs/namei.c
+++ b/fs/reiserfs/namei.c
@@ -578,8 +578,7 @@ static int new_inode_init(struct inode *inode, struct inode *dir, int mode)
} else {
inode->i_gid = current_fsgid();
}
- dquot_initialize(inode);
- return 0;
+ return dquot_initialize(inode);
}
static int reiserfs_create(struct inode *dir, struct dentry *dentry, int mode,
@@ -595,12 +594,18 @@ static int reiserfs_create(struct inode *dir, struct dentry *dentry, int mode,
struct reiserfs_transaction_handle th;
struct reiserfs_security_handle security;
- dquot_initialize(dir);
+ retval = dquot_initialize(dir);
+ if (retval)
+ return retval;
if (!(inode = new_inode(dir->i_sb))) {
return -ENOMEM;
}
- new_inode_init(inode, dir, mode);
+ retval = new_inode_init(inode, dir, mode);
+ if (retval) {
+ drop_new_inode(inode);
+ return retval;
+ }
jbegin_count += reiserfs_cache_default_acl(dir);
retval = reiserfs_security_init(dir, inode, &security);
@@ -669,12 +674,18 @@ static int reiserfs_mknod(struct inode *dir, struct dentry *dentry, int mode,
if (!new_valid_dev(rdev))
return -EINVAL;
- dquot_initialize(dir);
+ retval = dquot_initialize(dir);
+ if (retval)
+ return retval;
if (!(inode = new_inode(dir->i_sb))) {
return -ENOMEM;
}
- new_inode_init(inode, dir, mode);
+ retval = new_inode_init(inode, dir, mode);
+ if (retval) {
+ drop_new_inode(inode);
+ return retval;
+ }
jbegin_count += reiserfs_cache_default_acl(dir);
retval = reiserfs_security_init(dir, inode, &security);
@@ -744,7 +755,9 @@ static int reiserfs_mkdir(struct inode *dir, struct dentry *dentry, int mode)
2 * (REISERFS_QUOTA_INIT_BLOCKS(dir->i_sb) +
REISERFS_QUOTA_TRANS_BLOCKS(dir->i_sb));
- dquot_initialize(dir);
+ retval = dquot_initialize(dir);
+ if (retval)
+ return retval;
#ifdef DISPLACE_NEW_PACKING_LOCALITIES
/* set flag that new packing locality created and new blocks for the content * of that directory are not displaced yet */
@@ -754,7 +767,11 @@ static int reiserfs_mkdir(struct inode *dir, struct dentry *dentry, int mode)
if (!(inode = new_inode(dir->i_sb))) {
return -ENOMEM;
}
- new_inode_init(inode, dir, mode);
+ retval = new_inode_init(inode, dir, mode);
+ if (retval) {
+ drop_new_inode(inode);
+ return retval;
+ }
jbegin_count += reiserfs_cache_default_acl(dir);
retval = reiserfs_security_init(dir, inode, &security);
@@ -849,7 +866,9 @@ static int reiserfs_rmdir(struct inode *dir, struct dentry *dentry)
JOURNAL_PER_BALANCE_CNT * 2 + 2 +
4 * REISERFS_QUOTA_TRANS_BLOCKS(dir->i_sb);
- dquot_initialize(dir);
+ retval = dquot_initialize(dir);
+ if (retval)
+ return retval;
reiserfs_write_lock(dir->i_sb);
retval = journal_begin(&th, dir->i_sb, jbegin_count);
@@ -932,7 +951,9 @@ static int reiserfs_unlink(struct inode *dir, struct dentry *dentry)
unsigned long savelink;
int depth;
- dquot_initialize(dir);
+ retval = dquot_initialize(dir);
+ if (retval)
+ return retval;
inode = dentry->d_inode;
@@ -1035,12 +1056,18 @@ static int reiserfs_symlink(struct inode *parent_dir,
2 * (REISERFS_QUOTA_INIT_BLOCKS(parent_dir->i_sb) +
REISERFS_QUOTA_TRANS_BLOCKS(parent_dir->i_sb));
- dquot_initialize(parent_dir);
+ retval = dquot_initialize(parent_dir);
+ if (retval)
+ return retval;
if (!(inode = new_inode(parent_dir->i_sb))) {
return -ENOMEM;
}
- new_inode_init(inode, parent_dir, mode);
+ retval = new_inode_init(inode, parent_dir, mode);
+ if (retval) {
+ drop_new_inode(inode);
+ return retval;
+ }
retval = reiserfs_security_init(parent_dir, inode, &security);
if (retval < 0) {
@@ -1124,7 +1151,9 @@ static int reiserfs_link(struct dentry *old_dentry, struct inode *dir,
JOURNAL_PER_BALANCE_CNT * 3 +
2 * REISERFS_QUOTA_TRANS_BLOCKS(dir->i_sb);
- dquot_initialize(dir);
+ retval = dquot_initialize(dir);
+ if (retval)
+ return retval;
reiserfs_write_lock(dir->i_sb);
if (inode->i_nlink >= REISERFS_LINK_MAX) {
@@ -1250,8 +1279,13 @@ static int reiserfs_rename(struct inode *old_dir, struct dentry *old_dentry,
JOURNAL_PER_BALANCE_CNT * 3 + 5 +
4 * REISERFS_QUOTA_TRANS_BLOCKS(old_dir->i_sb);
- dquot_initialize(old_dir);
- dquot_initialize(new_dir);
+ retval = dquot_initialize(old_dir);
+ if (retval)
+ return retval;
+
+ retval = dquot_initialize(new_dir);
+ if (retval)
+ return retval;
old_inode = old_dentry->d_inode;
new_dentry_inode = new_dentry->d_inode;
diff --git a/fs/reiserfs/super.c b/fs/reiserfs/super.c
index 59125fb..9ccb948 100644
--- a/fs/reiserfs/super.c
+++ b/fs/reiserfs/super.c
@@ -247,7 +247,8 @@ static int finish_unfinished(struct super_block *s)
retval = remove_save_link_only(s, &save_link_key, 0);
continue;
}
- dquot_initialize(inode);
+ retval = dquot_initialize(inode);
+ /* XXX don't know what to do with error here */
if (truncate && S_ISDIR(inode->i_mode)) {
/* We got a truncate request for a dir which is impossible.
--
1.6.6.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 08/12] ocfs2: add error handling for dquot_initialize
2010-05-19 6:02 ` [PATCH 07/12] reiserfs: " Dmitry Monakhov
@ 2010-05-19 6:02 ` Dmitry Monakhov
2010-05-19 6:02 ` [PATCH 09/12] jfs: " Dmitry Monakhov
0 siblings, 1 reply; 20+ messages in thread
From: Dmitry Monakhov @ 2010-05-19 6:02 UTC (permalink / raw)
To: linux-fsdevel; +Cc: jack, hch, Dmitry Monakhov
Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
fs/ocfs2/file.c | 16 ++++++++---
fs/ocfs2/inode.c | 5 +++-
fs/ocfs2/namei.c | 64 ++++++++++++++++++++++++++++++++++++----------
fs/ocfs2/refcounttree.c | 5 ++-
4 files changed, 68 insertions(+), 22 deletions(-)
diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
index 92d91a1..02003c0 100644
--- a/fs/ocfs2/file.c
+++ b/fs/ocfs2/file.c
@@ -107,9 +107,11 @@ static int ocfs2_file_open(struct inode *inode, struct file *file)
mlog_entry("(0x%p, 0x%p, '%.*s')\n", inode, file,
file->f_path.dentry->d_name.len, file->f_path.dentry->d_name.name);
- if (file->f_mode & FMODE_WRITE)
- dquot_initialize(inode);
-
+ if (file->f_mode & FMODE_WRITE) {
+ status = dquot_initialize(inode);
+ if (status)
+ goto leave;
+ }
spin_lock(&oi->ip_lock);
/* Check that the inode hasn't been wiped from disk by another
@@ -978,8 +980,12 @@ int ocfs2_setattr(struct dentry *dentry, struct iattr *attr)
if (status)
return status;
- if (is_quota_modification(inode, attr))
- dquot_initialize(inode);
+ if (is_quota_modification(inode, attr)) {
+ status = dquot_initialize(inode);
+ if (status)
+ return status;
+ }
+
size_change = S_ISREG(inode->i_mode) && attr->ia_valid & ATTR_SIZE;
if (size_change) {
status = ocfs2_rw_lock(inode, 1);
diff --git a/fs/ocfs2/inode.c b/fs/ocfs2/inode.c
index af18988..605a12a 100644
--- a/fs/ocfs2/inode.c
+++ b/fs/ocfs2/inode.c
@@ -993,7 +993,10 @@ void ocfs2_delete_inode(struct inode *inode)
goto bail;
}
- dquot_initialize(inode);
+ status = dquot_initialize(inode);
+ /* Even when quota wasn't initialized we can't faile the operation */
+ if (status)
+ mlog_errno(status);
if (!ocfs2_inode_is_valid_to_delete(inode)) {
/* It's probably not necessary to truncate_inode_pages
diff --git a/fs/ocfs2/namei.c b/fs/ocfs2/namei.c
index 4cbb18f..e955b8a 100644
--- a/fs/ocfs2/namei.c
+++ b/fs/ocfs2/namei.c
@@ -190,11 +190,12 @@ bail:
static struct inode *ocfs2_get_init_inode(struct inode *dir, int mode)
{
struct inode *inode;
+ int status;
inode = new_inode(dir->i_sb);
if (!inode) {
mlog(ML_ERROR, "new_inode failed!\n");
- return NULL;
+ return ERR_PTR(-ENOMEM);
}
/* populate as many fields early on as possible - many of
@@ -212,7 +213,14 @@ static struct inode *ocfs2_get_init_inode(struct inode *dir, int mode)
} else
inode->i_gid = current_fsgid();
inode->i_mode = mode;
- dquot_initialize(inode);
+ status = dquot_initialize(inode);
+ if (status) {
+ clear_nlink(inode);
+ iput(inode);
+ inode = ERR_PTR(status);
+ mlog(ML_ERROR, "new_inode failed!\n");
+ }
+
return inode;
}
@@ -244,7 +252,11 @@ static int ocfs2_mknod(struct inode *dir,
(unsigned long)dev, dentry->d_name.len,
dentry->d_name.name);
- dquot_initialize(dir);
+ status = dquot_initialize(dir);
+ if (status) {
+ mlog_errno(status);
+ return status;
+ }
/* get our super block */
osb = OCFS2_SB(dir->i_sb);
@@ -291,9 +303,10 @@ static int ocfs2_mknod(struct inode *dir,
}
inode = ocfs2_get_init_inode(dir, mode);
- if (!inode) {
- status = -ENOMEM;
+ if (IS_ERR(inode)) {
+ status = PTR_ERR(inode);
mlog_errno(status);
+ inode = NULL;
goto leave;
}
@@ -645,7 +658,11 @@ static int ocfs2_link(struct dentry *old_dentry,
if (S_ISDIR(inode->i_mode))
return -EPERM;
- dquot_initialize(dir);
+ err = dquot_initialize(dir);
+ if (err) {
+ mlog_errno(err);
+ return err;
+ }
err = ocfs2_inode_lock_nested(dir, &parent_fe_bh, 1, OI_LS_PARENT);
if (err < 0) {
@@ -802,7 +819,11 @@ static int ocfs2_unlink(struct inode *dir,
mlog_entry("(0x%p, 0x%p, '%.*s')\n", dir, dentry,
dentry->d_name.len, dentry->d_name.name);
- dquot_initialize(dir);
+ status = dquot_initialize(dir);
+ if (status) {
+ mlog_errno(status);
+ return status;
+ }
BUG_ON(dentry->d_parent->d_inode != dir);
@@ -1064,8 +1085,17 @@ static int ocfs2_rename(struct inode *old_dir,
old_dentry->d_name.len, old_dentry->d_name.name,
new_dentry->d_name.len, new_dentry->d_name.name);
- dquot_initialize(old_dir);
- dquot_initialize(new_dir);
+ status = dquot_initialize(old_dir);
+ if (status) {
+ mlog_errno(status);
+ return status;
+ }
+ status = dquot_initialize(new_dir);
+ if (status) {
+ mlog_errno(status);
+ return status;
+ }
+
osb = OCFS2_SB(old_dir->i_sb);
@@ -1615,7 +1645,11 @@ static int ocfs2_symlink(struct inode *dir,
mlog_entry("(0x%p, 0x%p, symname='%s' actual='%.*s')\n", dir,
dentry, symname, dentry->d_name.len, dentry->d_name.name);
- dquot_initialize(dir);
+ status = dquot_initialize(dir);
+ if (status) {
+ mlog_errno(status);
+ return status;
+ }
sb = dir->i_sb;
osb = OCFS2_SB(sb);
@@ -1660,9 +1694,10 @@ static int ocfs2_symlink(struct inode *dir,
}
inode = ocfs2_get_init_inode(dir, S_IFLNK | S_IRWXUGO);
- if (!inode) {
- status = -ENOMEM;
+ if (IS_ERR(inode)) {
+ status = PTR_ERR(inode);
mlog_errno(status);
+ inode = NULL;
goto bail;
}
@@ -2123,9 +2158,10 @@ int ocfs2_create_inode_in_orphan(struct inode *dir,
}
inode = ocfs2_get_init_inode(dir, mode);
- if (!inode) {
- status = -ENOMEM;
+ if (IS_ERR(inode)) {
+ status = PTR_ERR(inode);
mlog_errno(status);
+ inode = NULL;
goto leave;
}
diff --git a/fs/ocfs2/refcounttree.c b/fs/ocfs2/refcounttree.c
index 5cbcd0f..7b4151c 100644
--- a/fs/ocfs2/refcounttree.c
+++ b/fs/ocfs2/refcounttree.c
@@ -4393,8 +4393,9 @@ static int ocfs2_vfs_reflink(struct dentry *old_dentry, struct inode *dir,
}
mutex_lock(&inode->i_mutex);
- dquot_initialize(dir);
- error = ocfs2_reflink(old_dentry, dir, new_dentry, preserve);
+ error = dquot_initialize(dir);
+ if (!error)
+ error = ocfs2_reflink(old_dentry, dir, new_dentry, preserve);
mutex_unlock(&inode->i_mutex);
if (!error)
fsnotify_create(dir, new_dentry);
--
1.6.6.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 09/12] jfs: add error handling for dquot_initialize
2010-05-19 6:02 ` [PATCH 08/12] ocfs2: " Dmitry Monakhov
@ 2010-05-19 6:02 ` Dmitry Monakhov
2010-05-19 6:02 ` [PATCH 10/12] ext4: " Dmitry Monakhov
0 siblings, 1 reply; 20+ messages in thread
From: Dmitry Monakhov @ 2010-05-19 6:02 UTC (permalink / raw)
To: linux-fsdevel; +Cc: jack, hch, Dmitry Monakhov
Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
fs/jfs/file.c | 7 ++++-
fs/jfs/jfs_inode.c | 5 +++-
fs/jfs/namei.c | 59 +++++++++++++++++++++++++++++++++++++--------------
3 files changed, 52 insertions(+), 19 deletions(-)
diff --git a/fs/jfs/file.c b/fs/jfs/file.c
index 85d9ec6..cb0da27 100644
--- a/fs/jfs/file.c
+++ b/fs/jfs/file.c
@@ -98,8 +98,11 @@ int jfs_setattr(struct dentry *dentry, struct iattr *iattr)
if (rc)
return rc;
- if (is_quota_modification(inode, iattr))
- dquot_initialize(inode);
+ if (is_quota_modification(inode, iattr)) {
+ rc = dquot_initialize(inode);
+ if (rc)
+ return rc;
+ }
if ((iattr->ia_valid & ATTR_UID && iattr->ia_uid != inode->i_uid) ||
(iattr->ia_valid & ATTR_GID && iattr->ia_gid != inode->i_gid)) {
rc = dquot_transfer(inode, iattr);
diff --git a/fs/jfs/jfs_inode.c b/fs/jfs/jfs_inode.c
index 829921b..8d18c95 100644
--- a/fs/jfs/jfs_inode.c
+++ b/fs/jfs/jfs_inode.c
@@ -116,7 +116,10 @@ struct inode *ialloc(struct inode *parent, umode_t mode)
/*
* Allocate inode to quota.
*/
- dquot_initialize(inode);
+ rc = dquot_initialize(inode);
+ if (rc)
+ goto fail_unlock;
+
rc = dquot_alloc_inode(inode);
if (rc)
goto fail_drop;
diff --git a/fs/jfs/namei.c b/fs/jfs/namei.c
index a9cf8e8..96d40a6 100644
--- a/fs/jfs/namei.c
+++ b/fs/jfs/namei.c
@@ -85,7 +85,9 @@ static int jfs_create(struct inode *dip, struct dentry *dentry, int mode,
jfs_info("jfs_create: dip:0x%p name:%s", dip, dentry->d_name.name);
- dquot_initialize(dip);
+ rc = dquot_initialize(dip);
+ if (rc)
+ goto out1;
/*
* search parent directory for entry/freespace
@@ -217,7 +219,9 @@ static int jfs_mkdir(struct inode *dip, struct dentry *dentry, int mode)
jfs_info("jfs_mkdir: dip:0x%p name:%s", dip, dentry->d_name.name);
- dquot_initialize(dip);
+ rc = dquot_initialize(dip);
+ if (rc)
+ goto out1;
/* link count overflow on parent directory ? */
if (dip->i_nlink == JFS_LINK_MAX) {
@@ -360,8 +364,12 @@ static int jfs_rmdir(struct inode *dip, struct dentry *dentry)
jfs_info("jfs_rmdir: dip:0x%p name:%s", dip, dentry->d_name.name);
/* Init inode for quota operations. */
- dquot_initialize(dip);
- dquot_initialize(ip);
+ rc = dquot_initialize(dip);
+ if (rc)
+ goto out;
+ rc = dquot_initialize(ip);
+ if (rc)
+ goto out;
/* directory must be empty to be removed */
if (!dtEmpty(ip)) {
@@ -488,8 +496,12 @@ static int jfs_unlink(struct inode *dip, struct dentry *dentry)
jfs_info("jfs_unlink: dip:0x%p name:%s", dip, dentry->d_name.name);
/* Init inode for quota operations. */
- dquot_initialize(dip);
- dquot_initialize(ip);
+ rc = dquot_initialize(dip);
+ if (rc)
+ goto out;
+ rc = dquot_initialize(ip);
+ if (rc)
+ goto out;
if ((rc = get_UCSname(&dname, dentry)))
goto out;
@@ -811,7 +823,9 @@ static int jfs_link(struct dentry *old_dentry,
if (ip->i_nlink == 0)
return -ENOENT;
- dquot_initialize(dir);
+ rc = dquot_initialize(dir);
+ if (rc)
+ return rc;
tid = txBegin(ip->i_sb, 0);
@@ -904,7 +918,9 @@ static int jfs_symlink(struct inode *dip, struct dentry *dentry,
jfs_info("jfs_symlink: dip:0x%p name:%s", dip, name);
- dquot_initialize(dip);
+ rc = dquot_initialize(dip);
+ if(rc)
+ goto out1;
ssize = strlen(name) + 1;
@@ -1097,12 +1113,24 @@ static int jfs_rename(struct inode *old_dir, struct dentry *old_dentry,
jfs_info("jfs_rename: %s %s", old_dentry->d_name.name,
new_dentry->d_name.name);
- dquot_initialize(old_dir);
- dquot_initialize(new_dir);
-
old_ip = old_dentry->d_inode;
new_ip = new_dentry->d_inode;
+ /* Init inode for quota operations. */
+ rc = dquot_initialize(old_dir);
+ if (rc)
+ goto out1;
+
+ rc = dquot_initialize(new_dir);
+ if(rc)
+ goto out1;
+
+ if (new_ip) {
+ rc = dquot_initialize(new_ip);
+ if(rc)
+ goto out1;
+ }
+
if ((rc = get_UCSname(&old_dname, old_dentry)))
goto out1;
@@ -1146,11 +1174,8 @@ static int jfs_rename(struct inode *old_dir, struct dentry *old_dentry,
rc = -EMLINK;
goto out3;
}
- } else if (new_ip) {
+ } else if (new_ip)
IWRITE_LOCK(new_ip, RDWRLOCK_NORMAL);
- /* Init inode for quota operations. */
- dquot_initialize(new_ip);
- }
/*
* The real work starts here
@@ -1373,7 +1398,9 @@ static int jfs_mknod(struct inode *dir, struct dentry *dentry,
jfs_info("jfs_mknod: %s", dentry->d_name.name);
- dquot_initialize(dir);
+ rc = dquot_initialize(dir);
+ if (rc)
+ goto out;
if ((rc = get_UCSname(&dname, dentry)))
goto out;
--
1.6.6.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 10/12] ext4: add error handling for dquot_initialize
2010-05-19 6:02 ` [PATCH 09/12] jfs: " Dmitry Monakhov
@ 2010-05-19 6:02 ` Dmitry Monakhov
2010-05-19 6:02 ` [PATCH 11/12] ext3: " Dmitry Monakhov
0 siblings, 1 reply; 20+ messages in thread
From: Dmitry Monakhov @ 2010-05-19 6:02 UTC (permalink / raw)
To: linux-fsdevel; +Cc: jack, hch, Dmitry Monakhov
Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
fs/ext4/ialloc.c | 5 +++-
fs/ext4/inode.c | 18 +++++++++++++---
fs/ext4/namei.c | 55 ++++++++++++++++++++++++++++++++++++++++-------------
fs/ext4/super.c | 9 +++++++-
4 files changed, 67 insertions(+), 20 deletions(-)
diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
index 57f6eef..83265e4 100644
--- a/fs/ext4/ialloc.c
+++ b/fs/ext4/ialloc.c
@@ -1029,7 +1029,10 @@ got:
ei->i_extra_isize = EXT4_SB(sb)->s_want_extra_isize;
ret = inode;
- dquot_initialize(inode);
+ err = dquot_initialize(inode);
+ if(err)
+ goto fail_drop;
+
err = dquot_alloc_inode(inode);
if (err)
goto fail_drop;
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 3e0f6af..10800e9 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -172,8 +172,15 @@ void ext4_delete_inode(struct inode *inode)
handle_t *handle;
int err;
- if (!is_bad_inode(inode))
- dquot_initialize(inode);
+ if (!is_bad_inode(inode)) {
+ int ret = dquot_initialize(inode);
+ if (ret)
+ ext4_error(inode->i_sb, "Fail to init_quota "
+ "for inode %lu, uid %d, gid:%d, error%d, thus "
+ "quota information is probably inconsistent, "
+ "Please run quotacheck(8)", inode->i_ino,
+ inode->i_uid, inode->i_gid, ret);
+ }
if (ext4_should_order_data(inode))
ext4_begin_ordered_truncate(inode, 0);
@@ -5425,8 +5432,11 @@ int ext4_setattr(struct dentry *dentry, struct iattr *attr)
if (error)
return error;
- if (is_quota_modification(inode, attr))
- dquot_initialize(inode);
+ if (is_quota_modification(inode, attr)) {
+ error = dquot_initialize(inode);
+ if (error)
+ return error;
+ }
if ((ia_valid & ATTR_UID && attr->ia_uid != inode->i_uid) ||
(ia_valid & ATTR_GID && attr->ia_gid != inode->i_gid)) {
handle_t *handle;
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index 0c070fa..7832549 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -1759,8 +1759,9 @@ static int ext4_create(struct inode *dir, struct dentry *dentry, int mode,
struct inode *inode;
int err, retries = 0;
- dquot_initialize(dir);
-
+ err = dquot_initialize(dir);
+ if (err)
+ return err;
retry:
handle = ext4_journal_start(dir, EXT4_DATA_TRANS_BLOCKS(dir->i_sb) +
EXT4_INDEX_EXTRA_TRANS_BLOCKS + 3 +
@@ -1795,7 +1796,9 @@ static int ext4_mknod(struct inode *dir, struct dentry *dentry,
if (!new_valid_dev(rdev))
return -EINVAL;
- dquot_initialize(dir);
+ err = dquot_initialize(dir);
+ if (err)
+ return err;
retry:
handle = ext4_journal_start(dir, EXT4_DATA_TRANS_BLOCKS(dir->i_sb) +
@@ -1834,7 +1837,9 @@ static int ext4_mkdir(struct inode *dir, struct dentry *dentry, int mode)
if (EXT4_DIR_LINK_MAX(dir))
return -EMLINK;
- dquot_initialize(dir);
+ err = dquot_initialize(dir);
+ if (err)
+ return err;
retry:
handle = ext4_journal_start(dir, EXT4_DATA_TRANS_BLOCKS(dir->i_sb) +
@@ -2143,8 +2148,13 @@ static int ext4_rmdir(struct inode *dir, struct dentry *dentry)
/* Initialize quotas before so that eventual writes go in
* separate transaction */
- dquot_initialize(dir);
- dquot_initialize(dentry->d_inode);
+ retval = dquot_initialize(dir);
+ if (retval)
+ return retval;
+
+ retval = dquot_initialize(dentry->d_inode);
+ if (retval)
+ return retval;
handle = ext4_journal_start(dir, EXT4_DELETE_TRANS_BLOCKS(dir->i_sb));
if (IS_ERR(handle))
@@ -2204,8 +2214,13 @@ static int ext4_unlink(struct inode *dir, struct dentry *dentry)
/* Initialize quotas before so that eventual writes go
* in separate transaction */
- dquot_initialize(dir);
- dquot_initialize(dentry->d_inode);
+ retval = dquot_initialize(dir);
+ if (retval)
+ return retval;
+
+ retval = dquot_initialize(dentry->d_inode);
+ if (retval)
+ return retval;
handle = ext4_journal_start(dir, EXT4_DELETE_TRANS_BLOCKS(dir->i_sb));
if (IS_ERR(handle))
@@ -2261,7 +2276,9 @@ static int ext4_symlink(struct inode *dir,
if (l > dir->i_sb->s_blocksize)
return -ENAMETOOLONG;
- dquot_initialize(dir);
+ err = dquot_initialize(dir);
+ if (err)
+ return err;
retry:
handle = ext4_journal_start(dir, EXT4_DATA_TRANS_BLOCKS(dir->i_sb) +
@@ -2321,7 +2338,9 @@ static int ext4_link(struct dentry *old_dentry,
if (inode->i_nlink >= EXT4_LINK_MAX)
return -EMLINK;
- dquot_initialize(dir);
+ err = dquot_initialize(dir);
+ if (err)
+ return err;
/*
* Return -ENOENT if we've raced with unlink and i_nlink is 0. Doing
@@ -2373,15 +2392,23 @@ static int ext4_rename(struct inode *old_dir, struct dentry *old_dentry,
struct ext4_dir_entry_2 *old_de, *new_de;
int retval, force_da_alloc = 0;
- dquot_initialize(old_dir);
- dquot_initialize(new_dir);
+ retval = dquot_initialize(old_dir);
+ if (retval)
+ return retval;
+
+ retval = dquot_initialize(new_dir);
+ if (retval)
+ return retval;
old_bh = new_bh = dir_bh = NULL;
/* Initialize quotas before so that eventual writes go
* in separate transaction */
- if (new_dentry->d_inode)
- dquot_initialize(new_dentry->d_inode);
+ if (new_dentry->d_inode) {
+ retval = dquot_initialize(new_dentry->d_inode);
+ if (retval)
+ return retval;
+ }
handle = ext4_journal_start(old_dir, 2 *
EXT4_DATA_TRANS_BLOCKS(old_dir->i_sb) +
EXT4_INDEX_EXTRA_TRANS_BLOCKS + 2);
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index a794707..d3a1920 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -2022,7 +2022,14 @@ static int ext4_orphan_cleanup(struct super_block *sb,
}
list_add(&EXT4_I(inode)->i_orphan, &EXT4_SB(sb)->s_orphan);
- dquot_initialize(inode);
+ ret = dquot_initialize(inode);
+ if (ret) {
+ ext4_msg(sb, KERN_ERR, "Can not initialize quota for "
+ "inode:%lu error %d",
+ inode->i_ino, ret);
+ if (!test_opt (sb, ERRORS_CONT))
+ goto out;
+ }
if (inode->i_nlink) {
ext4_msg(sb, KERN_DEBUG,
"%s: truncating inode %lu to %lld bytes",
--
1.6.6.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 11/12] ext3: add error handling for dquot_initialize
2010-05-19 6:02 ` [PATCH 10/12] ext4: " Dmitry Monakhov
@ 2010-05-19 6:02 ` Dmitry Monakhov
2010-05-19 6:02 ` [PATCH 12/12] ext2: " Dmitry Monakhov
2010-05-20 17:32 ` [PATCH 11/12] ext3: " Jan Kara
0 siblings, 2 replies; 20+ messages in thread
From: Dmitry Monakhov @ 2010-05-19 6:02 UTC (permalink / raw)
To: linux-fsdevel; +Cc: jack, hch, Dmitry Monakhov
Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
fs/ext3/ialloc.c | 6 +++++-
fs/ext3/inode.c | 19 ++++++++++++++-----
fs/ext3/namei.c | 54 ++++++++++++++++++++++++++++++++++++++++--------------
fs/ext3/super.c | 9 ++++++++-
4 files changed, 67 insertions(+), 21 deletions(-)
diff --git a/fs/ext3/ialloc.c b/fs/ext3/ialloc.c
index 0d0e97e..6757cde 100644
--- a/fs/ext3/ialloc.c
+++ b/fs/ext3/ialloc.c
@@ -590,7 +590,11 @@ got:
sizeof(struct ext3_inode) - EXT3_GOOD_OLD_INODE_SIZE : 0;
ret = inode;
- dquot_initialize(inode);
+
+ err = dquot_initialize(inode);
+ if (err)
+ goto fail_drop;
+
err = dquot_alloc_inode(inode);
if (err)
goto fail_drop;
diff --git a/fs/ext3/inode.c b/fs/ext3/inode.c
index 735f019..8497b03 100644
--- a/fs/ext3/inode.c
+++ b/fs/ext3/inode.c
@@ -196,9 +196,15 @@ void ext3_delete_inode (struct inode * inode)
{
handle_t *handle;
- if (!is_bad_inode(inode))
- dquot_initialize(inode);
-
+ if (!is_bad_inode(inode)) {
+ int ret = dquot_initialize(inode);
+ if (ret)
+ ext3_error(inode->i_sb, __func__, "Fail to init_quota "
+ "for inode %lu, uid %d, gid:%d, error%d, thus "
+ "quota information is probably inconsistent, "
+ "Please run quotacheck(8)", inode->i_ino,
+ inode->i_uid, inode->i_gid, ret);
+ }
truncate_inode_pages(&inode->i_data, 0);
if (is_bad_inode(inode))
@@ -3151,8 +3157,11 @@ int ext3_setattr(struct dentry *dentry, struct iattr *attr)
if (error)
return error;
- if (is_quota_modification(inode, attr))
- dquot_initialize(inode);
+ if (is_quota_modification(inode, attr)) {
+ error = dquot_initialize(inode);
+ if (error)
+ return error;
+ }
if ((ia_valid & ATTR_UID && attr->ia_uid != inode->i_uid) ||
(ia_valid & ATTR_GID && attr->ia_gid != inode->i_gid)) {
handle_t *handle;
diff --git a/fs/ext3/namei.c b/fs/ext3/namei.c
index ee18408..4813ebf 100644
--- a/fs/ext3/namei.c
+++ b/fs/ext3/namei.c
@@ -1696,8 +1696,9 @@ static int ext3_create (struct inode * dir, struct dentry * dentry, int mode,
struct inode * inode;
int err, retries = 0;
- dquot_initialize(dir);
-
+ err = dquot_initialize(dir);
+ if (err)
+ return err;
retry:
handle = ext3_journal_start(dir, EXT3_DATA_TRANS_BLOCKS(dir->i_sb) +
EXT3_INDEX_EXTRA_TRANS_BLOCKS + 3 +
@@ -1732,7 +1733,9 @@ static int ext3_mknod (struct inode * dir, struct dentry *dentry,
if (!new_valid_dev(rdev))
return -EINVAL;
- dquot_initialize(dir);
+ err = dquot_initialize(dir);
+ if (err)
+ return err;
retry:
handle = ext3_journal_start(dir, EXT3_DATA_TRANS_BLOCKS(dir->i_sb) +
@@ -1770,7 +1773,9 @@ static int ext3_mkdir(struct inode * dir, struct dentry * dentry, int mode)
if (dir->i_nlink >= EXT3_LINK_MAX)
return -EMLINK;
- dquot_initialize(dir);
+ err = dquot_initialize(dir);
+ if (err)
+ return err;
retry:
handle = ext3_journal_start(dir, EXT3_DATA_TRANS_BLOCKS(dir->i_sb) +
@@ -2066,8 +2071,13 @@ static int ext3_rmdir (struct inode * dir, struct dentry *dentry)
/* Initialize quotas before so that eventual writes go in
* separate transaction */
- dquot_initialize(dir);
- dquot_initialize(dentry->d_inode);
+ retval = dquot_initialize(dir);
+ if (retval)
+ return retval;
+
+ retval = dquot_initialize(dentry->d_inode);
+ if (retval)
+ return retval;
handle = ext3_journal_start(dir, EXT3_DELETE_TRANS_BLOCKS(dir->i_sb));
if (IS_ERR(handle))
@@ -2127,8 +2137,13 @@ static int ext3_unlink(struct inode * dir, struct dentry *dentry)
/* Initialize quotas before so that eventual writes go
* in separate transaction */
- dquot_initialize(dir);
- dquot_initialize(dentry->d_inode);
+ retval = dquot_initialize(dir);
+ if (retval)
+ return retval;
+
+ retval = dquot_initialize(dentry->d_inode);
+ if (retval)
+ return retval;
handle = ext3_journal_start(dir, EXT3_DELETE_TRANS_BLOCKS(dir->i_sb));
if (IS_ERR(handle))
@@ -2184,7 +2199,9 @@ static int ext3_symlink (struct inode * dir,
if (l > dir->i_sb->s_blocksize)
return -ENAMETOOLONG;
- dquot_initialize(dir);
+ err = dquot_initialize(dir);
+ if (err)
+ return err;
retry:
handle = ext3_journal_start(dir, EXT3_DATA_TRANS_BLOCKS(dir->i_sb) +
@@ -2241,7 +2258,9 @@ static int ext3_link (struct dentry * old_dentry,
if (inode->i_nlink >= EXT3_LINK_MAX)
return -EMLINK;
- dquot_initialize(dir);
+ err = dquot_initialize(dir);
+ if (err)
+ return err;
/*
* Return -ENOENT if we've raced with unlink and i_nlink is 0. Doing
@@ -2293,15 +2312,22 @@ static int ext3_rename (struct inode * old_dir, struct dentry *old_dentry,
struct ext3_dir_entry_2 * old_de, * new_de;
int retval, flush_file = 0;
- dquot_initialize(old_dir);
- dquot_initialize(new_dir);
+ retval = dquot_initialize(old_dir);
+ if (retval)
+ return retval;
+ retval = dquot_initialize(new_dir);
+ if (retval)
+ return retval;
old_bh = new_bh = dir_bh = NULL;
/* Initialize quotas before so that eventual writes go
* in separate transaction */
- if (new_dentry->d_inode)
- dquot_initialize(new_dentry->d_inode);
+ if (new_dentry->d_inode) {
+ retval = dquot_initialize(new_dentry->d_inode);
+ if (retval)
+ return retval;
+ }
handle = ext3_journal_start(old_dir, 2 *
EXT3_DATA_TRANS_BLOCKS(old_dir->i_sb) +
EXT3_INDEX_EXTRA_TRANS_BLOCKS + 2);
diff --git a/fs/ext3/super.c b/fs/ext3/super.c
index 5ca068f..dd17196 100644
--- a/fs/ext3/super.c
+++ b/fs/ext3/super.c
@@ -1498,7 +1498,14 @@ static int ext3_orphan_cleanup (struct super_block * sb,
}
list_add(&EXT3_I(inode)->i_orphan, &EXT3_SB(sb)->s_orphan);
- dquot_initialize(inode);
+ ret = dquot_initialize(inode);
+ if (ret) {
+ ext3_msg(sb, KERN_ERR, "Can not initialize quota for "
+ "inode:%lu error %d",
+ inode->i_ino, ret);
+ if (!test_opt (sb, ERRORS_CONT))
+ goto out;
+ }
if (inode->i_nlink) {
printk(KERN_DEBUG
"%s: truncating inode %lu to %Ld bytes\n",
--
1.6.6.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 12/12] ext2: add error handling for dquot_initialize
2010-05-19 6:02 ` [PATCH 11/12] ext3: " Dmitry Monakhov
@ 2010-05-19 6:02 ` Dmitry Monakhov
2010-05-20 17:30 ` Jan Kara
2010-05-20 17:32 ` [PATCH 11/12] ext3: " Jan Kara
1 sibling, 1 reply; 20+ messages in thread
From: Dmitry Monakhov @ 2010-05-19 6:02 UTC (permalink / raw)
To: linux-fsdevel; +Cc: jack, hch, Dmitry Monakhov
Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
fs/ext2/ialloc.c | 5 ++++-
fs/ext2/inode.c | 11 +++++++++--
fs/ext2/namei.c | 40 ++++++++++++++++++++++++++++++----------
3 files changed, 43 insertions(+), 13 deletions(-)
diff --git a/fs/ext2/ialloc.c b/fs/ext2/ialloc.c
index f0c5286..2b3e739 100644
--- a/fs/ext2/ialloc.c
+++ b/fs/ext2/ialloc.c
@@ -585,7 +585,10 @@ got:
goto fail_drop;
}
- dquot_initialize(inode);
+ err = dquot_initialize(inode);
+ if (err)
+ goto fail_drop;
+
err = dquot_alloc_inode(inode);
if (err)
goto fail_drop;
diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
index 527c46d..6f6cf0e 100644
--- a/fs/ext2/inode.c
+++ b/fs/ext2/inode.c
@@ -59,8 +59,15 @@ static inline int ext2_inode_is_fast_symlink(struct inode *inode)
*/
void ext2_delete_inode (struct inode * inode)
{
- if (!is_bad_inode(inode))
- dquot_initialize(inode);
+ if (!is_bad_inode(inode)) {
+ int ret = dquot_initialize(inode);
+ if (ret)
+ ext2_error(inode->i_sb, __func__, "Fail to init_quota "
+ "for inode %lu, uid %d, gid:%d, error%d, thus "
+ "quota information is probably inconsistent, "
+ "Please run quotacheck(8)", inode->i_ino,
+ inode->i_uid, inode->i_gid, ret);
+ }
truncate_inode_pages(&inode->i_data, 0);
if (is_bad_inode(inode))
diff --git a/fs/ext2/namei.c b/fs/ext2/namei.c
index 71efb0e..d47e549 100644
--- a/fs/ext2/namei.c
+++ b/fs/ext2/namei.c
@@ -101,8 +101,11 @@ struct dentry *ext2_get_parent(struct dentry *child)
static int ext2_create (struct inode * dir, struct dentry * dentry, int mode, struct nameidata *nd)
{
struct inode *inode;
+ int err;
- dquot_initialize(dir);
+ err = dquot_initialize(dir);
+ if (err)
+ return err;
inode = ext2_new_inode(dir, mode);
if (IS_ERR(inode))
@@ -131,7 +134,9 @@ static int ext2_mknod (struct inode * dir, struct dentry *dentry, int mode, dev_
if (!new_valid_dev(rdev))
return -EINVAL;
- dquot_initialize(dir);
+ err = dquot_initialize(dir);
+ if (err)
+ return err;
inode = ext2_new_inode (dir, mode);
err = PTR_ERR(inode);
@@ -157,7 +162,9 @@ static int ext2_symlink (struct inode * dir, struct dentry * dentry,
if (l > sb->s_blocksize)
goto out;
- dquot_initialize(dir);
+ err = dquot_initialize(dir);
+ if (err)
+ return err;
inode = ext2_new_inode (dir, S_IFLNK | S_IRWXUGO);
err = PTR_ERR(inode);
@@ -202,7 +209,9 @@ static int ext2_link (struct dentry * old_dentry, struct inode * dir,
if (inode->i_nlink >= EXT2_LINK_MAX)
return -EMLINK;
- dquot_initialize(dir);
+ err = dquot_initialize(dir);
+ if (err)
+ return err;
inode->i_ctime = CURRENT_TIME_SEC;
inode_inc_link_count(inode);
@@ -226,7 +235,9 @@ static int ext2_mkdir(struct inode * dir, struct dentry * dentry, int mode)
if (dir->i_nlink >= EXT2_LINK_MAX)
goto out;
- dquot_initialize(dir);
+ err = dquot_initialize(dir);
+ if (err)
+ return err;
inode_inc_link_count(dir);
@@ -274,7 +285,9 @@ static int ext2_unlink(struct inode * dir, struct dentry *dentry)
struct page * page;
int err = -ENOENT;
- dquot_initialize(dir);
+ err = dquot_initialize(dir);
+ if (err)
+ return err;
de = ext2_find_entry (dir, &dentry->d_name, &page);
if (!de)
@@ -316,14 +329,21 @@ static int ext2_rename (struct inode * old_dir, struct dentry * old_dentry,
struct ext2_dir_entry_2 * dir_de = NULL;
struct page * old_page;
struct ext2_dir_entry_2 * old_de;
- int err = -ENOENT;
+ int err;
- dquot_initialize(old_dir);
- dquot_initialize(new_dir);
+ err = dquot_initialize(old_dir);
+ if (err)
+ return err;
+
+ err = dquot_initialize(new_dir);
+ if (err)
+ return err;
old_de = ext2_find_entry (old_dir, &old_dentry->d_name, &old_page);
- if (!old_de)
+ if (!old_de) {
+ err = -ENOENT;
goto out;
+ }
if (S_ISDIR(old_inode->i_mode)) {
err = -EIO;
--
1.6.6.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 12/12] ext2: add error handling for dquot_initialize
2010-05-19 6:02 ` [PATCH 12/12] ext2: " Dmitry Monakhov
@ 2010-05-20 17:30 ` Jan Kara
0 siblings, 0 replies; 20+ messages in thread
From: Jan Kara @ 2010-05-20 17:30 UTC (permalink / raw)
To: Dmitry Monakhov; +Cc: linux-fsdevel, jack, hch
On Wed 19-05-10 10:02:08, Dmitry Monakhov wrote:
>
> Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
> ---
> fs/ext2/ialloc.c | 5 ++++-
> fs/ext2/inode.c | 11 +++++++++--
> fs/ext2/namei.c | 40 ++++++++++++++++++++++++++++++----------
> 3 files changed, 43 insertions(+), 13 deletions(-)
>
...
> diff --git a/fs/ext2/namei.c b/fs/ext2/namei.c
> index 71efb0e..d47e549 100644
> --- a/fs/ext2/namei.c
> +++ b/fs/ext2/namei.c
> @@ -157,7 +162,9 @@ static int ext2_symlink (struct inode * dir, struct dentry * dentry,
> if (l > sb->s_blocksize)
> goto out;
>
> - dquot_initialize(dir);
> + err = dquot_initialize(dir);
> + if (err)
> + return err;
Should be 'goto out' for consistency and in other functions below as
well..
Honza
--
Jan Kara <jack@suse.cz>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 11/12] ext3: add error handling for dquot_initialize
2010-05-19 6:02 ` [PATCH 11/12] ext3: " Dmitry Monakhov
2010-05-19 6:02 ` [PATCH 12/12] ext2: " Dmitry Monakhov
@ 2010-05-20 17:32 ` Jan Kara
1 sibling, 0 replies; 20+ messages in thread
From: Jan Kara @ 2010-05-20 17:32 UTC (permalink / raw)
To: Dmitry Monakhov; +Cc: linux-fsdevel, jack, hch
On Wed 19-05-10 10:02:07, Dmitry Monakhov wrote:
The patch looks good.
Honza
>
> Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
> ---
> fs/ext3/ialloc.c | 6 +++++-
> fs/ext3/inode.c | 19 ++++++++++++++-----
> fs/ext3/namei.c | 54 ++++++++++++++++++++++++++++++++++++++++--------------
> fs/ext3/super.c | 9 ++++++++-
> 4 files changed, 67 insertions(+), 21 deletions(-)
>
> diff --git a/fs/ext3/ialloc.c b/fs/ext3/ialloc.c
> index 0d0e97e..6757cde 100644
> --- a/fs/ext3/ialloc.c
> +++ b/fs/ext3/ialloc.c
> @@ -590,7 +590,11 @@ got:
> sizeof(struct ext3_inode) - EXT3_GOOD_OLD_INODE_SIZE : 0;
>
> ret = inode;
> - dquot_initialize(inode);
> +
> + err = dquot_initialize(inode);
> + if (err)
> + goto fail_drop;
> +
> err = dquot_alloc_inode(inode);
> if (err)
> goto fail_drop;
> diff --git a/fs/ext3/inode.c b/fs/ext3/inode.c
> index 735f019..8497b03 100644
> --- a/fs/ext3/inode.c
> +++ b/fs/ext3/inode.c
> @@ -196,9 +196,15 @@ void ext3_delete_inode (struct inode * inode)
> {
> handle_t *handle;
>
> - if (!is_bad_inode(inode))
> - dquot_initialize(inode);
> -
> + if (!is_bad_inode(inode)) {
> + int ret = dquot_initialize(inode);
> + if (ret)
> + ext3_error(inode->i_sb, __func__, "Fail to init_quota "
> + "for inode %lu, uid %d, gid:%d, error%d, thus "
> + "quota information is probably inconsistent, "
> + "Please run quotacheck(8)", inode->i_ino,
> + inode->i_uid, inode->i_gid, ret);
> + }
> truncate_inode_pages(&inode->i_data, 0);
>
> if (is_bad_inode(inode))
> @@ -3151,8 +3157,11 @@ int ext3_setattr(struct dentry *dentry, struct iattr *attr)
> if (error)
> return error;
>
> - if (is_quota_modification(inode, attr))
> - dquot_initialize(inode);
> + if (is_quota_modification(inode, attr)) {
> + error = dquot_initialize(inode);
> + if (error)
> + return error;
> + }
> if ((ia_valid & ATTR_UID && attr->ia_uid != inode->i_uid) ||
> (ia_valid & ATTR_GID && attr->ia_gid != inode->i_gid)) {
> handle_t *handle;
> diff --git a/fs/ext3/namei.c b/fs/ext3/namei.c
> index ee18408..4813ebf 100644
> --- a/fs/ext3/namei.c
> +++ b/fs/ext3/namei.c
> @@ -1696,8 +1696,9 @@ static int ext3_create (struct inode * dir, struct dentry * dentry, int mode,
> struct inode * inode;
> int err, retries = 0;
>
> - dquot_initialize(dir);
> -
> + err = dquot_initialize(dir);
> + if (err)
> + return err;
> retry:
> handle = ext3_journal_start(dir, EXT3_DATA_TRANS_BLOCKS(dir->i_sb) +
> EXT3_INDEX_EXTRA_TRANS_BLOCKS + 3 +
> @@ -1732,7 +1733,9 @@ static int ext3_mknod (struct inode * dir, struct dentry *dentry,
> if (!new_valid_dev(rdev))
> return -EINVAL;
>
> - dquot_initialize(dir);
> + err = dquot_initialize(dir);
> + if (err)
> + return err;
>
> retry:
> handle = ext3_journal_start(dir, EXT3_DATA_TRANS_BLOCKS(dir->i_sb) +
> @@ -1770,7 +1773,9 @@ static int ext3_mkdir(struct inode * dir, struct dentry * dentry, int mode)
> if (dir->i_nlink >= EXT3_LINK_MAX)
> return -EMLINK;
>
> - dquot_initialize(dir);
> + err = dquot_initialize(dir);
> + if (err)
> + return err;
>
> retry:
> handle = ext3_journal_start(dir, EXT3_DATA_TRANS_BLOCKS(dir->i_sb) +
> @@ -2066,8 +2071,13 @@ static int ext3_rmdir (struct inode * dir, struct dentry *dentry)
>
> /* Initialize quotas before so that eventual writes go in
> * separate transaction */
> - dquot_initialize(dir);
> - dquot_initialize(dentry->d_inode);
> + retval = dquot_initialize(dir);
> + if (retval)
> + return retval;
> +
> + retval = dquot_initialize(dentry->d_inode);
> + if (retval)
> + return retval;
>
> handle = ext3_journal_start(dir, EXT3_DELETE_TRANS_BLOCKS(dir->i_sb));
> if (IS_ERR(handle))
> @@ -2127,8 +2137,13 @@ static int ext3_unlink(struct inode * dir, struct dentry *dentry)
>
> /* Initialize quotas before so that eventual writes go
> * in separate transaction */
> - dquot_initialize(dir);
> - dquot_initialize(dentry->d_inode);
> + retval = dquot_initialize(dir);
> + if (retval)
> + return retval;
> +
> + retval = dquot_initialize(dentry->d_inode);
> + if (retval)
> + return retval;
>
> handle = ext3_journal_start(dir, EXT3_DELETE_TRANS_BLOCKS(dir->i_sb));
> if (IS_ERR(handle))
> @@ -2184,7 +2199,9 @@ static int ext3_symlink (struct inode * dir,
> if (l > dir->i_sb->s_blocksize)
> return -ENAMETOOLONG;
>
> - dquot_initialize(dir);
> + err = dquot_initialize(dir);
> + if (err)
> + return err;
>
> retry:
> handle = ext3_journal_start(dir, EXT3_DATA_TRANS_BLOCKS(dir->i_sb) +
> @@ -2241,7 +2258,9 @@ static int ext3_link (struct dentry * old_dentry,
> if (inode->i_nlink >= EXT3_LINK_MAX)
> return -EMLINK;
>
> - dquot_initialize(dir);
> + err = dquot_initialize(dir);
> + if (err)
> + return err;
>
> /*
> * Return -ENOENT if we've raced with unlink and i_nlink is 0. Doing
> @@ -2293,15 +2312,22 @@ static int ext3_rename (struct inode * old_dir, struct dentry *old_dentry,
> struct ext3_dir_entry_2 * old_de, * new_de;
> int retval, flush_file = 0;
>
> - dquot_initialize(old_dir);
> - dquot_initialize(new_dir);
> + retval = dquot_initialize(old_dir);
> + if (retval)
> + return retval;
> + retval = dquot_initialize(new_dir);
> + if (retval)
> + return retval;
>
> old_bh = new_bh = dir_bh = NULL;
>
> /* Initialize quotas before so that eventual writes go
> * in separate transaction */
> - if (new_dentry->d_inode)
> - dquot_initialize(new_dentry->d_inode);
> + if (new_dentry->d_inode) {
> + retval = dquot_initialize(new_dentry->d_inode);
> + if (retval)
> + return retval;
> + }
> handle = ext3_journal_start(old_dir, 2 *
> EXT3_DATA_TRANS_BLOCKS(old_dir->i_sb) +
> EXT3_INDEX_EXTRA_TRANS_BLOCKS + 2);
> diff --git a/fs/ext3/super.c b/fs/ext3/super.c
> index 5ca068f..dd17196 100644
> --- a/fs/ext3/super.c
> +++ b/fs/ext3/super.c
> @@ -1498,7 +1498,14 @@ static int ext3_orphan_cleanup (struct super_block * sb,
> }
>
> list_add(&EXT3_I(inode)->i_orphan, &EXT3_SB(sb)->s_orphan);
> - dquot_initialize(inode);
> + ret = dquot_initialize(inode);
> + if (ret) {
> + ext3_msg(sb, KERN_ERR, "Can not initialize quota for "
> + "inode:%lu error %d",
> + inode->i_ino, ret);
> + if (!test_opt (sb, ERRORS_CONT))
> + goto out;
> + }
> if (inode->i_nlink) {
> printk(KERN_DEBUG
> "%s: truncating inode %lu to %Ld bytes\n",
> --
> 1.6.6.1
>
--
Jan Kara <jack@suse.cz>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 05/12] ufs: add error handling for dquot_initialize
2010-05-19 6:02 ` [PATCH 05/12] ufs: add error handling for dquot_initialize Dmitry Monakhov
2010-05-19 6:02 ` [PATCH 06/12] udf: " Dmitry Monakhov
@ 2010-05-20 17:33 ` Jan Kara
1 sibling, 0 replies; 20+ messages in thread
From: Jan Kara @ 2010-05-20 17:33 UTC (permalink / raw)
To: Dmitry Monakhov; +Cc: linux-fsdevel, jack, hch
On Wed 19-05-10 10:02:01, Dmitry Monakhov wrote:
This patch won't be needed because quota support for ufs is
non-functional for several years and we're going to remove it soon...
Honza
>
> Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
> ---
> fs/ufs/ialloc.c | 5 ++++-
> fs/ufs/namei.c | 46 +++++++++++++++++++++++++++++++++-------------
> fs/ufs/truncate.c | 6 ++++--
> 3 files changed, 41 insertions(+), 16 deletions(-)
>
> diff --git a/fs/ufs/ialloc.c b/fs/ufs/ialloc.c
> index 230ecf6..33e6fab 100644
> --- a/fs/ufs/ialloc.c
> +++ b/fs/ufs/ialloc.c
> @@ -355,7 +355,10 @@ cg_found:
>
> unlock_super (sb);
>
> - dquot_initialize(inode);
> + err = dquot_initialize(inode);
> + if (err)
> + goto fail_without_unlock;
> +
> err = dquot_alloc_inode(inode);
> if (err) {
> dquot_drop(inode);
> diff --git a/fs/ufs/namei.c b/fs/ufs/namei.c
> index eabc02e..94edec7 100644
> --- a/fs/ufs/namei.c
> +++ b/fs/ufs/namei.c
> @@ -86,7 +86,9 @@ static int ufs_create (struct inode * dir, struct dentry * dentry, int mode,
>
> UFSD("BEGIN\n");
>
> - dquot_initialize(dir);
> + err = dquot_initialize(dir);
> + if (err)
> + return err;
>
> inode = ufs_new_inode(dir, mode);
> err = PTR_ERR(inode);
> @@ -112,7 +114,9 @@ static int ufs_mknod (struct inode * dir, struct dentry *dentry, int mode, dev_t
> if (!old_valid_dev(rdev))
> return -EINVAL;
>
> - dquot_initialize(dir);
> + err = dquot_initialize(dir);
> + if (err)
> + return err;
>
> inode = ufs_new_inode(dir, mode);
> err = PTR_ERR(inode);
> @@ -138,7 +142,9 @@ static int ufs_symlink (struct inode * dir, struct dentry * dentry,
> if (l > sb->s_blocksize)
> goto out_notlocked;
>
> - dquot_initialize(dir);
> + err = dquot_initialize(dir);
> + if (err)
> + goto out;
>
> lock_kernel();
> inode = ufs_new_inode(dir, S_IFLNK | S_IRWXUGO);
> @@ -181,17 +187,20 @@ static int ufs_link (struct dentry * old_dentry, struct inode * dir,
>
> lock_kernel();
> if (inode->i_nlink >= UFS_LINK_MAX) {
> - unlock_kernel();
> - return -EMLINK;
> + error = -EMLINK;
> + goto out_unlock;
> }
>
> - dquot_initialize(dir);
> + error = dquot_initialize(dir);
> + if (error)
> + goto out_unlock;
>
> inode->i_ctime = CURRENT_TIME_SEC;
> inode_inc_link_count(inode);
> atomic_inc(&inode->i_count);
>
> error = ufs_add_nondir(dentry, inode);
> +out_unlock:
> unlock_kernel();
> return error;
> }
> @@ -204,7 +213,9 @@ static int ufs_mkdir(struct inode * dir, struct dentry * dentry, int mode)
> if (dir->i_nlink >= UFS_LINK_MAX)
> goto out;
>
> - dquot_initialize(dir);
> + err = dquot_initialize(dir);
> + if (err)
> + goto out;
>
> lock_kernel();
> inode_inc_link_count(dir);
> @@ -248,10 +259,13 @@ static int ufs_unlink(struct inode *dir, struct dentry *dentry)
> struct inode * inode = dentry->d_inode;
> struct ufs_dir_entry *de;
> struct page *page;
> - int err = -ENOENT;
> + int err;
>
> - dquot_initialize(dir);
> + err = dquot_initialize(dir);
> + if (err)
> + goto out;
>
> + err = -ENOENT;
> de = ufs_find_entry(dir, &dentry->d_name, &page);
> if (!de)
> goto out;
> @@ -294,14 +308,20 @@ static int ufs_rename(struct inode *old_dir, struct dentry *old_dentry,
> struct ufs_dir_entry * dir_de = NULL;
> struct page *old_page;
> struct ufs_dir_entry *old_de;
> - int err = -ENOENT;
> + int err;
>
> - dquot_initialize(old_dir);
> - dquot_initialize(new_dir);
> + err = dquot_initialize(old_dir);
> + if (err)
> + goto out;
> + err = dquot_initialize(new_dir);
> + if (err)
> + goto out;
>
> old_de = ufs_find_entry(old_dir, &old_dentry->d_name, &old_page);
> - if (!old_de)
> + if (!old_de) {
> + err = -ENOENT;
> goto out;
> + }
>
> if (S_ISDIR(old_inode->i_mode)) {
> err = -EIO;
> diff --git a/fs/ufs/truncate.c b/fs/ufs/truncate.c
> index f294c44..2de89ee 100644
> --- a/fs/ufs/truncate.c
> +++ b/fs/ufs/truncate.c
> @@ -518,8 +518,10 @@ int ufs_setattr(struct dentry *dentry, struct iattr *attr)
> if (error)
> return error;
>
> - if (is_quota_modification(inode, attr))
> - dquot_initialize(inode);
> + if (is_quota_modification(inode, attr)) {
> + error = dquot_initialize(inode);
> + return error;
> + }
>
> if ((ia_valid & ATTR_UID && attr->ia_uid != inode->i_uid) ||
> (ia_valid & ATTR_GID && attr->ia_gid != inode->i_gid)) {
> --
> 1.6.6.1
>
--
Jan Kara <jack@suse.cz>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 06/12] udf: add error handling for dquot_initialize
2010-05-19 6:02 ` [PATCH 06/12] udf: " Dmitry Monakhov
2010-05-19 6:02 ` [PATCH 07/12] reiserfs: " Dmitry Monakhov
@ 2010-05-20 17:34 ` Jan Kara
1 sibling, 0 replies; 20+ messages in thread
From: Jan Kara @ 2010-05-20 17:34 UTC (permalink / raw)
To: Dmitry Monakhov; +Cc: linux-fsdevel, jack, hch
On Wed 19-05-10 10:02:02, Dmitry Monakhov wrote:
This is the same case as ufs. Quota support is non-functional for several
years so I already have a patch to remove all the quota code from it.
Honza
>
> Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
> ---
> fs/udf/file.c | 6 ++++--
> fs/udf/ialloc.c | 18 +++++++++++-------
> fs/udf/namei.c | 39 +++++++++++++++++++++++++++++----------
> 3 files changed, 44 insertions(+), 19 deletions(-)
>
> diff --git a/fs/udf/file.c b/fs/udf/file.c
> index 6ebc043..1ef70d6 100644
> --- a/fs/udf/file.c
> +++ b/fs/udf/file.c
> @@ -227,8 +227,10 @@ int udf_setattr(struct dentry *dentry, struct iattr *iattr)
> if (error)
> return error;
>
> - if (is_quota_modification(inode, iattr))
> - dquot_initialize(inode);
> + if (is_quota_modification(inode, iattr)) {
> + error = dquot_initialize(inode);
> + return error;
> + }
>
> if ((iattr->ia_valid & ATTR_UID && iattr->ia_uid != inode->i_uid) ||
> (iattr->ia_valid & ATTR_GID && iattr->ia_gid != inode->i_gid)) {
> diff --git a/fs/udf/ialloc.c b/fs/udf/ialloc.c
> index fb68c9c..dc78998 100644
> --- a/fs/udf/ialloc.c
> +++ b/fs/udf/ialloc.c
> @@ -153,17 +153,21 @@ struct inode *udf_new_inode(struct inode *dir, int mode, int *err)
> insert_inode_hash(inode);
> mark_inode_dirty(inode);
>
> - dquot_initialize(inode);
> + ret = dquot_initialize(inode);
> + if (ret)
> + goto out_put;
> ret = dquot_alloc_inode(inode);
> if (ret) {
> dquot_drop(inode);
> - inode->i_flags |= S_NOQUOTA;
> - inode->i_nlink = 0;
> - iput(inode);
> - *err = ret;
> - return NULL;
> + goto out_put;
> }
> -
> *err = 0;
> return inode;
> +out_put:
> + inode->i_flags |= S_NOQUOTA;
> + clear_nlink(inode);
> + iput(inode);
> + *err = ret;
> + return NULL;
> +
> }
> diff --git a/fs/udf/namei.c b/fs/udf/namei.c
> index 7581602..0cb8167 100644
> --- a/fs/udf/namei.c
> +++ b/fs/udf/namei.c
> @@ -563,7 +563,9 @@ static int udf_create(struct inode *dir, struct dentry *dentry, int mode,
> int err;
> struct udf_inode_info *iinfo;
>
> - dquot_initialize(dir);
> + err = dquot_initialize(dir);
> + if (err)
> + return err;
>
> lock_kernel();
> inode = udf_new_inode(dir, mode, &err);
> @@ -618,7 +620,9 @@ static int udf_mknod(struct inode *dir, struct dentry *dentry, int mode,
> if (!old_valid_dev(rdev))
> return -EINVAL;
>
> - dquot_initialize(dir);
> + err = dquot_initialize(dir);
> + if (err)
> + return err;
>
> lock_kernel();
> err = -EIO;
> @@ -666,7 +670,9 @@ static int udf_mkdir(struct inode *dir, struct dentry *dentry, int mode)
> struct udf_inode_info *dinfo = UDF_I(dir);
> struct udf_inode_info *iinfo;
>
> - dquot_initialize(dir);
> + err = dquot_initialize(dir);
> + if (err)
> + return err;
>
> lock_kernel();
> err = -EMLINK;
> @@ -805,7 +811,9 @@ static int udf_rmdir(struct inode *dir, struct dentry *dentry)
> struct fileIdentDesc *fi, cfi;
> struct kernel_lb_addr tloc;
>
> - dquot_initialize(dir);
> + retval = dquot_initialize(dir);
> + if (retval)
> + return retval;
>
> retval = -ENOENT;
> lock_kernel();
> @@ -853,7 +861,9 @@ static int udf_unlink(struct inode *dir, struct dentry *dentry)
> struct fileIdentDesc cfi;
> struct kernel_lb_addr tloc;
>
> - dquot_initialize(dir);
> + retval = dquot_initialize(dir);
> + if (retval)
> + return retval;
>
> retval = -ENOENT;
> lock_kernel();
> @@ -909,7 +919,9 @@ static int udf_symlink(struct inode *dir, struct dentry *dentry,
> struct buffer_head *bh;
> struct udf_inode_info *iinfo;
>
> - dquot_initialize(dir);
> + err = dquot_initialize(dir);
> + if (err)
> + return err;
>
> lock_kernel();
> inode = udf_new_inode(dir, S_IFLNK, &err);
> @@ -1081,7 +1093,9 @@ static int udf_link(struct dentry *old_dentry, struct inode *dir,
> int err;
> struct buffer_head *bh;
>
> - dquot_initialize(dir);
> + err = dquot_initialize(dir);
> + if (err)
> + return err;
>
> lock_kernel();
> if (inode->i_nlink >= (256 << sizeof(inode->i_nlink)) - 1) {
> @@ -1141,13 +1155,18 @@ static int udf_rename(struct inode *old_dir, struct dentry *old_dentry,
> struct fileIdentDesc *ofi = NULL, *nfi = NULL, *dir_fi = NULL;
> struct fileIdentDesc ocfi, ncfi;
> struct buffer_head *dir_bh = NULL;
> - int retval = -ENOENT;
> + int retval;
> struct kernel_lb_addr tloc;
> struct udf_inode_info *old_iinfo = UDF_I(old_inode);
>
> - dquot_initialize(old_dir);
> - dquot_initialize(new_dir);
> + retval = dquot_initialize(old_dir);
> + if (retval)
> + return retval;
> + retval = dquot_initialize(new_dir);
> + if (retval)
> + return retval;
>
> + retval = -ENOENT;
> lock_kernel();
> ofi = udf_find_entry(old_dir, &old_dentry->d_name, &ofibh, &ocfi);
> if (ofi) {
> --
> 1.6.6.1
>
--
Jan Kara <jack@suse.cz>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 03/12] ext3: handle errors in orphan_cleanup
2010-05-19 6:01 ` [PATCH 03/12] ext3: handle errors in orphan_cleanup Dmitry Monakhov
2010-05-19 6:02 ` [PATCH 04/12] ext4: " Dmitry Monakhov
@ 2010-05-20 17:35 ` Jan Kara
1 sibling, 0 replies; 20+ messages in thread
From: Jan Kara @ 2010-05-20 17:35 UTC (permalink / raw)
To: Dmitry Monakhov; +Cc: linux-fsdevel, jack, hch
On Wed 19-05-10 10:01:59, Dmitry Monakhov wrote:
> Orphan cleanup procedure is complex task and may fail due to number
> of reasons. Handle errors from orphan_cleanp according to
> predefined per-sb errors behavior flags.
The patch looks good.
Honza
>
> Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
> ---
> fs/ext3/super.c | 27 +++++++++++++++++++--------
> 1 files changed, 19 insertions(+), 8 deletions(-)
>
> diff --git a/fs/ext3/super.c b/fs/ext3/super.c
> index 0fc1293..5ca068f 100644
> --- a/fs/ext3/super.c
> +++ b/fs/ext3/super.c
> @@ -1438,23 +1438,24 @@ static int ext3_check_descriptors(struct super_block *sb)
> * e2fsck was run on this filesystem, and it must have already done the orphan
> * inode cleanup for us, so we can safely abort without any further action.
> */
> -static void ext3_orphan_cleanup (struct super_block * sb,
> +static int ext3_orphan_cleanup (struct super_block * sb,
> struct ext3_super_block * es)
> {
> unsigned int s_flags = sb->s_flags;
> int nr_orphans = 0, nr_truncates = 0;
> + int ret = 0;
> #ifdef CONFIG_QUOTA
> int i;
> #endif
> if (!es->s_last_orphan) {
> jbd_debug(4, "no orphan inodes to clean up\n");
> - return;
> + return ret;
> }
>
> if (bdev_read_only(sb->s_bdev)) {
> ext3_msg(sb, KERN_ERR, "error: write access "
> "unavailable, skipping orphan cleanup.");
> - return;
> + return ret;
> }
>
> if (EXT3_SB(sb)->s_mount_state & EXT3_ERROR_FS) {
> @@ -1463,7 +1464,7 @@ static void ext3_orphan_cleanup (struct super_block * sb,
> "clearing orphan list.\n");
> es->s_last_orphan = 0;
> jbd_debug(1, "Skipping orphan recovery on fs with errors.\n");
> - return;
> + return ret;
> }
>
> if (s_flags & MS_RDONLY) {
> @@ -1476,11 +1477,13 @@ static void ext3_orphan_cleanup (struct super_block * sb,
> /* Turn on quotas so that they are updated correctly */
> for (i = 0; i < MAXQUOTAS; i++) {
> if (EXT3_SB(sb)->s_qf_names[i]) {
> - int ret = ext3_quota_on_mount(sb, i);
> - if (ret < 0)
> + ret = ext3_quota_on_mount(sb, i);
> + if (ret < 0) {
> ext3_msg(sb, KERN_ERR,
> "error: cannot turn on journaled "
> "quota: %d", ret);
> + goto out;
> + }
> }
> }
> #endif
> @@ -1514,7 +1517,7 @@ static void ext3_orphan_cleanup (struct super_block * sb,
> }
> iput(inode); /* The delete magic happens here! */
> }
> -
> +out:
> #define PLURAL(x) (x), ((x)==1) ? "" : "s"
>
> if (nr_orphans)
> @@ -1523,6 +1526,9 @@ static void ext3_orphan_cleanup (struct super_block * sb,
> if (nr_truncates)
> ext3_msg(sb, KERN_INFO, "%d truncate%s cleaned up",
> PLURAL(nr_truncates));
> + if (ret)
> + ext3_msg(sb, KERN_ERR, "Error %d wile orphan cleanup", ret);
> +
> #ifdef CONFIG_QUOTA
> /* Turn quotas off */
> for (i = 0; i < MAXQUOTAS; i++) {
> @@ -1531,6 +1537,7 @@ static void ext3_orphan_cleanup (struct super_block * sb,
> }
> #endif
> sb->s_flags = s_flags; /* Restore MS_RDONLY status */
> + return ret;
> }
>
> /*
> @@ -2034,7 +2041,11 @@ static int ext3_fill_super (struct super_block *sb, void *data, int silent)
> ext3_setup_super (sb, es, sb->s_flags & MS_RDONLY);
>
> EXT3_SB(sb)->s_mount_state |= EXT3_ORPHAN_FS;
> - ext3_orphan_cleanup(sb, es);
> + ret = ext3_orphan_cleanup(sb, es);
> +
> + if (ret)
> + goto failed_mount3;
> +
> EXT3_SB(sb)->s_mount_state &= ~EXT3_ORPHAN_FS;
> if (needs_recovery)
> ext3_msg(sb, KERN_INFO, "recovery complete");
> --
> 1.6.6.1
>
--
Jan Kara <jack@suse.cz>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 01/12] quota: Add proper error handling on quota initialization.
2010-05-19 6:01 ` [PATCH 01/12] quota: Add proper error handling on quota initialization Dmitry Monakhov
2010-05-19 6:01 ` [PATCH 02/12] quota: Check what quota is properly initialized for inode before charge Dmitry Monakhov
@ 2010-05-20 18:05 ` Jan Kara
1 sibling, 0 replies; 20+ messages in thread
From: Jan Kara @ 2010-05-20 18:05 UTC (permalink / raw)
To: Dmitry Monakhov; +Cc: linux-fsdevel, jack, hch
On Wed 19-05-10 10:01:57, Dmitry Monakhov wrote:
> Currently dqget_initialize() is a black-box so IO errors are simply
> ignored. In order to pass to the caller real error codes quota
> interface must being redesigned in following way.
>
> - return real error code from dqget()
> - return real error code from dquot_initialize()
>
> Now filesystem it is filesystem's responsibility to dquot_initialize()
> return value.
>
> Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
...
> diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
> index 7c624e1..95aa445 100644
> --- a/fs/quota/dquot.c
> +++ b/fs/quota/dquot.c
> @@ -720,7 +720,7 @@ void dqput(struct dquot *dquot)
> {
> int ret;
>
> - if (!dquot)
> + if (!dquot || IS_ERR(dquot))
It is unusual for "put" functions to silently ignore IS_ERR pointer
(while it is common for them to ignore NULL). So I'd prefer to keep the
old test so that the behavior is consistent with the rest of kernel and
make sure no callers of dqput() pass IS_ERR pointer to it...
> @@ -1332,18 +1340,19 @@ static int info_bdq_free(struct dquot *dquot, qsize_t space)
> * It is better to call this function outside of any transaction as it
> * might need a lot of space in journal for dquot structure allocation.
> */
> -static void __dquot_initialize(struct inode *inode, int type)
> +static int __dquot_initialize(struct inode *inode, int type)
> {
> unsigned int id = 0;
> - int cnt;
> + int cnt, err = 0;
> struct dquot *got[MAXQUOTAS];
> struct super_block *sb = inode->i_sb;
> qsize_t rsv;
>
> +repeat:
> /* First test before acquiring mutex - solves deadlocks when we
> * re-enter the quota code and are already holding the mutex */
> if (!sb_any_quota_active(inode->i_sb) || IS_NOQUOTA(inode))
> - return;
> + return 0;
>
> /* First get references to structures we might need. */
> for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
> @@ -1362,35 +1371,50 @@ static void __dquot_initialize(struct inode *inode, int type)
> }
>
> down_write(&sb_dqopt(sb)->dqptr_sem);
> - if (IS_NOQUOTA(inode))
> + if (IS_NOQUOTA(inode)) {
> + err = 0;
> goto out_err;
> + }
> for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
> if (type != -1 && cnt != type)
> continue;
> /* Avoid races with quotaoff() */
> if (!sb_has_quota_active(sb, cnt))
> continue;
> - if (!inode->i_dquot[cnt]) {
> - inode->i_dquot[cnt] = got[cnt];
> - got[cnt] = NULL;
> - /*
> - * Make quota reservation system happy if someone
> - * did a write before quota was turned on
> - */
> - rsv = inode_get_rsv_space(inode);
> - if (unlikely(rsv))
> - dquot_resv_space(inode->i_dquot[cnt], rsv);
> + if (inode->i_dquot[cnt])
> + continue;
> + if (unlikely(IS_ERR(got[cnt]))) {
> + err = PTR_ERR(got[cnt]);
> + /* If dqget() was raced with quotaon() then we have to
> + * repeat lookup. */
If we race with quotaon(), then add_dquot_ref() is responsible for adding
proper dquot reference. Therefore there's no need to retry lookup.
Also I would do all the error checking just after dqget() because there's
not much point in doing it in this loop.
> + if (err == -ESRCH) {
> + err = 0;
> + up_write(&sb_dqopt(sb)->dqptr_sem);
> + dqput_all(got);
> + goto repeat;
> + }
> + goto out_err;
> }
> + inode->i_dquot[cnt] = got[cnt];
> + got[cnt] = NULL;
> + /*
> + * Make quota reservation system happy if someone
> + * did a write before quota was turned on
> + */
> + rsv = inode_get_rsv_space(inode);
> + if (unlikely(rsv))
> + dquot_resv_space(inode->i_dquot[cnt], rsv);
> }
> out_err:
> up_write(&sb_dqopt(sb)->dqptr_sem);
> /* Drop unused references */
> dqput_all(got);
> + return err;
> }
>
> -void dquot_initialize(struct inode *inode)
> +int dquot_initialize(struct inode *inode)
> {
> - __dquot_initialize(inode, -1);
> + return __dquot_initialize(inode, -1);
> }
> EXPORT_SYMBOL(dquot_initialize);
>
...
> @@ -2092,24 +2127,30 @@ static int vfs_load_quota_inode(struct inode *inode, int type, int format_id,
> }
> mutex_unlock(&dqopt->dqio_mutex);
> spin_lock(&dq_state_lock);
> + dqflags = dqopt->flags;
> dqopt->flags |= dquot_state_flag(flags, type);
> spin_unlock(&dq_state_lock);
>
> - add_dquot_ref(sb, type);
> + error = add_dquot_ref(sb, type);
> + if (error)
> + goto out_dquot_init;
This is actually wrong as quota state flags have already been set and
quota for some inodes is already initialized and even may have been already
changed. So I think you have no other option than release dqonoff_mutex and
call dquot_disable (that's the name after Christoph's cleanups) to cleanup
everything...
> mutex_unlock(&dqopt->dqonoff_mutex);
>
> return 0;
> -
> +out_dquot_init:
> + spin_lock(&dq_state_lock);
> + dqopt->flags = dqflags;
> + spin_unlock(&dq_state_lock);
> out_file_init:
> dqopt->files[type] = NULL;
> iput(inode);
> out_lock:
> - if (oldflags != -1) {
> + if (iflags != -1) {
> mutex_lock_nested(&inode->i_mutex, I_MUTEX_QUOTA);
> /* Set the flags back (in the case of accidental quotaon()
> * on a wrong file we don't want to mess up the flags) */
> inode->i_flags &= ~(S_NOATIME | S_NOQUOTA | S_IMMUTABLE);
> - inode->i_flags |= oldflags;
> + inode->i_flags |= iflags;
> mutex_unlock(&inode->i_mutex);
> }
> mutex_unlock(&dqopt->dqonoff_mutex);
Honza
--
Jan Kara <jack@suse.cz>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 02/12] quota: Check what quota is properly initialized for inode before charge
2010-05-19 6:01 ` [PATCH 02/12] quota: Check what quota is properly initialized for inode before charge Dmitry Monakhov
2010-05-19 6:01 ` [PATCH 03/12] ext3: handle errors in orphan_cleanup Dmitry Monakhov
@ 2010-05-20 18:13 ` Jan Kara
1 sibling, 0 replies; 20+ messages in thread
From: Jan Kara @ 2010-05-20 18:13 UTC (permalink / raw)
To: Dmitry Monakhov; +Cc: linux-fsdevel, jack, hch
On Wed 19-05-10 10:01:58, Dmitry Monakhov wrote:
> Due to previous IO error or other bugs an inode may not has quota
> pointer. This definite sign of quota inconsistency/corruption.
> In fact this condition must being checked in all charge/uncharge
> functions. And if error was detected it is reasonable to fail whole
> operation. But uncharge(free_space/claim_space/free_inode) functions
> has no fail nature. This is because caller can't handle errors from
> such functions. How can you handle error from following call-trace?
> write_page()->quota_claim_space()
>
> So alloc_space() and alloc_inode() are the only functions which we may
> reliably abort in case of any errors.
>
> This patch add corresponding checks only for charge functions.
Actually, I've realized that this patch has a problem if
dquot_alloc_space is called while add_dquot_ref() is running. Then it would
return EIO although it should just silently succeed (and yes, quotas will
be inconsistent but that's "life" when quota isn't filesystem metadata...).
So I don't think this is a move in the right direction at least for now...
Honza
>
> Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
> ---
> fs/quota/dquot.c | 41 +++++++++++++++++++++++++++++++++++------
> 1 files changed, 35 insertions(+), 6 deletions(-)
>
> diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
> index 95aa445..f0e68b3 100644
> --- a/fs/quota/dquot.c
> +++ b/fs/quota/dquot.c
> @@ -1541,7 +1541,7 @@ static void inode_decr_space(struct inode *inode, qsize_t number, int reserve)
> int __dquot_alloc_space(struct inode *inode, qsize_t number,
> int warn, int reserve)
> {
> - int cnt, ret = 0;
> + int cnt, active, ret = 0;
> char warntype[MAXQUOTAS];
>
> /*
> @@ -1554,13 +1554,28 @@ int __dquot_alloc_space(struct inode *inode, qsize_t number,
> }
>
> down_read(&sb_dqopt(inode->i_sb)->dqptr_sem);
> + /* Now recheck reliably when holding dqptr_sem */
> + active = sb_any_quota_active(inode->i_sb);
> + if (!active || IS_NOQUOTA(inode)) {
> + inode_incr_space(inode, number, reserve);
> + goto out;
> + }
> +
> for (cnt = 0; cnt < MAXQUOTAS; cnt++)
> warntype[cnt] = QUOTA_NL_NOWARN;
>
> spin_lock(&dq_data_lock);
> for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
> - if (!inode->i_dquot[cnt])
> + if (!(active & (1 << cnt)))
> continue;
> + /*
> + * Given quota type is active, so quota must being
> + * initialized for this inode
> + */
> + if (!inode->i_dquot[cnt]) {
> + ret = -EIO;
> + goto out_flush_warn;
> + }
> ret = check_bdq(inode->i_dquot[cnt], number, !warn,
> warntype+cnt);
> if (ret) {
> @@ -1569,7 +1584,7 @@ int __dquot_alloc_space(struct inode *inode, qsize_t number,
> }
> }
> for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
> - if (!inode->i_dquot[cnt])
> + if (!(active & (1 << cnt)))
> continue;
> if (reserve)
> dquot_resv_space(inode->i_dquot[cnt], number);
> @@ -1595,7 +1610,7 @@ EXPORT_SYMBOL(__dquot_alloc_space);
> */
> int dquot_alloc_inode(const struct inode *inode)
> {
> - int cnt, ret = 0;
> + int cnt, active, ret = 0;
> char warntype[MAXQUOTAS];
>
> /* First test before acquiring mutex - solves deadlocks when we
> @@ -1605,17 +1620,31 @@ int dquot_alloc_inode(const struct inode *inode)
> for (cnt = 0; cnt < MAXQUOTAS; cnt++)
> warntype[cnt] = QUOTA_NL_NOWARN;
> down_read(&sb_dqopt(inode->i_sb)->dqptr_sem);
> + /* Now recheck reliably when holding dqptr_sem */
> + active = sb_any_quota_active(inode->i_sb);
> + if (!active || IS_NOQUOTA(inode)) {
> + return 0;
> + }
> +
> spin_lock(&dq_data_lock);
> for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
> - if (!inode->i_dquot[cnt])
> + if (!(active & (1 < cnt)))
> continue;
> + /*
> + * Given quota type is active, so quota must being
> + * initialized for this inode
> + */
> + if (!inode->i_dquot[cnt]) {
> + ret = -EIO;
> + goto warn_put_all;
> + }
> ret = check_idq(inode->i_dquot[cnt], 1, warntype + cnt);
> if (ret)
> goto warn_put_all;
> }
>
> for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
> - if (!inode->i_dquot[cnt])
> + if ((!active & (1 < cnt)))
> continue;
> dquot_incr_inodes(inode->i_dquot[cnt], 1);
> }
> --
> 1.6.6.1
>
--
Jan Kara <jack@suse.cz>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2010-05-20 18:13 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-05-19 6:01 [PATCH 00/12] quota: Redesign IO error handling interface V2 Dmitry Monakhov
2010-05-19 6:01 ` [PATCH 01/12] quota: Add proper error handling on quota initialization Dmitry Monakhov
2010-05-19 6:01 ` [PATCH 02/12] quota: Check what quota is properly initialized for inode before charge Dmitry Monakhov
2010-05-19 6:01 ` [PATCH 03/12] ext3: handle errors in orphan_cleanup Dmitry Monakhov
2010-05-19 6:02 ` [PATCH 04/12] ext4: " Dmitry Monakhov
2010-05-19 6:02 ` [PATCH 05/12] ufs: add error handling for dquot_initialize Dmitry Monakhov
2010-05-19 6:02 ` [PATCH 06/12] udf: " Dmitry Monakhov
2010-05-19 6:02 ` [PATCH 07/12] reiserfs: " Dmitry Monakhov
2010-05-19 6:02 ` [PATCH 08/12] ocfs2: " Dmitry Monakhov
2010-05-19 6:02 ` [PATCH 09/12] jfs: " Dmitry Monakhov
2010-05-19 6:02 ` [PATCH 10/12] ext4: " Dmitry Monakhov
2010-05-19 6:02 ` [PATCH 11/12] ext3: " Dmitry Monakhov
2010-05-19 6:02 ` [PATCH 12/12] ext2: " Dmitry Monakhov
2010-05-20 17:30 ` Jan Kara
2010-05-20 17:32 ` [PATCH 11/12] ext3: " Jan Kara
2010-05-20 17:34 ` [PATCH 06/12] udf: " Jan Kara
2010-05-20 17:33 ` [PATCH 05/12] ufs: " Jan Kara
2010-05-20 17:35 ` [PATCH 03/12] ext3: handle errors in orphan_cleanup Jan Kara
2010-05-20 18:13 ` [PATCH 02/12] quota: Check what quota is properly initialized for inode before charge Jan Kara
2010-05-20 18:05 ` [PATCH 01/12] quota: Add proper error handling on quota initialization Jan Kara
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.