On Fri, Oct 30, 2015 at 08:36:08AM -0400, Brian Foster wrote: > On Fri, Oct 30, 2015 at 10:35:48AM +1100, Dave Chinner wrote: > > On Thu, Oct 29, 2015 at 10:27:58AM -0400, Brian Foster wrote: > > > On Mon, Oct 19, 2015 at 02:27:14PM +1100, Dave Chinner wrote: > > > > From: Dave Chinner <dchinner@xxxxxxxxxx> > > > > > ... > > > > > I suspect there's no use case to pass XFS_BMAPI_ZERO for new unwritten > > > allocations, but if the flag is passed, doesn't this cause duplicate > > > block zeroing? > > > > It probably would, but.... > > > > > Perhaps we should drop the zero flag from 'flags' after > > > allocation in xfs_bmapi_write() just to ensure this executes in one > > > place or the other..? > > > > I think that if we hit this, we're doing something else wrong - why > > would we allocate unwritten extents and still need to initialise > > them to zero? > > > > No idea, really (as noted above). ;) It just looked like it could be > invoked twice per bmapi call, nothing else I saw prevented it, and it > looks easily avoidable. Maybe somebody down the road decides to turn on > block zeroing unconditionally in the block allocator due to hardware > support or some such. Or maybe we'll never hit the problem. The point is > that this code will inevitably be modified/enhanced down the road and > nobody is going to remember that the zeroing is invoked twice in a > particular prealloc codepath. > > If we don't want to mess with the flags, how about an assert somewhere > so it's explicit the bmapi implementation doesn't expect this > combination of flags? Easy enough. I'll add this to the initial asserts in xfs_bmapi_write(): + /* + * we can allocate unwritten extents or pre-zero allocated blocks, + * but it makes no sense to do both at once. This would result in + * zeroing the unwritten extent twice, but it still being an + * unwritten extent.... + */ + ASSERT((flags & (XFS_BMAPI_PREALLOC|XFS_BMAPI_ZERO)) != + (XFS_BMAPI_PREALLOC|XFS_BMAPI_ZERO)); Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs
