On 2020/9/23 0:18, Darrick J. Wong wrote:
> On Tue, Sep 22, 2020 at 05:04:02PM +0800, xiakaixu1987@xxxxxxxxx wrote:
>> From: Kaixu Xia <kaixuxia@xxxxxxxxxxx>
>>
>> We attach the dquot(s) to the given inode and return udquot, gdquot
>> or pdquot with references taken by dqget or dqhold in xfs_qm_vop_dqalloc()
>> funciton. Actually, we only need to do dqget or dqhold for the specified
>> dquots, it is unnecessary if the passed-in O_{u,g,p}dqpp value is NULL.
>
> When would a caller pass in (for example) XFS_QMOPT_UQUOTA, a different
> uid than the one currently associated with the inode, but a null
> O_udqpp? It doesn't make sense to say "Prepare to change this file's
> uid, but don't give me the dquot of the new uid."
>
> None of the callers do that today, so I don't see the point of this
> patch. Perhaps the function could ASSERT the arguments a little more
> closely?
Yeah, ASSERT the arguments is better, will do that in the next version.
Thanks,
Kaixu
>
> --D
>
>> Signed-off-by: Kaixu Xia <kaixuxia@xxxxxxxxxxx>
>> ---
>> fs/xfs/xfs_qm.c | 13 ++++---------
>> 1 file changed, 4 insertions(+), 9 deletions(-)
>>
>> diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
>> index 44509decb4cd..38380fc29b4d 100644
>> --- a/fs/xfs/xfs_qm.c
>> +++ b/fs/xfs/xfs_qm.c
>> @@ -1661,7 +1661,7 @@ xfs_qm_vop_dqalloc(
>> }
>> }
>>
>> - if ((flags & XFS_QMOPT_UQUOTA) && XFS_IS_UQUOTA_ON(mp)) {
>> + if ((flags & XFS_QMOPT_UQUOTA) && XFS_IS_UQUOTA_ON(mp) && O_udqpp) {
>> if (!uid_eq(inode->i_uid, uid)) {
>> /*
>> * What we need is the dquot that has this uid, and
>> @@ -1694,7 +1694,7 @@ xfs_qm_vop_dqalloc(
>> uq = xfs_qm_dqhold(ip->i_udquot);
>> }
>> }
>> - if ((flags & XFS_QMOPT_GQUOTA) && XFS_IS_GQUOTA_ON(mp)) {
>> + if ((flags & XFS_QMOPT_GQUOTA) && XFS_IS_GQUOTA_ON(mp) && O_gdqpp) {
>> if (!gid_eq(inode->i_gid, gid)) {
>> xfs_iunlock(ip, lockflags);
>> error = xfs_qm_dqget(mp, from_kgid(user_ns, gid),
>> @@ -1711,7 +1711,7 @@ xfs_qm_vop_dqalloc(
>> gq = xfs_qm_dqhold(ip->i_gdquot);
>> }
>> }
>> - if ((flags & XFS_QMOPT_PQUOTA) && XFS_IS_PQUOTA_ON(mp)) {
>> + if ((flags & XFS_QMOPT_PQUOTA) && XFS_IS_PQUOTA_ON(mp) && O_pdqpp) {
>> if (ip->i_d.di_projid != prid) {
>> xfs_iunlock(ip, lockflags);
>> error = xfs_qm_dqget(mp, prid,
>> @@ -1733,16 +1733,11 @@ xfs_qm_vop_dqalloc(
>> xfs_iunlock(ip, lockflags);
>> if (O_udqpp)
>> *O_udqpp = uq;
>> - else
>> - xfs_qm_dqrele(uq);
>> if (O_gdqpp)
>> *O_gdqpp = gq;
>> - else
>> - xfs_qm_dqrele(gq);
>> if (O_pdqpp)
>> *O_pdqpp = pq;
>> - else
>> - xfs_qm_dqrele(pq);
>> +
>> return 0;
>>
>> error_rele:
>> --
>> 2.20.0
>>
--
kaixuxia
