Re: [PATCH 2/10] xfs: xfs_bulkstat_single consolidation

[Dave Chinner <david@fromorbit.com>]

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@oracle.com>
> >>
> >> 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@oracle.com>
> >> ---
> > 
> > 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;

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

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs