On Tue, Jan 17, 2017 at 12:14:51PM -0500, Brian Foster wrote: > Got it, thanks. So all of the writeback stuff is protected via > page/buffer locks, and even if we still had those locks, it doesn't > matter because the same extent is obviously covered by many page/buffer > objects. Yes. > Yeah, and doing otherwise may break the assumption that larger delallocs > produce larger physical allocs (re: cowextsz hint and potentially > preallocation). Yes - especially for the COW case this might be very important. > That seems reasonable so long as we skip the parts of the loop that are > expecting a real (non-hole) startblock. Agreed. > Things like the above had me thinking it might be more clear to > explicitly read the extent and check for delalloc in the caller while > under the appropriate lock (and if XFS_COW_FORK). That's kind of what I > was alluding to above wrt to closing the race. That's just an idea, > however, and doesn't necessarily improve the error handling in the way > that this patch does (to avoid the transaction overrun). Given that, I'm > not against what this patch is currently doing so long as we fix up the > rest of the loop. Your idea of xfs_bmapi_convert() or some such sounds > like a nice potential cleanup at some point too. I don't like that idea because it just means even more extent lookups. xfs_bmapi_write has to read in the extents anyway, so instead of doing another read under the same lock we'd better reuse this one. -- 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
