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

Matthew Wilcox
2019-11-12T14:36:24Z

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.