On Wed, 6 Oct 2010 15:30:18 +0200, Jan Kara <jack@xxxxxxx> wrote:
> On Tue 05-10-10 22:20:25, Dmitry Monakhov wrote:
> > @@ -1762,14 +1810,16 @@ int __dquot_transfer(struct inode *inode, struct dquot **transfer_to)
> > return 0;
> > }
> > spin_lock(&sb_dqopt(inode->i_sb)->dq_data_lock);
> > + inode_dquot_lock(inode, transfer_from);
> > cur_space = inode_get_bytes(inode);
> > rsv_space = inode_get_rsv_space(inode);
> > space = cur_space + rsv_space;
> > + dquot_lock_all(transfer_to);
> One more problem I've spotted - here you cannot lock new and old dquot
> structures in an arbitrary order as that would lead to deadlocks when two
> dquot_transfers run in parallel. So you have to establish some ordering and
> follow that while locking...
Absolutely, WOW, i've even written a remiander for that somewhere,
And as usually simply forgot about that :(
Minor issue: I've also forgot to place different quota types to different
lockdep classes. Which result complain from lockdep. Also will fix.
>
> Honza
>
> > /* Build the transfer_from list and check the limits */
> > +
> > for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
> > if (!transfer_to[cnt])
> > continue;
> > - transfer_from[cnt] = inode->i_dquot[cnt];
> > ret = check_idq(transfer_to[cnt], 1, warntype_to + cnt);
> > if (ret)
> > goto over_quota;
> > @@ -1806,6 +1856,8 @@ int __dquot_transfer(struct inode *inode, struct dquot **transfer_to)
> >
> > inode->i_dquot[cnt] = transfer_to[cnt];
> > }
> > + dquot_unlock_all(transfer_to);
> > + dquot_unlock_all(transfer_from);
> > spin_unlock(&sb_dqopt(inode->i_sb)->dq_data_lock);
> > up_write(&sb_dqopt(inode->i_sb)->dqptr_sem);
> >
> > @@ -1820,6 +1872,8 @@ warn:
> > flush_warnings(transfer_from, warntype_from_space);
> > return ret;
> > over_quota:
> > + dquot_unlock_all(transfer_to);
> > + dquot_unlock_all(transfer_from);
> > spin_unlock(&sb_dqopt(inode->i_sb)->dq_data_lock);
> > up_write(&sb_dqopt(inode->i_sb)->dqptr_sem);
> > goto warn;
> > @@ -2363,6 +2417,7 @@ static void do_get_dqblk(struct dquot *dquot, struct fs_disk_quota *di)
> > di->d_id = dquot->dq_id;
> >
> > spin_lock(&dq_opt(dquot)->dq_data_lock);
> > + spin_lock(&dquot->dq_lock);
> > di->d_blk_hardlimit = stoqb(dm->dqb_bhardlimit);
> > di->d_blk_softlimit = stoqb(dm->dqb_bsoftlimit);
> > di->d_ino_hardlimit = dm->dqb_ihardlimit;
> > @@ -2371,6 +2426,7 @@ static void do_get_dqblk(struct dquot *dquot, struct fs_disk_quota *di)
> > di->d_icount = dm->dqb_curinodes;
> > di->d_btimer = dm->dqb_btime;
> > di->d_itimer = dm->dqb_itime;
> > + spin_unlock(&dquot->dq_lock);
> > spin_unlock(&dq_opt(dquot)->dq_data_lock);
> > }
> >
> > @@ -2415,6 +2471,7 @@ static int do_set_dqblk(struct dquot *dquot, struct fs_disk_quota *di)
> > return -ERANGE;
> >
> > spin_lock(&dq_opt(dquot)->dq_data_lock);
> > + spin_lock(&dquot->dq_lock);
> > if (di->d_fieldmask & FS_DQ_BCOUNT) {
> > dm->dqb_curspace = di->d_bcount - dm->dqb_rsvspace;
> > check_blim = 1;
> > @@ -2480,6 +2537,7 @@ static int do_set_dqblk(struct dquot *dquot, struct fs_disk_quota *di)
> > clear_bit(DQ_FAKE_B, &dquot->dq_flags);
> > else
> > set_bit(DQ_FAKE_B, &dquot->dq_flags);
> > + spin_unlock(&dquot->dq_lock);
> > spin_unlock(&dq_opt(dquot)->dq_data_lock);
> > mark_dquot_dirty(dquot);
> >
> > diff --git a/fs/quota/quota_tree.c b/fs/quota/quota_tree.c
> > index 8b04f24..1643c30 100644
> > --- a/fs/quota/quota_tree.c
> > +++ b/fs/quota/quota_tree.c
> > @@ -376,7 +376,9 @@ int qtree_write_dquot(struct qtree_mem_dqinfo *info, struct dquot *dquot)
> > }
> > }
> > spin_lock(&sb_dqopt(sb)->dq_data_lock);
> > + spin_lock(&dquot->dq_lock);
> > info->dqi_ops->mem2disk_dqblk(ddquot, dquot);
> > + spin_unlock(&dquot->dq_lock);
> > spin_unlock(&sb_dqopt(sb)->dq_data_lock);
> > ret = sb->s_op->quota_write(sb, type, ddquot, info->dqi_entry_size,
> > dquot->dq_off);
> > @@ -632,12 +634,14 @@ int qtree_read_dquot(struct qtree_mem_dqinfo *info, struct dquot *dquot)
> > goto out;
> > }
> > spin_lock(&sb_dqopt(sb)->dq_data_lock);
> > + spin_lock(&dquot->dq_lock);
> > info->dqi_ops->disk2mem_dqblk(dquot, ddquot);
> > if (!dquot->dq_dqb.dqb_bhardlimit &&
> > !dquot->dq_dqb.dqb_bsoftlimit &&
> > !dquot->dq_dqb.dqb_ihardlimit &&
> > !dquot->dq_dqb.dqb_isoftlimit)
> > set_bit(DQ_FAKE_B, &dquot->dq_flags);
> > + spin_unlock(&dquot->dq_lock);
> > spin_unlock(&sb_dqopt(sb)->dq_data_lock);
> > kfree(ddquot);
> > out:
> > diff --git a/include/linux/quota.h b/include/linux/quota.h
> > index 9e7a102..197660f 100644
> > --- a/include/linux/quota.h
> > +++ b/include/linux/quota.h
> > @@ -294,6 +294,7 @@ struct dquot {
> > unsigned long dq_flags; /* See DQ_* */
> > short dq_type; /* Type of quota */
> > struct mem_dqblk dq_dqb; /* Diskquota usage */
> > + spinlock_t dq_lock; /* protect in mem_dqblk */
> > };
> >
> > /* Operations which must be implemented by each quota format */
> > --
> > 1.6.6.1
> >
> --
> Jan Kara <jack@xxxxxxx>
> SUSE Labs, CR
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
