On Tue, Dec 17, 2024 at 11:25 PM Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx> wrote: > > 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. I meant WARN in free_unref_page_commit() if any other FPI flags are used. Anyway, I don't feel strongly about this as I mentioned earlier.
