On Sun, Mar 1, 2020 at 1:51 AM Bart Van Assche <bvanassche@xxxxxxx> wrote:
>
> On 2020-02-21 02:47, Jack Wang wrote:
> > +struct rtrs_clt {
> > + struct list_head /* __rcu */ paths_list;
>
> The commented out __rcu is confusing. Please remove it and add an
> elaborate comment if paths_list is a list head with nonstandard behavior.
Will change to a normal comment, we want to use rculist, but no such
annotation usage for normal list_head, only hlist_head in kernel tree,
Do you know why?
>
> > +#define PERMIT_SIZE(clt) (sizeof(struct rtrs_permit) + (clt)->pdu_sz)
> > +#define GET_PERMIT(clt, idx) ((clt)->permits + PERMIT_SIZE(clt) * (idx))
>
> Can these macros be changed into inline functions?
will try.
>
> > +static inline void rtrs_clt_decrease_inflight(struct rtrs_clt_stats *s)
> > +{
> > + atomic_dec(&s->inflight);
> > +}
>
> The name of this function is longer than its implementation. Consider to
> inline this function.
Ok, we can use the atomic_dec directly.
>
> Thanks,
>
> Bart.
Thanks!
