Hi Kirill,
On Tue, Oct 09, 2018 at 08:46:56PM +0300, Kirill Tkhai wrote:
> This will allow to use generic refcount_t interfaces
> to check counters overflow instead of currently existing
> VM_BUG_ON(). The only difference after the patch is
> VM_BUG_ON() may cause BUG(), while refcount_t fires
> with WARN().
refcount_{sub_and_test,inc_not_zero}() are documented to provide
"slightly" more relaxed ordering than their atomic_* counterpart,
c.f.,
Documentation/core-api/refcount-vs-atomic.rst
lib/refcount.c (inline comments)
IIUC, this semantic change won't cause problems here (but please
double-check? ;D ).
Andrea
>But this seems not to be significant here,
> since such the problems are usually caught by syzbot
> with panic-on-warn enabled.
>
> Signed-off-by: Kirill Tkhai <ktkhai@xxxxxxxxxxxxx>
> ---
> include/linux/memcontrol.h | 2 +-
> mm/memcontrol.c | 10 ++++------
> 2 files changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> index 4399cc3f00e4..7ab2120155a4 100644
> --- a/include/linux/memcontrol.h
> +++ b/include/linux/memcontrol.h
> @@ -78,7 +78,7 @@ struct mem_cgroup_reclaim_cookie {
>
> struct mem_cgroup_id {
> int id;
> - atomic_t ref;
> + refcount_t ref;
> };
>
> /*
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 7bebe2ddec05..aa728d5b3d72 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -4299,14 +4299,12 @@ static void mem_cgroup_id_remove(struct mem_cgroup *memcg)
>
> static void mem_cgroup_id_get_many(struct mem_cgroup *memcg, unsigned int n)
> {
> - VM_BUG_ON(atomic_read(&memcg->id.ref) <= 0);
> - atomic_add(n, &memcg->id.ref);
> + refcount_add(n, &memcg->id.ref);
> }
>
> static void mem_cgroup_id_put_many(struct mem_cgroup *memcg, unsigned int n)
> {
> - VM_BUG_ON(atomic_read(&memcg->id.ref) < n);
> - if (atomic_sub_and_test(n, &memcg->id.ref)) {
> + if (refcount_sub_and_test(n, &memcg->id.ref)) {
> mem_cgroup_id_remove(memcg);
>
> /* Memcg ID pins CSS */
> @@ -4523,7 +4521,7 @@ static int mem_cgroup_css_online(struct cgroup_subsys_state *css)
> }
>
> /* Online state pins memcg ID, memcg ID pins CSS */
> - atomic_set(&memcg->id.ref, 1);
> + refcount_set(&memcg->id.ref, 1);
> css_get(css);
> return 0;
> }
> @@ -6357,7 +6355,7 @@ subsys_initcall(mem_cgroup_init);
> #ifdef CONFIG_MEMCG_SWAP
> static struct mem_cgroup *mem_cgroup_id_get_online(struct mem_cgroup *memcg)
> {
> - while (!atomic_inc_not_zero(&memcg->id.ref)) {
> + while (!refcount_inc_not_zero(&memcg->id.ref)) {
> /*
> * The root cgroup cannot be destroyed, so it's refcount must
> * always be >= 1.
>
