On Thu, Jan 02, 2014 at 03:23:46PM +0800, Jeff Liu wrote:
> 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);
} while (XFS_INO_TO_AGNO(mp, lastino) < mp->m_sb.sb_agcount);
> 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.
Right - when you parallelise the quotacheck, then bulkstat
termination conditions are determined by the caller not
xfs_bulkstat(). Hence removing the "done" flag and letting the
callers determine temination conditions via the lastino being
returned seems like the right thing to do as a preparation step.
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
