Messages in this thread | | | Date | Fri, 19 Apr 2024 09:46:11 +0800 | Subject | Re: [PATCH] quota: fix to propagate error of mark_dquot_dirty() to caller | From | "wangjianjian (C)" <> |
| |
On 2024/4/12 21:01, Jan Kara wrote: > On Fri 12-04-24 14:15:17, Jan Kara wrote: >> On Fri 12-04-24 17:49:42, Chao Yu wrote: >>> in order to let caller be aware of failure of mark_dquot_dirty(). >>> >>> Signed-off-by: Chao Yu <chao@kernel.org> >> >> Thanks. I've added the patch to my tree. > > So this patch was buggy because mark_all_dquots() dirty was returning 1 in > case some dquot was indeed dirtied which resulted in e.g. > dquot_alloc_inode() to return 1 and consequently __ext4_new_inode() to fail This is what I try to say in my another f2fs patch, some callers use if return value is zero not *less than zero* to check success or not. Luckily, maybe f2fs doesn't use it this way.
> and eventually we've crashed in ext4_create(). I've fixed up the patch to > make mark_all_dquots() return 0 or error. > > Honza > >>> --- >>> fs/quota/dquot.c | 21 ++++++++++++++------- >>> 1 file changed, 14 insertions(+), 7 deletions(-) >>> >>> diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c >>> index dacbee455c03..b2a109d8b198 100644 >>> --- a/fs/quota/dquot.c >>> +++ b/fs/quota/dquot.c >>> @@ -1737,7 +1737,7 @@ int __dquot_alloc_space(struct inode *inode, qsize_t number, int flags) >>> >>> if (reserve) >>> goto out_flush_warn; >>> - mark_all_dquot_dirty(dquots); >>> + ret = mark_all_dquot_dirty(dquots); >>> out_flush_warn: >>> srcu_read_unlock(&dquot_srcu, index); >>> flush_warnings(warn); >>> @@ -1786,7 +1786,7 @@ int dquot_alloc_inode(struct inode *inode) >>> warn_put_all: >>> spin_unlock(&inode->i_lock); >>> if (ret == 0) >>> - mark_all_dquot_dirty(dquots); >>> + ret = mark_all_dquot_dirty(dquots); >>> srcu_read_unlock(&dquot_srcu, index); >>> flush_warnings(warn); >>> return ret; >>> @@ -1990,7 +1990,7 @@ int __dquot_transfer(struct inode *inode, struct dquot **transfer_to) >>> qsize_t inode_usage = 1; >>> struct dquot __rcu **dquots; >>> struct dquot *transfer_from[MAXQUOTAS] = {}; >>> - int cnt, index, ret = 0; >>> + int cnt, index, ret = 0, err; >>> char is_valid[MAXQUOTAS] = {}; >>> struct dquot_warn warn_to[MAXQUOTAS]; >>> struct dquot_warn warn_from_inodes[MAXQUOTAS]; >>> @@ -2087,8 +2087,12 @@ int __dquot_transfer(struct inode *inode, struct dquot **transfer_to) >>> * mark_all_dquot_dirty(). >>> */ >>> index = srcu_read_lock(&dquot_srcu); >>> - mark_all_dquot_dirty((struct dquot __rcu **)transfer_from); >>> - mark_all_dquot_dirty((struct dquot __rcu **)transfer_to); >>> + err = mark_all_dquot_dirty((struct dquot __rcu **)transfer_from); >>> + if (err < 0) >>> + ret = err; >>> + err = mark_all_dquot_dirty((struct dquot __rcu **)transfer_to); >>> + if (err < 0) >>> + ret = err; >>> srcu_read_unlock(&dquot_srcu, index); >>> >>> flush_warnings(warn_to); >>> @@ -2098,7 +2102,7 @@ int __dquot_transfer(struct inode *inode, struct dquot **transfer_to) >>> for (cnt = 0; cnt < MAXQUOTAS; cnt++) >>> if (is_valid[cnt]) >>> transfer_to[cnt] = transfer_from[cnt]; >>> - return 0; >>> + return ret; >>> over_quota: >>> /* Back out changes we already did */ >>> for (cnt--; cnt >= 0; cnt--) { >>> @@ -2726,6 +2730,7 @@ static int do_set_dqblk(struct dquot *dquot, struct qc_dqblk *di) >>> struct mem_dqblk *dm = &dquot->dq_dqb; >>> int check_blim = 0, check_ilim = 0; >>> struct mem_dqinfo *dqi = &sb_dqopt(dquot->dq_sb)->info[dquot->dq_id.type]; >>> + int ret; >>> >>> if (di->d_fieldmask & ~VFS_QC_MASK) >>> return -EINVAL; >>> @@ -2807,7 +2812,9 @@ static int do_set_dqblk(struct dquot *dquot, struct qc_dqblk *di) >>> else >>> set_bit(DQ_FAKE_B, &dquot->dq_flags); >>> spin_unlock(&dquot->dq_dqb_lock); >>> - mark_dquot_dirty(dquot); >>> + ret = mark_dquot_dirty(dquot); >>> + if (ret < 0) >>> + return ret; >>> >>> return 0; >>> } >>> -- >>> 2.40.1 >>> >> -- >> Jan Kara <jack@suse.com> >> SUSE Labs, CR >> -- Regards
| |