On Tue, Dec 17, 2024 at 10:49 PM Yosry Ahmed <yosryahmed@xxxxxxxxxx> wrote: > > On Tue, Dec 17, 2024 at 10:37 PM Alexei Starovoitov > <alexei.starovoitov@xxxxxxxxx> wrote: > > > > On Tue, Dec 17, 2024 at 9:58 PM Yosry Ahmed <yosryahmed@xxxxxxxxxx> wrote: > > > > > > What I mean is, functions like __free_unref_page() and > > > free_unref_page_commit() now accept fpi_flags, but any flags other > > > than FPI_TRYLOCK are essentially ignored, also not very clear. > > > > They're not ignored. They are just not useful in this context. > > I think they are. For example, if you pass FPI_SKIP_REPORT_NOTIFY to > __free_unref_page(), page_reporting_notify_free() will still be called > when the page is eventually freed to the buddy allocator. Same goes > for FPI_NO_TAIL. free_pcppages_bulk()->page_reporting_notify_free() will _not_ be called when FPI_TRYLOCK is specified. They are internal flags. The callers cannot make try_alloc_pages() pass these extra flags. The flags are more or less exclusive. > > The code rules over comment. If you have a concrete suggestion on > > how to improve the comment please say so. > > What I had in mind is adding a WARN in the pcp freeing functions if > any FPI flag but FPI_TRYLOCK is passed, and/or explicitly calling out > that other flags should not be passed as they have no effect in this > context (whether at the function definition, above the WARN, or at the > flag definitions). pcp freeing functions? In particular? tbh this sounds like defensive programming... BUILD_BUG_ON is a good thing when api is misused, but WARN will be wasting run-time cycles on something that should have been caught during code review. free_unref_page_commit() is a shallow function when FPI_TRYLOCK is used. There is no need to propagate fpi_flags further into free_pcppages_bulk just to issue a WARN.
