Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH] mm: use in_atomic() in print_vma_addr()
To:     Andrew Morton <akpm@linux-foundation.org>
Cc:     Michal Hocko <mhocko@kernel.org>, mingo@redhat.com,
        linux-mm@kvack.org, linux-kernel@vger.kernel.org,
        Joe Perches <joe@perches.com>
References: <1509572313-102989-1-git-send-email-yang.s@alibaba-inc.com>
 <20171102075744.whhxjmqbdkfaxghd@dhcp22.suse.cz>
 <ace5b078-652b-cbc0-176a-25f69612f7fa@alibaba-inc.com>
 <20171103110245.7049460a05cc18c7e8a9feb2@linux-foundation.org>
From:   "Yang Shi" <yang.s@alibaba-inc.com>
Message-ID: <39320f9c-95fd-9cf6-6bd9-e31655168d43@alibaba-inc.com>
Date:   Sat, 04 Nov 2017 02:16:45 +0800
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:52.0)
 Gecko/20100101 Thunderbird/52.2.1
MIME-Version: 1.0
In-Reply-To: <20171103110245.7049460a05cc18c7e8a9feb2@linux-foundation.org>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


On 11/3/17 11:02 AM, Andrew Morton wrote:
> On Fri, 03 Nov 2017 01:44:44 +0800 "Yang Shi" <yang.s@alibaba-inc.com> 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@alibaba-inc.com>
> : 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@alibaba-inc.com
> : Signed-off-by: Yang Shi <yang.s@alibaba-inc.com>
> : Acked-by: Michal Hocko <mhocko@suse.com>
> : Cc: Frederic Weisbecker <fweisbec@gmail.com>
> : Cc: Ingo Molnar <mingo@elte.hu>

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
> 

From 1583068977036411109@xxx Fri Nov 03 18:03:44 +0000 2017
X-GM-THRID: 1582901365152158645
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread