Re: [PATCH 09/11] quota: protect dquot mem info with objects's lock

[Jan Kara <jack@suse.cz>]

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...

								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@suse.cz>
SUSE Labs, CR