[bug report] perf: Rewrite core context handling

[Dan Carpenter <error27@gmail.com>]

[ Resending two weeks of email.  Crap.  -dan ]

Hello Peter Zijlstra,

The patch bd2756811766: "perf: Rewrite core context handling" from
Oct 8, 2022, leads to the following Smatch static checker warning:

kernel/events/core.c:13190 inherit_event() warn: 'pmu_ctx' is an error pointer or valid

kernel/events/core.c
    13146 static struct perf_event *
    13147 inherit_event(struct perf_event *parent_event,
    13148               struct task_struct *parent,
    13149               struct perf_event_context *parent_ctx,
    13150               struct task_struct *child,
    13151               struct perf_event *group_leader,
    13152               struct perf_event_context *child_ctx)
    13153 {
    13154         enum perf_event_state parent_state = parent_event->state;
    13155         struct perf_event_pmu_context *pmu_ctx;
    13156         struct perf_event *child_event;
    13157         unsigned long flags;
    13158 
    13159         /*
    13160          * Instead of creating recursive hierarchies of events,
    13161          * we link inherited events back to the original parent,
    13162          * which has a filp for sure, which we use as the reference
    13163          * count:
    13164          */
    13165         if (parent_event->parent)
    13166                 parent_event = parent_event->parent;
    13167 
    13168         child_event = perf_event_alloc(&parent_event->attr,
    13169                                            parent_event->cpu,
    13170                                            child,
    13171                                            group_leader, parent_event,
    13172                                            NULL, NULL, -1);
    13173         if (IS_ERR(child_event))
    13174                 return child_event;
                          ^^^^^^^^^^^^^^^^^^^
inherit_event() has historically returned a mix of error pointers and
NULL.  I am not clear on the rules.

    13175 
    13176         pmu_ctx = find_get_pmu_context(child_event->pmu, child_ctx, child_event);
--> 13177         if (!pmu_ctx) {

The find_get_pmu_context() function only returns error pointers so this
is straight forward, but I don't know if this path should return an
error pointer or NULL or what.  Probably easiest to just report it to
you.  :)

    13178                 free_event(child_event);
    13179                 return NULL;
    13180         }
    13181         child_event->pmu_ctx = pmu_ctx;
    13182 
    13183         /*
    13184          * is_orphaned_event() and list_add_tail(&parent_event->child_list)
    13185          * must be under the same lock in order to serialize against
    13186          * perf_event_release_kernel(), such that either we must observe
    13187          * is_orphaned_event() or they will observe us on the child_list.
    13188          */
    13189         mutex_lock(&parent_event->child_mutex);
    13190         if (is_orphaned_event(parent_event) ||
    13191             !atomic_long_inc_not_zero(&parent_event->refcount)) {
    13192                 mutex_unlock(&parent_event->child_mutex);
    13193                 /* task_ctx_data is freed with child_ctx */
    13194                 free_event(child_event);
    13195                 return NULL;
    13196         }
    13197 
    13198         get_ctx(child_ctx);
    13199 
    13200         /*
    13201          * Make the child state follow the state of the parent event,
    13202          * not its attr.disabled bit.  We hold the parent's mutex,
    13203          * so we won't race with perf_event_{en, dis}able_family.
    13204          */
    13205         if (parent_state >= PERF_EVENT_STATE_INACTIVE)
    13206                 child_event->state = PERF_EVENT_STATE_INACTIVE;
    13207         else
    13208                 child_event->state = PERF_EVENT_STATE_OFF;
    13209 
    13210         if (parent_event->attr.freq) {
    13211                 u64 sample_period = parent_event->hw.sample_period;
    13212                 struct hw_perf_event *hwc = &child_event->hw;
    13213 
    13214                 hwc->sample_period = sample_period;
    13215                 hwc->last_period   = sample_period;
    13216 
    13217                 local64_set(&hwc->period_left, sample_period);
    13218         }
    13219 
    13220         child_event->ctx = child_ctx;
    13221         child_event->overflow_handler = parent_event->overflow_handler;
    13222         child_event->overflow_handler_context
    13223                 = parent_event->overflow_handler_context;
    13224 
    13225         /*
    13226          * Precalculate sample_data sizes
    13227          */
    13228         perf_event__header_size(child_event);
    13229         perf_event__id_header_size(child_event);
    13230 
    13231         /*
    13232          * Link it up in the child's context:
    13233          */
    13234         raw_spin_lock_irqsave(&child_ctx->lock, flags);
    13235         add_event_to_ctx(child_event, child_ctx);
    13236         child_event->attach_state |= PERF_ATTACH_CHILD;
    13237         raw_spin_unlock_irqrestore(&child_ctx->lock, flags);
    13238 
    13239         /*
    13240          * Link this into the parent event's child list
    13241          */
    13242         list_add_tail(&child_event->child_list, &parent_event->child_list);
    13243         mutex_unlock(&parent_event->child_mutex);
    13244 
    13245         return child_event;
    13246 }

regards,
dan carpenter