Re: [PATCH v4 01/48] mm: move some shrinker-related function declarations to mm/internal.h
> On Aug 7, 2023, at 19:08, Qi Zheng wrote:
>
> The following functions are only used inside the mm subsystem, so it's
> better to move their declarations to the mm/internal.h file.
>
> 1. shrinker_debugfs_add()
> 2. shrinker_debugfs_detach()
> 3. shrinker_debugfs_remove()
>
> Signed-off-by: Qi Zheng
Reviewed-by: Muchun Song
One nit bellow.
[...]
> +
> +/*
> + * shrinker related functions
> + */
This is a multi-comment format. "/* shrinker related functions. */" is
the right one-line format of comment.
> +
> +#ifdef CONFIG_SHRINKER_DEBUG
> +extern int shrinker_debugfs_add(struct shrinker *shrinker);
> +extern struct dentry *shrinker_debugfs_detach(struct shrinker *shrinker,
> + int *debugfs_id);
> +extern void shrinker_debugfs_remove(struct dentry *debugfs_entry,
> +int debugfs_id);
> +#else /* CONFIG_SHRINKER_DEBUG */
> +static inline int shrinker_debugfs_add(struct shrinker *shrinker)
> +{
> + return 0;
> +}
> +static inline struct dentry *shrinker_debugfs_detach(struct shrinker
> *shrinker,
> + int *debugfs_id)
> +{
> + *debugfs_id = -1;
> + return NULL;
> +}
> +static inline void shrinker_debugfs_remove(struct dentry *debugfs_entry,
> + int debugfs_id)
> +{
> +}
> +#endif /* CONFIG_SHRINKER_DEBUG */
> +
> #endif /* __MM_INTERNAL_H */
> --
> 2.30.2
>
Re: [PATCH v4 02/48] mm: vmscan: move shrinker-related code into a separate file
> On Aug 7, 2023, at 19:08, Qi Zheng wrote: > > The mm/vmscan.c file is too large, so separate the shrinker-related > code from it into a separate file. No functional changes. > > Signed-off-by: Qi Zheng Reviewed-by: Muchun Song
Re: [PATCH v4 12/48] gfs2: dynamically allocate the gfs2-qd shrinker
> On Aug 7, 2023, at 19:09, Qi Zheng wrote: > > Use new APIs to dynamically allocate the gfs2-qd shrinker. > > Signed-off-by: Qi Zheng Reviewed-by: Muchun Song
Re: [PATCH v4 19/48] rcu: dynamically allocate the rcu-kfree shrinker
> On Aug 7, 2023, at 19:09, Qi Zheng wrote: > > Use new APIs to dynamically allocate the rcu-kfree shrinker. > > Signed-off-by: Qi Zheng Reviewed-by: Muchun Song
Re: [PATCH v4 06/48] binder: dynamically allocate the android-binder shrinker
> On Aug 7, 2023, at 19:08, Qi Zheng wrote: > > Use new APIs to dynamically allocate the android-binder shrinker. > > Signed-off-by: Qi Zheng Reviewed-by: Muchun Song
Re: [PATCH v2 40/47] fs: super: dynamically allocate the s_shrink
On 2023/7/24 17:43, Qi Zheng wrote: In preparation for implementing lockless slab shrink, use new APIs to dynamically allocate the s_shrink, so that it can be freed asynchronously using kfree_rcu(). Then it doesn't need to wait for RCU read-side critical section when releasing the struct super_block. Signed-off-by: Qi Zheng Reviewed-by: Muchun Song
Re: [PATCH v2 31/47] mbcache: dynamically allocate the mbcache shrinker
> On Jul 24, 2023, at 17:43, Qi Zheng wrote: > > In preparation for implementing lockless slab shrink, use new APIs to > dynamically allocate the mbcache shrinker, so that it can be freed > asynchronously using kfree_rcu(). Then it doesn't need to wait for RCU > read-side critical section when releasing the struct mb_cache. > > Signed-off-by: Qi Zheng Reviewed-by: Muchun Song
Re: [PATCH v2 28/47] bcache: dynamically allocate the md-bcache shrinker
On 2023/7/24 17:43, Qi Zheng wrote:
In preparation for implementing lockless slab shrink, use new APIs to
dynamically allocate the md-bcache shrinker, so that it can be freed
asynchronously using kfree_rcu(). Then it doesn't need to wait for RCU
read-side critical section when releasing the struct cache_set.
Signed-off-by: Qi Zheng
---
drivers/md/bcache/bcache.h | 2 +-
drivers/md/bcache/btree.c | 27 ---
drivers/md/bcache/sysfs.c | 3 ++-
3 files changed, 19 insertions(+), 13 deletions(-)
diff --git a/drivers/md/bcache/bcache.h b/drivers/md/bcache/bcache.h
index 5a79bb3c272f..c622bc50f81b 100644
--- a/drivers/md/bcache/bcache.h
+++ b/drivers/md/bcache/bcache.h
@@ -541,7 +541,7 @@ struct cache_set {
struct bio_set bio_split;
/* For the btree cache */
- struct shrinker shrink;
+ struct shrinker *shrink;
/* For the btree cache and anything allocation related */
struct mutexbucket_lock;
diff --git a/drivers/md/bcache/btree.c b/drivers/md/bcache/btree.c
index fd121a61f17c..c176c7fc77d9 100644
--- a/drivers/md/bcache/btree.c
+++ b/drivers/md/bcache/btree.c
@@ -667,7 +667,7 @@ static int mca_reap(struct btree *b, unsigned int
min_order, bool flush)
static unsigned long bch_mca_scan(struct shrinker *shrink,
struct shrink_control *sc)
{
- struct cache_set *c = container_of(shrink, struct cache_set, shrink);
+ struct cache_set *c = shrink->private_data;
struct btree *b, *t;
unsigned long i, nr = sc->nr_to_scan;
unsigned long freed = 0;
@@ -734,7 +734,7 @@ static unsigned long bch_mca_scan(struct shrinker *shrink,
static unsigned long bch_mca_count(struct shrinker *shrink,
struct shrink_control *sc)
{
- struct cache_set *c = container_of(shrink, struct cache_set, shrink);
+ struct cache_set *c = shrink->private_data;
if (c->shrinker_disabled)
return 0;
@@ -752,8 +752,8 @@ void bch_btree_cache_free(struct cache_set *c)
closure_init_stack();
- if (c->shrink.list.next)
- unregister_shrinker(>shrink);
+ if (c->shrink)
+ shrinker_unregister(c->shrink);
mutex_lock(>bucket_lock);
@@ -828,14 +828,19 @@ int bch_btree_cache_alloc(struct cache_set *c)
c->verify_data = NULL;
#endif
- c->shrink.count_objects = bch_mca_count;
- c->shrink.scan_objects = bch_mca_scan;
- c->shrink.seeks = 4;
- c->shrink.batch = c->btree_pages * 2;
+ c->shrink = shrinker_alloc(0, "md-bcache:%pU", c->set_uuid);
+ if (!c->shrink) {
+ pr_warn("bcache: %s: could not allocate shrinker\n", __func__);
+ return -ENOMEM;
Seems you have cheanged the semantic of this. In the past,
it is better to have a shrinker, but now it becomes a mandatory.
Right? I don't know if it is acceptable. From my point of view,
just do the cleanup, don't change any behaviour.
+ }
+
+ c->shrink->count_objects = bch_mca_count;
+ c->shrink->scan_objects = bch_mca_scan;
+ c->shrink->seeks = 4;
+ c->shrink->batch = c->btree_pages * 2;
+ c->shrink->private_data = c;
- if (register_shrinker(>shrink, "md-bcache:%pU", c->set_uuid))
- pr_warn("bcache: %s: could not register shrinker\n",
- __func__);
+ shrinker_register(c->shrink);
return 0;
}
diff --git a/drivers/md/bcache/sysfs.c b/drivers/md/bcache/sysfs.c
index 0e2c1880f60b..45d8af755de6 100644
--- a/drivers/md/bcache/sysfs.c
+++ b/drivers/md/bcache/sysfs.c
@@ -866,7 +866,8 @@ STORE(__bch_cache_set)
sc.gfp_mask = GFP_KERNEL;
sc.nr_to_scan = strtoul_or_return(buf);
- c->shrink.scan_objects(>shrink, );
+ if (c->shrink)
+ c->shrink->scan_objects(c->shrink, );
}
sysfs_strtoul_clamp(congested_read_threshold_us,
Re: [PATCH v2 43/47] mm: shrinker: add a secondary array for shrinker_info::{map, nr_deferred}
> On Jul 24, 2023, at 17:43, Qi Zheng wrote:
>
> Currently, we maintain two linear arrays per node per memcg, which are
> shrinker_info::map and shrinker_info::nr_deferred. And we need to resize
> them when the shrinker_nr_max is exceeded, that is, allocate a new array,
> and then copy the old array to the new array, and finally free the old
> array by RCU.
>
> For shrinker_info::map, we do set_bit() under the RCU lock, so we may set
> the value into the old map which is about to be freed. This may cause the
> value set to be lost. The current solution is not to copy the old map when
> resizing, but to set all the corresponding bits in the new map to 1. This
> solves the data loss problem, but bring the overhead of more pointless
> loops while doing memcg slab shrink.
>
> For shrinker_info::nr_deferred, we will only modify it under the read lock
> of shrinker_rwsem, so it will not run concurrently with the resizing. But
> after we make memcg slab shrink lockless, there will be the same data loss
> problem as shrinker_info::map, and we can't work around it like the map.
>
> For such resizable arrays, the most straightforward idea is to change it
> to xarray, like we did for list_lru [1]. We need to do xa_store() in the
> list_lru_add()-->set_shrinker_bit(), but this will cause memory
> allocation, and the list_lru_add() doesn't accept failure. A possible
> solution is to pre-allocate, but the location of pre-allocation is not
> well determined.
>
> Therefore, this commit chooses to introduce a secondary array for
> shrinker_info::{map, nr_deferred}, so that we only need to copy this
> secondary array every time the size is resized. Then even if we get the
> old secondary array under the RCU lock, the found map and nr_deferred are
> also true, so no data is lost.
>
> [1].
> https://lore.kernel.org/all/20220228122126.37293-13-songmuc...@bytedance.com/
>
> Signed-off-by: Qi Zheng
Reviewed-by: Muchun Song
Re: [PATCH v2 42/47] drm/ttm: introduce pool_shrink_rwsem
> On Jul 24, 2023, at 17:43, Qi Zheng wrote: > > Currently, the synchronize_shrinkers() is only used by TTM pool. It only > requires that no shrinkers run in parallel. > > After we use RCU+refcount method to implement the lockless slab shrink, > we can not use shrinker_rwsem or synchronize_rcu() to guarantee that all > shrinker invocations have seen an update before freeing memory. > > So we introduce a new pool_shrink_rwsem to implement a private > synchronize_shrinkers(), so as to achieve the same purpose. > > Signed-off-by: Qi Zheng Reviewed-by: Muchun Song
Re: [PATCH v2 33/47] jbd2, ext4: dynamically allocate the jbd2-journal shrinker
> On Jul 24, 2023, at 17:43, Qi Zheng wrote: > > In preparation for implementing lockless slab shrink, use new APIs to > dynamically allocate the jbd2-journal shrinker, so that it can be freed > asynchronously using kfree_rcu(). Then it doesn't need to wait for RCU > read-side critical section when releasing the struct journal_s. > > Signed-off-by: Qi Zheng Reviewed-by: Muchun Song
Re: [PATCH v2 41/47] mm: shrinker: remove old APIs
> On Jul 24, 2023, at 17:43, Qi Zheng wrote: > > Now no users are using the old APIs, just remove them. > > Signed-off-by: Qi Zheng Reviewed-by: Muchun Song
Re: [PATCH v2 38/47] xfs: dynamically allocate the xfs-qm shrinker
> On Jul 24, 2023, at 17:43, Qi Zheng wrote: > > In preparation for implementing lockless slab shrink, use new APIs to > dynamically allocate the xfs-qm shrinker, so that it can be freed > asynchronously using kfree_rcu(). Then it doesn't need to wait for RCU > read-side critical section when releasing the struct xfs_quotainfo. > > Signed-off-by: Qi Zheng Reviewed-by: Muchun Song
Re: [PATCH v2 36/47] xfs: dynamically allocate the xfs-buf shrinker
> On Jul 24, 2023, at 17:43, Qi Zheng wrote: > > In preparation for implementing lockless slab shrink, use new APIs to > dynamically allocate the xfs-buf shrinker, so that it can be freed > asynchronously using kfree_rcu(). Then it doesn't need to wait for RCU > read-side critical section when releasing the struct xfs_buftarg. > > Signed-off-by: Qi Zheng Reviewed-by: Muchun Song
Re: [PATCH v2 29/47] vmw_balloon: dynamically allocate the vmw-balloon shrinker
> On Jul 24, 2023, at 17:43, Qi Zheng wrote: > > In preparation for implementing lockless slab shrink, use new APIs to > dynamically allocate the vmw-balloon shrinker, so that it can be freed > asynchronously using kfree_rcu(). Then it doesn't need to wait for RCU > read-side critical section when releasing the struct vmballoon. > > And we can simply exit vmballoon_init() when registering the shrinker > fails. So the shrinker_registered indication is redundant, just remove it. > > Signed-off-by: Qi Zheng Nice cleanup. Reviewed-by: Muchun Song
Re: [PATCH v2 32/47] ext4: dynamically allocate the ext4-es shrinker
> On Jul 24, 2023, at 17:43, Qi Zheng wrote: > > In preparation for implementing lockless slab shrink, use new APIs to > dynamically allocate the ext4-es shrinker, so that it can be freed > asynchronously using kfree_rcu(). Then it doesn't need to wait for RCU > read-side critical section when releasing the struct ext4_sb_info. > > Signed-off-by: Qi Zheng Reviewed-by: Muchun Song
Re: [PATCH v2 30/47] virtio_balloon: dynamically allocate the virtio-balloon shrinker
> On Jul 24, 2023, at 17:43, Qi Zheng wrote: > > In preparation for implementing lockless slab shrink, use new APIs to > dynamically allocate the virtio-balloon shrinker, so that it can be freed > asynchronously using kfree_rcu(). Then it doesn't need to wait for RCU > read-side critical section when releasing the struct virtio_balloon. > > Signed-off-by: Qi Zheng Reviewed-by: Muchun Song
Re: [PATCH v2 39/47] zsmalloc: dynamically allocate the mm-zspool shrinker
> On Jul 24, 2023, at 17:43, Qi Zheng wrote: > > In preparation for implementing lockless slab shrink, use new APIs to > dynamically allocate the mm-zspool shrinker, so that it can be freed > asynchronously using kfree_rcu(). Then it doesn't need to wait for RCU > read-side critical section when releasing the struct zs_pool. > > Signed-off-by: Qi Zheng Reviewed-by: Muchun Song
Re: [PATCH v2 37/47] xfs: dynamically allocate the xfs-inodegc shrinker
> On Jul 24, 2023, at 17:43, Qi Zheng wrote: > > In preparation for implementing lockless slab shrink, use new APIs to > dynamically allocate the xfs-inodegc shrinker, so that it can be freed > asynchronously using kfree_rcu(). Then it doesn't need to wait for RCU > read-side critical section when releasing the struct xfs_mount. > > Signed-off-by: Qi Zheng Reviewed-by: Muchun Song
Re: [PATCH v2 23/47] drm/msm: dynamically allocate the drm-msm_gem shrinker
On 2023/7/24 17:43, Qi Zheng wrote:
In preparation for implementing lockless slab shrink, use new APIs to
dynamically allocate the drm-msm_gem shrinker, so that it can be freed
asynchronously using kfree_rcu(). Then it doesn't need to wait for RCU
read-side critical section when releasing the struct msm_drm_private.
Signed-off-by: Qi Zheng
Reviewed-by: Muchun Song
A nit bellow.
---
drivers/gpu/drm/msm/msm_drv.c | 4 ++-
drivers/gpu/drm/msm/msm_drv.h | 4 +--
drivers/gpu/drm/msm/msm_gem_shrinker.c | 36 --
3 files changed, 28 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 891eff8433a9..7f6933be703f 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -461,7 +461,9 @@ static int msm_drm_init(struct device *dev, const struct
drm_driver *drv)
if (ret)
goto err_msm_uninit;
- msm_gem_shrinker_init(ddev);
+ ret = msm_gem_shrinker_init(ddev);
+ if (ret)
+ goto err_msm_uninit;
if (priv->kms_init) {
ret = priv->kms_init(ddev);
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index e13a8cbd61c9..84523d4a1e58 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -217,7 +217,7 @@ struct msm_drm_private {
} vram;
struct notifier_block vmap_notifier;
- struct shrinker shrinker;
+ struct shrinker *shrinker;
struct drm_atomic_state *pm_state;
@@ -279,7 +279,7 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
unsigned long msm_gem_shrinker_shrink(struct drm_device *dev, unsigned long
nr_to_scan);
#endif
-void msm_gem_shrinker_init(struct drm_device *dev);
+int msm_gem_shrinker_init(struct drm_device *dev);
void msm_gem_shrinker_cleanup(struct drm_device *dev);
int msm_gem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma);
diff --git a/drivers/gpu/drm/msm/msm_gem_shrinker.c
b/drivers/gpu/drm/msm/msm_gem_shrinker.c
index f38296ad8743..7daab1298c11 100644
--- a/drivers/gpu/drm/msm/msm_gem_shrinker.c
+++ b/drivers/gpu/drm/msm/msm_gem_shrinker.c
@@ -34,8 +34,7 @@ static bool can_block(struct shrink_control *sc)
static unsigned long
msm_gem_shrinker_count(struct shrinker *shrinker, struct shrink_control *sc)
{
- struct msm_drm_private *priv =
- container_of(shrinker, struct msm_drm_private, shrinker);
+ struct msm_drm_private *priv = shrinker->private_data;
unsigned count = priv->lru.dontneed.count;
if (can_swap())
@@ -100,8 +99,7 @@ active_evict(struct drm_gem_object *obj)
static unsigned long
msm_gem_shrinker_scan(struct shrinker *shrinker, struct shrink_control *sc)
{
- struct msm_drm_private *priv =
- container_of(shrinker, struct msm_drm_private, shrinker);
+ struct msm_drm_private *priv = shrinker->private_data;
struct {
struct drm_gem_lru *lru;
bool (*shrink)(struct drm_gem_object *obj);
@@ -148,10 +146,11 @@ msm_gem_shrinker_shrink(struct drm_device *dev, unsigned
long nr_to_scan)
struct shrink_control sc = {
.nr_to_scan = nr_to_scan,
};
- int ret;
+ unsigned long ret = SHRINK_STOP;
fs_reclaim_acquire(GFP_KERNEL);
- ret = msm_gem_shrinker_scan(>shrinker, );
+ if (priv->shrinker)
+ ret = msm_gem_shrinker_scan(priv->shrinker, );
fs_reclaim_release(GFP_KERNEL);
return ret;
@@ -210,16 +209,27 @@ msm_gem_shrinker_vmap(struct notifier_block *nb, unsigned
long event, void *ptr)
*
* This function registers and sets up the msm shrinker.
*/
-void msm_gem_shrinker_init(struct drm_device *dev)
+int msm_gem_shrinker_init(struct drm_device *dev)
{
struct msm_drm_private *priv = dev->dev_private;
- priv->shrinker.count_objects = msm_gem_shrinker_count;
- priv->shrinker.scan_objects = msm_gem_shrinker_scan;
- priv->shrinker.seeks = DEFAULT_SEEKS;
- WARN_ON(register_shrinker(>shrinker, "drm-msm_gem"));
+
+ priv->shrinker = shrinker_alloc(0, "drm-msm_gem");
+ if (!priv->shrinker) {
Just "if (WARN_ON(!priv->shrinker))"
+ WARN_ON(1);
+ return -ENOMEM;
+ }
+
+ priv->shrinker->count_objects = msm_gem_shrinker_count;
+ priv->shrinker->scan_objects = msm_gem_shrinker_scan;
+ priv->shrinker->seeks = DEFAULT_SEEKS;
+ priv->shrinker->private_data = priv;
+
+ shrinker_register(priv->shrinker);
priv->vmap_notifier.notifier_call = msm_gem_shrinker_vmap;
WARN_ON(register_vmap_purge_notifier(>vmap_notifier));
+
+ return 0;
}
/**
@@ -232,8 +242,8 @@ void msm_gem_shrinker_cleanup(struct drm_device *dev)
{
stru
Re: [PATCH v2 13/47] nfs: dynamically allocate the nfs-acl shrinker
> On Jul 24, 2023, at 17:43, Qi Zheng wrote: > > Use new APIs to dynamically allocate the nfs-acl shrinker. > > Signed-off-by: Qi Zheng Reviewed-by: Muchun Song
Re: [PATCH v2 19/47] mm: thp: dynamically allocate the thp-related shrinkers
On 2023/7/24 17:43, Qi Zheng wrote:
Use new APIs to dynamically allocate the thp-zero and thp-deferred_split
shrinkers.
Signed-off-by: Qi Zheng
---
mm/huge_memory.c | 69 +++-
1 file changed, 45 insertions(+), 24 deletions(-)
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 8c94b34024a2..4db5a1834d81 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -65,7 +65,11 @@ unsigned long transparent_hugepage_flags __read_mostly =
(1<
-static struct shrinker deferred_split_shrinker;
+static struct shrinker *deferred_split_shrinker;
+static unsigned long deferred_split_count(struct shrinker *shrink,
+ struct shrink_control *sc);
+static unsigned long deferred_split_scan(struct shrinker *shrink,
+struct shrink_control *sc);
static atomic_t huge_zero_refcount;
struct page *huge_zero_page __read_mostly;
@@ -229,11 +233,7 @@ static unsigned long shrink_huge_zero_page_scan(struct
shrinker *shrink,
return 0;
}
-static struct shrinker huge_zero_page_shrinker = {
- .count_objects = shrink_huge_zero_page_count,
- .scan_objects = shrink_huge_zero_page_scan,
- .seeks = DEFAULT_SEEKS,
-};
+static struct shrinker *huge_zero_page_shrinker;
Same as patch #17.
#ifdef CONFIG_SYSFS
static ssize_t enabled_show(struct kobject *kobj,
@@ -454,6 +454,40 @@ static inline void hugepage_exit_sysfs(struct kobject
*hugepage_kobj)
}
#endif /* CONFIG_SYSFS */
+static int thp_shrinker_init(void)
Better to declare it as __init.
+{
+ huge_zero_page_shrinker = shrinker_alloc(0, "thp-zero");
+ if (!huge_zero_page_shrinker)
+ return -ENOMEM;
+
+ deferred_split_shrinker = shrinker_alloc(SHRINKER_NUMA_AWARE |
+SHRINKER_MEMCG_AWARE |
+SHRINKER_NONSLAB,
+"thp-deferred_split");
+ if (!deferred_split_shrinker) {
+ shrinker_free_non_registered(huge_zero_page_shrinker);
+ return -ENOMEM;
+ }
+
+ huge_zero_page_shrinker->count_objects = shrink_huge_zero_page_count;
+ huge_zero_page_shrinker->scan_objects = shrink_huge_zero_page_scan;
+ huge_zero_page_shrinker->seeks = DEFAULT_SEEKS;
+ shrinker_register(huge_zero_page_shrinker);
+
+ deferred_split_shrinker->count_objects = deferred_split_count;
+ deferred_split_shrinker->scan_objects = deferred_split_scan;
+ deferred_split_shrinker->seeks = DEFAULT_SEEKS;
+ shrinker_register(deferred_split_shrinker);
+
+ return 0;
+}
+
+static void thp_shrinker_exit(void)
Same as here.
+{
+ shrinker_unregister(huge_zero_page_shrinker);
+ shrinker_unregister(deferred_split_shrinker);
+}
+
static int __init hugepage_init(void)
{
int err;
@@ -482,12 +516,9 @@ static int __init hugepage_init(void)
if (err)
goto err_slab;
- err = register_shrinker(_zero_page_shrinker, "thp-zero");
- if (err)
- goto err_hzp_shrinker;
- err = register_shrinker(_split_shrinker, "thp-deferred_split");
+ err = thp_shrinker_init();
if (err)
- goto err_split_shrinker;
+ goto err_shrinker;
/*
* By default disable transparent hugepages on smaller systems,
@@ -505,10 +536,8 @@ static int __init hugepage_init(void)
return 0;
err_khugepaged:
- unregister_shrinker(_split_shrinker);
-err_split_shrinker:
- unregister_shrinker(_zero_page_shrinker);
-err_hzp_shrinker:
+ thp_shrinker_exit();
+err_shrinker:
khugepaged_destroy();
err_slab:
hugepage_exit_sysfs(hugepage_kobj);
@@ -2851,7 +2880,7 @@ void deferred_split_folio(struct folio *folio)
#ifdef CONFIG_MEMCG
if (memcg)
set_shrinker_bit(memcg, folio_nid(folio),
-deferred_split_shrinker.id);
+deferred_split_shrinker->id);
#endif
}
spin_unlock_irqrestore(_queue->split_queue_lock, flags);
@@ -2925,14 +2954,6 @@ static unsigned long deferred_split_scan(struct shrinker
*shrink,
return split;
}
-static struct shrinker deferred_split_shrinker = {
- .count_objects = deferred_split_count,
- .scan_objects = deferred_split_scan,
- .seeks = DEFAULT_SEEKS,
- .flags = SHRINKER_NUMA_AWARE | SHRINKER_MEMCG_AWARE |
-SHRINKER_NONSLAB,
-};
-
#ifdef CONFIG_DEBUG_FS
static void split_huge_pages_all(void)
{
Re: [PATCH v2 17/47] rcu: dynamically allocate the rcu-lazy shrinker
> On Jul 24, 2023, at 17:43, Qi Zheng wrote:
>
> Use new APIs to dynamically allocate the rcu-lazy shrinker.
>
> Signed-off-by: Qi Zheng
> ---
> kernel/rcu/tree_nocb.h | 19 +++
> 1 file changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/kernel/rcu/tree_nocb.h b/kernel/rcu/tree_nocb.h
> index 43229d2b0c44..919f17561733 100644
> --- a/kernel/rcu/tree_nocb.h
> +++ b/kernel/rcu/tree_nocb.h
> @@ -1397,12 +1397,7 @@ lazy_rcu_shrink_scan(struct shrinker *shrink, struct
> shrink_control *sc)
> return count ? count : SHRINK_STOP;
> }
>
> -static struct shrinker lazy_rcu_shrinker = {
> - .count_objects = lazy_rcu_shrink_count,
> - .scan_objects = lazy_rcu_shrink_scan,
> - .batch = 0,
> - .seeks = DEFAULT_SEEKS,
> -};
> +static struct shrinker *lazy_rcu_shrinker;
Seems there is no users of this variable, maybe we could drop
this.
Re: [PATCH v2 27/47] md/raid5: dynamically allocate the md-raid5 shrinker
> On Jul 24, 2023, at 17:43, Qi Zheng wrote: > > In preparation for implementing lockless slab shrink, use new APIs to > dynamically allocate the md-raid5 shrinker, so that it can be freed > asynchronously using kfree_rcu(). Then it doesn't need to wait for RCU > read-side critical section when releasing the struct r5conf. > > Signed-off-by: Qi Zheng Reviewed-by: Muchun Song
Re: [PATCH v2 25/47] dm: dynamically allocate the dm-bufio shrinker
> On Jul 24, 2023, at 17:43, Qi Zheng wrote: > > In preparation for implementing lockless slab shrink, use new APIs to > dynamically allocate the dm-bufio shrinker, so that it can be freed > asynchronously using kfree_rcu(). Then it doesn't need to wait for RCU > read-side critical section when releasing the struct dm_bufio_client. > > Signed-off-by: Qi Zheng Reviewed-by: Muchun Song
Re: [PATCH v2 20/47] sunrpc: dynamically allocate the sunrpc_cred shrinker
> On Jul 24, 2023, at 17:43, Qi Zheng wrote: > > Use new APIs to dynamically allocate the sunrpc_cred shrinker. > > Signed-off-by: Qi Zheng Reviewed-by: Muchun Song
Re: [PATCH v2 18/47] rcu: dynamically allocate the rcu-kfree shrinker
> On Jul 24, 2023, at 17:43, Qi Zheng wrote:
>
> Use new APIs to dynamically allocate the rcu-kfree shrinker.
>
> Signed-off-by: Qi Zheng
> ---
> kernel/rcu/tree.c | 21 +
> 1 file changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index 1449cb69a0e0..d068ce3567fc 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -3445,12 +3445,7 @@ kfree_rcu_shrink_scan(struct shrinker *shrink, struct
> shrink_control *sc)
> return freed == 0 ? SHRINK_STOP : freed;
> }
>
> -static struct shrinker kfree_rcu_shrinker = {
> - .count_objects = kfree_rcu_shrink_count,
> - .scan_objects = kfree_rcu_shrink_scan,
> - .batch = 0,
> - .seeks = DEFAULT_SEEKS,
> -};
> +static struct shrinker *kfree_rcu_shrinker;
Same as patch #17.
Re: [PATCH v2 12/47] NFSv4.2: dynamically allocate the nfs-xattr shrinkers
> On Jul 24, 2023, at 17:43, Qi Zheng wrote: > > Use new APIs to dynamically allocate the nfs-xattr shrinkers. > > Signed-off-by: Qi Zheng Reviewed-by: Muchun Song
Re: [PATCH v2 07/47] xenbus/backend: dynamically allocate the xen-backend shrinker
> On Jul 24, 2023, at 17:43, Qi Zheng wrote: > > Use new APIs to dynamically allocate the xen-backend shrinker. > > Signed-off-by: Qi Zheng Reviewed-by: Muchun Song Thanks.
Re: [PATCH v2 26/47] dm zoned: dynamically allocate the dm-zoned-meta shrinker
> On Jul 24, 2023, at 17:43, Qi Zheng wrote: > > In preparation for implementing lockless slab shrink, use new APIs to > dynamically allocate the dm-zoned-meta shrinker, so that it can be freed > asynchronously using kfree_rcu(). Then it doesn't need to wait for RCU > read-side critical section when releasing the struct dmz_metadata. > > Signed-off-by: Qi Zheng Reviewed-by: Muchun Song
Re: [PATCH v2 14/47] nfsd: dynamically allocate the nfsd-filecache shrinker
> On Jul 24, 2023, at 17:43, Qi Zheng wrote: > > Use new APIs to dynamically allocate the nfsd-filecache shrinker. > > Signed-off-by: Qi Zheng Reviewed-by: Muchun Song
Re: [PATCH v2 06/47] drm/ttm: dynamically allocate the drm-ttm_pool shrinker
> On Jul 24, 2023, at 17:43, Qi Zheng wrote: > > Use new APIs to dynamically allocate the drm-ttm_pool shrinker. > > Signed-off-by: Qi Zheng Reviewed-by: Muchun Song Thanks.
Re: [PATCH v2 11/47] gfs2: dynamically allocate the gfs2-qd shrinker
On 2023/7/24 17:43, Qi Zheng wrote:
Use new APIs to dynamically allocate the gfs2-qd shrinker.
Signed-off-by: Qi Zheng
---
fs/gfs2/main.c | 6 +++---
fs/gfs2/quota.c | 26 --
fs/gfs2/quota.h | 3 ++-
3 files changed, 25 insertions(+), 10 deletions(-)
diff --git a/fs/gfs2/main.c b/fs/gfs2/main.c
index afcb32854f14..e47b1cc79f59 100644
--- a/fs/gfs2/main.c
+++ b/fs/gfs2/main.c
@@ -147,7 +147,7 @@ static int __init init_gfs2_fs(void)
if (!gfs2_trans_cachep)
goto fail_cachep8;
- error = register_shrinker(_qd_shrinker, "gfs2-qd");
+ error = gfs2_qd_shrinker_init();
if (error)
goto fail_shrinker;
@@ -196,7 +196,7 @@ static int __init init_gfs2_fs(void)
fail_wq2:
destroy_workqueue(gfs_recovery_wq);
fail_wq1:
- unregister_shrinker(_qd_shrinker);
+ gfs2_qd_shrinker_exit();
fail_shrinker:
kmem_cache_destroy(gfs2_trans_cachep);
fail_cachep8:
@@ -229,7 +229,7 @@ static int __init init_gfs2_fs(void)
static void __exit exit_gfs2_fs(void)
{
- unregister_shrinker(_qd_shrinker);
+ gfs2_qd_shrinker_exit();
gfs2_glock_exit();
gfs2_unregister_debugfs();
unregister_filesystem(_fs_type);
diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
index 704192b73605..bc9883cea847 100644
--- a/fs/gfs2/quota.c
+++ b/fs/gfs2/quota.c
@@ -186,13 +186,27 @@ static unsigned long gfs2_qd_shrink_count(struct shrinker
*shrink,
return vfs_pressure_ratio(list_lru_shrink_count(_qd_lru, sc));
}
-struct shrinker gfs2_qd_shrinker = {
- .count_objects = gfs2_qd_shrink_count,
- .scan_objects = gfs2_qd_shrink_scan,
- .seeks = DEFAULT_SEEKS,
- .flags = SHRINKER_NUMA_AWARE,
-};
+static struct shrinker *gfs2_qd_shrinker;
+
+int gfs2_qd_shrinker_init(void)
It's better to declare this as __init.
+{
+ gfs2_qd_shrinker = shrinker_alloc(SHRINKER_NUMA_AWARE, "gfs2-qd");
+ if (!gfs2_qd_shrinker)
+ return -ENOMEM;
+
+ gfs2_qd_shrinker->count_objects = gfs2_qd_shrink_count;
+ gfs2_qd_shrinker->scan_objects = gfs2_qd_shrink_scan;
+ gfs2_qd_shrinker->seeks = DEFAULT_SEEKS;
+
+ shrinker_register(gfs2_qd_shrinker);
+ return 0;
+}
+
+void gfs2_qd_shrinker_exit(void)
+{
+ shrinker_unregister(gfs2_qd_shrinker);
+}
static u64 qd2index(struct gfs2_quota_data *qd)
{
diff --git a/fs/gfs2/quota.h b/fs/gfs2/quota.h
index 21ada332d555..f9cb863373f7 100644
--- a/fs/gfs2/quota.h
+++ b/fs/gfs2/quota.h
@@ -59,7 +59,8 @@ static inline int gfs2_quota_lock_check(struct gfs2_inode *ip,
}
extern const struct quotactl_ops gfs2_quotactl_ops;
-extern struct shrinker gfs2_qd_shrinker;
+int gfs2_qd_shrinker_init(void);
+void gfs2_qd_shrinker_exit(void);
extern struct list_lru gfs2_qd_lru;
extern void __init gfs2_quota_hash_init(void);
Re: [PATCH v2 21/47] mm: workingset: dynamically allocate the mm-shadow shrinker
> On Jul 24, 2023, at 17:43, Qi Zheng wrote:
>
> Use new APIs to dynamically allocate the mm-shadow shrinker.
>
> Signed-off-by: Qi Zheng
> ---
> mm/workingset.c | 26 ++
> 1 file changed, 14 insertions(+), 12 deletions(-)
>
> diff --git a/mm/workingset.c b/mm/workingset.c
> index 4686ae363000..4bc85f739b13 100644
> --- a/mm/workingset.c
> +++ b/mm/workingset.c
> @@ -762,12 +762,7 @@ static unsigned long scan_shadow_nodes(struct shrinker
> *shrinker,
> NULL);
> }
>
> -static struct shrinker workingset_shadow_shrinker = {
> - .count_objects = count_shadow_nodes,
> - .scan_objects = scan_shadow_nodes,
> - .seeks = 0, /* ->count reports only fully expendable nodes */
> - .flags = SHRINKER_NUMA_AWARE | SHRINKER_MEMCG_AWARE,
> -};
> +static struct shrinker *workingset_shadow_shrinker;
Same as patch #17.
Re: [PATCH v2 04/47] kvm: mmu: dynamically allocate the x86-mmu shrinker
> On Jul 24, 2023, at 17:43, Qi Zheng wrote: > > Use new APIs to dynamically allocate the x86-mmu shrinker. > > Signed-off-by: Qi Zheng Reviewed-by: Muchun Song Thanks.
Re: [PATCH v2 15/47] quota: dynamically allocate the dquota-cache shrinker
> On Jul 24, 2023, at 17:43, Qi Zheng wrote: > > Use new APIs to dynamically allocate the dquota-cache shrinker. > > Signed-off-by: Qi Zheng Reviewed-by: Muchun Song
Re: [PATCH v2 16/47] ubifs: dynamically allocate the ubifs-slab shrinker
> On Jul 24, 2023, at 17:43, Qi Zheng wrote: > > Use new APIs to dynamically allocate the ubifs-slab shrinker. > > Signed-off-by: Qi Zheng Reviewed-by: Muchun Song
Re: [PATCH v2 22/47] drm/i915: dynamically allocate the i915_gem_mm shrinker
> On Jul 24, 2023, at 17:43, Qi Zheng wrote: > > In preparation for implementing lockless slab shrink, use new APIs to > dynamically allocate the i915_gem_mm shrinker, so that it can be freed > asynchronously using kfree_rcu(). Then it doesn't need to wait for RCU > read-side critical section when releasing the struct drm_i915_private. > > Signed-off-by: Qi Zheng Reviewed-by: Muchun Song
Re: [PATCH v2 10/47] gfs2: dynamically allocate the gfs2-glock shrinker
> On Jul 24, 2023, at 17:43, Qi Zheng wrote: > > Use new APIs to dynamically allocate the gfs2-glock shrinker. > > Signed-off-by: Qi Zheng Reviewed-by: Muchun Song Thanks.
Re: [PATCH v2 09/47] f2fs: dynamically allocate the f2fs-shrinker
> On Jul 24, 2023, at 17:43, Qi Zheng wrote: > > Use new APIs to dynamically allocate the f2fs-shrinker. > > Signed-off-by: Qi Zheng Reviewed-by: Muchun Song Thanks.
Re: [PATCH v2 03/47] mm: shrinker: add infrastructure for dynamically allocating shrinker
On 2023/7/24 17:43, Qi Zheng wrote:
Currently, the shrinker instances can be divided into the following three
types:
a) global shrinker instance statically defined in the kernel, such as
workingset_shadow_shrinker.
b) global shrinker instance statically defined in the kernel modules, such
as mmu_shrinker in x86.
c) shrinker instance embedded in other structures.
For case a, the memory of shrinker instance is never freed. For case b,
the memory of shrinker instance will be freed after synchronize_rcu() when
the module is unloaded. For case c, the memory of shrinker instance will
be freed along with the structure it is embedded in.
In preparation for implementing lockless slab shrink, we need to
dynamically allocate those shrinker instances in case c, then the memory
can be dynamically freed alone by calling kfree_rcu().
So this commit adds the following new APIs for dynamically allocating
shrinker, and add a private_data field to struct shrinker to record and
get the original embedded structure.
1. shrinker_alloc()
Used to allocate shrinker instance itself and related memory, it will
return a pointer to the shrinker instance on success and NULL on failure.
2. shrinker_free_non_registered()
Used to destroy the non-registered shrinker instance.
At least I don't like this name. I know you want to tell others
this function only should be called when shrinker has not been
registed but allocated. Maybe shrinker_free() is more simple.
And and a comment to tell the users when to use it.
3. shrinker_register()
Used to register the shrinker instance, which is same as the current
register_shrinker_prepared().
4. shrinker_unregister()
Used to unregister and free the shrinker instance.
In order to simplify shrinker-related APIs and make shrinker more
independent of other kernel mechanisms, subsequent submissions will use
the above API to convert all shrinkers (including case a and b) to
dynamically allocated, and then remove all existing APIs.
This will also have another advantage mentioned by Dave Chinner:
```
The other advantage of this is that it will break all the existing
out of tree code and third party modules using the old API and will
no longer work with a kernel using lockless slab shrinkers. They
need to break (both at the source and binary levels) to stop bad
things from happening due to using uncoverted shrinkers in the new
setup.
```
Signed-off-by: Qi Zheng
---
include/linux/shrinker.h | 6 +++
mm/shrinker.c| 113 +++
2 files changed, 119 insertions(+)
diff --git a/include/linux/shrinker.h b/include/linux/shrinker.h
index 961cb84e51f5..296f5e163861 100644
--- a/include/linux/shrinker.h
+++ b/include/linux/shrinker.h
@@ -70,6 +70,8 @@ struct shrinker {
int seeks; /* seeks to recreate an obj */
unsigned flags;
+ void *private_data;
+
/* These are for internal use */
struct list_head list;
#ifdef CONFIG_MEMCG
@@ -98,6 +100,10 @@ struct shrinker {
unsigned long shrink_slab(gfp_t gfp_mask, int nid, struct mem_cgroup *memcg,
int priority);
+struct shrinker *shrinker_alloc(unsigned int flags, const char *fmt, ...);
+void shrinker_free_non_registered(struct shrinker *shrinker);
+void shrinker_register(struct shrinker *shrinker);
+void shrinker_unregister(struct shrinker *shrinker);
extern int __printf(2, 3) prealloc_shrinker(struct shrinker *shrinker,
const char *fmt, ...);
diff --git a/mm/shrinker.c b/mm/shrinker.c
index 0a32ef42f2a7..d820e4cc5806 100644
--- a/mm/shrinker.c
+++ b/mm/shrinker.c
@@ -548,6 +548,119 @@ unsigned long shrink_slab(gfp_t gfp_mask, int nid, struct
mem_cgroup *memcg,
return freed;
}
+struct shrinker *shrinker_alloc(unsigned int flags, const char *fmt, ...)
+{
+ struct shrinker *shrinker;
+ unsigned int size;
+ va_list __maybe_unused ap;
+ int err;
+
+ shrinker = kzalloc(sizeof(struct shrinker), GFP_KERNEL);
+ if (!shrinker)
+ return NULL;
+
+#ifdef CONFIG_SHRINKER_DEBUG
+ va_start(ap, fmt);
+ shrinker->name = kvasprintf_const(GFP_KERNEL, fmt, ap);
+ va_end(ap);
+ if (!shrinker->name)
+ goto err_name;
+#endif
So why not introduce another helper to handle this and declare it
as a void function when !CONFIG_SHRINKER_DEBUG? Something like the
following:
#ifdef CONFIG_SHRINKER_DEBUG
static int shrinker_debugfs_name_alloc(struct shrinker *shrinker, const
char *fmt,
va_list vargs)
{
shrinker->name = kvasprintf_const(GFP_KERNEL, fmt, vargs);
return shrinker->name ? 0 : -ENOMEM;
}
#else
static int shrinker_debugfs_name_alloc(struct shrinker *shrinker, const
char *fmt,
va_list vargs)
{
return 0;
}
#endif
+ shrinker->flags = flags;
+
+ if (flags & SHRINKER_MEMCG_AWARE) {
+
Re: [PATCH v2 08/47] erofs: dynamically allocate the erofs-shrinker
> On Jul 24, 2023, at 17:43, Qi Zheng wrote: > > Use new APIs to dynamically allocate the erofs-shrinker. > > Signed-off-by: Qi Zheng Reviewed-by: Muchun Song Thanks.
Re: [PATCH v2 01/47] mm: vmscan: move shrinker-related code into a separate file
> On Jul 24, 2023, at 17:43, Qi Zheng wrote:
>
> The mm/vmscan.c file is too large, so separate the shrinker-related
> code from it into a separate file. No functional changes.
>
> Signed-off-by: Qi Zheng
> ---
> include/linux/shrinker.h | 3 +
> mm/Makefile | 4 +-
> mm/shrinker.c| 707 +++
> mm/vmscan.c | 701 --
> 4 files changed, 712 insertions(+), 703 deletions(-)
> create mode 100644 mm/shrinker.c
>
> diff --git a/include/linux/shrinker.h b/include/linux/shrinker.h
> index 224293b2dd06..961cb84e51f5 100644
> --- a/include/linux/shrinker.h
> +++ b/include/linux/shrinker.h
> @@ -96,6 +96,9 @@ struct shrinker {
> */
> #define SHRINKER_NONSLAB (1 << 3)
>
> +unsigned long shrink_slab(gfp_t gfp_mask, int nid, struct mem_cgroup *memcg,
> +int priority);
A good cleanup, vmscan.c is so huge.
I'd like to introduce a new header in mm/ directory and contains those
declarations of functions (like this and other debug function in
shrinker_debug.c) since they are used internally across mm.
Thanks.
Re: [PATCH v2 01/47] mm: vmscan: move shrinker-related code into a separate file
> On Jul 25, 2023, at 11:09, Qi Zheng wrote:
>
>
>
> On 2023/7/25 10:35, Muchun Song wrote:
>>> On Jul 24, 2023, at 17:43, Qi Zheng wrote:
>>>
>>> The mm/vmscan.c file is too large, so separate the shrinker-related
>>> code from it into a separate file. No functional changes.
>>>
>>> Signed-off-by: Qi Zheng
>>> ---
>>> include/linux/shrinker.h | 3 +
>>> mm/Makefile | 4 +-
>>> mm/shrinker.c| 707 +++
>>> mm/vmscan.c | 701 --
>>> 4 files changed, 712 insertions(+), 703 deletions(-)
>>> create mode 100644 mm/shrinker.c
>>>
>>> diff --git a/include/linux/shrinker.h b/include/linux/shrinker.h
>>> index 224293b2dd06..961cb84e51f5 100644
>>> --- a/include/linux/shrinker.h
>>> +++ b/include/linux/shrinker.h
>>> @@ -96,6 +96,9 @@ struct shrinker {
>>> */
>>> #define SHRINKER_NONSLAB (1 << 3)
>>>
>>> +unsigned long shrink_slab(gfp_t gfp_mask, int nid, struct mem_cgroup
>>> *memcg,
>>> +int priority);
>> A good cleanup, vmscan.c is so huge.
>> I'd like to introduce a new header in mm/ directory and contains those
>> declarations of functions (like this and other debug function in
>> shrinker_debug.c) since they are used internally across mm.
>
> How about putting them in the mm/internal.h file?
Either is fine to me.
>
>> Thanks.
Re: [PATCH v2 02/47] mm: shrinker: remove redundant shrinker_rwsem in debugfs operations
> On Jul 24, 2023, at 17:43, Qi Zheng wrote: > > The debugfs_remove_recursive() will wait for debugfs_file_put() to return, > so the shrinker will not be freed when doing debugfs operations (such as > shrinker_debugfs_count_show() and shrinker_debugfs_scan_write()), so there > is no need to hold shrinker_rwsem during debugfs operations. > > Signed-off-by: Qi Zheng Reviewed-by: Muchun Song Thanks.
Re: [PATCH 2/3] mm/slab: delete cache_alloc_debugcheck_before()
On Sun, Jun 05, 2022 at 05:25:38PM +0200, Daniel Vetter wrote: > It only does a might_sleep_if(GFP_RECLAIM) check, which is already > covered by the might_alloc() in slab_pre_alloc_hook(). And all callers > of cache_alloc_debugcheck_before() call that beforehand already. > > Signed-off-by: Daniel Vetter Nice cleanup. Reviewed-by: Muchun Song Thanks.
Re: [PATCH 03/27] mm: remove pointless includes from
On Thu, Feb 10, 2022 at 3:28 PM Christoph Hellwig wrote: > > hmm.h pulls in the world for no good reason at all. Remove the > includes and push a few ones into the users instead. > > Signed-off-by: Christoph Hellwig > Reviewed-by: Logan Gunthorpe > Reviewed-by: Jason Gunthorpe > Reviewed-by: Chaitanya Kulkarni Reviewed-by: Muchun Song
Re: [PATCH 4/8] mm: move free_devmap_managed_page to memremap.c
On Mon, Feb 7, 2022 at 2:42 PM Christoph Hellwig wrote: > > free_devmap_managed_page has nothing to do with the code in swap.c, > move it to live with the rest of the code for devmap handling. > > Signed-off-by: Christoph Hellwig Reviewed-by: Muchun Song Thanks.
Re: [PATCH 2/8] mm: remove the __KERNEL__ guard from
On Mon, Feb 7, 2022 at 2:42 PM Christoph Hellwig wrote: > > __KERNEL__ ifdefs don't make sense outside of include/uapi/. > > Signed-off-by: Christoph Hellwig Reviewed-by: Muchun Song Thanks.
Re: [PATCH 1/8] mm: remove a pointless CONFIG_ZONE_DEVICE check in memremap_pages
On Mon, Feb 7, 2022 at 2:36 PM Christoph Hellwig wrote: > > memremap.c is only built when CONFIG_ZONE_DEVICE is set, so remove > the superflous extra check. > > Signed-off-by: Christoph Hellwig Reviewed-by: Muchun Song Thanks.
Re: [External] [PATCH v4] dma-buf: Add DmaBufTotal counter in meminfo
On Sat, Apr 17, 2021 at 9:44 PM wrote:
>
> On 4/17/21 3:07 PM, Muchun Song wrote:
> > On Sat, Apr 17, 2021 at 6:41 PM Peter Enderborg
> > wrote:
> >> This adds a total used dma-buf memory. Details
> >> can be found in debugfs, however it is not for everyone
> >> and not always available. dma-buf are indirect allocated by
> >> userspace. So with this value we can monitor and detect
> >> userspace applications that have problems.
> >>
> >> Signed-off-by: Peter Enderborg
> >> ---
> >> drivers/dma-buf/dma-buf.c | 13 +
> >> fs/proc/meminfo.c | 5 -
> >> include/linux/dma-buf.h | 1 +
> >> 3 files changed, 18 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> >> index f264b70c383e..197e5c45dd26 100644
> >> --- a/drivers/dma-buf/dma-buf.c
> >> +++ b/drivers/dma-buf/dma-buf.c
> >> @@ -37,6 +37,7 @@ struct dma_buf_list {
> >> };
> >>
> >> static struct dma_buf_list db_list;
> >> +static atomic_long_t dma_buf_global_allocated;
> >>
> >> static char *dmabuffs_dname(struct dentry *dentry, char *buffer, int
> >> buflen)
> >> {
> >> @@ -79,6 +80,7 @@ static void dma_buf_release(struct dentry *dentry)
> >> if (dmabuf->resv == (struct dma_resv *)[1])
> >> dma_resv_fini(dmabuf->resv);
> >>
> >> + atomic_long_sub(dmabuf->size, _buf_global_allocated);
> >> module_put(dmabuf->owner);
> >> kfree(dmabuf->name);
> >> kfree(dmabuf);
> >> @@ -586,6 +588,7 @@ struct dma_buf *dma_buf_export(const struct
> >> dma_buf_export_info *exp_info)
> >> mutex_lock(_list.lock);
> >> list_add(>list_node, _list.head);
> >> mutex_unlock(_list.lock);
> >> + atomic_long_add(dmabuf->size, _buf_global_allocated);
> >>
> >> return dmabuf;
> >>
> >> @@ -1346,6 +1349,16 @@ void dma_buf_vunmap(struct dma_buf *dmabuf, struct
> >> dma_buf_map *map)
> >> }
> >> EXPORT_SYMBOL_GPL(dma_buf_vunmap);
> >>
> >> +/**
> >> + * dma_buf_allocated_pages - Return the used nr of pages
> >> + * allocated for dma-buf
> >> + */
> >> +long dma_buf_allocated_pages(void)
> >> +{
> >> + return atomic_long_read(_buf_global_allocated) >> PAGE_SHIFT;
> >> +}
> >> +EXPORT_SYMBOL_GPL(dma_buf_allocated_pages);
> > dma_buf_allocated_pages is only called from fs/proc/meminfo.c.
> > I am confused why it should be exported. If it won't be called
> > from the driver module, we should not export it.
>
> Ah. I thought you did not want the GPL restriction. I don't have real
> opinion about it. It's written to be following the rest of the module.
> It is not needed for the usage of dma-buf in kernel module. But I
> don't see any reason for hiding it either.
The modules do not need dma_buf_allocated_pages, hiding it
can prevent the module from calling it. So I think that
EXPORT_SYMBOL_GPL is unnecessary. If one day someone
want to call it from the module, maybe it’s not too late to export
it at that time.
>
>
> > Thanks.
> >
> >> +
> >> #ifdef CONFIG_DEBUG_FS
> >> static int dma_buf_debug_show(struct seq_file *s, void *unused)
> >> {
> >> diff --git a/fs/proc/meminfo.c b/fs/proc/meminfo.c
> >> index 6fa761c9cc78..ccc7c40c8db7 100644
> >> --- a/fs/proc/meminfo.c
> >> +++ b/fs/proc/meminfo.c
> >> @@ -16,6 +16,7 @@
> >> #ifdef CONFIG_CMA
> >> #include
> >> #endif
> >> +#include
> >> #include
> >> #include "internal.h"
> >>
> >> @@ -145,7 +146,9 @@ static int meminfo_proc_show(struct seq_file *m, void
> >> *v)
> >> show_val_kb(m, "CmaFree:",
> >> global_zone_page_state(NR_FREE_CMA_PAGES));
> >> #endif
> >> -
> >> +#ifdef CONFIG_DMA_SHARED_BUFFER
> >> + show_val_kb(m, "DmaBufTotal:", dma_buf_allocated_pages());
> >> +#endif
> >> hugetlb_report_meminfo(m);
> >>
> >> arch_report_meminfo(m);
> >> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> >> index efdc56b9d95f..5b05816bd2cd 100644
> >> --- a/include/linux/dma-buf.h
> >> +++ b/include/linux/dma-buf.h
> >> @@ -507,4 +507,5 @@ int dma_buf_mmap(struct dma_buf *, struct
> >> vm_area_struct *,
> >> unsigned long);
> >> int dma_buf_vmap(struct dma_buf *dmabuf, struct dma_buf_map *map);
> >> void dma_buf_vunmap(struct dma_buf *dmabuf, struct dma_buf_map *map);
> >> +long dma_buf_allocated_pages(void);
> >> #endif /* __DMA_BUF_H__ */
> >> --
> >> 2.17.1
> >>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [External] [PATCH v4] dma-buf: Add DmaBufTotal counter in meminfo
On Sat, Apr 17, 2021 at 6:41 PM Peter Enderborg
wrote:
>
> This adds a total used dma-buf memory. Details
> can be found in debugfs, however it is not for everyone
> and not always available. dma-buf are indirect allocated by
> userspace. So with this value we can monitor and detect
> userspace applications that have problems.
>
> Signed-off-by: Peter Enderborg
> ---
> drivers/dma-buf/dma-buf.c | 13 +
> fs/proc/meminfo.c | 5 -
> include/linux/dma-buf.h | 1 +
> 3 files changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index f264b70c383e..197e5c45dd26 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -37,6 +37,7 @@ struct dma_buf_list {
> };
>
> static struct dma_buf_list db_list;
> +static atomic_long_t dma_buf_global_allocated;
>
> static char *dmabuffs_dname(struct dentry *dentry, char *buffer, int buflen)
> {
> @@ -79,6 +80,7 @@ static void dma_buf_release(struct dentry *dentry)
> if (dmabuf->resv == (struct dma_resv *)[1])
> dma_resv_fini(dmabuf->resv);
>
> + atomic_long_sub(dmabuf->size, _buf_global_allocated);
> module_put(dmabuf->owner);
> kfree(dmabuf->name);
> kfree(dmabuf);
> @@ -586,6 +588,7 @@ struct dma_buf *dma_buf_export(const struct
> dma_buf_export_info *exp_info)
> mutex_lock(_list.lock);
> list_add(>list_node, _list.head);
> mutex_unlock(_list.lock);
> + atomic_long_add(dmabuf->size, _buf_global_allocated);
>
> return dmabuf;
>
> @@ -1346,6 +1349,16 @@ void dma_buf_vunmap(struct dma_buf *dmabuf, struct
> dma_buf_map *map)
> }
> EXPORT_SYMBOL_GPL(dma_buf_vunmap);
>
> +/**
> + * dma_buf_allocated_pages - Return the used nr of pages
> + * allocated for dma-buf
> + */
> +long dma_buf_allocated_pages(void)
> +{
> + return atomic_long_read(_buf_global_allocated) >> PAGE_SHIFT;
> +}
> +EXPORT_SYMBOL_GPL(dma_buf_allocated_pages);
dma_buf_allocated_pages is only called from fs/proc/meminfo.c.
I am confused why it should be exported. If it won't be called
from the driver module, we should not export it.
Thanks.
> +
> #ifdef CONFIG_DEBUG_FS
> static int dma_buf_debug_show(struct seq_file *s, void *unused)
> {
> diff --git a/fs/proc/meminfo.c b/fs/proc/meminfo.c
> index 6fa761c9cc78..ccc7c40c8db7 100644
> --- a/fs/proc/meminfo.c
> +++ b/fs/proc/meminfo.c
> @@ -16,6 +16,7 @@
> #ifdef CONFIG_CMA
> #include
> #endif
> +#include
> #include
> #include "internal.h"
>
> @@ -145,7 +146,9 @@ static int meminfo_proc_show(struct seq_file *m, void *v)
> show_val_kb(m, "CmaFree:",
> global_zone_page_state(NR_FREE_CMA_PAGES));
> #endif
> -
> +#ifdef CONFIG_DMA_SHARED_BUFFER
> + show_val_kb(m, "DmaBufTotal:", dma_buf_allocated_pages());
> +#endif
> hugetlb_report_meminfo(m);
>
> arch_report_meminfo(m);
> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> index efdc56b9d95f..5b05816bd2cd 100644
> --- a/include/linux/dma-buf.h
> +++ b/include/linux/dma-buf.h
> @@ -507,4 +507,5 @@ int dma_buf_mmap(struct dma_buf *, struct vm_area_struct
> *,
> unsigned long);
> int dma_buf_vmap(struct dma_buf *dmabuf, struct dma_buf_map *map);
> void dma_buf_vunmap(struct dma_buf *dmabuf, struct dma_buf_map *map);
> +long dma_buf_allocated_pages(void);
> #endif /* __DMA_BUF_H__ */
> --
> 2.17.1
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [External] [PATCH v3] dma-buf: Add DmaBufTotal counter in meminfo
On Sat, Apr 17, 2021 at 12:08 AM Peter Enderborg
wrote:
>
> This adds a total used dma-buf memory. Details
> can be found in debugfs, however it is not for everyone
> and not always available. dma-buf are indirect allocated by
> userspace. So with this value we can monitor and detect
> userspace applications that have problems.
I want to know more details about the problems.
Can you share what problems you have encountered?
Thanks.
>
> Signed-off-by: Peter Enderborg
> ---
> drivers/dma-buf/dma-buf.c | 12
> fs/proc/meminfo.c | 5 -
> include/linux/dma-buf.h | 1 +
> 3 files changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index f264b70c383e..d40fff2ae1fa 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -37,6 +37,7 @@ struct dma_buf_list {
> };
>
> static struct dma_buf_list db_list;
> +static atomic_long_t dma_buf_global_allocated;
>
> static char *dmabuffs_dname(struct dentry *dentry, char *buffer, int buflen)
> {
> @@ -79,6 +80,7 @@ static void dma_buf_release(struct dentry *dentry)
> if (dmabuf->resv == (struct dma_resv *)[1])
> dma_resv_fini(dmabuf->resv);
>
> + atomic_long_sub(dmabuf->size, _buf_global_allocated);
> module_put(dmabuf->owner);
> kfree(dmabuf->name);
> kfree(dmabuf);
> @@ -586,6 +588,7 @@ struct dma_buf *dma_buf_export(const struct
> dma_buf_export_info *exp_info)
> mutex_lock(_list.lock);
> list_add(>list_node, _list.head);
> mutex_unlock(_list.lock);
> + atomic_long_add(dmabuf->size, _buf_global_allocated);
>
> return dmabuf;
>
> @@ -1346,6 +1349,15 @@ void dma_buf_vunmap(struct dma_buf *dmabuf, struct
> dma_buf_map *map)
> }
> EXPORT_SYMBOL_GPL(dma_buf_vunmap);
>
> +/**
> + * dma_buf_get_size - Return the used nr pages by dma-buf
> + */
> +long dma_buf_allocated_pages(void)
> +{
> + return atomic_long_read(_buf_global_allocated) >> PAGE_SHIFT;
> +}
> +EXPORT_SYMBOL_GPL(dma_buf_allocated_pages);
Why need "EXPORT_SYMBOL_GPL"?
> +
> #ifdef CONFIG_DEBUG_FS
> static int dma_buf_debug_show(struct seq_file *s, void *unused)
> {
> diff --git a/fs/proc/meminfo.c b/fs/proc/meminfo.c
> index 6fa761c9cc78..ccc7c40c8db7 100644
> --- a/fs/proc/meminfo.c
> +++ b/fs/proc/meminfo.c
> @@ -16,6 +16,7 @@
> #ifdef CONFIG_CMA
> #include
> #endif
> +#include
> #include
> #include "internal.h"
>
> @@ -145,7 +146,9 @@ static int meminfo_proc_show(struct seq_file *m, void *v)
> show_val_kb(m, "CmaFree:",
> global_zone_page_state(NR_FREE_CMA_PAGES));
> #endif
> -
> +#ifdef CONFIG_DMA_SHARED_BUFFER
> + show_val_kb(m, "DmaBufTotal:", dma_buf_allocated_pages());
> +#endif
> hugetlb_report_meminfo(m);
>
> arch_report_meminfo(m);
> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> index efdc56b9d95f..5b05816bd2cd 100644
> --- a/include/linux/dma-buf.h
> +++ b/include/linux/dma-buf.h
> @@ -507,4 +507,5 @@ int dma_buf_mmap(struct dma_buf *, struct vm_area_struct
> *,
> unsigned long);
> int dma_buf_vmap(struct dma_buf *dmabuf, struct dma_buf_map *map);
> void dma_buf_vunmap(struct dma_buf *dmabuf, struct dma_buf_map *map);
> +long dma_buf_allocated_pages(void);
> #endif /* __DMA_BUF_H__ */
> --
> 2.17.1
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3] drm/virtio: fix missing dma_fence_put() in virtio_gpu_execbuffer_ioctl()
On Tue, Jul 21, 2020 at 6:17 PM Xin He wrote:
>
> From: Qi Liu
>
> We should put the reference count of the fence after calling
> virtio_gpu_cmd_submit(). So add the missing dma_fence_put().
>
> Fixes: 2cd7b6f08bc4 ("drm/virtio: add in/out fence support for explicit
> synchronization")
> Co-developed-by: Xin He
> Signed-off-by: Xin He
> Signed-off-by: Qi Liu
> Reviewed-by: Muchun Song
> ---
>
> changelog in v3:
> 1) Change the subject from "drm/virtio: fixed memory leak in
> virtio_gpu_execbuffer_ioctl()" to
>"drm/virtio: fix missing dma_fence_put() in virtio_gpu_execbuffer_ioctl()"
> 2) Rework the commit log
>
> changelog in v2:
> 1) Add a change description
>
> drivers/gpu/drm/virtio/virtgpu_ioctl.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> index 5df722072ba0..19c5bc01eb79 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> @@ -179,6 +179,7 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device
> *dev, void *data,
>
> virtio_gpu_cmd_submit(vgdev, buf, exbuf->size,
> vfpriv->ctx_id, buflist, out_fence);
> + dma_fence_put(_fence->f);
> virtio_gpu_notify(vgdev);
> return 0;
>
> --
> 2.21.1 (Apple Git-122.3)
>
Ping guys. Any comments or suggestions？
--
Yours,
Muchun
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
