Subject: Re: [PATCH] mm/memory-hotplug: Switch locking to a percpu rwsem
To: Thomas Gleixner <tglx@linutronix.de>
Cc: Michal Hocko <mhocko@kernel.org>, LKML <linux-kernel@vger.kernel.org>,
        "linux-mm@kvack.org" <linux-mm@kvack.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        Vlastimil Babka <vbabka@suse.cz>,
        Vladimir Davydov <vdavydov.dev@gmail.com>,
        Heiko Carstens <heiko.carstens@de.ibm.com>
References: <alpine.DEB.2.20.1706291803380.1861@nanos>
 <20170630092747.GD22917@dhcp22.suse.cz>
 <alpine.DEB.2.20.1706301210210.1748@nanos>
 <3f2395c6-bbe0-23c1-fe06-d17ffbf619c3@virtuozzo.com>
 <alpine.DEB.2.20.1706301418190.1748@nanos>
From: Andrey Ryabinin <aryabinin@virtuozzo.com>
Message-ID: <fc6676da-dc20-a80c-82b3-ae479af3e6ad@virtuozzo.com>
Date: Fri, 30 Jun 2017 18:56:44 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.2.0
MIME-Version: 1.0
In-Reply-To: <alpine.DEB.2.20.1706301418190.1748@nanos>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
SpamDiagnosticOutput: 1:99
SpamDiagnosticMetadata: NSPM
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Jun 2017 15:54:30.7494 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM4PR0801MB2724
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 2103
Lines: 60

On 06/30/2017 04:00 PM, Thomas Gleixner wrote:
> On Fri, 30 Jun 2017, Andrey Ryabinin wrote:
>> On 06/30/2017 01:15 PM, Thomas Gleixner wrote:
>>> On Fri, 30 Jun 2017, Michal Hocko wrote:
>>>> So I like this simplification a lot! Even if we can get rid of the
>>>> stop_machine eventually this patch would be an improvement. A short
>>>> comment on why the per-cpu semaphore over the regular one is better
>>>> would be nice.
>>>
>>> Yes, will add one.
>>>
>>> The main point is that the current locking construct is evading lockdep due
>>> to the ability to support recursive locking, which I did not observe so
>>> far.
>>>
>>
>> Like this?
> 
> Cute.....
> 
>> [  131.023034] Call Trace:
>> [  131.023034]  dump_stack+0x85/0xc7
>> [  131.023034]  __lock_acquire+0x1747/0x17a0
>> [  131.023034]  ? lru_add_drain_all+0x3d/0x190
>> [  131.023034]  ? __mutex_lock+0x218/0x940
>> [  131.023034]  ? trace_hardirqs_on+0xd/0x10
>> [  131.023034]  lock_acquire+0x103/0x200
>> [  131.023034]  ? lock_acquire+0x103/0x200
>> [  131.023034]  ? lru_add_drain_all+0x42/0x190
>> [  131.023034]  cpus_read_lock+0x3d/0x80
>> [  131.023034]  ? lru_add_drain_all+0x42/0x190
>> [  131.023034]  lru_add_drain_all+0x42/0x190
>> [  131.023034]  __offline_pages.constprop.25+0x5de/0x870
>> [  131.023034]  offline_pages+0xc/0x10
>> [  131.023034]  memory_subsys_offline+0x43/0x70
>> [  131.023034]  device_offline+0x83/0xb0
>> [  131.023034]  store_mem_state+0xdb/0xe0
>> [  131.023034]  dev_attr_store+0x13/0x20
>> [  131.023034]  sysfs_kf_write+0x40/0x50
>> [  131.023034]  kernfs_fop_write+0x130/0x1b0
>> [  131.023034]  __vfs_write+0x23/0x130
>> [  131.023034]  ? rcu_read_lock_sched_held+0x6d/0x80
>> [  131.023034]  ? rcu_sync_lockdep_assert+0x2a/0x50
>> [  131.023034]  ? __sb_start_write+0xd4/0x1c0
>> [  131.023034]  ? vfs_write+0x1a8/0x1d0
>> [  131.023034]  vfs_write+0xc8/0x1d0
>> [  131.023034]  SyS_write+0x44/0xa0
> 
> Why didn't trigger that here? Bah, I should have become suspicious due to
> not seeing a splat ....
> 
> The patch below should cure that.
> 

FWIW, it works for me.

> Thanks,
> 
> 	tglx
>