quota: Propagate errors when creating quota entry
Subsystems (6)
Jan Kara
maintainercover quota: Propagate errors when creating quota entry
Hello,
this patch set makes quota code report errors when quota entry creation fails (upto
now such errors were silently ignored). Filesystems can then properly handle the errors and report them to userspace. Patch set also includes patches to all filesystems to properly handle the errors. Review by respective fs maintainers is welcome.
If noone objects, I will queue these patches in my tree for the next merge window.
Honza
Jan Kara
maintainerPATCH 1/6 quota: Propagate error from ->acquire_dquot()
From: Jan Kara <[email protected]>
Currently when some error happened in ->acquire_dquot(), dqget() just returned NULL. That was indistinguishable from a case when e.g. someone run quotaoff and so was generally silently ignored. However ->acquire_dquot() can fail because of ENOSPC or EIO in which case user should better know. So propagate error up from ->acquire_dquot properly.
--- fs/ocfs2/file.c | 8 ++--- fs/ocfs2/quota_local.c | 4 +-- fs/quota/dquot.c | 88 ++++++++++++++++++++++++++++++++++-------------- include/linux/quotaops.h | 2 +- 4 files changed, 70 insertions(+), 32 deletions(-)
fs/ocfs2/file.c +4 -4
@@ -1209,8 +1209,8 @@ int ocfs2_setattr(struct dentry *dentry, struct iattr *attr) && OCFS2_HAS_RO_COMPAT_FEATURE(sb, OCFS2_FEATURE_RO_COMPAT_USRQUOTA)) { transfer_to[USRQUOTA ] =dqget (sb,make_kqid_uid (attr->ia_uid)); -if (!transfer_to[USRQUOTA ]) { - status = -ESRCH ; +if (IS_ERR (transfer_to[USRQUOTA ])) { + status =PTR_ERR (transfer_to[USRQUOTA ]); goto bail_unlock; } }
@@ -1218,8 +1218,8 @@ int ocfs2_setattr(struct dentry *dentry, struct iattr *attr) && OCFS2_HAS_RO_COMPAT_FEATURE(sb, OCFS2_FEATURE_RO_COMPAT_GRPQUOTA)) { transfer_to[GRPQUOTA ] =dqget (sb,make_kqid_gid (attr->ia_gid)); -if (!transfer_to[GRPQUOTA ]) { - status = -ESRCH ; +if (IS_ERR (transfer_to[GRPQUOTA ])) { + status =PTR_ERR (transfer_to[GRPQUOTA ]); goto bail_unlock; } }
fs/ocfs2/quota_local.c +2 -2
@@ -499,8 +499,8 @@ static int ocfs2_recover_local_quota_file(struct inode *lqinode, dquot =dqget (sb,make_kqid (&init_user_ns, type,le64_to_cpu (dqblk->dqb_id))); -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",
fs/quota/dquot.c +63 -25
183 lines not on this page · raw message
include/linux/quotaops.h +1 -1
@@ -43,7 +43,7 @@ void inode_claim_rsv_space(struct inode *inode, qsize_t number);void inode_sub_rsv_space (struct inode *inode,qsize_t number);void inode_reclaim_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,struct kqid qid);static inline struct dquot *dqgrab (struct dquot *dquot)
Signature · 1 line
2.1.4
Jan Kara
maintainerPATCH 2/6 ext2: Handle error from dquot_initalize()
In reply to Jan Karadquot_initialize() can now return error. Handle it where possible.
--- fs/ext2/ialloc.c | 5 ++++- fs/ext2/inode.c | 7 +++++-- fs/ext2/namei.c | 46 ++++++++++++++++++++++++++++++++++------------ 3 files changed, 43 insertions(+), 15 deletions(-)
fs/ext2/ialloc.c +4 -1
@@ -577,7 +577,10 @@ got: goto fail; } -dquot_initialize (inode); + err =dquot_initialize (inode); +if (err) + goto fail_drop; + err =dquot_alloc_inode (inode);if (err) goto fail_drop;
fs/ext2/inode.c +5 -2
@@ -1552,8 +1552,11 @@ int ext2_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); +if (error) +return error; + }if ((iattr->ia_valid &ATTR_UID && !uid_eq (iattr->ia_uid, inode->i_uid)) || (iattr->ia_valid &ATTR_GID && !gid_eq (iattr->ia_gid, inode->i_gid))) { error =dquot_transfer (inode, iattr);
fs/ext2/namei.c +34 -12
97 lines not on this page · raw message
Signature · 1 line
2.1.4
Jan Kara
maintainerPATCH 3/6 ext4: Handle error from dquot_initialize()
In reply to Jan Karadquot_initialize() can now return error. Handle it where possible.
--- fs/ext4/ialloc.c | 6 ++++-- fs/ext4/inode.c | 7 +++++-- fs/ext4/namei.c | 63 ++++++++++++++++++++++++++++++++++++++++++-------------- 3 files changed, 56 insertions(+), 20 deletions(-)
fs/ext4/ialloc.c +4 -2
@@ -721,7 +721,7 @@ struct inode *__ext4_new_inode(handle_t *handle, struct inode *dir,struct ext4_group_desc *gdp =NULL ;struct ext4_inode_info *ei;struct ext4_sb_info *sbi; -int ret2, err =0 ; +int ret2, err;struct inode *ret;ext4_group_t i;ext4_group_t flex_group;
@@ -769,7 +769,9 @@ struct inode *__ext4_new_inode(handle_t *handle, struct inode *dir, inode->i_gid = dir->i_gid; }else inode_init_owner (inode, dir, mode); -dquot_initialize (inode); + err =dquot_initialize (inode); +if (err) + goto out;if (!goal) goal = sbi->s_inode_goal;
fs/ext4/inode.c +5 -2
@@ -4661,8 +4661,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 && !uid_eq (attr->ia_uid, inode->i_uid)) || (ia_valid &ATTR_GID && !gid_eq (attr->ia_gid, inode->i_gid))) {handle_t *handle;
fs/ext4/namei.c +47 -16
126 lines not on this page · raw message
Signature · 1 line
2.1.4
Re: [PATCH 3/6] ext4: Handle error from dquot_initialize()
Quoted 3 lines · Jan Kara
dquot_initialize()can now return error. Handle it where possible.Signed-off-by Jan Kara <[email protected]>
Thanks!!
- Ted
Jan Kara
maintainerPATCH 4/6 ocfs2: Handle error from dquot_initialize()
In reply to Jan Karadquot_initialize() can now return error. Handle it where possible.
--- fs/ocfs2/file.c | 14 ++++++++---- fs/ocfs2/namei.c | 59 +++++++++++++++++++++++++++++++++++++------------ fs/ocfs2/refcounttree.c | 5 +++-- 3 files changed, 58 insertions(+), 20 deletions(-)
fs/ocfs2/file.c +10 -4
@@ -105,8 +105,11 @@ static int ocfs2_file_open(struct inode *inode, struct file *file) file->f_path.dentry->d_name.len, file->f_path.dentry->d_name.name, mode); -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);
@@ -1155,8 +1158,11 @@ 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 );
fs/ocfs2/namei.c +45 -14
123 lines not on this page · raw message
fs/ocfs2/refcounttree.c +3 -2
@@ -4419,8 +4419,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);
Signature · 1 line
2.1.4
Re: [Ocfs2-devel] [PATCH 4/6] ocfs2: Handle error from dquot_initialize()
Quoted 3 lines · Jan Kara
dquot_initialize()can now return error. Handle it where possible.Signed-off-by Jan Kara <[email protected]>
Looks good.
Quoted 192 lines
--- fs/ocfs2/file.c | 14 ++++++++---- fs/ocfs2/namei.c | 59 +++++++++++++++++++++++++++++++++++++------------ fs/ocfs2/refcounttree.c | 5 +++-- 3 files changed, 58 insertions(+), 20 deletions(-)fs/ocfs2/file.c +10
-4@@ -105,8 +105,11 @@ static int ocfs2_file_open(struct inode *inode, struct file *file) file->f_path.dentry->d_name.len, file->f_path.dentry->d_name.name, mode); -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);@@ -1155,8 +1158,11 @@ 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 );fs/ocfs2/namei.c +45
-14@@ -200,11 +200,12 @@ bail:static struct inode *ocfs2_get_init_inode (struct inode *dir,umode_t 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 @@ -213,7 +214,10 @@ static struct inode *ocfs2_get_init_inode(struct inode *dir, umode_t mode)if (S_ISDIR (mode))set_nlink (inode,2 );inode_init_owner (inode, dir, mode); -dquot_initialize (inode); + status =dquot_initialize (inode); +if (status) +return ERR_PTR (status); +return inode; }@@ -264,7 +268,11 @@ static int ocfs2_mknod(struct inode *dir, (unsigned long long )OCFS2_I (dir)->ip_blkno, (unsigned long)dev, mode); -dquot_initialize (dir); + status =dquot_initialize (dir); +if (status) { +mlog_errno (status); +return status; + }/* get our super block */ osb =OCFS2_SB (dir->i_sb);@@ -311,8 +319,9 @@ 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); + inode =NULL ;mlog_errno (status); goto leave; }@@ -708,7 +717,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_double_lock (osb, &old_dir_bh, old_dir, &parent_fe_bh, dir,0 );@@ -896,7 +909,11 @@ static int ocfs2_unlink(struct inode *dir, (unsigned long long )OCFS2_I (dir)->ip_blkno, (unsigned long long)OCFS2_I(inode)->ip_blkno); -dquot_initialize (dir); + status =dquot_initialize (dir); +if (status) { +mlog_errno (status); +return status; + }BUG_ON (d_inode (dentry->d_parent) != dir);@@ -1230,8 +1247,16 @@ 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); + goto bail; + } + status =dquot_initialize (new_dir); +if (status) { +mlog_errno (status); + goto bail; + } osb =OCFS2_SB (old_dir->i_sb);@@ -1786,7 +1811,11 @@ static int ocfs2_symlink(struct inode *dir,trace_ocfs2_symlink_begin (dir, dentry, symname, dentry->d_name.len, dentry->d_name.name); -dquot_initialize (dir); + status =dquot_initialize (dir); +if (status) { +mlog_errno (status); + goto bail; + } sb = dir->i_sb; osb =OCFS2_SB (sb);@@ -1831,8 +1860,9 @@ 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); + inode =NULL ;mlog_errno (status); goto bail; }@@ -2485,8 +2515,9 @@ 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); + inode =NULL ;mlog_errno (status); goto leave; }fs/ocfs2/refcounttree.c +3
-2@@ -4419,8 +4419,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);
Jan Kara
maintainerPATCH 5/6 jfs: Handle error from dquot_initialize()
In reply to Jan Karadquot_initialize() can now return error. Handle it where possible.
--- fs/jfs/file.c | 7 +++++-- fs/jfs/jfs_inode.c | 4 +++- fs/jfs/namei.c | 53 +++++++++++++++++++++++++++++++++++++++-------------- 3 files changed, 47 insertions(+), 17 deletions(-)
fs/jfs/file.c +5 -2
@@ -107,8 +107,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 && !uid_eq (iattr->ia_uid, inode->i_uid)) || (iattr->ia_valid &ATTR_GID && !gid_eq (iattr->ia_gid, inode->i_gid))) { rc =dquot_transfer (inode, iattr);
fs/jfs/jfs_inode.c +3 -1
@@ -109,7 +109,9 @@ struct inode *ialloc(struct inode *parent, umode_t mode)/* * Allocate inode to quota. */ -dquot_initialize (inode); + rc =dquot_initialize (inode); +if (rc) + goto fail_drop; rc =dquot_alloc_inode (inode);if (rc) goto fail_drop;
fs/jfs/namei.c +39 -14
124 lines not on this page · raw message
Signature · 1 line
2.1.4
Re: [PATCH 5/6] jfs: Handle error from dquot_initialize()
Quoted 1 line · Jan Kara
dquot_initialize()can now return error. Handle it where possible.
Looks good to me.
Quoted 1 line
Signed-off-by Jan Kara <[email protected]>
Quoted 179 lines
--- fs/jfs/file.c | 7 +++++-- fs/jfs/jfs_inode.c | 4 +++- fs/jfs/namei.c | 53 +++++++++++++++++++++++++++++++++++++++-------------- 3 files changed, 47 insertions(+), 17 deletions(-)fs/jfs/file.c +5
-2@@ -107,8 +107,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 && !uid_eq (iattr->ia_uid, inode->i_uid)) || (iattr->ia_valid &ATTR_GID && !gid_eq (iattr->ia_gid, inode->i_gid))) { rc =dquot_transfer (inode, iattr);fs/jfs/jfs_inode.c +3
-1@@ -109,7 +109,9 @@ struct inode *ialloc(struct inode *parent, umode_t mode)/* * Allocate inode to quota. */ -dquot_initialize (inode); + rc =dquot_initialize (inode); +if (rc) + goto fail_drop; rc =dquot_alloc_inode (inode);if (rc) goto fail_drop;fs/jfs/namei.c +39
-14@@ -86,7 +86,9 @@ static int jfs_create(struct inode *dip, struct dentry *dentry, umode_t mode,jfs_info ("jfs_create: dip:0x%p name:%pd" , dip, dentry); -dquot_initialize (dip); + rc =dquot_initialize (dip); +if (rc) + goto out1;/* * search parent directory for entry/freespace @@ -218,7 +220,9 @@ static int jfs_mkdir(struct inode *dip, struct dentry *dentry, umode_t mode)jfs_info ("jfs_mkdir: dip:0x%p name:%pd" , dip, dentry); -dquot_initialize (dip); + rc =dquot_initialize (dip); +if (rc) + goto out1;/* * search parent directory for entry/freespace @@ -355,8 +359,12 @@ static int jfs_rmdir(struct inode *dip, struct dentry *dentry)jfs_info ("jfs_rmdir: dip:0x%p name:%pd" , dip, dentry);/* 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)) {@@ -483,8 +491,12 @@ static int jfs_unlink(struct inode *dip, struct dentry *dentry)jfs_info ("jfs_unlink: dip:0x%p name:%pd" , dip, dentry);/* 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;@@ -799,7 +811,9 @@ static int jfs_link(struct dentry *old_dentry,jfs_info ("jfs_link: %pd %pd" , old_dentry, dentry); -dquot_initialize (dir); + rc =dquot_initialize (dir); +if (rc) + goto out; tid =txBegin (ip->i_sb,0 );@@ -810,7 +824,7 @@ static int jfs_link(struct dentry *old_dentry,* scan parent directory for entry/freespace */ if ((rc =get_UCSname (&dname, dentry))) - goto out; + goto out_tx;if ((rc =dtSearch (dir, &dname, &ino, &btstack,JFS_CREATE ))) goto free_dname;@@ -842,12 +856,13 @@ static int jfs_link(struct dentry *old_dentry, free_dname:free_UCSname (&dname); - out: + out_tx:txEnd (tid);mutex_unlock (&JFS_IP (ip)->commit_mutex);mutex_unlock (&JFS_IP (dir)->commit_mutex); + out:jfs_info ("jfs_link: rc:%d" , rc);return rc; }@@ -891,7 +906,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 ;@@ -1082,8 +1099,12 @@ static int jfs_rename(struct inode *old_dir, struct dentry *old_dentry,jfs_info ("jfs_rename: %pd %pd" , old_dentry, new_dentry); -dquot_initialize (old_dir); -dquot_initialize (new_dir); + rc =dquot_initialize (old_dir); +if (rc) + goto out1; + rc =dquot_initialize (new_dir); +if (rc) + goto out1; old_ip =d_inode (old_dentry); new_ip =d_inode (new_dentry);@@ -1130,7 +1151,9 @@ static int jfs_rename(struct inode *old_dir, struct dentry *old_dentry, }else if (new_ip) {IWRITE_LOCK (new_ip,RDWRLOCK_NORMAL );/* Init inode for quota operations. */ -dquot_initialize (new_ip); + rc =dquot_initialize (new_ip); +if (rc) + goto out3; }/* @@ -1354,7 +1377,9 @@ static int jfs_mknod(struct inode *dir, struct dentry *dentry,jfs_info ("jfs_mknod: %pd" , dentry); -dquot_initialize (dir); + rc =dquot_initialize (dir); +if (rc) + goto out;if ((rc =get_UCSname (&dname, dentry))) goto out;
Re: [PATCH 5/6] jfs: Handle error from dquot_initialize()
Quoted 4 lines · Dave Kleikamp
Quoted 1 line · Jan Kara
dquot_initialize()can now return error. Handle it where possible.Looks good to me.
No, I take that back. Reviewing this I found an existing problem with the error paths in jfs_rename, and your patch adds to the problem. See below.
Quoted 149 lines
Quoted 2 lines
Signed-off-by Jan Kara <[email protected]>Reviewed-by Dave Kleikamp <[email protected]>Quoted 145 lines
--- fs/jfs/file.c | 7 +++++-- fs/jfs/jfs_inode.c | 4 +++- fs/jfs/namei.c | 53 +++++++++++++++++++++++++++++++++++++++-------------- 3 files changed, 47 insertions(+), 17 deletions(-)fs/jfs/file.c +5
-2@@ -107,8 +107,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 && !uid_eq (iattr->ia_uid, inode->i_uid)) || (iattr->ia_valid &ATTR_GID && !gid_eq (iattr->ia_gid, inode->i_gid))) { rc =dquot_transfer (inode, iattr);fs/jfs/jfs_inode.c +3
-1@@ -109,7 +109,9 @@ struct inode *ialloc(struct inode *parent, umode_t mode)/* * Allocate inode to quota. */ -dquot_initialize (inode); + rc =dquot_initialize (inode); +if (rc) + goto fail_drop; rc =dquot_alloc_inode (inode);if (rc) goto fail_drop;fs/jfs/namei.c +27
-10@@ -86,7 +86,9 @@ static int jfs_create(struct inode *dip, struct dentry *dentry, umode_t mode,jfs_info ("jfs_create: dip:0x%p name:%pd" , dip, dentry); -dquot_initialize (dip); + rc =dquot_initialize (dip); +if (rc) + goto out1;/* * search parent directory for entry/freespace @@ -218,7 +220,9 @@ static int jfs_mkdir(struct inode *dip, struct dentry *dentry, umode_t mode)jfs_info ("jfs_mkdir: dip:0x%p name:%pd" , dip, dentry); -dquot_initialize (dip); + rc =dquot_initialize (dip); +if (rc) + goto out1;/* * search parent directory for entry/freespace @@ -355,8 +359,12 @@ static int jfs_rmdir(struct inode *dip, struct dentry *dentry)jfs_info ("jfs_rmdir: dip:0x%p name:%pd" , dip, dentry);/* 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)) {@@ -483,8 +491,12 @@ static int jfs_unlink(struct inode *dip, struct dentry *dentry)jfs_info ("jfs_unlink: dip:0x%p name:%pd" , dip, dentry);/* 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;@@ -799,7 +811,9 @@ static int jfs_link(struct dentry *old_dentry,jfs_info ("jfs_link: %pd %pd" , old_dentry, dentry); -dquot_initialize (dir); + rc =dquot_initialize (dir); +if (rc) + goto out; tid =txBegin (ip->i_sb,0 );@@ -810,7 +824,7 @@ static int jfs_link(struct dentry *old_dentry,* scan parent directory for entry/freespace */ if ((rc =get_UCSname (&dname, dentry))) - goto out; + goto out_tx;if ((rc =dtSearch (dir, &dname, &ino, &btstack,JFS_CREATE ))) goto free_dname;@@ -842,12 +856,13 @@ static int jfs_link(struct dentry *old_dentry, free_dname:free_UCSname (&dname); - out: + out_tx:txEnd (tid);mutex_unlock (&JFS_IP (ip)->commit_mutex);mutex_unlock (&JFS_IP (dir)->commit_mutex); + out:jfs_info ("jfs_link: rc:%d" , rc);return rc; }@@ -891,7 +906,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 ;@@ -1082,8 +1099,12 @@ static int jfs_rename(struct inode *old_dir, struct dentry *old_dentry,jfs_info ("jfs_rename: %pd %pd" , old_dentry, new_dentry);
Quoted 8 lines
fs/jfs/namei.c static int jfs_rename(struct inode *old_dir, struct dentry *old_dentry, · line 1107
Quoted 8 lines
fs/jfs/namei.c +6
-2-dquot_initialize (old_dir); -dquot_initialize (new_dir); + rc =dquot_initialize (old_dir); +if (rc) + goto out1; + rc =dquot_initialize (new_dir); +if (rc) + goto out1;
out1: conditionally calls IWRITE_UNLOCK after checking new_ip, which would here be uninitialized. That IWRITE_UNLOCK needs to be in another place anyway. I'll send a patch that fixes that and then adjust your patch on top of that.
Quoted 26 lines
Quoted 26 lines
fs/jfs/namei.c +6
-2old_ip =d_inode (old_dentry); new_ip =d_inode (new_dentry);@@ -1130,7 +1151,9 @@ static int jfs_rename(struct inode *old_dir, struct dentry *old_dentry, }else if (new_ip) {IWRITE_LOCK (new_ip,RDWRLOCK_NORMAL );/* Init inode for quota operations. */ -dquot_initialize (new_ip); + rc =dquot_initialize (new_ip); +if (rc) + goto out3; }/* @@ -1354,7 +1377,9 @@ static int jfs_mknod(struct inode *dir, struct dentry *dentry,jfs_info ("jfs_mknod: %pd" , dentry); -dquot_initialize (dir); + rc =dquot_initialize (dir); +if (rc) + goto out;if ((rc =get_UCSname (&dname, dentry))) goto out;
PATCH 1/1 jfs: clean up jfs_rename and fix out of order unlock
In reply to Jan KaraSubject: [PATCH] jfs: clean up jfs_rename and fix out of order unlock
[ Jan, I'll push this right away. You can carry it along until it gets merged. I want this pushed to stable as well.]
The end of jfs_rename(), which is also used by the error paths, included a call to IWRITE_UNLOCK(new_ip) after labels out1, out2 and out3. If we come in through these labels, IWRITE_LOCK() has not been called yet.
In moving that call to the correct spot, I also moved some exceptional truncate code earlier as well, since the early error paths don't need to deal with it, and I renamed out4: to out_tx: so a future patch by Jan Kara doesn't need to deal with renumbering or confusing out-of-order labels.
--- fs/jfs/namei.c | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-)
fs/jfs/namei.c +13 -14
@@ -1160,7 +1160,7 @@ static int jfs_rename(struct inode *old_dir, struct dentry *old_dentry, rc =dtModify (tid, new_dir, &new_dname, &ino, old_ip->i_ino,JFS_RENAME );if (rc) - goto out4; + goto out_tx;drop_nlink (new_ip);if (S_ISDIR (new_ip->i_mode)) {drop_nlink (new_ip);
@@ -1185,7 +1185,7 @@ static int jfs_rename(struct inode *old_dir, struct dentry *old_dentry,if ((new_size =commitZeroLink (tid, new_ip)) <0 ) {txAbort (tid,1 );/* Marks FS Dirty */ rc = new_size; - goto out4; + goto out_tx; } tblk =tid_to_tblock (tid); tblk->xflag |=COMMIT_DELETE ;
@@ -1203,7 +1203,7 @@ static int jfs_rename(struct inode *old_dir, struct dentry *old_dentry,if (rc) {jfs_err ("jfs_rename didn't expect dtSearch to fail " "w/rc = %d" , rc); - goto out4; + goto out_tx; } ino = old_ip->i_ino;
@@ -1211,7 +1211,7 @@ static int jfs_rename(struct inode *old_dir, struct dentry *old_dentry,if (rc) {if (rc == -EIO )jfs_err ("jfs_rename: dtInsert returned -EIO" ); - goto out4; + goto out_tx; }if (S_ISDIR (old_ip->i_mode))inc_nlink (new_dir);
@@ -1226,7 +1226,7 @@ static int jfs_rename(struct inode *old_dir, struct dentry *old_dentry,jfs_err ("jfs_rename did not expect dtDelete to return rc = %d" , rc);txAbort (tid,1 );/* Marks Filesystem dirty */ - goto out4; + goto out_tx; }if (S_ISDIR (old_ip->i_mode)) {drop_nlink (old_dir);
@@ -1285,7 +1285,7 @@ static int jfs_rename(struct inode *old_dir, struct dentry *old_dentry, rc =txCommit (tid, ipcount, iplist, commit_flag); - out4: + out_tx:txEnd (tid);if (new_ip)mutex_unlock (&JFS_IP (new_ip)->commit_mutex);
@@ -1308,13 +1308,6 @@ static int jfs_rename(struct inode *old_dir, struct dentry *old_dentry, }if (new_ip && (new_ip->i_nlink ==0 ))set_cflag (COMMIT_Nolink, new_ip); - out3: -free_UCSname (&new_dname); - out2: -free_UCSname (&old_dname); - out1: -if (new_ip && !S_ISDIR (new_ip->i_mode)) -IWRITE_UNLOCK (new_ip);/* * Truncating the directory index table is not guaranteed. It * may need to be done iteratively
@@ -1325,7 +1318,13 @@ static int jfs_rename(struct inode *old_dir, struct dentry *old_dentry,clear_cflag (COMMIT_Stale, old_dir); } - +if (new_ip && !S_ISDIR (new_ip->i_mode)) +IWRITE_UNLOCK (new_ip); + out3: +free_UCSname (&new_dname); + out2: +free_UCSname (&old_dname); + out1:jfs_info ("jfs_rename: returning %d" , rc);return rc; }
Signature · 1 line
2.4.5
Re: [PATCH] jfs: clean up jfs_rename and fix out of order unlock
Trimmed the cc list
Quoted 4 lines · Dave Kleikamp
Subject: [PATCH] jfs: clean up
jfs_renameand fix out of order unlock[ Jan, I'll push this right away. You can carry it along until it gets merged. I want this pushed to stable as well.]
This is in mainline now. I changed my mind about stable since this didn't fix an observed problem, but a potential yet unlikely one.
Shaggy
PATCH 1/1 dquot_initialize() can now return error. Handle it where possible
In reply to Jan KaraSlightly modified by Dave Kleikamp due to needed jfs_rename() error path fix.
--- fs/jfs/file.c | 7 +++++-- fs/jfs/jfs_inode.c | 4 +++- fs/jfs/namei.c | 54 ++++++++++++++++++++++++++++++++++++++++-------------- 3 files changed, 48 insertions(+), 17 deletions(-)
fs/jfs/file.c +5 -2
@@ -107,8 +107,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 && !uid_eq (iattr->ia_uid, inode->i_uid)) || (iattr->ia_valid &ATTR_GID && !gid_eq (iattr->ia_gid, inode->i_gid))) { rc =dquot_transfer (inode, iattr);
fs/jfs/jfs_inode.c +3 -1
@@ -109,7 +109,9 @@ struct inode *ialloc(struct inode *parent, umode_t mode)/* * Allocate inode to quota. */ -dquot_initialize (inode); + rc =dquot_initialize (inode); +if (rc) + goto fail_drop; rc =dquot_alloc_inode (inode);if (rc) goto fail_drop;
fs/jfs/namei.c +40 -14
131 lines not on this page · raw message
Signature · 1 line
2.4.5
Jan Kara
reviewerRe: [PATCH] dquot_initialize() can now return error. Handle it where possible
Quoted 4 lines · Dave Kleikamp
Slightly modified by Dave Kleikamp due to needed
jfs_rename()error path fix.Signed-off-by Jan Kara <[email protected]>Reviewed-by Dave Kleikamp <[email protected]>
Thanks! I have picked up both patches for now so that I have the series consistent. I will wait for a few days before pushing my series to linux-next so that your patch has time to reach Linus and I don't create unnecessary conflicts...
Honza
Quoted 190 lines
--- fs/jfs/file.c | 7 +++++-- fs/jfs/jfs_inode.c | 4 +++- fs/jfs/namei.c | 54 ++++++++++++++++++++++++++++++++++++++++-------------- 3 files changed, 48 insertions(+), 17 deletions(-)fs/jfs/file.c +5
-2@@ -107,8 +107,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 && !uid_eq (iattr->ia_uid, inode->i_uid)) || (iattr->ia_valid &ATTR_GID && !gid_eq (iattr->ia_gid, inode->i_gid))) { rc =dquot_transfer (inode, iattr);fs/jfs/jfs_inode.c +3
-1@@ -109,7 +109,9 @@ struct inode *ialloc(struct inode *parent, umode_t mode)/* * Allocate inode to quota. */ -dquot_initialize (inode); + rc =dquot_initialize (inode); +if (rc) + goto fail_drop; rc =dquot_alloc_inode (inode);if (rc) goto fail_drop;fs/jfs/namei.c +40
-14@@ -86,7 +86,9 @@ static int jfs_create(struct inode *dip, struct dentry *dentry, umode_t mode,jfs_info ("jfs_create: dip:0x%p name:%pd" , dip, dentry); -dquot_initialize (dip); + rc =dquot_initialize (dip); +if (rc) + goto out1;/* * search parent directory for entry/freespace @@ -218,7 +220,9 @@ static int jfs_mkdir(struct inode *dip, struct dentry *dentry, umode_t mode)jfs_info ("jfs_mkdir: dip:0x%p name:%pd" , dip, dentry); -dquot_initialize (dip); + rc =dquot_initialize (dip); +if (rc) + goto out1;/* * search parent directory for entry/freespace @@ -355,8 +359,12 @@ static int jfs_rmdir(struct inode *dip, struct dentry *dentry)jfs_info ("jfs_rmdir: dip:0x%p name:%pd" , dip, dentry);/* 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)) {@@ -483,8 +491,12 @@ static int jfs_unlink(struct inode *dip, struct dentry *dentry)jfs_info ("jfs_unlink: dip:0x%p name:%pd" , dip, dentry);/* 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;@@ -799,7 +811,9 @@ static int jfs_link(struct dentry *old_dentry,jfs_info ("jfs_link: %pd %pd" , old_dentry, dentry); -dquot_initialize (dir); + rc =dquot_initialize (dir); +if (rc) + goto out; tid =txBegin (ip->i_sb,0 );@@ -810,7 +824,7 @@ static int jfs_link(struct dentry *old_dentry,* scan parent directory for entry/freespace */ if ((rc =get_UCSname (&dname, dentry))) - goto out; + goto out_tx;if ((rc =dtSearch (dir, &dname, &ino, &btstack,JFS_CREATE ))) goto free_dname;@@ -842,12 +856,13 @@ static int jfs_link(struct dentry *old_dentry, free_dname:free_UCSname (&dname); - out: + out_tx:txEnd (tid);mutex_unlock (&JFS_IP (ip)->commit_mutex);mutex_unlock (&JFS_IP (dir)->commit_mutex); + out:jfs_info ("jfs_link: rc:%d" , rc);return rc; }@@ -891,7 +906,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 ;@@ -1082,8 +1099,12 @@ static int jfs_rename(struct inode *old_dir, struct dentry *old_dentry,jfs_info ("jfs_rename: %pd %pd" , old_dentry, new_dentry); -dquot_initialize (old_dir); -dquot_initialize (new_dir); + rc =dquot_initialize (old_dir); +if (rc) + goto out1; + rc =dquot_initialize (new_dir); +if (rc) + goto out1; old_ip =d_inode (old_dentry); new_ip =d_inode (new_dentry);@@ -1130,7 +1151,9 @@ static int jfs_rename(struct inode *old_dir, struct dentry *old_dentry, }else if (new_ip) {IWRITE_LOCK (new_ip,RDWRLOCK_NORMAL );/* Init inode for quota operations. */ -dquot_initialize (new_ip); + rc =dquot_initialize (new_ip); +if (rc) + goto out_unlock; }/* @@ -1318,6 +1341,7 @@ static int jfs_rename(struct inode *old_dir, struct dentry *old_dentry,clear_cflag (COMMIT_Stale, old_dir); } + out_unlock:if (new_ip && !S_ISDIR (new_ip->i_mode))IWRITE_UNLOCK (new_ip); out3:@@ -1353,7 +1377,9 @@ static int jfs_mknod(struct inode *dir, struct dentry *dentry,jfs_info ("jfs_mknod: %pd" , dentry); -dquot_initialize (dir); + rc =dquot_initialize (dir); +if (rc) + goto out;if ((rc =get_UCSname (&dname, dentry))) goto out;Signature · 1 line
2.4.5
Signature · 2 lines
Jan Kara <[email protected]> SUSE Labs, CR
Jan Kara
maintainerPATCH 6/6 reiserfs: Handle error from dquot_initialize()
In reply to Jan Karadquot_initialize() can now return error. Handle it where possible.
--- fs/reiserfs/inode.c | 7 ++++-- fs/reiserfs/namei.c | 63 ++++++++++++++++++++++++++++++++++++++++------------- 2 files changed, 53 insertions(+), 17 deletions(-)
fs/reiserfs/inode.c +5 -2
@@ -3319,8 +3319,11 @@ int reiserfs_setattr(struct dentry *dentry, struct iattr *attr)/* must be turned off for recursive notify_change calls */ ia_valid = attr->ia_valid &= ~(ATTR_KILL_SUID |ATTR_KILL_SGID ); -if (is_quota_modification (inode, attr)) -dquot_initialize (inode); +if (is_quota_modification (inode, attr)) { + error =dquot_initialize (inode); +if (error) +return error; + }reiserfs_write_lock (inode->i_sb);if (attr->ia_valid &ATTR_SIZE ) {/*
fs/reiserfs/namei.c +48 -15
135 lines not on this page · raw message
Signature · 1 line
2.1.4