[patch 0/2] replace work queue synchronization with synchronize

linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

* [patch 0/2] replace work queue synchronization with synchronize_rcu
@ 2022-02-22 14:47 Marcelo Tosatti
  2022-02-22 14:47 ` [patch 1/2] mm: protect local lock sections with rcu_read_lock (on RT) Marcelo Tosatti
  2022-02-22 14:47 ` [patch 2/2] mm: lru_cache_disable: replace work queue synchronization with synchronize_rcu Marcelo Tosatti
  0 siblings, 2 replies; 7+ messages in thread
From: Marcelo Tosatti @ 2022-02-22 14:47 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-mm, Minchan Kim, Matthew Wilcox, Mel Gorman,
	Nicolas Saenz Julienne, Juri Lelli, Thomas Gleixner,
	Sebastian Andrzej Siewior, Paul E. McKenney

On systems that run FIFO:1 applications that busy loop 
on isolated CPUs, executing tasks on such CPUs under
lower priority is undesired (since that will either
hang the system, or cause longer interruption to the
FIFO task due to execution of lower priority task 
with very small sched slices).


Commit d479960e44f27e0e52ba31b21740b703c538027c ("mm: disable LRU 
pagevec during the migration temporarily") relies on 
queueing work items on all online CPUs to ensure visibility
of lru_disable_count.

However, its possible to use synchronize_rcu which will provide the same
guarantees:

    * synchronize_rcu() waits for preemption disabled
    * and RCU read side critical sections
    * For the users of lru_disable_count:
    *
    * preempt_disable, local_irq_disable() [bh_lru_lock()]
    * rcu_read_lock                        [lru_pvecs CONFIG_PREEMPT_RT]
    * preempt_disable                      [lru_pvecs !CONFIG_PREEMPT_RT]
    *
    *
    * so any calls of lru_cache_disabled wrapped by
    * local_lock+rcu_read_lock or preemption disabled would be
    * ordered by that. 


Fixes:

[ 1873.243925] INFO: task kworker/u160:0:9 blocked for more than 622 seconds.
[ 1873.243927]       Tainted: G          I      --------- ---  5.14.0-31.rt21.31.el9.x86_64 #1
[ 1873.243929] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[ 1873.243929] task:kworker/u160:0  state:D stack:    0 pid:    9 ppid:     2 flags:0x00004000
[ 1873.243932] Workqueue: cpuset_migrate_mm cpuset_migrate_mm_workfn
[ 1873.243936] Call Trace:
[ 1873.243938]  __schedule+0x21b/0x5b0
[ 1873.243941]  schedule+0x43/0xe0
[ 1873.243943]  schedule_timeout+0x14d/0x190
[ 1873.243946]  ? resched_curr+0x20/0xe0
[ 1873.243953]  ? __prepare_to_swait+0x4b/0x70
[ 1873.243958]  wait_for_completion+0x84/0xe0
[ 1873.243962]  __flush_work.isra.0+0x146/0x200
[ 1873.243966]  ? flush_workqueue_prep_pwqs+0x130/0x130
[ 1873.243971]  __lru_add_drain_all+0x158/0x1f0
[ 1873.243978]  do_migrate_pages+0x3d/0x2d0
[ 1873.243985]  ? pick_next_task_fair+0x39/0x3b0
[ 1873.243989]  ? put_prev_task_fair+0x1e/0x30
[ 1873.243992]  ? pick_next_task+0xb30/0xbd0
[ 1873.243995]  ? __tick_nohz_task_switch+0x1e/0x70
[ 1873.244000]  ? raw_spin_rq_unlock+0x18/0x60
[ 1873.244002]  ? finish_task_switch.isra.0+0xc1/0x2d0
[ 1873.244005]  ? __switch_to+0x12f/0x510
[ 1873.244013]  cpuset_migrate_mm_workfn+0x22/0x40
[ 1873.244016]  process_one_work+0x1e0/0x410
[ 1873.244019]  worker_thread+0x50/0x3b0
[ 1873.244022]  ? process_one_work+0x410/0x410
[ 1873.244024]  kthread+0x173/0x190
[ 1873.244027]  ? set_kthread_struct+0x40/0x40
[ 1873.244031]  ret_from_fork+0x1f/0x30 

diffstat:

 swap.c |  110 +++++++++++++++++++++++++++++++++++++++++++++++------------------
 1 file changed, 80 insertions(+), 30 deletions(-)




^ permalink raw reply	[flat|nested] 7+ messages in thread

* [patch 1/2] mm: protect local lock sections with rcu_read_lock (on RT)
  2022-02-22 14:47 [patch 0/2] replace work queue synchronization with synchronize_rcu Marcelo Tosatti
@ 2022-02-22 14:47 ` Marcelo Tosatti
  2022-02-22 15:21   ` Nicolas Saenz Julienne
  2022-02-22 14:47 ` [patch 2/2] mm: lru_cache_disable: replace work queue synchronization with synchronize_rcu Marcelo Tosatti
  1 sibling, 1 reply; 7+ messages in thread
From: Marcelo Tosatti @ 2022-02-22 14:47 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-mm, Minchan Kim, Matthew Wilcox, Mel Gorman,
	Nicolas Saenz Julienne, Juri Lelli, Thomas Gleixner,
	Sebastian Andrzej Siewior, Paul E. McKenney, Marcelo Tosatti

For the per-CPU LRU page vectors, augment the local lock protected
code sections with rcu_read_lock.

This makes it possible to replace the queueing of work items on all 
CPUs by synchronize_rcu (which is necessary to run FIFO:1 applications
uninterrupted on isolated CPUs).

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>


Index: linux-rt-devel/mm/swap.c
===================================================================
--- linux-rt-devel.orig/mm/swap.c
+++ linux-rt-devel/mm/swap.c
@@ -73,6 +73,48 @@ static DEFINE_PER_CPU(struct lru_pvecs,
 	.lock = INIT_LOCAL_LOCK(lock),
 };
 
+#ifdef CONFIG_PREEMPT_RT
+
+#define lru_local_lock(lock)		\
+	do {				\
+		rcu_read_lock();	\
+		local_lock(lock);	\
+	} while (0)
+
+#define lru_local_unlock(lock)		\
+	do {				\
+		local_unlock(lock);	\
+		rcu_read_unlock();	\
+	} while (0)
+
+#define lru_local_lock_irqsave(lock, flags)		\
+	do {						\
+		rcu_read_lock();			\
+		local_lock_irqsave(lock, flags);	\
+	} while (0)
+
+#define lru_local_unlock_irqrestore(lock, flags)		\
+	do {							\
+		local_unlock_irqrestore(lock, flags);		\
+		rcu_read_unlock();				\
+	} while (0)
+
+#else
+
+#define lru_local_lock(lock)		\
+	local_lock(lock)
+
+#define lru_local_unlock(lock)		\
+	local_unlock(lock)
+
+#define lru_local_lock_irqsave(lock, flag)		\
+	local_lock_irqsave(lock, flags)
+
+#define lru_local_unlock_irqrestore(lock, flags)	\
+	local_unlock_irqrestore(lock, flags)
+
+#endif
+
 /*
  * This path almost never happens for VM activity - pages are normally
  * freed via pagevecs.  But it gets used by networking.
@@ -255,11 +297,11 @@ void folio_rotate_reclaimable(struct fol
 		unsigned long flags;
 
 		folio_get(folio);
-		local_lock_irqsave(&lru_rotate.lock, flags);
+		lru_local_lock_irqsave(&lru_rotate.lock, flags);
 		pvec = this_cpu_ptr(&lru_rotate.pvec);
 		if (pagevec_add_and_need_flush(pvec, &folio->page))
 			pagevec_lru_move_fn(pvec, pagevec_move_tail_fn);
-		local_unlock_irqrestore(&lru_rotate.lock, flags);
+		lru_local_unlock_irqrestore(&lru_rotate.lock, flags);
 	}
 }
 
@@ -351,11 +393,11 @@ static void folio_activate(struct folio
 		struct pagevec *pvec;
 
 		folio_get(folio);
-		local_lock(&lru_pvecs.lock);
+		lru_local_lock(&lru_pvecs.lock);
 		pvec = this_cpu_ptr(&lru_pvecs.activate_page);
 		if (pagevec_add_and_need_flush(pvec, &folio->page))
 			pagevec_lru_move_fn(pvec, __activate_page);
-		local_unlock(&lru_pvecs.lock);
+		lru_local_unlock(&lru_pvecs.lock);
 	}
 }
 
@@ -382,7 +424,7 @@ static void __lru_cache_activate_folio(s
 	struct pagevec *pvec;
 	int i;
 
-	local_lock(&lru_pvecs.lock);
+	lru_local_lock(&lru_pvecs.lock);
 	pvec = this_cpu_ptr(&lru_pvecs.lru_add);
 
 	/*
@@ -404,7 +446,7 @@ static void __lru_cache_activate_folio(s
 		}
 	}
 
-	local_unlock(&lru_pvecs.lock);
+	lru_local_unlock(&lru_pvecs.lock);
 }
 
 /*
@@ -463,11 +505,11 @@ void folio_add_lru(struct folio *folio)
 	VM_BUG_ON_FOLIO(folio_test_lru(folio), folio);
 
 	folio_get(folio);
-	local_lock(&lru_pvecs.lock);
+	lru_local_lock(&lru_pvecs.lock);
 	pvec = this_cpu_ptr(&lru_pvecs.lru_add);
 	if (pagevec_add_and_need_flush(pvec, &folio->page))
 		__pagevec_lru_add(pvec);
-	local_unlock(&lru_pvecs.lock);
+	lru_local_unlock(&lru_pvecs.lock);
 }
 EXPORT_SYMBOL(folio_add_lru);
 
@@ -618,9 +660,9 @@ void lru_add_drain_cpu(int cpu)
 		unsigned long flags;
 
 		/* No harm done if a racing interrupt already did this */
-		local_lock_irqsave(&lru_rotate.lock, flags);
+		lru_local_lock_irqsave(&lru_rotate.lock, flags);
 		pagevec_lru_move_fn(pvec, pagevec_move_tail_fn);
-		local_unlock_irqrestore(&lru_rotate.lock, flags);
+		lru_local_unlock_irqrestore(&lru_rotate.lock, flags);
 	}
 
 	pvec = &per_cpu(lru_pvecs.lru_deactivate_file, cpu);
@@ -658,12 +700,12 @@ void deactivate_file_page(struct page *p
 	if (likely(get_page_unless_zero(page))) {
 		struct pagevec *pvec;
 
-		local_lock(&lru_pvecs.lock);
+		lru_local_lock(&lru_pvecs.lock);
 		pvec = this_cpu_ptr(&lru_pvecs.lru_deactivate_file);
 
 		if (pagevec_add_and_need_flush(pvec, page))
 			pagevec_lru_move_fn(pvec, lru_deactivate_file_fn);
-		local_unlock(&lru_pvecs.lock);
+		lru_local_unlock(&lru_pvecs.lock);
 	}
 }
 
@@ -680,12 +722,12 @@ void deactivate_page(struct page *page)
 	if (PageLRU(page) && PageActive(page) && !PageUnevictable(page)) {
 		struct pagevec *pvec;
 
-		local_lock(&lru_pvecs.lock);
+		lru_local_lock(&lru_pvecs.lock);
 		pvec = this_cpu_ptr(&lru_pvecs.lru_deactivate);
 		get_page(page);
 		if (pagevec_add_and_need_flush(pvec, page))
 			pagevec_lru_move_fn(pvec, lru_deactivate_fn);
-		local_unlock(&lru_pvecs.lock);
+		lru_local_unlock(&lru_pvecs.lock);
 	}
 }
 
@@ -702,20 +744,20 @@ void mark_page_lazyfree(struct page *pag
 	    !PageSwapCache(page) && !PageUnevictable(page)) {
 		struct pagevec *pvec;
 
-		local_lock(&lru_pvecs.lock);
+		lru_local_lock(&lru_pvecs.lock);
 		pvec = this_cpu_ptr(&lru_pvecs.lru_lazyfree);
 		get_page(page);
 		if (pagevec_add_and_need_flush(pvec, page))
 			pagevec_lru_move_fn(pvec, lru_lazyfree_fn);
-		local_unlock(&lru_pvecs.lock);
+		lru_local_unlock(&lru_pvecs.lock);
 	}
 }
 
 void lru_add_drain(void)
 {
-	local_lock(&lru_pvecs.lock);
+	lru_local_lock(&lru_pvecs.lock);
 	lru_add_drain_cpu(smp_processor_id());
-	local_unlock(&lru_pvecs.lock);
+	lru_local_unlock(&lru_pvecs.lock);
 }
 
 /*
@@ -726,18 +768,18 @@ void lru_add_drain(void)
  */
 static void lru_add_and_bh_lrus_drain(void)
 {
-	local_lock(&lru_pvecs.lock);
+	lru_local_lock(&lru_pvecs.lock);
 	lru_add_drain_cpu(smp_processor_id());
-	local_unlock(&lru_pvecs.lock);
+	lru_local_unlock(&lru_pvecs.lock);
 	invalidate_bh_lrus_cpu();
 }
 
 void lru_add_drain_cpu_zone(struct zone *zone)
 {
-	local_lock(&lru_pvecs.lock);
+	lru_local_lock(&lru_pvecs.lock);
 	lru_add_drain_cpu(smp_processor_id());
 	drain_local_pages(zone);
-	local_unlock(&lru_pvecs.lock);
+	lru_local_unlock(&lru_pvecs.lock);
 }
 
 #ifdef CONFIG_SMP



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [patch 1/2] mm: protect local lock sections with rcu_read_lock (on RT)
  2022-02-22 14:47 ` [patch 1/2] mm: protect local lock sections with rcu_read_lock (on RT) Marcelo Tosatti
@ 2022-02-22 15:21   ` Nicolas Saenz Julienne
  2022-02-22 15:51     ` Marcelo Tosatti
  0 siblings, 1 reply; 7+ messages in thread
From: Nicolas Saenz Julienne @ 2022-02-22 15:21 UTC (permalink / raw)
  To: Marcelo Tosatti, linux-kernel
  Cc: linux-mm, Minchan Kim, Matthew Wilcox, Mel Gorman, Juri Lelli,
	Thomas Gleixner, Sebastian Andrzej Siewior, Paul E. McKenney

On Tue, 2022-02-22 at 11:47 -0300, Marcelo Tosatti wrote:
> For the per-CPU LRU page vectors, augment the local lock protected
> code sections with rcu_read_lock.
> 
> This makes it possible to replace the queueing of work items on all 
> CPUs by synchronize_rcu (which is necessary to run FIFO:1 applications
> uninterrupted on isolated CPUs).

I don't think this is needed. In RT local_locks use a spinlock. See
kernel/locking/spinlock_rt.c:

"The RT [spinlock] substitutions explicitly disable migration and take
rcu_read_lock() across the lock held section."

Regards,

-- 
Nicolás Sáenz


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [patch 1/2] mm: protect local lock sections with rcu_read_lock (on RT)
  2022-02-22 15:21   ` Nicolas Saenz Julienne
@ 2022-02-22 15:51     ` Marcelo Tosatti
  2022-02-22 16:16       ` Nicolas Saenz Julienne
  0 siblings, 1 reply; 7+ messages in thread
From: Marcelo Tosatti @ 2022-02-22 15:51 UTC (permalink / raw)
  To: Nicolas Saenz Julienne
  Cc: linux-kernel, linux-mm, Minchan Kim, Matthew Wilcox, Mel Gorman,
	Juri Lelli, Thomas Gleixner, Sebastian Andrzej Siewior,
	Paul E. McKenney

On Tue, Feb 22, 2022 at 04:21:26PM +0100, Nicolas Saenz Julienne wrote:
> On Tue, 2022-02-22 at 11:47 -0300, Marcelo Tosatti wrote:
> > For the per-CPU LRU page vectors, augment the local lock protected
> > code sections with rcu_read_lock.
> > 
> > This makes it possible to replace the queueing of work items on all 
> > CPUs by synchronize_rcu (which is necessary to run FIFO:1 applications
> > uninterrupted on isolated CPUs).
> 
> I don't think this is needed. In RT local_locks use a spinlock. See
> kernel/locking/spinlock_rt.c:
> 
> "The RT [spinlock] substitutions explicitly disable migration and take
> rcu_read_lock() across the lock held section."

Nice! Then the migrate_disable from __local_lock and friends seems unnecessary as
well

#define __local_lock(__lock)                                    \
        do {                                                    \
                migrate_disable();                              \
                spin_lock(this_cpu_ptr((__lock)));              \
        } while (0)

Since:

static __always_inline void __rt_spin_lock(spinlock_t *lock)
{
        rtlock_might_resched();
        rtlock_lock(&lock->lock); 
        rcu_read_lock();
        migrate_disable();
}

Will resend -v2.



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [patch 1/2] mm: protect local lock sections with rcu_read_lock (on RT)
  2022-02-22 15:51     ` Marcelo Tosatti
@ 2022-02-22 16:16       ` Nicolas Saenz Julienne
  0 siblings, 0 replies; 7+ messages in thread
From: Nicolas Saenz Julienne @ 2022-02-22 16:16 UTC (permalink / raw)
  To: Marcelo Tosatti
  Cc: linux-kernel, linux-mm, Minchan Kim, Matthew Wilcox, Mel Gorman,
	Juri Lelli, Thomas Gleixner, Sebastian Andrzej Siewior,
	Paul E. McKenney

On Tue, 2022-02-22 at 12:51 -0300, Marcelo Tosatti wrote:
> On Tue, Feb 22, 2022 at 04:21:26PM +0100, Nicolas Saenz Julienne wrote:
> > On Tue, 2022-02-22 at 11:47 -0300, Marcelo Tosatti wrote:
> > > For the per-CPU LRU page vectors, augment the local lock protected
> > > code sections with rcu_read_lock.
> > > 
> > > This makes it possible to replace the queueing of work items on all 
> > > CPUs by synchronize_rcu (which is necessary to run FIFO:1 applications
> > > uninterrupted on isolated CPUs).
> > 
> > I don't think this is needed. In RT local_locks use a spinlock. See
> > kernel/locking/spinlock_rt.c:
> > 
> > "The RT [spinlock] substitutions explicitly disable migration and take
> > rcu_read_lock() across the lock held section."
> 
> Nice! Then the migrate_disable from __local_lock and friends seems unnecessary as
> well
>
> #define __local_lock(__lock)                                    \
>         do {                                                    \
>                 migrate_disable();                              \
>                 spin_lock(this_cpu_ptr((__lock)));              \
>         } while (0)
> 

It's needed as you might migrate between:

	cpu1_lock = this_cpu_ptr(__lock);
	// migrate here to cpu2
	spin_lock(cpu1_lock);
	// unprotected write into cpu2 lists

Regards,

-- 
Nicolás Sáenz


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [patch 2/2] mm: lru_cache_disable: replace work queue synchronization with synchronize_rcu
  2022-02-22 14:47 [patch 0/2] replace work queue synchronization with synchronize_rcu Marcelo Tosatti
  2022-02-22 14:47 ` [patch 1/2] mm: protect local lock sections with rcu_read_lock (on RT) Marcelo Tosatti
@ 2022-02-22 14:47 ` Marcelo Tosatti
  2022-02-22 15:53   ` Nicolas Saenz Julienne
  1 sibling, 1 reply; 7+ messages in thread
From: Marcelo Tosatti @ 2022-02-22 14:47 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-mm, Minchan Kim, Matthew Wilcox, Mel Gorman,
	Nicolas Saenz Julienne, Juri Lelli, Thomas Gleixner,
	Sebastian Andrzej Siewior, Paul E. McKenney, Marcelo Tosatti

On systems that run FIFO:1 applications that busy loop 
on isolated CPUs, executing tasks on such CPUs under
lower priority is undesired (since that will either
hang the system, or cause longer interruption to the
FIFO task due to execution of lower priority task 
with very small sched slices).


Commit d479960e44f27e0e52ba31b21740b703c538027c ("mm: disable LRU 
pagevec during the migration temporarily") relies on 
queueing work items on all online CPUs to ensure visibility
of lru_disable_count.

However, its possible to use synchronize_rcu which will provide the same
guarantees:

    * synchronize_rcu() waits for preemption disabled
    * and RCU read side critical sections
    * For the users of lru_disable_count:
    *
    * preempt_disable, local_irq_disable() [bh_lru_lock()]
    * rcu_read_lock                        [lru_pvecs CONFIG_PREEMPT_RT]
    * preempt_disable                      [lru_pvecs !CONFIG_PREEMPT_RT]
    *
    *
    * so any calls of lru_cache_disabled wrapped by
    * local_lock+rcu_read_lock or preemption disabled would be
    * ordered by that. 


Fixes:

[ 1873.243925] INFO: task kworker/u160:0:9 blocked for more than 622 seconds.
[ 1873.243927]       Tainted: G          I      --------- ---  5.14.0-31.rt21.31.el9.x86_64 #1
[ 1873.243929] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[ 1873.243929] task:kworker/u160:0  state:D stack:    0 pid:    9 ppid:     2 flags:0x00004000
[ 1873.243932] Workqueue: cpuset_migrate_mm cpuset_migrate_mm_workfn
[ 1873.243936] Call Trace:
[ 1873.243938]  __schedule+0x21b/0x5b0
[ 1873.243941]  schedule+0x43/0xe0
[ 1873.243943]  schedule_timeout+0x14d/0x190
[ 1873.243946]  ? resched_curr+0x20/0xe0
[ 1873.243953]  ? __prepare_to_swait+0x4b/0x70
[ 1873.243958]  wait_for_completion+0x84/0xe0
[ 1873.243962]  __flush_work.isra.0+0x146/0x200
[ 1873.243966]  ? flush_workqueue_prep_pwqs+0x130/0x130
[ 1873.243971]  __lru_add_drain_all+0x158/0x1f0
[ 1873.243978]  do_migrate_pages+0x3d/0x2d0
[ 1873.243985]  ? pick_next_task_fair+0x39/0x3b0
[ 1873.243989]  ? put_prev_task_fair+0x1e/0x30
[ 1873.243992]  ? pick_next_task+0xb30/0xbd0
[ 1873.243995]  ? __tick_nohz_task_switch+0x1e/0x70
[ 1873.244000]  ? raw_spin_rq_unlock+0x18/0x60
[ 1873.244002]  ? finish_task_switch.isra.0+0xc1/0x2d0
[ 1873.244005]  ? __switch_to+0x12f/0x510
[ 1873.244013]  cpuset_migrate_mm_workfn+0x22/0x40
[ 1873.244016]  process_one_work+0x1e0/0x410
[ 1873.244019]  worker_thread+0x50/0x3b0
[ 1873.244022]  ? process_one_work+0x410/0x410
[ 1873.244024]  kthread+0x173/0x190
[ 1873.244027]  ? set_kthread_struct+0x40/0x40
[ 1873.244031]  ret_from_fork+0x1f/0x30

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>

Index: linux-rt-devel/mm/swap.c
===================================================================
--- linux-rt-devel.orig/mm/swap.c
+++ linux-rt-devel/mm/swap.c
@@ -873,8 +873,7 @@ inline void __lru_add_drain_all(bool for
 	for_each_online_cpu(cpu) {
 		struct work_struct *work = &per_cpu(lru_add_drain_work, cpu);
 
-		if (force_all_cpus ||
-		    pagevec_count(&per_cpu(lru_pvecs.lru_add, cpu)) ||
+		if (pagevec_count(&per_cpu(lru_pvecs.lru_add, cpu)) ||
 		    data_race(pagevec_count(&per_cpu(lru_rotate.pvec, cpu))) ||
 		    pagevec_count(&per_cpu(lru_pvecs.lru_deactivate_file, cpu)) ||
 		    pagevec_count(&per_cpu(lru_pvecs.lru_deactivate, cpu)) ||
@@ -918,14 +917,23 @@ atomic_t lru_disable_count = ATOMIC_INIT
 void lru_cache_disable(void)
 {
 	atomic_inc(&lru_disable_count);
+	synchronize_rcu();
 #ifdef CONFIG_SMP
 	/*
-	 * lru_add_drain_all in the force mode will schedule draining on
-	 * all online CPUs so any calls of lru_cache_disabled wrapped by
-	 * local_lock or preemption disabled would be ordered by that.
-	 * The atomic operation doesn't need to have stronger ordering
-	 * requirements because that is enforced by the scheduling
-	 * guarantees.
+	 * synchronize_rcu() waits for preemption disabled
+	 * and RCU read side critical sections
+	 * For the users of lru_disable_count:
+	 *
+	 * preempt_disable, local_irq_disable() [bh_lru_lock()]
+	 * rcu_read_lock			[lru_pvecs CONFIG_PREEMPT_RT]
+	 * preempt_disable			[lru_pvecs !CONFIG_PREEMPT_RT]
+	 *
+	 *
+	 * so any calls of lru_cache_disabled wrapped by
+	 * local_lock+rcu_read_lock or preemption disabled would be
+	 * ordered by that. The atomic operation doesn't need to have
+	 * stronger ordering requirements because that is enforced
+	 * by the scheduling guarantees.
 	 */
 	__lru_add_drain_all(true);
 #else



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [patch 2/2] mm: lru_cache_disable: replace work queue synchronization with synchronize_rcu
  2022-02-22 14:47 ` [patch 2/2] mm: lru_cache_disable: replace work queue synchronization with synchronize_rcu Marcelo Tosatti
@ 2022-02-22 15:53   ` Nicolas Saenz Julienne
  0 siblings, 0 replies; 7+ messages in thread
From: Nicolas Saenz Julienne @ 2022-02-22 15:53 UTC (permalink / raw)
  To: Marcelo Tosatti, linux-kernel
  Cc: linux-mm, Minchan Kim, Matthew Wilcox, Mel Gorman, Juri Lelli,
	Thomas Gleixner, Sebastian Andrzej Siewior, Paul E. McKenney

On Tue, 2022-02-22 at 11:47 -0300, Marcelo Tosatti wrote:
> @@ -918,14 +917,23 @@ atomic_t lru_disable_count = ATOMIC_INIT
>  void lru_cache_disable(void)
>  {
>  	atomic_inc(&lru_disable_count);
> +	synchronize_rcu();
>  #ifdef CONFIG_SMP
>  	/*
> -	 * lru_add_drain_all in the force mode will schedule draining on
> -	 * all online CPUs so any calls of lru_cache_disabled wrapped by
> -	 * local_lock or preemption disabled would be ordered by that.
> -	 * The atomic operation doesn't need to have stronger ordering
> -	 * requirements because that is enforced by the scheduling
> -	 * guarantees.
> +	 * synchronize_rcu() waits for preemption disabled
> +	 * and RCU read side critical sections
> +	 * For the users of lru_disable_count:
> +	 *
> +	 * preempt_disable, local_irq_disable() [bh_lru_lock()]
> +	 * rcu_read_lock			[lru_pvecs CONFIG_PREEMPT_RT]
> +	 * preempt_disable			[lru_pvecs !CONFIG_PREEMPT_RT]
> +	 *
> +	 *
> +	 * so any calls of lru_cache_disabled wrapped by
> +	 * local_lock+rcu_read_lock or preemption disabled would be
> +	 * ordered by that. The atomic operation doesn't need to have
> +	 * stronger ordering requirements because that is enforced
> +	 * by the scheduling guarantees.

"The atomic operation doesn't need to have stronger ordering requirements
because that is enforced by the scheduling guarantees."

This is no longer needed.

Regards,

-- 
Nicolás Sáenz


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-02-22 16:17 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-22 14:47 [patch 0/2] replace work queue synchronization with synchronize_rcu Marcelo Tosatti
2022-02-22 14:47 ` [patch 1/2] mm: protect local lock sections with rcu_read_lock (on RT) Marcelo Tosatti
2022-02-22 15:21   ` Nicolas Saenz Julienne
2022-02-22 15:51     ` Marcelo Tosatti
2022-02-22 16:16       ` Nicolas Saenz Julienne
2022-02-22 14:47 ` [patch 2/2] mm: lru_cache_disable: replace work queue synchronization with synchronize_rcu Marcelo Tosatti
2022-02-22 15:53   ` Nicolas Saenz Julienne

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).