| Message ID | 20241205171653.3179945-5-willy@infradead.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Convert ocfs2 to use folios | expand |
On 2024/12/6 01:16, Matthew Wilcox (Oracle) wrote: > From: Mark Tinguely <mark.tinguely@oracle.com> > > Convert to the new APIs, saving at least one hidden call to > compound_head(). > > Signed-off-by: Mark Tinguely <mark.tinguely@oracle.com> > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> > --- > fs/ocfs2/aops.c | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c > index 5f7a33335385..a1fad246765a 100644 > --- a/fs/ocfs2/aops.c > +++ b/fs/ocfs2/aops.c > @@ -869,30 +869,30 @@ static int ocfs2_alloc_write_ctxt(struct ocfs2_write_ctxt **wcp, > * and dirty so they'll be written out (in order to prevent uninitialised > * block data from leaking). And clear the new bit. > */ > -static void ocfs2_zero_new_buffers(struct folio *folio, unsigned from, unsigned to) > +static void ocfs2_zero_new_buffers(struct folio *folio, size_t from, size_t to) Don't see why we have to change 'unsigned' to 'size_t'. Thanks, Joseph > { > - struct page *page = &folio->page; > unsigned int block_start, block_end; > struct buffer_head *head, *bh; > > - BUG_ON(!PageLocked(page)); > - if (!page_has_buffers(page)) > + BUG_ON(!folio_test_locked(folio)); > + head = folio_buffers(folio); > + if (!head) > return; > > - bh = head = page_buffers(page); > + bh = head; > block_start = 0; > do { > block_end = block_start + bh->b_size; > > if (buffer_new(bh)) { > if (block_end > from && block_start < to) { > - if (!PageUptodate(page)) { > + if (!folio_test_uptodate(folio)) { > unsigned start, end; > > start = max(from, block_start); > end = min(to, block_end); > > - zero_user_segment(page, start, end); > + folio_zero_segment(folio, start, end); > set_buffer_uptodate(bh); > } >
On Sat, Dec 14, 2024 at 10:01:20PM +0800, Joseph Qi wrote: > > -static void ocfs2_zero_new_buffers(struct folio *folio, unsigned from, unsigned to) > > +static void ocfs2_zero_new_buffers(struct folio *folio, size_t from, size_t to) > > Don't see why we have to change 'unsigned' to 'size_t'. This is just part of the folio conversion process. Three reasons: 1. size_t is an indication to the human reader that this is a count of the number of bytes in memory, as opposed to anything else that an 'unsigned int' might be. 2. Prepares us for folios which are larger than 2GB in size. Yes, not likely to be something that ocfs2 ever supports, but on arm64 with a 16KiB page size, hugetlbfs at the PUD level has folios which are 64GB in size. So we need to use size_t within the mm & vfs, and we should continue that into filesystems. 3. It's actually more efficient. The CPU has to insert a lot of zero-extend instructions when calling core code which is using size_t.
On 2024/12/14 23:32, Matthew Wilcox wrote: > On Sat, Dec 14, 2024 at 10:01:20PM +0800, Joseph Qi wrote: >>> -static void ocfs2_zero_new_buffers(struct folio *folio, unsigned from, unsigned to) >>> +static void ocfs2_zero_new_buffers(struct folio *folio, size_t from, size_t to) >> >> Don't see why we have to change 'unsigned' to 'size_t'. > > This is just part of the folio conversion process. Three reasons: > > 1. size_t is an indication to the human reader that this is a count of the > number of bytes in memory, as opposed to anything else that an 'unsigned > int' might be. > > 2. Prepares us for folios which are larger than 2GB in size. Yes, not > likely to be something that ocfs2 ever supports, but on arm64 with a > 16KiB page size, hugetlbfs at the PUD level has folios which are 64GB in > size. So we need to use size_t within the mm & vfs, and we should > continue that into filesystems. > > 3. It's actually more efficient. The CPU has to insert a lot of > zero-extend instructions when calling core code which is using size_t. Thanks for the explanation. Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>
diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c index 5f7a33335385..a1fad246765a 100644 --- a/fs/ocfs2/aops.c +++ b/fs/ocfs2/aops.c @@ -869,30 +869,30 @@ static int ocfs2_alloc_write_ctxt(struct ocfs2_write_ctxt **wcp, * and dirty so they'll be written out (in order to prevent uninitialised * block data from leaking). And clear the new bit. */ -static void ocfs2_zero_new_buffers(struct folio *folio, unsigned from, unsigned to) +static void ocfs2_zero_new_buffers(struct folio *folio, size_t from, size_t to) { - struct page *page = &folio->page; unsigned int block_start, block_end; struct buffer_head *head, *bh; - BUG_ON(!PageLocked(page)); - if (!page_has_buffers(page)) + BUG_ON(!folio_test_locked(folio)); + head = folio_buffers(folio); + if (!head) return; - bh = head = page_buffers(page); + bh = head; block_start = 0; do { block_end = block_start + bh->b_size; if (buffer_new(bh)) { if (block_end > from && block_start < to) { - if (!PageUptodate(page)) { + if (!folio_test_uptodate(folio)) { unsigned start, end; start = max(from, block_start); end = min(to, block_end); - zero_user_segment(page, start, end); + folio_zero_segment(folio, start, end); set_buffer_uptodate(bh); }