Messages in this thread | | | Date | Mon, 2 Dec 2019 14:16:46 +0100 | From | Peter Zijlstra <> | Subject | Re: [RFC PATCH 2/8] perf: Helpers for alloc/init/fini PMU specific data |
| |
On Thu, Nov 28, 2019 at 07:14:25AM -0800, kan.liang@linux.intel.com wrote:
> +static int > +__alloc_task_ctx_data_rcu(struct task_struct *task, > + size_t ctx_size, gfp_t flags) > +{ > + struct perf_ctx_data *ctx_data = task->perf_ctx_data; > + int ret; > + > + lockdep_assert_held_once(&task->perf_ctx_data_lock); > + > + ret = alloc_perf_ctx_data(ctx_size, flags, &ctx_data); > + if (ret) > + return ret; > + > + ctx_data->refcount = 1; > + > + rcu_assign_pointer(task->perf_ctx_data, ctx_data); > + > + return 0; > +}
> +static int > +__init_task_ctx_data_rcu(struct task_struct *task, size_t ctx_size, gfp_t flags) > +{ > + struct perf_ctx_data *ctx_data = task->perf_ctx_data; > + > + lockdep_assert_held_once(&task->perf_ctx_data_lock); > + > + if (ctx_data) { > + ctx_data->refcount++; > + return 0; > + } > + > + return __alloc_task_ctx_data_rcu(task, ctx_size, flags); > +}
> +/** > + * Free perf_ctx_data RCU pointer for a task > + * @task: Target Task > + * @force: Unconditionally free perf_ctx_data > + * > + * If force is set, free perf_ctx_data unconditionally. > + * Otherwise, free perf_ctx_data when there are no users. > + * Lock is required to sync the writers of perf_ctx_data RCU pointer > + * and refcount. > + */ > +static void > +fini_task_ctx_data_rcu(struct task_struct *task, bool force) > +{ > + struct perf_ctx_data *ctx_data; > + unsigned long flags; > + > + raw_spin_lock_irqsave(&task->perf_ctx_data_lock, flags); > + > + ctx_data = task->perf_ctx_data; > + if (!ctx_data) > + goto unlock; > + > + if (!force && --ctx_data->refcount) > + goto unlock; > + > + RCU_INIT_POINTER(task->perf_ctx_data, NULL); > + call_rcu(&ctx_data->rcu_head, free_perf_ctx_data); > + > +unlock: > + raw_spin_unlock_irqrestore(&task->perf_ctx_data_lock, flags); > +}
All this refcount under lock is an anti-pattern. Also the naming is insane.
| |