Re: [PATCH v4 3/9] mm/lru: replace pgdat lru_lock with lruvec lock

[Alex Shi <alex.shi@linux.alibaba.com>]

在 2019/11/20 上午12:44, Johannes Weiner 写道:
> On Tue, Nov 19, 2019 at 07:57:04AM -0800, Matthew Wilcox wrote:
>> On Tue, Nov 19, 2019 at 08:23:17PM +0800, Alex Shi wrote:
>>> +static inline struct lruvec *lock_page_lruvec_irqsave(struct page *page,
>>> +				struct pglist_data *pgdat, unsigned long *flags)
>>> +{
>>> +	struct lruvec *lruvec = mem_cgroup_page_lruvec(page, pgdat);
>>> +
>>> +	spin_lock_irqsave(&lruvec->lru_lock, *flags);
>>> +
>>> +	return lruvec;
>>> +}
>>
>> This should be a macro, not a function.  You basically can't do this;
>> spin_lock_irqsave needs to write to a variable which can then be passed
>> to spin_unlock_irqrestore().  What you're doing here will dereference the
>> pointer in _this_ function, but won't propagate the modified value back to
>> the caller.  I suppose you could do something like this ...
> 
> This works because spin_lock_irqsave and local_irq_save() are
> macros. It boils down to '*flags = arch_local_irq_save()' in this
> function, and therefor does the right thing.
> 
> We exploit that in quite a few places:
> 
> $ git grep 'spin_lock_irqsave(.*\*flags' | wc -l
> 39
> 

Yes, thank you all for point this!