Messages in this thread | | | Date | Fri, 23 Jun 2023 21:24:25 +0800 | Subject | Re: [PATCH 29/29] mm: shrinker: move shrinker-related code into a separate file | From | Qi Zheng <> |
| |
Hi Sergey,
On 2023/6/23 13:25, Sergey Senozhatsky wrote: > On (23/06/22 16:53), Qi Zheng wrote: >> +/* >> + * Remove one >> + */ >> +void unregister_shrinker(struct shrinker *shrinker) >> +{ >> + struct dentry *debugfs_entry; >> + int debugfs_id; >> + >> + if (!(shrinker->flags & SHRINKER_REGISTERED)) >> + return; >> + >> + shrinker_put(shrinker); >> + wait_for_completion(&shrinker->completion_wait); >> + >> + mutex_lock(&shrinker_mutex); >> + list_del_rcu(&shrinker->list); > > Should this function wait for RCU grace period(s) before it goes > touching shrinker fields?
Why? We will free this shrinker instance by rcu after executing unregister_shrinker(). So it is safe to touch shrinker fields here.
> >> + shrinker->flags &= ~SHRINKER_REGISTERED; >> + if (shrinker->flags & SHRINKER_MEMCG_AWARE) >> + unregister_memcg_shrinker(shrinker); >> + debugfs_entry = shrinker_debugfs_detach(shrinker, &debugfs_id); >> + mutex_unlock(&shrinker_mutex); >> + >> + shrinker_debugfs_remove(debugfs_entry, debugfs_id); >> + >> + kfree(shrinker->nr_deferred); >> + shrinker->nr_deferred = NULL; >> +} >> +EXPORT_SYMBOL(unregister_shrinker); > > [..] > >> +void shrinker_free(struct shrinker *shrinker) >> +{ >> + kfree(shrinker); >> +} >> +EXPORT_SYMBOL(shrinker_free); >> + >> +void unregister_and_free_shrinker(struct shrinker *shrinker) >> +{ >> + unregister_shrinker(shrinker); >> + kfree_rcu(shrinker, rcu); >> +} > > Seems like this > > unregister_shrinker(); > shrinker_free(); > > is not exact equivalent of this > > unregister_and_free_shrinker();
Yes, my original intention is that shrinker_free() is only used to handle the case where register_shrinker() returns failure.
I will implement the method suggested by Dave in 02/29. Those APIs are more concise and will bring more benefits. :)
Thanks, Qi
| |