Re: [PATCH 1/2] mm/vmscan: add sync_shrinkers function v2

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





Am 26.08.21 um 17:34 schrieb Daniel Vetter:
On Thu, Aug 26, 2021 at 04:58:06PM +0200, Christian König wrote:
Am 26.08.21 um 15:28 schrieb Daniel Vetter:
On Thu, Aug 26, 2021 at 03:27:30PM +0200, Daniel Vetter wrote:
On Fri, Aug 20, 2021 at 02:05:27PM +0200, Christian König wrote:
From: Christian König <ckoenig.leichtzumerken@xxxxxxxxx>

While unplugging a device the TTM shrinker implementation
needs a barrier to make sure that all concurrent shrink
operations are done and no other CPU is referring to a
device specific pool any more.

Taking and releasing the shrinker semaphore on the write
side after unmapping and freeing all pages from the device
pool should make sure that no shrinker is running in
paralell.

This allows us to avoid the contented mutex in the TTM pool
implementation for every alloc/free operation.

v2: rework the commit message to make clear why we need this

Signed-off-by: Christian König <christian.koenig@xxxxxxx>
Acked-by: Huang Rui <ray.huang@xxxxxxx>
Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
---
   include/linux/shrinker.h |  1 +
   mm/vmscan.c              | 10 ++++++++++
   2 files changed, 11 insertions(+)

diff --git a/include/linux/shrinker.h b/include/linux/shrinker.h
index 9814fff58a69..1de17f53cdbc 100644
--- a/include/linux/shrinker.h
+++ b/include/linux/shrinker.h
@@ -93,4 +93,5 @@ extern void register_shrinker_prepared(struct shrinker *shrinker);
   extern int register_shrinker(struct shrinker *shrinker);
   extern void unregister_shrinker(struct shrinker *shrinker);
   extern void free_prealloced_shrinker(struct shrinker *shrinker);
+extern void sync_shrinkers(void);
   #endif
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 4620df62f0ff..fde1aabcfa7f 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -638,6 +638,16 @@ void unregister_shrinker(struct shrinker *shrinker)
   }
   EXPORT_SYMBOL(unregister_shrinker);
+/**
+ * sync_shrinker - Wait for all running shrinkers to complete.
I think it would be good to add a bit more text here maybe:

"This is equivalent to calling unregister_shrink() and
register_shrinker(), but atomically and with less overhead. This is useful
to guarantee that all shrinker invocations have seen an update, before
freeing memory, similar to rcu."

Also a bit a bikeshed, but if we look at the equivalent in irq land it's
called synchronize_irq() and synchronize_hardirq(). I think it'd be good
to bikeshed that for more conceptual consistency.
Oh also synchronize_*rcu* also spells them all out, so even more reasons
to do the same.
I will just go with the explanation above.

The synchronize_rcu() explanation is so extensive that most people will
probably stop reading after the first paragraph.
Ack, my comment was only about the function name (spelled out instead of
abbreviated), not about pulling the entire kerneldoc in from these.

Ah, good point. Going to change that as well.

Christian.

-Daniel

Thanks,
Christian.

-Daniel

+ */
+void sync_shrinkers(void)
+{
+	down_write(&shrinker_rwsem);
+	up_write(&shrinker_rwsem);
+}
+EXPORT_SYMBOL(sync_shrinkers);
+
   #define SHRINK_BATCH 128
   static unsigned long do_shrink_slab(struct shrink_control *shrinkctl,
--
2.25.1

--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux