Thread (96 messages) 96 messages, 6 authors, 2021-06-08

Re: [PATCH v10 01/33] mm: Introduce struct folio

From: Matthew Wilcox <willy@infradead.org>
Date: 2021-05-14 11:48:06
Also in: linux-fsdevel, lkml

On Fri, May 14, 2021 at 12:40:05PM +0200, Vlastimil Babka wrote:
On 5/11/21 11:47 PM, Matthew Wilcox (Oracle) wrote:
quoted
+/**
+ * folio_page - Return a page from a folio.
+ * @folio: The folio.
+ * @n: The page number to return.
+ *
+ * @n is relative to the start of the folio.  It should be between
+ * 0 and folio_nr_pages(@folio) - 1, but this is not checked for.
+ */
+#define folio_page(folio, n)	nth_page(&(folio)->page, n)
BTW, would it make sense to have also a folio_page(folio) wrapper? Or is
"&folio->page" used in later patches sufficiently elegant and stable enough for
the future?
Ah!  If you see &folio->page in a patch, it's "a bad smell" [1].  At
this stage, it probably indicates "This other thing I need isn't
converted entirely to folios yet".  I consider it fine in
implementations of utility functions like this:

+static inline unsigned int folio_order(struct folio *folio)
+{
+       return compound_order(&folio->page);
+}

but when we see it here:

+void folio_unlock(struct folio *folio)
 {
        BUILD_BUG_ON(PG_waiters != 7);
-       page = compound_head(page);
-       VM_BUG_ON_PAGE(!PageLocked(page), page);
-       if (clear_bit_unlock_is_negative_byte(PG_locked, &page->flags))
-               wake_up_page_bit(page, PG_locked);
+       VM_BUG_ON_FOLIO(!folio_locked(folio), folio);
+       if (clear_bit_unlock_is_negative_byte(PG_locked, folio_flags(folio, 0)))
+               wake_up_page_bit(&folio->page, PG_locked);
 }

that's an indication that wake_up_page_bit() needs to be converted to
folio_wake_bit(), which happens in a later patch.  I could probably
avoid this temporary problem with a different ordering of the patches,
but it's not clear to me that's a good use of my time.

The existing folio_page() is a way of distinguishing between "this
function i need to call doesn't have a folio equivalent yet" and "this
function i need to call needs to deal specifically with one page in
this folio".  For the former, use &folio->page; for the latter, use
folio_page() or folio_file_page().

[1] https://en.wikipedia.org/wiki/Code_smell
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help