Re: [PATCH v2 4/8] mm/lru: only change the lru_lock iff page's lruvec is different

[Matthew Wilcox <willy@xxxxxxxxxxxxx>]

On Tue, Nov 12, 2019 at 10:06:24PM +0800, Alex Shi wrote:
> +/* Don't lock again iff page's lruvec locked */
> +static inline struct lruvec *relock_page_lruvec_irq(struct page *page,
> +					struct lruvec *locked_lruvec)
> +{
> +	struct pglist_data *pgdat = page_pgdat(page);
> +	struct lruvec *lruvec;
> +
> +	rcu_read_lock();
> +	lruvec = mem_cgroup_page_lruvec(page, pgdat);
> +
> +	if (locked_lruvec == lruvec) {
> +		rcu_read_unlock();
> +		return lruvec;
> +	}
> +	rcu_read_unlock();

Why not simply:

	rcu_read_lock();
	lruvec = mem_cgroup_page_lruvec(page, pgdat);
	rcu_read_unlock();

	if (locked_lruvec == lruvec)
		return lruvec;

Also, why are you bothering to re-enable interrupts here?  Surely if
you're holding lock A with interrupts disabled , you can just drop lock A,
acquire lock B and leave the interrupts alone.  That way you only need
one of this variety of function, and not the separate irq/irqsave variants.