[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <cb49a573-8d24-d289-199d-8058cd79b89b@linux.alibaba.com>
Date: Wed, 29 Jul 2020 08:48:00 +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>,
Michal Hocko <mhocko@...nel.org>,
Vladimir Davydov <vdavydov.dev@...il.com>
Subject: Re: [PATCH v17 17/21] mm/lru: replace pgdat lru_lock with lruvec lock
在 2020/7/28 下午11:55, Alexander Duyck 写道:
>> /*
>> @@ -511,11 +511,11 @@ static bool compact_lock_irqsave(spinlock_t *lock, unsigned long *flags,
>> * scheduled)
>> */
>> static bool compact_unlock_should_abort(spinlock_t *lock,
>> - unsigned long flags, bool *locked, struct compact_control *cc)
>> + unsigned long flags, void **locked, struct compact_control *cc)
> Instead of passing both a void pointer and the lock why not just pass
> the pointer to the lock pointer? You could combine lock and locked
> into a single argument and save yourself some extra effort.
>
the passed locked pointer could be rewrite in the func, that is unacceptable if it is a lock which could
be used other place.
And it is alreay dangerous to NULL a local pointer. In fact, I perfer the orignal verion, not so smart
but rebust enough for future changes, right?
Thanks
Alex
>> {
>> if (*locked) {
>> spin_unlock_irqrestore(lock, flags);
>> - *locked = false;
>> + *locked = NULL;
>> }
>>
>> if (fatal_signal_pending(current)) {
Powered by blists - more mailing lists