lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 6 Aug 2020 09:39:12 +0800 From: Alex Shi <alex.shi@...ux.alibaba.com> To: Alexander Duyck <alexander.duyck@...il.com> Cc: Andrew Morton <akpm@...ux-foundation.org>, Mel Gorman <mgorman@...hsingularity.net>, Tejun Heo <tj@...nel.org>, Hugh Dickins <hughd@...gle.com>, Konstantin Khlebnikov <khlebnikov@...dex-team.ru>, Daniel Jordan <daniel.m.jordan@...cle.com>, Yang Shi <yang.shi@...ux.alibaba.com>, Matthew Wilcox <willy@...radead.org>, Johannes Weiner <hannes@...xchg.org>, kbuild test robot <lkp@...el.com>, linux-mm <linux-mm@...ck.org>, LKML <linux-kernel@...r.kernel.org>, cgroups@...r.kernel.org, Shakeel Butt <shakeelb@...gle.com>, Joonsoo Kim <iamjoonsoo.kim@....com>, Wei Yang <richard.weiyang@...il.com>, "Kirill A. Shutemov" <kirill@...temov.name>, Rong Chen <rong.a.chen@...el.com>, Andrey Ryabinin <aryabinin@...tuozzo.com>, Jann Horn <jannh@...gle.com> Subject: Re: [PATCH v17 21/21] mm/lru: revise the comments of lru_lock 在 2020/8/4 下午10:29, Alexander Duyck 写道: > On Tue, Aug 4, 2020 at 3:04 AM Alex Shi <alex.shi@...ux.alibaba.com> wrote: >> >> >> >> 在 2020/8/4 上午6:37, Alexander Duyck 写道: >>>> >>>> shrink_inactive_list() also diverts any unevictable pages that it finds on the >>>> -inactive lists to the appropriate zone's unevictable list. >>>> +inactive lists to the appropriate node's unevictable list. >>>> >>>> shrink_inactive_list() should only see SHM_LOCK'd pages that became SHM_LOCK'd >>>> after shrink_active_list() had moved them to the inactive list, or pages mapped >>> Same here. >> >> lruvec is used per memcg per node actually, and it fallback to node if memcg disabled. >> So the comments are still right. >> >> And most of changes just fix from zone->lru_lock to pgdat->lru_lock change. > > Actually in my mind one thing that might work better would be to > explain what the lruvec is and where it resides. Then replace zone > with lruvec since that is really where the unevictable list resides. > Then it would be correct for both the memcg and pgdat case. Could you like to revise the doc as your thought? > >>> >>>> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h >>>> index 64ede5f150dc..44738cdb5a55 100644 >>>> --- a/include/linux/mm_types.h >>>> +++ b/include/linux/mm_types.h >>>> @@ -78,7 +78,7 @@ struct page { >>>> struct { /* Page cache and anonymous pages */ >>>> /** >>>> * @lru: Pageout list, eg. active_list protected by >>>> - * pgdat->lru_lock. Sometimes used as a generic list >>>> + * lruvec->lru_lock. Sometimes used as a generic list >>>> * by the page owner. >>>> */ >>>> struct list_head lru; >>>> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h >>>> index 8af956aa13cf..c92289a4e14d 100644 >>>> --- a/include/linux/mmzone.h >>>> +++ b/include/linux/mmzone.h >>>> @@ -115,7 +115,7 @@ static inline bool free_area_empty(struct free_area *area, int migratetype) >>>> struct pglist_data; >>>> >>>> /* >>>> - * zone->lock and the zone lru_lock are two of the hottest locks in the kernel. >>>> + * zone->lock and the lru_lock are two of the hottest locks in the kernel. >>>> * So add a wild amount of padding here to ensure that they fall into separate >>>> * cachelines. There are very few zone structures in the machine, so space >>>> * consumption is not a concern here. >>> So I don't believe you are using ZONE_PADDING in any way to try and >>> protect the LRU lock currently. At least you aren't using it in the >>> lruvec. As such it might make sense to just drop the reference to the >>> lru_lock here. That reminds me that we still need to review the >>> placement of the lru_lock and determine if there might be a better >>> placement and/or padding that might improve performance when under >>> heavy stress. >>> >> >> Right, is it the following looks better? >> >> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h >> index ccc76590f823..0ed520954843 100644 >> --- a/include/linux/mmzone.h >> +++ b/include/linux/mmzone.h >> @@ -113,8 +113,7 @@ static inline bool free_area_empty(struct free_area *area, int migratetype) >> struct pglist_data; >> >> /* >> - * zone->lock and the lru_lock are two of the hottest locks in the kernel. >> - * So add a wild amount of padding here to ensure that they fall into separate >> + * Add a wild amount of padding here to ensure datas fall into separate >> * cachelines. There are very few zone structures in the machine, so space >> * consumption is not a concern here. >> */ >> >> Thanks! >> Alex > > I would maybe tweak it to make sure it is clear that we are using this > to pad out items that are likely to cause cache thrash such as various > hot spinocks and such. > I appreciate if you like to change the doc better. :) Thanks Alex
Powered by blists - more mailing lists