Re: [PATCH v4 07/25] ibtrs: client: statistics functions

[Jinpu Wang <jinpu.wang@cloud.ionos.com>]

On Tue, Sep 24, 2019 at 1:15 AM Bart Van Assche <bvanassche@acm.org> wrote:
>
> On 6/20/19 8:03 AM, Jack Wang wrote:
> > +void ibtrs_clt_update_rdma_lat(struct ibtrs_clt_stats *stats, bool read,
> > +                            unsigned long ms)
> > +{
> > +     struct ibtrs_clt_stats_pcpu *s;
> > +     int id;
> > +
> > +     id = ibtrs_clt_ms_to_id(ms);
> > +     s = this_cpu_ptr(stats->pcpu_stats);
> > +     if (read) {
> > +             s->rdma_lat_distr[id].read++;
> > +             if (s->rdma_lat_max.read < ms)
> > +                     s->rdma_lat_max.read = ms;
> > +     } else {
> > +             s->rdma_lat_distr[id].write++;
> > +             if (s->rdma_lat_max.write < ms)
> > +                     s->rdma_lat_max.write = ms;
> > +     }
> > +}
>
> Can it happen that this function is called simultaneously from thread
> context and from interrupt context?
This can't happen, we only call the function from complete_rdma_req, and
complete_rdma_req is call from cq callback except fail_all_outstanding_reqs,
cq callback context is softirq, fail_all_outstanding_reqs is process
context, but we
disconnect and drain_qp before call into fail_all_outstading_reqs

>
> > +void ibtrs_clt_update_wc_stats(struct ibtrs_clt_con *con)
> > +{
> > +     struct ibtrs_clt_sess *sess = to_clt_sess(con->c.sess);
> > +     struct ibtrs_clt_stats *stats = &sess->stats;
> > +     struct ibtrs_clt_stats_pcpu *s;
> > +     int cpu;
> > +
> > +     cpu = raw_smp_processor_id();
> > +     s = this_cpu_ptr(stats->pcpu_stats);
> > +     s->wc_comp.cnt++;
> > +     s->wc_comp.total_cnt++;
> > +     if (unlikely(con->cpu != cpu)) {
> > +             s->cpu_migr.to++;
> > +
> > +             /* Careful here, override s pointer */
> > +             s = per_cpu_ptr(stats->pcpu_stats, con->cpu);
> > +             atomic_inc(&s->cpu_migr.from);
> > +     }
> > +}
>
> Same question here.
The function is only called from cq done callback,
>
> > +void ibtrs_clt_inc_failover_cnt(struct ibtrs_clt_stats *stats)
> > +{
> > +     struct ibtrs_clt_stats_pcpu *s;
> > +
> > +     s = this_cpu_ptr(stats->pcpu_stats);
> > +     s->rdma.failover_cnt++;
> > +}
>
> And here ...
this function only call from process context.

>
> Thanks,
>
> Bart.
Thanks,
Jinpu