| Message ID | 20190725184253.21160-2-lpf.vector@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/10] mm/page_alloc: use unsigned int for "order" in should_compact_retry() | expand |
On Fri, Jul 26, 2019 at 02:42:44AM +0800, Pengfei Li wrote: > static inline bool > -should_compact_retry(struct alloc_context *ac, int order, int alloc_flags, > - enum compact_result compact_result, > - enum compact_priority *compact_priority, > - int *compaction_retries) > +should_compact_retry(struct alloc_context *ac, unsigned int order, > + int alloc_flags, enum compact_result compact_result, > + enum compact_priority *compact_priority, int *compaction_retries) > { > int max_retries = MAX_COMPACT_RETRIES; One tab here is insufficient indentation. It should be at least two. Some parts of the kernel insist on lining up arguments with the opening parenthesis of the function; I don't know if mm really obeys this rule, but you're indenting function arguments to the same level as the opening variables of the function, which is confusing.
On Fri, Jul 26, 2019 at 2:58 AM Matthew Wilcox <willy@infradead.org> wrote: > > On Fri, Jul 26, 2019 at 02:42:44AM +0800, Pengfei Li wrote: > > static inline bool > > -should_compact_retry(struct alloc_context *ac, int order, int alloc_flags, > > - enum compact_result compact_result, > > - enum compact_priority *compact_priority, > > - int *compaction_retries) > > +should_compact_retry(struct alloc_context *ac, unsigned int order, > > + int alloc_flags, enum compact_result compact_result, > > + enum compact_priority *compact_priority, int *compaction_retries) > > { > > int max_retries = MAX_COMPACT_RETRIES; > > One tab here is insufficient indentation. It should be at least two. Thanks for your comments. > Some parts of the kernel insist on lining up arguments with the opening > parenthesis of the function; I don't know if mm really obeys this rule, > but you're indenting function arguments to the same level as the opening > variables of the function, which is confusing. I will use two tabs in the next version. -- Pengfei
diff --git a/include/trace/events/oom.h b/include/trace/events/oom.h index 26a11e4a2c36..b7fa989349c7 100644 --- a/include/trace/events/oom.h +++ b/include/trace/events/oom.h @@ -154,7 +154,7 @@ TRACE_EVENT(skip_task_reaping, #ifdef CONFIG_COMPACTION TRACE_EVENT(compact_retry, - TP_PROTO(int order, + TP_PROTO(unsigned int order, enum compact_priority priority, enum compact_result result, int retries, @@ -164,7 +164,7 @@ TRACE_EVENT(compact_retry, TP_ARGS(order, priority, result, retries, max_retries, ret), TP_STRUCT__entry( - __field( int, order) + __field(unsigned int, order) __field( int, priority) __field( int, result) __field( int, retries) @@ -181,7 +181,7 @@ TRACE_EVENT(compact_retry, __entry->ret = ret; ), - TP_printk("order=%d priority=%s compaction_result=%s retries=%d max_retries=%d should_retry=%d", + TP_printk("order=%u priority=%s compaction_result=%s retries=%d max_retries=%d should_retry=%d", __entry->order, __print_symbolic(__entry->priority, COMPACTION_PRIORITY), __print_symbolic(__entry->result, COMPACTION_FEEDBACK), diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 272c6de1bf4e..75c18f4fd66a 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -3940,10 +3940,9 @@ __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order, } static inline bool -should_compact_retry(struct alloc_context *ac, int order, int alloc_flags, - enum compact_result compact_result, - enum compact_priority *compact_priority, - int *compaction_retries) +should_compact_retry(struct alloc_context *ac, unsigned int order, + int alloc_flags, enum compact_result compact_result, + enum compact_priority *compact_priority, int *compaction_retries) { int max_retries = MAX_COMPACT_RETRIES; int min_priority;
Like another should_compact_retry(), use unsigned int for "order". And modify trace_compact_retry() accordingly. Signed-off-by: Pengfei Li <lpf.vector@gmail.com> --- include/trace/events/oom.h | 6 +++--- mm/page_alloc.c | 7 +++---- 2 files changed, 6 insertions(+), 7 deletions(-)