Re: [PATCH] mm: use in_atomic() in print_vma_addr()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 11/3/17 11:02 AM, Andrew Morton wrote:
On Fri, 03 Nov 2017 01:44:44 +0800 "Yang Shi" <yang.s@xxxxxxxxxxxxxxx> wrote:

I may not articulate it in the commit log

You should have done so ;)

Yes, definitely. I could done it much better.


Here's the changelog I ended up with:

: From: "Yang Shi" <yang.s@xxxxxxxxxxxxxxx>
: Subject: mm: use in_atomic() in print_vma_addr()
:
: 3e51f3c4004c9b ("sched/preempt: Remove PREEMPT_ACTIVE unmasking off
: in_atomic()") uses in_atomic() just check the preempt count, so it is not
: necessary to use preempt_count() in print_vma_addr() any more.  Replace
: preempt_count() to in_atomic() which is a generic API for checking atomic
: context.
:
: in_atomic() is the preferred API for checking atomic context instead of
: preempt_count() which should be used for retrieving the preemption count
: value.
:
: If we go through the kernel code, almost everywhere "in_atomic" is used
: for such use case already, except two places:
:
: - print_vma_addr()
: - debug_smp_processor_id()
:
: Both came from Ingo long time ago before 3e51f3c4004c9b01 ("sched/preempt:
: Remove PREEMPT_ACTIVE unmasking off in_atomic()").  But, after this commit
: was merged, use in_atomic() to follow the convention.
:
: Link: http://lkml.kernel.org/r/1509572313-102989-1-git-send-email-yang.s@xxxxxxxxxxxxxxx
: Signed-off-by: Yang Shi <yang.s@xxxxxxxxxxxxxxx>
: Acked-by: Michal Hocko <mhocko@xxxxxxxx>
: Cc: Frederic Weisbecker <fweisbec@xxxxxxxxx>
: Cc: Ingo Molnar <mingo@xxxxxxx>

Thanks a lot for reworking the commit log.




Also, checkpatch says

WARNING: use of in_atomic() is incorrect outside core kernel code
#43: FILE: mm/memory.c:4491:
+       if (in_atomic())

I don't recall why we did that, but perhaps this should be revisited?

I think the rule for in_atomic is obsolete in checkpatch.pl. A quick grep shows in_atomic() is used by arch, drivers, crypto, even though the comment in include/linux/preempt.h says in_atomic() should be not used by drivers.

However, the message could be ignored with --ignore=IN_ATOMIC. But, it sounds better to fix the wrong rule and maybe even the comment in include/linux/preempt.h since it sounds confusing.

Thanks,
Yang


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]
  Powered by Linux