On Tue, Nov 15, 2016 at 01:11:01PM -0500, Brian Foster wrote: > On Tue, Nov 15, 2016 at 06:16:21AM -0800, Christoph Hellwig wrote: > > > + if (imap->br_startoff != got.br_startoff || > > > + imap->br_blockcount != got.br_blockcount) > > > xfs_inode_set_cowblocks_tag(ip); > > > > Can't got.br_blockcount be smaller than imap->br_blockcount if we have > > an existing COW fork reservation lying around behind the whole we're > > filling? Also they way xfs_bmapi_reserve_delalloc works the startoff > > will be the same. E.g. this check should probably be: > > > > Good point, though I think it can be smaller or larger without > necessarily having preallocation due to being merged with surrounding > extents. I'm not quite sure what the right answer for that is with > regard to tagging, but I do think it's better to have false positive > tagging than false negatives. Good point, merging can change both the start and length. Based on that I think tagging in the caller of xfs_bmapi_reserve_delalloc is wrong, and we need to do it inside xfs_bmapi_reserve_delalloc where we know if we did speculative preallocation. -- 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