Re: [PATCH 23/63] xfs: implement deferred bmbt map/unmap operations

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Sep 30, 2016 at 09:34:48PM +0100, Roger Willcocks wrote:
> On Fri, 2016-09-30 at 10:38 -0700, Darrick J. Wong wrote:
> > On Fri, Sep 30, 2016 at 12:34:04AM -0700, Christoph Hellwig wrote:
> > > > +/* Deferred mapping is only for real extents in the data fork. */
> > > > +static bool
> > > > +xfs_bmap_is_update_needed(
> > > > +	int			whichfork,
> > > > +	struct xfs_bmbt_irec	*bmap)
> > > > +{
> > > > +	ASSERT(whichfork == XFS_DATA_FORK);
> > > > +
> > > > +	return  bmap->br_startblock != HOLESTARTBLOCK &&
> > > > +		bmap->br_startblock != DELAYSTARTBLOCK;
> > > > +}
> > > 
> > > Passing in an argument just to assert on it seems weird.
> > > And except for that a better name might be xfs_bmbt_is_real or similar,
> > > and I bet we'd have other users for it as well.
> > 
> > xfs_bmap_*map_extent are the only callers, and the only whichfork
> > values are XFS_DATA_FORK.  I might as well just tear out all those
> > asserts since they're never going to trigger anyway.
> > 
> 
> Um, isn't that the point of an assertion ?

There's no point in checking whichfork since all callers pass
XFS_DATA_FORK, making the function argument unnecessary.

--D

> 
> --
> Roger
> 
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]
  Powered by Linux