On Wed, Oct 12, 2016 at 10:12:34AM -0400, Brian Foster wrote: > > + if (xfs_is_reflink_inode(ip)) { > > + bool shared; > > + > > + end_fsb = min(XFS_B_TO_FSB(mp, offset + count), > > + maxbytes_fsb); > > + xfs_trim_extent(&got, offset_fsb, end_fsb - offset_fsb); > > + error = xfs_reflink_reserve_cow(ip, &got, &shared); > > + if (error) > > + goto out_unlock; > > All in all this seems fine, but I don't see why we need to get all the > way down through xfs_reflink_reserve_cow() -> > xfs_reflink_trim_around_shared() to handle the basic delalloc overwrite > case on a reflink inode. Could we enhance the is_reflink_inode() helper > or create a new one that can consider whether the data fork extent is a > hole or delalloc? Do you mean delalloc non-overwrite? We could skip non-overwrite extents by factoring out a helper that checks for extent types that don't need to be overwritten. But this would defeat the COW fork speculative preallocation logic, which causes additional COW operations even for extents we would not nessecarily have to COW. So we'll always have to look at the COW fork first if we already have an allocation to implement that scheme (and we should probably document it better). xfs_reflink_trim_around_shared does a check for the non-COWable extent types as the very first thing, so that's where we are done with the COW overhead for a non-overwrite that doesn't have a speculative preallocation in the COW fork. -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html