On 01/02 2014 09:12 AM, Dave Chinner wrote:
> On Tue, Dec 31, 2013 at 05:51:33PM +0800, Jeff Liu wrote:
>> On 2013年12月31日 02:35, Mark Tinguely wrote:
>>> On 12/28/13 05:20, Jeff Liu wrote:
>>>> From: Jie Liu<jeff.liu@xxxxxxxxxx>
>>>>
>>>> In xfs_bulkstat_single(), xfs_bulkstat_one() and xfs_bulkstat() might
>>>> return different error if either call failed, we'd better return the
>>>> proper error in this case. Moreover, the function argument done is
>>>> useless in terms of xfs_ioc_bulkstat(), hence we can get rid of it.
>>>>
>>>> Signed-off-by: Jie Liu<jeff.liu@xxxxxxxxxx>
>>>> ---
>>>
>>> Yes, I know dmapi is not loved here but SGI still uses it and it wants
>>> the done flag still..
>> My mistake. At that time, I noticed that there has comments about this
>> in xfs_ioc_bulkstat(), i.e,
>>
>> /* done = 1 if there are more stats to get and if bulkstat */
>> /* should be called again (unused here, but used in dmapi) */
>>
>> However, I failed to find out why it would be called by going through
>> the dmapi source code...
>>
>> I'll keep this argument in next round of post.
>
> Well, let's consider how DMAPI uses it first.
>
> dmapi_ioctl()
> use_rvp = 0;
> case DM_GET_BULKALL:
> use_rvp = 1;
> dm_get_bulkattr_rvp(*rvp)
> fsys_vector->get_bulkattr_rvp(rvp)
> xfs_dm_get_bulkall_rvp(*rvalp)
> xfs_bulkstat(&done)
> *rvalp = !done ? 1 : 0;
Thanks for the clarification. Now I can understand the use scenarios
via DMAPI.
>
> if (use_rvp && !error)
> return rvp;
>
>
> Ok, so it returns the "done" status to userspace. How is "done"
> calculated?
>
> if (agno >= mp->m_sb.sb_agcount) {
> /*
> * If we ran out of filesystem, mark lastino as off
> * the end of the filesystem, so the next call
> * will return immediately.
> */
> *lastinop = (xfs_ino_t)XFS_AGINO_TO_INO(mp, agno, 0);
> *done = 1;
> } else
> *lastinop = (xfs_ino_t)lastino;
>
> Oh, so it's nothing special - the lastinop is pointed outside the
> current filesystem bounds and done is set to 1. IOWs, the dmapi code
> could easily generate the "done" value based on the returned
> lastinop value. i.e. xfs_dm_get_bulkall_rvp() can do this after the
> xfs_bulkstat() call:
>
> if (XFS_INO_TO_AGNO(mp, lastinop) >= mp->m_sb.sb_agcount)
> *rvalp = 0;
> else
> *rvalp = 1;
>
> And that means we can remove the done parameter from xfs_bulkstat()
> and no longer have to care about what DMAPI requires. Hence I think
> the patch as it stands does not impact on DMAPI functionality and so
> it just fine to clean up...
Yep. Except DMAPI, the only user of the done parameter is quota check, i.e,
xfs_qm_quotacheck():
do {
/*
* Iterate thru all the inodes in the file system,
* adjusting the corresponding dquot counters in core.
*/
error = xfs_bulkstat(mp, &lastino, &count,
xfs_qm_dqusage_adjust,
structsz, NULL, &done);
if (error)
break;
} while (!done);
But if we finally could perform quota check in parallel, the done parameter
can totally be removed as flush_workqueue() can ensure that is completed.
Actually, I just did it as the last patch in parallel quota check which is
not yet posted.
Thanks,
-Jeff
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
