| Message ID | 20210712030701.4000097-11-willy@infradead.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Memory folios | expand |
On Mon, Jul 12, 2021 at 04:04:54AM +0100, Matthew Wilcox (Oracle) wrote: > +/* Whether there are one or multiple pages in a folio */ > +static inline bool folio_single(struct folio *folio) > +{ > + return !folio_head(folio); > +} Reading more converted code in the series, I keep tripping over the new non-camelcased flag testers. It's not an issue when it's adjectives: folio_uptodate(), folio_referenced(), folio_locked() etc. - those are obvious. But nouns and words that overlap with struct member names can easily be confused with non-bool accessors and lookups. Pop quiz: flag test or accessor? folio_private() folio_lru() folio_nid() folio_head() folio_mapping() folio_slab() folio_waiters() This requires a lot of double-taking on what is actually being queried. Bool types, ! etc. don't help, since we test pointers for NULL/non-NULL all the time. I see in a later patch you changed the existing page_lru() (which returns an enum) to folio_lru_list() to avoid the obvious collision with the PG_lru flag test. page_private() has the same problem but it changed into folio_get_private() (no refcounting involved). There doesn't seem to be a consistent, future-proof scheme to avoid this new class of collisions between flag testing and member accessors. There is also an inconsistency between flag test and set that makes me pause to think if they're actually testing and setting the same thing: if (folio_idle(folio)) folio_clear_idle_flag(folio); Compare this to check_move_unevictable_pages(), where we do if (page_evictable(page)) ClearPageUnevictable(page); where one queries a more complex, contextual userpage state and the other updates the corresponding pageframe bit flag. The camelcase stuff we use for page flag testing is unusual for kernel code. But the page API is also unusually rich and sprawling. What would actually come close? task? inode? Having those multiple namespaces to structure and organize the API has been quite helpful. On top of losing the flagops namespacing, this series also disappears many <verb>_page() operations (which currently optically distinguish themselves from page_<noun>() accessors) into the shared folio_ namespace. This further increases the opportunities for collisions, which force undesirable naming compromises and/or ambiguity. More double-taking when the verb can be read as a noun: lock_folio() vs folio_lock(). Now, is anybody going to mistake folio_lock() for an accessor? Not once they think about it. Can you figure out and remember what folio_head() returns? Probably. What about all the examples above at the same time? Personally, I'm starting to struggle. It certainly eliminates syntactic help and pattern matching, and puts much more weight on semantic analysis and remembering API definitions. What about functions like shrink_page_list() which are long sequences of page queries and manipulations? Many lines would be folio_<foo> with no further cue whether you're looking at tests, accessors, or a high-level state change that is being tested for success. There are fewer visual anchors to orient yourself when you page up and down. It quite literally turns some code into blah_(), blah_(), blah_(): if (!folio_active(folio) && !folio_unevictable(folio)) { folio_del_from_lru_list(folio, lruvec); folio_set_active_flag(folio); folio_add_to_lru_list(folio, lruvec); trace_mm_lru_activate(&folio->page); } Think about the mental strain of reading and writing complicated memory management code with such a degree of syntactic parsimony, let alone the repetetive monotony. In those few lines of example code alone, readers will pause on things that should be obvious, and miss grave errors that should stand out. Add compatible return types to similarly named functions and we'll provoke subtle bugs that the compiler won't catch either. There are warts and inconsistencies in our naming patterns that could use cleanups. But I think this compresses a vast API into one template that isn't nearly expressive enough to adequately communicate and manage the complexity of the underlying structure and its operations.
On Mon, Jul 12, 2021 at 08:24:09PM -0400, Johannes Weiner wrote: > On Mon, Jul 12, 2021 at 04:04:54AM +0100, Matthew Wilcox (Oracle) wrote: > > +/* Whether there are one or multiple pages in a folio */ > > +static inline bool folio_single(struct folio *folio) > > +{ > > + return !folio_head(folio); > > +} > > Reading more converted code in the series, I keep tripping over the > new non-camelcased flag testers. Added PeterZ as he asked for it. https://lore.kernel.org/linux-mm/20210419135528.GC2531743@casper.infradead.org/ > It's not an issue when it's adjectives: folio_uptodate(), > folio_referenced(), folio_locked() etc. - those are obvious. But nouns > and words that overlap with struct member names can easily be confused > with non-bool accessors and lookups. Pop quiz: flag test or accessor? > > folio_private() > folio_lru() > folio_nid() > folio_head() > folio_mapping() > folio_slab() > folio_waiters() I know the answers to each of those, but your point is valid. So what's your preferred alternative? folio_is_lru(), folio_is_uptodate(), folio_is_slab(), etc? I've seen suggestions for folio_test_lru(), folio_test_uptodate(), and I don't much care for that alternative. > This requires a lot of double-taking on what is actually being > queried. Bool types, ! etc. don't help, since we test pointers for > NULL/non-NULL all the time. > > I see in a later patch you changed the existing page_lru() (which > returns an enum) to folio_lru_list() to avoid the obvious collision > with the PG_lru flag test. page_private() has the same problem but it > changed into folio_get_private() (no refcounting involved). There > doesn't seem to be a consistent, future-proof scheme to avoid this new > class of collisions between flag testing and member accessors. > > There is also an inconsistency between flag test and set that makes me > pause to think if they're actually testing and setting the same thing: > > if (folio_idle(folio)) > folio_clear_idle_flag(folio); > > Compare this to check_move_unevictable_pages(), where we do > > if (page_evictable(page)) > ClearPageUnevictable(page); > > where one queries a more complex, contextual userpage state and the > other updates the corresponding pageframe bit flag. > > The camelcase stuff we use for page flag testing is unusual for kernel > code. But the page API is also unusually rich and sprawling. What > would actually come close? task? inode? Having those multiple > namespaces to structure and organize the API has been quite helpful. > > On top of losing the flagops namespacing, this series also disappears > many <verb>_page() operations (which currently optically distinguish > themselves from page_<noun>() accessors) into the shared folio_ > namespace. This further increases the opportunities for collisions, > which force undesirable naming compromises and/or ambiguity. > > More double-taking when the verb can be read as a noun: lock_folio() > vs folio_lock(). > > Now, is anybody going to mistake folio_lock() for an accessor? Not > once they think about it. Can you figure out and remember what > folio_head() returns? Probably. What about all the examples above at > the same time? Personally, I'm starting to struggle. It certainly > eliminates syntactic help and pattern matching, and puts much more > weight on semantic analysis and remembering API definitions. Other people have given the opposite advice. For example, https://lore.kernel.org/linux-mm/YMmfQNjExNs3cuyq@kroah.com/ > What about functions like shrink_page_list() which are long sequences > of page queries and manipulations? Many lines would be folio_<foo> > with no further cue whether you're looking at tests, accessors, or a > high-level state change that is being tested for success. There are > fewer visual anchors to orient yourself when you page up and down. It > quite literally turns some code into blah_(), blah_(), blah_(): > > if (!folio_active(folio) && !folio_unevictable(folio)) { > folio_del_from_lru_list(folio, lruvec); > folio_set_active_flag(folio); > folio_add_to_lru_list(folio, lruvec); > trace_mm_lru_activate(&folio->page); > } I actually like the way that looks (other than the trace_mm_lru_activate() which is pending a conversion from page to folio). But I have my head completely down in it, and I can't tell what works for someone who's fresh to it. I do know that it's hard to change from an API you're used to (and that's part of the cost of changing an API), and I don't know how to balance that against making a more discoverable API. > Think about the mental strain of reading and writing complicated > memory management code with such a degree of syntactic parsimony, let > alone the repetetive monotony. > > In those few lines of example code alone, readers will pause on things > that should be obvious, and miss grave errors that should stand out. > > Add compatible return types to similarly named functions and we'll > provoke subtle bugs that the compiler won't catch either. > > There are warts and inconsistencies in our naming patterns that could > use cleanups. But I think this compresses a vast API into one template > that isn't nearly expressive enough to adequately communicate and > manage the complexity of the underlying structure and its operations. I don't want to dismiss your concerns. I just don't agree with them. If there's a consensus on folio_verb() vs verb_folio(), I'm happy to go back through all these patches and do the rename.
On Tue, Jul 13, 2021 at 03:15:10AM +0100, Matthew Wilcox wrote: > On Mon, Jul 12, 2021 at 08:24:09PM -0400, Johannes Weiner wrote: > > On Mon, Jul 12, 2021 at 04:04:54AM +0100, Matthew Wilcox (Oracle) wrote: > > > +/* Whether there are one or multiple pages in a folio */ > > > +static inline bool folio_single(struct folio *folio) > > > +{ > > > + return !folio_head(folio); > > > +} > > > > Reading more converted code in the series, I keep tripping over the > > new non-camelcased flag testers. > > Added PeterZ as he asked for it. > > https://lore.kernel.org/linux-mm/20210419135528.GC2531743@casper.infradead.org/ Aye; I hate me some Camels with a passion. And Linux Coding style explicitly not having Camels these things were always a sore spot. I'm very glad to see them go. > > It's not an issue when it's adjectives: folio_uptodate(), > > folio_referenced(), folio_locked() etc. - those are obvious. But nouns > > and words that overlap with struct member names can easily be confused > > with non-bool accessors and lookups. Pop quiz: flag test or accessor? > > > > folio_private() > > folio_lru() > > folio_nid() > > folio_head() > > folio_mapping() > > folio_slab() > > folio_waiters() > > I know the answers to each of those, but your point is valid. So what's > your preferred alternative? folio_is_lru(), folio_is_uptodate(), > folio_is_slab(), etc? I've seen suggestions for folio_test_lru(), > folio_test_uptodate(), and I don't much care for that alternative. Either _is_ or _test_ works for me, with a slight preference to _is_ on account it of being shorter. > > Now, is anybody going to mistake folio_lock() for an accessor? Not > > once they think about it. Can you figure out and remember what > > folio_head() returns? Probably. What about all the examples above at > > the same time? Personally, I'm starting to struggle. It certainly > > eliminates syntactic help and pattern matching, and puts much more > > weight on semantic analysis and remembering API definitions. > > Other people have given the opposite advice. For example, > https://lore.kernel.org/linux-mm/YMmfQNjExNs3cuyq@kroah.com/ Yes, we -tip folk tend to also prefer consistent prefix_ naming, and every time something big gets refactorered we make sure to make it so. Look at it like a namespace; you can read it like folio::del_from_lru_list() if you want. Obviously there's nothing like 'using folio' for this being C and not C++. > > What about functions like shrink_page_list() which are long sequences > > of page queries and manipulations? Many lines would be folio_<foo> > > with no further cue whether you're looking at tests, accessors, or a > > high-level state change that is being tested for success. There are > > fewer visual anchors to orient yourself when you page up and down. It > > quite literally turns some code into blah_(), blah_(), blah_(): > > > > if (!folio_active(folio) && !folio_unevictable(folio)) { > > folio_del_from_lru_list(folio, lruvec); > > folio_set_active_flag(folio); > > folio_add_to_lru_list(folio, lruvec); > > trace_mm_lru_activate(&folio->page); > > } > > I actually like the way that looks (other than the trace_mm_lru_activate() > which is pending a conversion from page to folio). But I have my head > completely down in it, and I can't tell what works for someone who's > fresh to it. I do know that it's hard to change from an API you're > used to (and that's part of the cost of changing an API), and I don't > know how to balance that against making a more discoverable API. Yeah, I don't particularly have a problem with the repeated folio_ thing either, it's something you'll get used to. I agree that significantly changing the naming of things is a majoy PITA, but given the level of refactoring at that, I think folio_ beats pageymcpageface_. Give it some time to get used to it...
On Tue, Jul 13, 2021 at 11:15:33AM +0200, Peter Zijlstra wrote: > On Tue, Jul 13, 2021 at 03:15:10AM +0100, Matthew Wilcox wrote: > > On Mon, Jul 12, 2021 at 08:24:09PM -0400, Johannes Weiner wrote: > > > On Mon, Jul 12, 2021 at 04:04:54AM +0100, Matthew Wilcox (Oracle) wrote: > > > > +/* Whether there are one or multiple pages in a folio */ > > > > +static inline bool folio_single(struct folio *folio) > > > > +{ > > > > + return !folio_head(folio); > > > > +} > > > > > > Reading more converted code in the series, I keep tripping over the > > > new non-camelcased flag testers. > > > > Added PeterZ as he asked for it. > > > > https://lore.kernel.org/linux-mm/20210419135528.GC2531743@casper.infradead.org/ > > Aye; I hate me some Camels with a passion. And Linux Coding style > explicitly not having Camels these things were always a sore spot. I'm > very glad to see them go. > > > > It's not an issue when it's adjectives: folio_uptodate(), > > > folio_referenced(), folio_locked() etc. - those are obvious. But nouns > > > and words that overlap with struct member names can easily be confused > > > with non-bool accessors and lookups. Pop quiz: flag test or accessor? > > > > > > folio_private() > > > folio_lru() > > > folio_nid() > > > folio_head() > > > folio_mapping() > > > folio_slab() > > > folio_waiters() > > > > I know the answers to each of those, but your point is valid. So what's > > your preferred alternative? folio_is_lru(), folio_is_uptodate(), > > folio_is_slab(), etc? I've seen suggestions for folio_test_lru(), > > folio_test_uptodate(), and I don't much care for that alternative. > > Either _is_ or _test_ works for me, with a slight preference to _is_ on > account it of being shorter. I agree that _is_ reads nicer by itself, but paired with other ops such as testset, _test_ might be better. For example, in __set_page_dirty_no_writeback() if (folio_is_dirty()) return !folio_testset_dirty() is less clear about what's going on than would be: if (folio_test_dirty()) return !folio_testset_dirty() My other example wasn't quoted, but IMO set and clear naming should also match testing to not cause confusion. I.e. the current: if (folio_idle()) folio_clear_idle_flag() can make you think two different things are being tested and modified (as in if (page_evictable()) ClearPageUnevictable()). IMO easier: if (folio_test_idle()) folio_clear_idle() Non-atomics would have the __ modifier in front of folio rather than read __clear or __set, which works I suppose? __folio_clear_dirty() With all that, we'd have something like: folio_test_foo() folio_set_foo() folio_clear_foo() folio_testset_foo() folio_testclear_foo() __folio_test_foo() __folio_set_foo() __folio_clear_foo() Would that be a workable compromise for everybody? > > > Now, is anybody going to mistake folio_lock() for an accessor? Not > > > once they think about it. Can you figure out and remember what > > > folio_head() returns? Probably. What about all the examples above at > > > the same time? Personally, I'm starting to struggle. It certainly > > > eliminates syntactic help and pattern matching, and puts much more > > > weight on semantic analysis and remembering API definitions. > > > > Other people have given the opposite advice. For example, > > https://lore.kernel.org/linux-mm/YMmfQNjExNs3cuyq@kroah.com/ > > Yes, we -tip folk tend to also prefer consistent prefix_ naming, and > every time something big gets refactorered we make sure to make it so. > > Look at it like a namespace; you can read it like > folio::del_from_lru_list() if you want. Obviously there's nothing like > 'using folio' for this being C and not C++. Yeah the lack of `using` is my concern. Namespacing is nice for more contained APIs. Classic class + method type deals, with non-namespaced private helpers implementing public methods, and public methods not layered past trivial stuff like foo_insert() calling __foo_insert() with a lock held. memcg, vmalloc, kobject, you name it. But the page api is pretty sprawling with sizable overlaps between interface and implementation, and heavy layering in both. `using` would be great to avoid excessive repetition where file or function context already does plenty of namespacing. Alas, it's not an option. So IMO we're taking a concept of more stringent object-oriented encapsulation to a large, heavily layered public API without having the tools e.g. C++ provides to manage exactly such situations. If everybody agrees we'll be fine, I won't stand in the way. But I do think the page API is a bit unusual in that regard. And while it is nice for the outward-facing filesystem interface - and I can see why fs people love it - the cost of it seems to be carried by the MM implementation code. > > > What about functions like shrink_page_list() which are long sequences > > > of page queries and manipulations? Many lines would be folio_<foo> > > > with no further cue whether you're looking at tests, accessors, or a > > > high-level state change that is being tested for success. There are > > > fewer visual anchors to orient yourself when you page up and down. It > > > quite literally turns some code into blah_(), blah_(), blah_(): > > > > > > if (!folio_active(folio) && !folio_unevictable(folio)) { > > > folio_del_from_lru_list(folio, lruvec); > > > folio_set_active_flag(folio); > > > folio_add_to_lru_list(folio, lruvec); > > > trace_mm_lru_activate(&folio->page); > > > } > > > > I actually like the way that looks (other than the trace_mm_lru_activate() > > which is pending a conversion from page to folio). But I have my head > > completely down in it, and I can't tell what works for someone who's > > fresh to it. I do know that it's hard to change from an API you're > > used to (and that's part of the cost of changing an API), and I don't > > know how to balance that against making a more discoverable API. > > Yeah, I don't particularly have a problem with the repeated folio_ thing > either, it's something you'll get used to. Yeah I won't stand in the way if everybody agrees this is fine. Although I will say, folio_del_from_lru_list() reads a bit like 'a'.append_to(string) to me. lruvec_add_folio() would match more conventional object hierarchy for container/collection/list/array interactions, like with list_add, xa_store, rb_insert, etc. Taking all of the above, we'd have: if (!folio_test_active(folio) && !folio_test_unevictable(folio)) { lruvec_del_folio(folio, lruvec); folio_set_active(folio); lruvec_add_folio(folio, lruvec); trace_mm_lru_activate(&folio->page); } which reads a little better overall, IMO. Is that a direction we could agree on? It still loses the visual anchoring of page state changes. These are often the "commit" part of multi-step transactions, and having those cut through the procedural grind a bit is nice - to see more easily what the code is fundamentally about, what is prerequisite for the transaction, and what is post-transactional housekeeping noise: if (!PageActive(page) && !PageUnevictable(page)) { del_page_from_lru_list(page, lruvec); SetPageActive(page); add_page_to_lru_list(page, lruvec); trace_mm_lru_activate(page); } Similar for isolation clearing PG_lru (empties, comments, locals removed): if (page_zonenum(page) > sc->reclaim_idx) { list_move(&page->lru, &pages_skipped); nr_skipped[page_zonenum(page)] += nr_pages; continue; } scan += nr_pages; if (!__isolate_lru_page_prepare(page, mode)) { list_move(&page->lru, src); continue; } if (unlikely(!get_page_unless_zero(page))) { list_move(&page->lru, src); continue; } if (!TestClearPageLRU(page)) { put_page(page); list_move(&page->lru, src); continue; } nr_taken += nr_pages; nr_zone_taken[page_zonenum(page)] += nr_pages; list_move(&page->lru, dst); Or writeback clearing PG_writeback: lock_page_memcg(page); if (mapping && mapping_use_writeback_tags(mapping)) { xa_lock_irqsave(&mapping->i_pages, flags); ret = TestClearPageWriteback(page); if (ret) { __xa_clear_mark(&mapping->i_pages, page_index(page), PAGECACHE_TAG_WRITEBACK); if (bdi->capabilities & BDI_CAP_WRITEBACK_ACCT) { dec_wb_stat(wb, WB_WRITEBACK); __wb_writeout_inc(wb); } } if (mapping->host && !mapping_tagged(mapping, PAGECACHE_TAG_WRITEBACK)) sb_clear_inode_writeback(mapping->host); xa_unlock_irqrestore(&mapping->i_pages, flags); } else { ret = TestClearPageWriteback(page); } if (ret) { dec_lruvec_page_state(page, NR_WRITEBACK); dec_zone_page_state(page, NR_ZONE_WRITE_PENDING); inc_node_page_state(page, NR_WRITTEN); } unlock_page_memcg(page); It's somewhat unfortunate to lose that bit of extra help when navigating the code, but I suppose we can live without it. > I agree that significantly changing the naming of things is a majoy > PITA, but given the level of refactoring at that, I think folio_ beats > pageymcpageface_. Give it some time to get used to it... I'll try ;-)
On Tue, Jul 13, 2021 at 11:55:04AM -0400, Johannes Weiner wrote: > On Tue, Jul 13, 2021 at 11:15:33AM +0200, Peter Zijlstra wrote: > > On Tue, Jul 13, 2021 at 03:15:10AM +0100, Matthew Wilcox wrote: > > > On Mon, Jul 12, 2021 at 08:24:09PM -0400, Johannes Weiner wrote: > > > > On Mon, Jul 12, 2021 at 04:04:54AM +0100, Matthew Wilcox (Oracle) wrote: > > > > > +/* Whether there are one or multiple pages in a folio */ > > > > > +static inline bool folio_single(struct folio *folio) > > > > > +{ > > > > > + return !folio_head(folio); > > > > > +} > > > > > > > > Reading more converted code in the series, I keep tripping over the > > > > new non-camelcased flag testers. > > > > > > Added PeterZ as he asked for it. > > > > > > https://lore.kernel.org/linux-mm/20210419135528.GC2531743@casper.infradead.org/ > > > > Aye; I hate me some Camels with a passion. And Linux Coding style > > explicitly not having Camels these things were always a sore spot. I'm > > very glad to see them go. > > > > > > It's not an issue when it's adjectives: folio_uptodate(), > > > > folio_referenced(), folio_locked() etc. - those are obvious. But nouns > > > > and words that overlap with struct member names can easily be confused > > > > with non-bool accessors and lookups. Pop quiz: flag test or accessor? > > > > > > > > folio_private() > > > > folio_lru() > > > > folio_nid() > > > > folio_head() > > > > folio_mapping() > > > > folio_slab() > > > > folio_waiters() > > > > > > I know the answers to each of those, but your point is valid. So what's > > > your preferred alternative? folio_is_lru(), folio_is_uptodate(), > > > folio_is_slab(), etc? I've seen suggestions for folio_test_lru(), > > > folio_test_uptodate(), and I don't much care for that alternative. > > > > Either _is_ or _test_ works for me, with a slight preference to _is_ on > > account it of being shorter. > > I agree that _is_ reads nicer by itself, but paired with other ops > such as testset, _test_ might be better. > > For example, in __set_page_dirty_no_writeback() > > if (folio_is_dirty()) > return !folio_testset_dirty() > > is less clear about what's going on than would be: > > if (folio_test_dirty()) > return !folio_testset_dirty() > > My other example wasn't quoted, but IMO set and clear naming should > also match testing to not cause confusion. I.e. the current: > > if (folio_idle()) > folio_clear_idle_flag() > > can make you think two different things are being tested and modified > (as in if (page_evictable()) ClearPageUnevictable()). IMO easier: > > if (folio_test_idle()) > folio_clear_idle() > > Non-atomics would have the __ modifier in front of folio rather than > read __clear or __set, which works I suppose? > > __folio_clear_dirty() > > With all that, we'd have something like: > > folio_test_foo() > folio_set_foo() > folio_clear_foo() > folio_testset_foo() > folio_testclear_foo() > > __folio_test_foo() BTW, this one doesn't exist. > __folio_set_foo() > __folio_clear_foo() > > Would that be a workable compromise for everybody? I think it has to be, because not all these work (marked with *): folio_is_locked() folio_is_referenced() folio_is_uptodate() folio_is_dirty() * folio_is_lru() folio_is_active() folio_is_workingset() * folio_is_waiters() folio_is_error() folio_is_slab() * folio_is_owner_priv_1() * folio_is_arch_1() folio_is_reserved() * folio_is_private() * folio_is_private_2() folio_is_writeback() + folio_is_head() folio_is_mappedtodisk() * folio_is_reclaim() folio_is_swapbacked() folio_is_unevictable() folio_is_mlocked() folio_is_uncached() * folio_is_hwpoison() folio_is_young() folio_is_idle() folio_is_arch_2() * folio_is_skip_kasan_poison() folio_is_readahead() folio_is_checked() folio_is_swapcache() folio_is_fscache() folio_is_pinned() folio_is_savepinned() folio_is_foreign() folio_is_xen_remapped() folio_is_slob_free() folio_is_double_map() folio_is_isolated() * folio_is_reported() > > Yes, we -tip folk tend to also prefer consistent prefix_ naming, and > > every time something big gets refactorered we make sure to make it so. > > > > Look at it like a namespace; you can read it like > > folio::del_from_lru_list() if you want. Obviously there's nothing like > > 'using folio' for this being C and not C++. > > Yeah the lack of `using` is my concern. > > Namespacing is nice for more contained APIs. Classic class + method > type deals, with non-namespaced private helpers implementing public > methods, and public methods not layered past trivial stuff like > foo_insert() calling __foo_insert() with a lock held. > > memcg, vmalloc, kobject, you name it. > > But the page api is pretty sprawling with sizable overlaps between > interface and implementation, and heavy layering in both. `using` > would be great to avoid excessive repetition where file or function > context already does plenty of namespacing. Alas, it's not an option. I mean, we could do ... #include <linux/using_folio.h> which makes bool test_writeback(struct folio *) an alias of folio_test_writeback. But I don't know that's a great thing to do. It makes it hard for people to get started in mm, hard to move code between mm and other parts of the kernel, or between mm/ and include/ Maybe I'm missing something important about 'using'. It's been over twenty years since I wrote Java in earnest and twenty-five since I wrote a single line of Ada, so I'm a little rusty with the concept of namespacing. > If everybody agrees we'll be fine, I won't stand in the way. But I do > think the page API is a bit unusual in that regard. And while it is > nice for the outward-facing filesystem interface - and I can see why > fs people love it - the cost of it seems to be carried by the MM > implementation code. I'm actually OK with that tradeoff. There are more filesystem people than MM people, and their concern is with how to implement their filesystem, not with how the page cache works. So if the MM side of the house needs to be a little more complicated to make filesystems simpler, then that's fine with me. > Although I will say, folio_del_from_lru_list() reads a bit like > 'a'.append_to(string) to me. lruvec_add_folio() would match more > conventional object hierarchy for container/collection/list/array > interactions, like with list_add, xa_store, rb_insert, etc. > > Taking all of the above, we'd have: > > if (!folio_test_active(folio) && !folio_test_unevictable(folio)) { > lruvec_del_folio(folio, lruvec); > folio_set_active(folio); > lruvec_add_folio(folio, lruvec); > trace_mm_lru_activate(&folio->page); > } > > which reads a little better overall, IMO. > > Is that a direction we could agree on? Yes! I have that ordering already with filemap_add_folio(). I don't mind doing that for lruvec too. But, it should then be: lruvec_del_folio(lruvec, folio); folio_set_active(folio); lruvec_add_folio(lruvec, folio); trace_mm_lru_activate(folio);
On Tue, 13 Jul 2021 11:55:04 -0400 Johannes Weiner <hannes@cmpxchg.org> wrote: > On Tue, Jul 13, 2021 at 11:15:33AM +0200, Peter Zijlstra wrote: > > On Tue, Jul 13, 2021 at 03:15:10AM +0100, Matthew Wilcox wrote: > > > On Mon, Jul 12, 2021 at 08:24:09PM -0400, Johannes Weiner wrote: > > > > On Mon, Jul 12, 2021 at 04:04:54AM +0100, Matthew Wilcox (Oracle) wrote: > > > > > +/* Whether there are one or multiple pages in a folio */ > > > > > +static inline bool folio_single(struct folio *folio) > > > > > +{ > > > > > + return !folio_head(folio); > > > > > +} > > > > > > > > Reading more converted code in the series, I keep tripping over the > > > > new non-camelcased flag testers. > > > > > > Added PeterZ as he asked for it. > > > > > > https://lore.kernel.org/linux-mm/20210419135528.GC2531743@casper.infradead.org/ > > > > Aye; I hate me some Camels with a passion. And Linux Coding style > > explicitly not having Camels these things were always a sore spot. I'm > > very glad to see them go. > > > > > > It's not an issue when it's adjectives: folio_uptodate(), > > > > folio_referenced(), folio_locked() etc. - those are obvious. But nouns > > > > and words that overlap with struct member names can easily be confused > > > > with non-bool accessors and lookups. Pop quiz: flag test or accessor? > > > > > > > > folio_private() > > > > folio_lru() > > > > folio_nid() > > > > folio_head() > > > > folio_mapping() > > > > folio_slab() > > > > folio_waiters() > > > > > > I know the answers to each of those, but your point is valid. So what's > > > your preferred alternative? folio_is_lru(), folio_is_uptodate(), > > > folio_is_slab(), etc? I've seen suggestions for folio_test_lru(), > > > folio_test_uptodate(), and I don't much care for that alternative. > > > > Either _is_ or _test_ works for me, with a slight preference to _is_ on > > account it of being shorter. Useful discussion, and quite important. Thanks for bringing it up. > I agree that _is_ reads nicer by itself, but paired with other ops > such as testset, _test_ might be better. > > For example, in __set_page_dirty_no_writeback() > > if (folio_is_dirty()) > return !folio_testset_dirty() > > is less clear about what's going on than would be: > > if (folio_test_dirty()) > return !folio_testset_dirty() I like folio_is_foo(). As long as it is used consistently, we'll get used to it quickly. Some GNU tools are careful about appending "_p" to functions-which-test-something (stands for "predicate"). Having spent a lot of time a long time ago with my nose in this stuff, I found the convention to be very useful. I think foo_is_bar() is as good as foo_bar_p() in this regard. > > folio_test_foo() > folio_set_foo() > folio_clear_foo() > folio_testset_foo() > folio_testclear_foo() Agree with everyone else about prefixing every symbol with "folio_". Although at times there will be heartache over which subsystem the function actually belongs to. For example, a hypothetical function which writes back a folio to disk could be writeback_folio() or folio_writeback(). Really it's a part of writeback so should be writeback_folio(). Plus folio isn't really a subsystem. But then, neither is spin_lock much, and that naming works OK. And sure, the CaMeLcAsE is fugly, but it sure is useful. set_page_dirty() is very different from SetPageDirty() and boy that visual differentiation is a relief.
Johannes Weiner <hannes@cmpxchg.org> wrote: > For example, in __set_page_dirty_no_writeback() > > if (folio_is_dirty()) > return !folio_testset_dirty() > > is less clear about what's going on than would be: > > if (folio_test_dirty()) > return !folio_testset_dirty() "if (folio_is_dirty())" reads better to me as that's more or less how you'd structure a sentence beginning with "if" in English. On the other hand, folio_test_xxx() fits in with a folio_testset_xxx() naming style. English doesn't really have test-and-set operator words. David
On Tue, Jul 13, 2021 at 06:56:28PM -0700, Andrew Morton wrote: > On Tue, 13 Jul 2021 11:55:04 -0400 Johannes Weiner <hannes@cmpxchg.org> wrote: > > I agree that _is_ reads nicer by itself, but paired with other ops > > such as testset, _test_ might be better. > > > > For example, in __set_page_dirty_no_writeback() > > > > if (folio_is_dirty()) > > return !folio_testset_dirty() > > > > is less clear about what's going on than would be: > > > > if (folio_test_dirty()) > > return !folio_testset_dirty() > > I like folio_is_foo(). As long as it is used consistently, we'll get > used to it quickly. I'm not sure that folio_is_private(), folio_is_lru(), folio_is_waiters(), or folio_is_reclaim() really work. > Some GNU tools are careful about appending "_p" to > functions-which-test-something (stands for "predicate"). Having spent > a lot of time a long time ago with my nose in this stuff, I found the > convention to be very useful. I think foo_is_bar() is as good as > foo_bar_p() in this regard. I just wish C let us put '?' on the end of a function name, but I recognise the ambiguity with foo?bar:baz; > And sure, the CaMeLcAsE is fugly, but it sure is useful. > set_page_dirty() is very different from SetPageDirty() and boy that > visual differentiation is a relief. Oh, I'm glad you brought that up </sarcasm> In folios, here's how that ends up looking: SetPageDirty() -> folio_set_dirty_flag() (johannes proposes folio_set_dirty instead) set_page_dirty() -> folio_mark_dirty() aops->set_page_dirty() -> aops->dirty_folio() __set_page_dirty() -> __folio_mark_dirty() __set_page_dirty_buffers() -> block_dirty_folio() __set_page_dirty_nobuffers() -> filemap_dirty_folio() __set_page_dirty_no_writeback() -> dirty_folio_no_writeback() I kind of feel that last one should be nowb_dirty_folio(), but I'm also hoping to eliminate it; if the filesystem sets AS_NO_WRITEBACK_TAGS in mapping->flags, then we just inline the no-writeback case into folio_mark_dirty() (which already has it for the !mapping case).
diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h index 70ede8345538..fb914468b302 100644 --- a/include/linux/page-flags.h +++ b/include/linux/page-flags.h @@ -143,6 +143,8 @@ enum pageflags { #endif __NR_PAGEFLAGS, + PG_readahead = PG_reclaim, + /* Filesystems */ PG_checked = PG_owner_priv_1, @@ -243,6 +245,15 @@ static inline void page_init_poison(struct page *page, size_t size) } #endif +static unsigned long *folio_flags(struct folio *folio, unsigned n) +{ + struct page *page = &folio->page; + + VM_BUG_ON_PGFLAGS(PageTail(page), page); + VM_BUG_ON_PGFLAGS(n > 0 && !test_bit(PG_head, &page->flags), page); + return &page[n].flags; +} + /* * Page flags policies wrt compound pages * @@ -287,36 +298,64 @@ static inline void page_init_poison(struct page *page, size_t size) VM_BUG_ON_PGFLAGS(!PageHead(page), page); \ PF_POISONED_CHECK(&page[1]); }) +/* Which page is the flag stored in */ +#define FOLIO_PF_ANY 0 +#define FOLIO_PF_HEAD 0 +#define FOLIO_PF_ONLY_HEAD 0 +#define FOLIO_PF_NO_TAIL 0 +#define FOLIO_PF_NO_COMPOUND 0 +#define FOLIO_PF_SECOND 1 + /* * Macros to create function definitions for page flags */ #define TESTPAGEFLAG(uname, lname, policy) \ +static __always_inline bool folio_##lname(struct folio *folio) \ +{ return test_bit(PG_##lname, folio_flags(folio, FOLIO_##policy)); } \ static __always_inline int Page##uname(struct page *page) \ - { return test_bit(PG_##lname, &policy(page, 0)->flags); } +{ return test_bit(PG_##lname, &policy(page, 0)->flags); } #define SETPAGEFLAG(uname, lname, policy) \ +static __always_inline \ +void folio_set_##lname##_flag(struct folio *folio) \ +{ set_bit(PG_##lname, folio_flags(folio, FOLIO_##policy)); } \ static __always_inline void SetPage##uname(struct page *page) \ - { set_bit(PG_##lname, &policy(page, 1)->flags); } +{ set_bit(PG_##lname, &policy(page, 1)->flags); } #define CLEARPAGEFLAG(uname, lname, policy) \ +static __always_inline \ +void folio_clear_##lname##_flag(struct folio *folio) \ +{ clear_bit(PG_##lname, folio_flags(folio, FOLIO_##policy)); } \ static __always_inline void ClearPage##uname(struct page *page) \ - { clear_bit(PG_##lname, &policy(page, 1)->flags); } +{ clear_bit(PG_##lname, &policy(page, 1)->flags); } #define __SETPAGEFLAG(uname, lname, policy) \ +static __always_inline \ +void __folio_set_##lname##_flag(struct folio *folio) \ +{ __set_bit(PG_##lname, folio_flags(folio, FOLIO_##policy)); } \ static __always_inline void __SetPage##uname(struct page *page) \ - { __set_bit(PG_##lname, &policy(page, 1)->flags); } +{ __set_bit(PG_##lname, &policy(page, 1)->flags); } #define __CLEARPAGEFLAG(uname, lname, policy) \ +static __always_inline \ +void __folio_clear_##lname##_flag(struct folio *folio) \ +{ __clear_bit(PG_##lname, folio_flags(folio, FOLIO_##policy)); } \ static __always_inline void __ClearPage##uname(struct page *page) \ - { __clear_bit(PG_##lname, &policy(page, 1)->flags); } +{ __clear_bit(PG_##lname, &policy(page, 1)->flags); } #define TESTSETFLAG(uname, lname, policy) \ +static __always_inline \ +bool folio_test_set_##lname##_flag(struct folio *folio) \ +{ return test_and_set_bit(PG_##lname, folio_flags(folio, FOLIO_##policy)); } \ static __always_inline int TestSetPage##uname(struct page *page) \ - { return test_and_set_bit(PG_##lname, &policy(page, 1)->flags); } +{ return test_and_set_bit(PG_##lname, &policy(page, 1)->flags); } #define TESTCLEARFLAG(uname, lname, policy) \ +static __always_inline \ +bool folio_test_clear_##lname##_flag(struct folio *folio) \ +{ return test_and_clear_bit(PG_##lname, folio_flags(folio, FOLIO_##policy)); } \ static __always_inline int TestClearPage##uname(struct page *page) \ - { return test_and_clear_bit(PG_##lname, &policy(page, 1)->flags); } +{ return test_and_clear_bit(PG_##lname, &policy(page, 1)->flags); } #define PAGEFLAG(uname, lname, policy) \ TESTPAGEFLAG(uname, lname, policy) \ @@ -332,29 +371,37 @@ static __always_inline int TestClearPage##uname(struct page *page) \ TESTSETFLAG(uname, lname, policy) \ TESTCLEARFLAG(uname, lname, policy) -#define TESTPAGEFLAG_FALSE(uname) \ +#define TESTPAGEFLAG_FALSE(uname, lname) \ +static inline bool folio_##lname(const struct folio *folio) { return 0; } \ static inline int Page##uname(const struct page *page) { return 0; } -#define SETPAGEFLAG_NOOP(uname) \ +#define SETPAGEFLAG_NOOP(uname, lname) \ +static inline void folio_set_##lname##_flag(struct folio *folio) { } \ static inline void SetPage##uname(struct page *page) { } -#define CLEARPAGEFLAG_NOOP(uname) \ +#define CLEARPAGEFLAG_NOOP(uname, lname) \ +static inline void folio_clear_##lname##_flag(struct folio *folio) { } \ static inline void ClearPage##uname(struct page *page) { } -#define __CLEARPAGEFLAG_NOOP(uname) \ +#define __CLEARPAGEFLAG_NOOP(uname, lname) \ +static inline void __folio_clear_##lname_flags(struct folio *folio) { } \ static inline void __ClearPage##uname(struct page *page) { } -#define TESTSETFLAG_FALSE(uname) \ +#define TESTSETFLAG_FALSE(uname, lname) \ +static inline bool folio_test_set_##lname##_flag(struct folio *folio) \ +{ return 0; } \ static inline int TestSetPage##uname(struct page *page) { return 0; } -#define TESTCLEARFLAG_FALSE(uname) \ +#define TESTCLEARFLAG_FALSE(uname, lname) \ +static inline bool folio_test_clear_##lname##_flag(struct folio *folio) \ +{ return 0; } \ static inline int TestClearPage##uname(struct page *page) { return 0; } -#define PAGEFLAG_FALSE(uname) TESTPAGEFLAG_FALSE(uname) \ - SETPAGEFLAG_NOOP(uname) CLEARPAGEFLAG_NOOP(uname) +#define PAGEFLAG_FALSE(uname, lname) TESTPAGEFLAG_FALSE(uname, lname) \ + SETPAGEFLAG_NOOP(uname, lname) CLEARPAGEFLAG_NOOP(uname, lname) -#define TESTSCFLAG_FALSE(uname) \ - TESTSETFLAG_FALSE(uname) TESTCLEARFLAG_FALSE(uname) +#define TESTSCFLAG_FALSE(uname, lname) \ + TESTSETFLAG_FALSE(uname, lname) TESTCLEARFLAG_FALSE(uname, lname) __PAGEFLAG(Locked, locked, PF_NO_TAIL) PAGEFLAG(Waiters, waiters, PF_ONLY_HEAD) __CLEARPAGEFLAG(Waiters, waiters, PF_ONLY_HEAD) @@ -410,8 +457,8 @@ PAGEFLAG(MappedToDisk, mappedtodisk, PF_NO_TAIL) /* PG_readahead is only used for reads; PG_reclaim is only for writes */ PAGEFLAG(Reclaim, reclaim, PF_NO_TAIL) TESTCLEARFLAG(Reclaim, reclaim, PF_NO_TAIL) -PAGEFLAG(Readahead, reclaim, PF_NO_COMPOUND) - TESTCLEARFLAG(Readahead, reclaim, PF_NO_COMPOUND) +PAGEFLAG(Readahead, readahead, PF_NO_COMPOUND) + TESTCLEARFLAG(Readahead, readahead, PF_NO_COMPOUND) #ifdef CONFIG_HIGHMEM /* @@ -420,22 +467,25 @@ PAGEFLAG(Readahead, reclaim, PF_NO_COMPOUND) */ #define PageHighMem(__p) is_highmem_idx(page_zonenum(__p)) #else -PAGEFLAG_FALSE(HighMem) +PAGEFLAG_FALSE(HighMem, highmem) #endif #ifdef CONFIG_SWAP -static __always_inline int PageSwapCache(struct page *page) +static __always_inline bool folio_swapcache(struct folio *folio) { -#ifdef CONFIG_THP_SWAP - page = compound_head(page); -#endif - return PageSwapBacked(page) && test_bit(PG_swapcache, &page->flags); + return folio_swapbacked(folio) && + test_bit(PG_swapcache, folio_flags(folio, 0)); +} +static __always_inline bool PageSwapCache(struct page *page) +{ + return folio_swapcache(page_folio(page)); } + SETPAGEFLAG(SwapCache, swapcache, PF_NO_TAIL) CLEARPAGEFLAG(SwapCache, swapcache, PF_NO_TAIL) #else -PAGEFLAG_FALSE(SwapCache) +PAGEFLAG_FALSE(SwapCache, swapcache) #endif PAGEFLAG(Unevictable, unevictable, PF_HEAD) @@ -447,14 +497,14 @@ PAGEFLAG(Mlocked, mlocked, PF_NO_TAIL) __CLEARPAGEFLAG(Mlocked, mlocked, PF_NO_TAIL) TESTSCFLAG(Mlocked, mlocked, PF_NO_TAIL) #else -PAGEFLAG_FALSE(Mlocked) __CLEARPAGEFLAG_NOOP(Mlocked) - TESTSCFLAG_FALSE(Mlocked) +PAGEFLAG_FALSE(Mlocked, mlocked) __CLEARPAGEFLAG_NOOP(Mlocked, mlocked) + TESTSCFLAG_FALSE(Mlocked, mlocked) #endif #ifdef CONFIG_ARCH_USES_PG_UNCACHED PAGEFLAG(Uncached, uncached, PF_NO_COMPOUND) #else -PAGEFLAG_FALSE(Uncached) +PAGEFLAG_FALSE(Uncached, uncached) #endif #ifdef CONFIG_MEMORY_FAILURE @@ -463,7 +513,7 @@ TESTSCFLAG(HWPoison, hwpoison, PF_ANY) #define __PG_HWPOISON (1UL << PG_hwpoison) extern bool take_page_off_buddy(struct page *page); #else -PAGEFLAG_FALSE(HWPoison) +PAGEFLAG_FALSE(HWPoison, hwpoison) #define __PG_HWPOISON 0 #endif @@ -477,7 +527,7 @@ PAGEFLAG(Idle, idle, PF_ANY) #ifdef CONFIG_KASAN_HW_TAGS PAGEFLAG(SkipKASanPoison, skip_kasan_poison, PF_HEAD) #else -PAGEFLAG_FALSE(SkipKASanPoison) +PAGEFLAG_FALSE(SkipKASanPoison, skip_kasan_poison) #endif /* @@ -515,10 +565,14 @@ static __always_inline int PageMappingFlags(struct page *page) return ((unsigned long)page->mapping & PAGE_MAPPING_FLAGS) != 0; } -static __always_inline int PageAnon(struct page *page) +static __always_inline bool folio_anon(struct folio *folio) +{ + return ((unsigned long)folio->mapping & PAGE_MAPPING_ANON) != 0; +} + +static __always_inline bool PageAnon(struct page *page) { - page = compound_head(page); - return ((unsigned long)page->mapping & PAGE_MAPPING_ANON) != 0; + return folio_anon(page_folio(page)); } static __always_inline int __PageMovable(struct page *page) @@ -534,30 +588,32 @@ static __always_inline int __PageMovable(struct page *page) * is found in VM_MERGEABLE vmas. It's a PageAnon page, pointing not to any * anon_vma, but to that page's node of the stable tree. */ -static __always_inline int PageKsm(struct page *page) +static __always_inline bool folio_ksm(struct folio *folio) { - page = compound_head(page); - return ((unsigned long)page->mapping & PAGE_MAPPING_FLAGS) == + return ((unsigned long)folio->mapping & PAGE_MAPPING_FLAGS) == PAGE_MAPPING_KSM; } + +static __always_inline bool PageKsm(struct page *page) +{ + return folio_ksm(page_folio(page)); +} #else -TESTPAGEFLAG_FALSE(Ksm) +TESTPAGEFLAG_FALSE(Ksm, ksm) #endif u64 stable_page_flags(struct page *page); -static inline int PageUptodate(struct page *page) +static inline bool folio_uptodate(struct folio *folio) { - int ret; - page = compound_head(page); - ret = test_bit(PG_uptodate, &(page)->flags); + bool ret = test_bit(PG_uptodate, folio_flags(folio, 0)); /* - * Must ensure that the data we read out of the page is loaded - * _after_ we've loaded page->flags to check for PageUptodate. - * We can skip the barrier if the page is not uptodate, because + * Must ensure that the data we read out of the folio is loaded + * _after_ we've loaded folio->flags to check the uptodate bit. + * We can skip the barrier if the folio is not uptodate, because * we wouldn't be reading anything from it. * - * See SetPageUptodate() for the other side of the story. + * See folio_mark_uptodate() for the other side of the story. */ if (ret) smp_rmb(); @@ -565,23 +621,36 @@ static inline int PageUptodate(struct page *page) return ret; } -static __always_inline void __SetPageUptodate(struct page *page) +static inline int PageUptodate(struct page *page) +{ + return folio_uptodate(page_folio(page)); +} + +static __always_inline void __folio_mark_uptodate(struct folio *folio) { - VM_BUG_ON_PAGE(PageTail(page), page); smp_wmb(); - __set_bit(PG_uptodate, &page->flags); + __set_bit(PG_uptodate, folio_flags(folio, 0)); } -static __always_inline void SetPageUptodate(struct page *page) +static __always_inline void folio_mark_uptodate(struct folio *folio) { - VM_BUG_ON_PAGE(PageTail(page), page); /* * Memory barrier must be issued before setting the PG_uptodate bit, - * so that all previous stores issued in order to bring the page - * uptodate are actually visible before PageUptodate becomes true. + * so that all previous stores issued in order to bring the folio + * uptodate are actually visible before folio_uptodate becomes true. */ smp_wmb(); - set_bit(PG_uptodate, &page->flags); + set_bit(PG_uptodate, folio_flags(folio, 0)); +} + +static __always_inline void __SetPageUptodate(struct page *page) +{ + __folio_mark_uptodate((struct folio *)page); +} + +static __always_inline void SetPageUptodate(struct page *page) +{ + folio_mark_uptodate((struct folio *)page); } CLEARPAGEFLAG(Uptodate, uptodate, PF_NO_TAIL) @@ -606,6 +675,17 @@ static inline void set_page_writeback_keepwrite(struct page *page) __PAGEFLAG(Head, head, PF_ANY) CLEARPAGEFLAG(Head, head, PF_ANY) +/* Whether there are one or multiple pages in a folio */ +static inline bool folio_single(struct folio *folio) +{ + return !folio_head(folio); +} + +static inline bool folio_multi(struct folio *folio) +{ + return folio_head(folio); +} + static __always_inline void set_compound_head(struct page *page, struct page *head) { WRITE_ONCE(page->compound_head, (unsigned long)head + 1); @@ -629,12 +709,15 @@ static inline void ClearPageCompound(struct page *page) #ifdef CONFIG_HUGETLB_PAGE int PageHuge(struct page *page); int PageHeadHuge(struct page *page); +static inline bool folio_hugetlb(struct folio *folio) +{ + return PageHeadHuge(&folio->page); +} #else -TESTPAGEFLAG_FALSE(Huge) -TESTPAGEFLAG_FALSE(HeadHuge) +TESTPAGEFLAG_FALSE(Huge, hugetlb) +TESTPAGEFLAG_FALSE(HeadHuge, headhuge) #endif - #ifdef CONFIG_TRANSPARENT_HUGEPAGE /* * PageHuge() only returns true for hugetlbfs pages, but not for @@ -650,6 +733,11 @@ static inline int PageTransHuge(struct page *page) return PageHead(page); } +static inline bool folio_transhuge(struct folio *folio) +{ + return folio_head(folio); +} + /* * PageTransCompound returns true for both transparent huge pages * and hugetlbfs pages, so it should only be called when it's known @@ -723,12 +811,12 @@ static inline int PageTransTail(struct page *page) PAGEFLAG(DoubleMap, double_map, PF_SECOND) TESTSCFLAG(DoubleMap, double_map, PF_SECOND) #else -TESTPAGEFLAG_FALSE(TransHuge) -TESTPAGEFLAG_FALSE(TransCompound) -TESTPAGEFLAG_FALSE(TransCompoundMap) -TESTPAGEFLAG_FALSE(TransTail) -PAGEFLAG_FALSE(DoubleMap) - TESTSCFLAG_FALSE(DoubleMap) +TESTPAGEFLAG_FALSE(TransHuge, transhuge) +TESTPAGEFLAG_FALSE(TransCompound, transcompound) +TESTPAGEFLAG_FALSE(TransCompoundMap, transcompoundmap) +TESTPAGEFLAG_FALSE(TransTail, transtail) +PAGEFLAG_FALSE(DoubleMap, double_map) + TESTSCFLAG_FALSE(DoubleMap, double_map) #endif /* @@ -903,6 +991,11 @@ static inline int page_has_private(struct page *page) return !!(page->flags & PAGE_FLAGS_PRIVATE); } +static inline bool folio_has_private(struct folio *folio) +{ + return page_has_private(&folio->page); +} + #undef PF_ANY #undef PF_HEAD #undef PF_ONLY_HEAD