Re: [PATCH v5 2/9] mm/swap: Add cluster lock

From: Huang\, Ying
Date: Wed Jan 11 2017 - 20:24:26 EST


Hi, Jonathan,

Jonathan Corbet <corbet@xxxxxxx> writes:

> On Wed, 11 Jan 2017 15:00:29 -0800
> Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
>
>> hm, bit_spin_lock() is a nasty thing. It is slow and it doesn't have
>> all the lockdep support.
>>
>> Would the world end if we added a spinlock to swap_cluster_info?
>
> FWIW, I asked the same question in December, this is what I got:

Sorry I made a mistake in the following email. I have sent another
email to correct this before from my another email address,
huang.ying.caritas@xxxxxxxxx, have you received it, copied below,

From: huang ying <huang.ying.caritas@xxxxxxxxx>
Subject: Re: [PATCH v2 2/8] mm/swap: Add cluster lock
To: "Huang, Ying" <ying.huang@xxxxxxxxx>
CC: Jonathan Corbet <corbet@xxxxxxx>, Tim Chen <tim.c.chen@xxxxxxxxxxxxxxx>,
Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>, <dave.hansen@xxxxxxxxx>, "Andi
Kleen" <ak@xxxxxxxxxxxxxxx>, Aaron Lu <aaron.lu@xxxxxxxxx>,
<linux-mm@xxxxxxxxx>, LKML <linux-kernel@xxxxxxxxxxxxxxx>, Hugh Dickins
<hughd@xxxxxxxxxx>, Shaohua Li <shli@xxxxxxxxxx>, Minchan Kim
<minchan@xxxxxxxxxx>, Rik van Riel <riel@xxxxxxxxxx>, Andrea Arcangeli
<aarcange@xxxxxxxxxx>, "Kirill A . Shutemov"
<kirill.shutemov@xxxxxxxxxxxxxxx>, Vladimir Davydov <vdavydov.dev@xxxxxxxxx>,
Johannes Weiner <hannes@xxxxxxxxxxx>, Michal Hocko <mhocko@xxxxxxxxxx>, Hillf
Danton <hillf.zj@xxxxxxxxxxxxxxx>
Date: Wed, 28 Dec 2016 11:34:01 +0800 (2 weeks, 21 hours, 45 minutes ago)

Hi, Jonathan,

On Tue, Oct 25, 2016 at 10:05 AM, Huang, Ying <ying.huang@xxxxxxxxx> wrote:
> Hi, Jonathan,
>
> Thanks for review.
>
> Jonathan Corbet <corbet@xxxxxxx> writes:
>
>> On Thu, 20 Oct 2016 16:31:41 -0700
>> Tim Chen <tim.c.chen@xxxxxxxxxxxxxxx> wrote:
>>
>>> From: "Huang, Ying" <ying.huang@xxxxxxxxx>
>>>
>>> This patch is to reduce the lock contention of swap_info_struct->lock
>>> via using a more fine grained lock in swap_cluster_info for some swap
>>> operations. swap_info_struct->lock is heavily contended if multiple

[...]

>> The cost, of course, is the growth of this structure, but you've already
>> noted that the overhead isn't all that high; seems like it could be worth
>> it.
>
> Yes. The data structure you proposed is much easier to be used than the
> current one. The main concern is the RAM usage. The size of the data
> structure you proposed is about 80 bytes, while that of the current one
> is about 8 bytes. There will be one struct swap_cluster_info for every
> 1MB swap space, so for 1TB swap space, the total size will be 80M
> compared with 8M of current implementation.

Sorry, I turned on the lockdep when measure the size change, so the
previous size change data is wrong. The size of the data structure
you proposed is 12 bytes. While that of the current one is 8 bytes on
64 bit platform and 4 bytes on 32 bit platform.

Best Regards,
Huang, Ying