Re: [PATCH 34/63] xfs: support removing extents from CoW fork

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

 



On Fri, Sep 30, 2016 at 12:46:42AM -0700, Christoph Hellwig wrote:
> >  /*
> > + * xfs_bunmapi_cow() -- Remove the relevant parts of the CoW fork.
> > + *			See xfs_bmap_del_extent.
> > + * @ip: XFS inode.
> > + * @idx: Extent number to delete.
> > + * @del: Extent to remove.
> > + */
> 
> Not quite a kerneldoc comment, not quite a normal one either..
> 
> That being sai the function seems mostly like a copy of the delay
> part of xfs_bmap_del_extent and the duplication seems unfortunate.
> 
> As I plan to do a major rework of that area for the next merge
> window I don't mind it for now, though.  

Yes, it is cobbled together from bmap_del_extent.  I wasn't sure at the
time whether it was worse to have two annoyingly similar functions, or
to sprinkle if statements all throughout the data fork one to special
case the COW fork.  Anyhow, when you rework the whole mess, cc: me and
I'll review it.

> > +	xfs_bmbt_irec_t		*del)
> 
> Same for these uses of the old typedefs in new code.  I'd rather
> avoid those, but instead of introducing churn now I'll sort this
> out later.

I'll at least fix those while I'm running through the entire tree.

--D

> 
> So:
> 
> Reviewed-by: Christoph Hellwig <hch@xxxxxx>
--
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