Re: [PATCH v2 02/28] mm: Add functions to zero portions of a folio
From: "Darrick J. Wong" <djwong@kernel.org>
Date: 2021-11-18 17:26:19
Also in:
linux-fsdevel, linux-xfs, lkml
On Thu, Nov 18, 2021 at 03:55:12PM +0000, Matthew Wilcox wrote:
On Wed, Nov 17, 2021 at 09:07:07AM -0800, Darrick J. Wong wrote:quoted
I've started using 'next', or changing the code to make 'end' be the last element in the range the caller wants to act upon. The thing is, those are all iterators, so 'next' fits, whereas it doesn't fit so well for range zeroing where that might have been all the zeroing we wanted to do.Yeah, it doesn't really work so well for one of the patches in this series: if (buffer_new(bh)) { ... folio_zero_segments(folio, to, block_end, block_start, from); ("zero between block_start and block_end, except for the region specified by 'from' and 'to'"). Except that for some reason the ranges are specified backwards, so it's not obvious what's going on. Converting that to folio_zero_ranges() would be a possibility, at the expense of complexity in the caller, or using 'max' instead of 'end' would also add complexity to the callers.
The call above looks like it is preparing to copy some data into the middle of a buffer by zero-initializing the bytes before and the bytes after that middle region. Admittedly my fs-addled brain actually finds this hot mess easier to understand: folio_zero_segments(folio, to, blocksize - 1, block_start, from - 1); but I suppose the xend method involves less subtraction everywhere.
quoted hunk ↗ jump to hunk
quoted
Though. 'xend' (shorthand for 'excluded end') is different enough to signal that the reader should pay attention. Ok, how about xend then?Done!@@ -367,26 +367,26 @@ static inline void memzero_page(struct page *page, size_toffset, size_t len) * folio_zero_segments() - Zero two byte ranges in a folio. * @folio: The folio to write to. * @start1: The first byte to zero. - * @end1: One more than the last byte in the first range. + * @xend1: One more than the last byte in the first range. * @start2: The first byte to zero in the second range. - * @end2: One more than the last byte in the second range. + * @xend2: One more than the last byte in the second range. */ static inline void folio_zero_segments(struct folio *folio, - size_t start1, size_t end1, size_t start2, size_t end2) + size_t start1, size_t xend1, size_t start2, size_t xend2) { - zero_user_segments(&folio->page, start1, end1, start2, end2); + zero_user_segments(&folio->page, start1, xend1, start2, xend2); } /** * folio_zero_segment() - Zero a byte range in a folio. * @folio: The folio to write to. * @start: The first byte to zero. - * @end: One more than the last byte in the first range. + * @xend: One more than the last byte to zero. */ static inline void folio_zero_segment(struct folio *folio, - size_t start, size_t end) + size_t start, size_t xend) { - zero_user_segments(&folio->page, start, end, 0, 0); + zero_user_segments(&folio->page, start, xend, 0, 0);
Works for me, Reviewed-by: Darrick J. Wong <djwong@kernel.org> --D
} /**