Messages in this thread | | | Date | Fri, 2 Oct 2020 14:41:18 +0100 | From | Matthew Wilcox <> | Subject | Re: [PATCH v1 1/5] mm/page_alloc: convert "report" flag of __free_one_page() to a proper flag |
| |
On Mon, Sep 28, 2020 at 08:21:06PM +0200, David Hildenbrand wrote: > Let's prepare for additional flags and avoid long parameter lists of bools. > Follow-up patches will also make use of the flags in __free_pages_ok(), > however, I wasn't able to come up with a better name for the type - should > be good enough for internal purposes.
> +/* Free One Page flags: for internal, non-pcp variants of free_pages(). */ > +typedef int __bitwise fop_t;
That invites confusion with f_op. There's no reason to use _t as a suffix here ... why not free_f?
> +/* > + * Skip free page reporting notification for the (possibly merged) page. (will > + * *not* mark the page reported, only skip the notification).
... Don't you mean "will not skip marking the page as reported, only skip the notification"?
*reads code*
No, I'm still confused. What does this sentence mean?
Would it help to have a FOP_DEFAULT that has FOP_REPORT_NOTIFY set and then a FOP_SKIP_REPORT_NOTIFY define that is 0?
> -static inline void __free_one_page(struct page *page, > - unsigned long pfn, > - struct zone *zone, unsigned int order, > - int migratetype, bool report) > +static inline void __free_one_page(struct page *page, unsigned long pfn, > + struct zone *zone, unsigned int order, > + int migratetype, fop_t fop_flags)
Please don't over-indent like this.
static inline void __free_one_page(struct page *page, unsigned long pfn, struct zone *zone, unsigned int order, int migratetype, fop_t fop_flags)
reads just as well and then if someone needs to delete the 'static' later, they don't need to fiddle around with subsequent lines getting the whitespace to line up again.
| |