Matthew Wilcox
2020-Feb-19 14:41 UTC
[Ocfs2-devel] [PATCH v6 07/19] mm: Put readahead pages in cache earlier
On Tue, Feb 18, 2020 at 04:01:43PM -0800, John Hubbard wrote:> How about this instead? It uses the "for" loop fully and more naturally, > and is easier to read. And it does the same thing: > > static inline struct page *readahead_page(struct readahead_control *rac) > { > struct page *page; > > if (!rac->_nr_pages) > return NULL; > > page = xa_load(&rac->mapping->i_pages, rac->_start); > VM_BUG_ON_PAGE(!PageLocked(page), page); > rac->_batch_count = hpage_nr_pages(page); > > return page; > } > > static inline struct page *readahead_next(struct readahead_control *rac) > { > rac->_nr_pages -= rac->_batch_count; > rac->_start += rac->_batch_count; > > return readahead_page(rac); > } > > #define readahead_for_each(rac, page) \ > for (page = readahead_page(rac); page != NULL; \ > page = readahead_page(rac))I'll go you one better ... how about we do this instead: static inline struct page *readahead_page(struct readahead_control *rac) { struct page *page; BUG_ON(rac->_batch_count > rac->_nr_pages); rac->_nr_pages -= rac->_batch_count; rac->_index += rac->_batch_count; rac->_batch_count = 0; if (!rac->_nr_pages) return NULL; page = xa_load(&rac->mapping->i_pages, rac->_index); VM_BUG_ON_PAGE(!PageLocked(page), page); rac->_batch_count = hpage_nr_pages(page); return page; } #define readahead_for_each(rac, page) \ while ((page = readahead_page(rac))) No more readahead_next() to forget to add to filesystems which don't use the readahead_for_each() iterator. Ahem.
Christoph Hellwig
2020-Feb-19 14:52 UTC
[Ocfs2-devel] [PATCH v6 07/19] mm: Put readahead pages in cache earlier
On Wed, Feb 19, 2020 at 06:41:17AM -0800, Matthew Wilcox wrote:> #define readahead_for_each(rac, page) \ > while ((page = readahead_page(rac))) > > No more readahead_next() to forget to add to filesystems which don't use > the readahead_for_each() iterator. Ahem.And then kill readahead_for_each and open code the above to make it even more obvious?