2020-12-22 05:22:34

by Alex Shi

[permalink] [raw]
Subject: [PATCH v2 1/3] mm/memcg: revise the using condition of lock_page_lruvec function series

lock_page_lruvec() and its variants are safe to use under the same
conditions as commit_charge(): add lock_page_memcg() to the comment.

Polished with Hugh Dickins' suggestions, thanks!

Signed-off-by: Alex Shi <[email protected]>
Acked-by: Hugh Dickins <[email protected]>
Cc: Hugh Dickins <[email protected]>
Cc: Johannes Weiner <[email protected]>
Cc: Michal Hocko <[email protected]>
Cc: Vladimir Davydov <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
mm/memcontrol.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index b80328f52fb4..8d400efc81b9 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -1345,10 +1345,11 @@ void lruvec_memcg_debug(struct lruvec *lruvec, struct page *page)
* lock_page_lruvec - lock and return lruvec for a given page.
* @page: the page
*
- * This series functions should be used in either conditions:
- * PageLRU is cleared or unset
- * or page->_refcount is zero
- * or page is locked.
+ * These functions are safe to use under any of the following conditions:
+ * - page locked
+ * - PageLRU cleared
+ * - lock_page_memcg()
+ * - page->_refcount is zero
*/
struct lruvec *lock_page_lruvec(struct page *page)
{
--
2.29.GIT


2020-12-22 05:23:22

by Alex Shi

[permalink] [raw]
Subject: [PATCH v2 2/3] mm/memcg: remove rcu locking for lock_page_lruvec function series

lock_page_lruvec() and its variants used rcu_read_lock() with the
intention of safeguarding against the mem_cgroup being destroyed
concurrently; but so long as they are called under the specified
conditions (as they are), there is no way for the page's mem_cgroup
to be destroyed. Delete the unnecessary rcu_read_lock() and _unlock().

Hugh Dickin's polished the commit log, Thanks a lot!

Signed-off-by: Alex Shi <[email protected]>
Acked-by: Hugh Dickins <[email protected]>
Cc: Hugh Dickins <[email protected]>
Cc: Johannes Weiner <[email protected]>
Cc: Michal Hocko <[email protected]>
Cc: Vladimir Davydov <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
mm/memcontrol.c | 6 ------
1 file changed, 6 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 8d400efc81b9..0af13c4fe4b3 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -1356,10 +1356,8 @@ struct lruvec *lock_page_lruvec(struct page *page)
struct lruvec *lruvec;
struct pglist_data *pgdat = page_pgdat(page);

- rcu_read_lock();
lruvec = mem_cgroup_page_lruvec(page, pgdat);
spin_lock(&lruvec->lru_lock);
- rcu_read_unlock();

lruvec_memcg_debug(lruvec, page);

@@ -1371,10 +1369,8 @@ struct lruvec *lock_page_lruvec_irq(struct page *page)
struct lruvec *lruvec;
struct pglist_data *pgdat = page_pgdat(page);

- rcu_read_lock();
lruvec = mem_cgroup_page_lruvec(page, pgdat);
spin_lock_irq(&lruvec->lru_lock);
- rcu_read_unlock();

lruvec_memcg_debug(lruvec, page);

@@ -1386,10 +1382,8 @@ struct lruvec *lock_page_lruvec_irqsave(struct page *page, unsigned long *flags)
struct lruvec *lruvec;
struct pglist_data *pgdat = page_pgdat(page);

- rcu_read_lock();
lruvec = mem_cgroup_page_lruvec(page, pgdat);
spin_lock_irqsave(&lruvec->lru_lock, *flags);
- rcu_read_unlock();

lruvec_memcg_debug(lruvec, page);

--
2.29.GIT

2020-12-22 05:25:26

by Alex Shi

[permalink] [raw]
Subject: [PATCH v2 3/3] mm/compaction: remove rcu_read_lock during page compaction

isolate_migratepages_block() used rcu_read_lock() with the intention
of safeguarding against the mem_cgroup being destroyed concurrently;
but its TestClearPageLRU already protects against that. Delete the
unnecessary rcu_read_lock() and _unlock().

Hugh Dickin' helped on commit log polishing, Thanks!

Signed-off-by: Alex Shi <[email protected]>
Acked-by: Hugh Dickins <[email protected]>
Cc: Hugh Dickins <[email protected]>
Cc: Johannes Weiner <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
mm/compaction.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/mm/compaction.c b/mm/compaction.c
index 8049d3530812..02af220fb992 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -995,7 +995,6 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,
if (!TestClearPageLRU(page))
goto isolate_fail_put;

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

/* If we already hold the lock, we can skip some rechecking */
@@ -1005,7 +1004,6 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,

compact_lock_irqsave(&lruvec->lru_lock, &flags, cc);
locked = lruvec;
- rcu_read_unlock();

lruvec_memcg_debug(lruvec, page);

@@ -1026,8 +1024,7 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,
SetPageLRU(page);
goto isolate_fail_put;
}
- } else
- rcu_read_unlock();
+ }

/* The whole page is taken off the LRU; skip the tail pages. */
if (PageCompound(page))
--
2.29.GIT

2020-12-22 08:01:44

by Alex Shi

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] mm/memcg: remove rcu locking for lock_page_lruvec function series

Cc: Hui Su and Alexander Duyck as Hugh suggested.

?? 2020/12/22 ????1:20, Alex Shi ะด??:
> lock_page_lruvec() and its variants used rcu_read_lock() with the
> intention of safeguarding against the mem_cgroup being destroyed
> concurrently; but so long as they are called under the specified
> conditions (as they are), there is no way for the page's mem_cgroup
> to be destroyed. Delete the unnecessary rcu_read_lock() and _unlock().
>
> Hugh Dickin's polished the commit log, Thanks a lot!
>
> Signed-off-by: Alex Shi <[email protected]>
> Acked-by: Hugh Dickins <[email protected]>
> Cc: Hugh Dickins <[email protected]>
> Cc: Johannes Weiner <[email protected]>
> Cc: Michal Hocko <[email protected]>
> Cc: Vladimir Davydov <[email protected]>
> Cc: Andrew Morton <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> ---
> mm/memcontrol.c | 6 ------
> 1 file changed, 6 deletions(-)
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 8d400efc81b9..0af13c4fe4b3 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -1356,10 +1356,8 @@ struct lruvec *lock_page_lruvec(struct page *page)
> struct lruvec *lruvec;
> struct pglist_data *pgdat = page_pgdat(page);
>
> - rcu_read_lock();
> lruvec = mem_cgroup_page_lruvec(page, pgdat);
> spin_lock(&lruvec->lru_lock);
> - rcu_read_unlock();
>
> lruvec_memcg_debug(lruvec, page);
>
> @@ -1371,10 +1369,8 @@ struct lruvec *lock_page_lruvec_irq(struct page *page)
> struct lruvec *lruvec;
> struct pglist_data *pgdat = page_pgdat(page);
>
> - rcu_read_lock();
> lruvec = mem_cgroup_page_lruvec(page, pgdat);
> spin_lock_irq(&lruvec->lru_lock);
> - rcu_read_unlock();
>
> lruvec_memcg_debug(lruvec, page);
>
> @@ -1386,10 +1382,8 @@ struct lruvec *lock_page_lruvec_irqsave(struct page *page, unsigned long *flags)
> struct lruvec *lruvec;
> struct pglist_data *pgdat = page_pgdat(page);
>
> - rcu_read_lock();
> lruvec = mem_cgroup_page_lruvec(page, pgdat);
> spin_lock_irqsave(&lruvec->lru_lock, *flags);
> - rcu_read_unlock();
>
> lruvec_memcg_debug(lruvec, page);
>
>