Re: [PATCH v2 net-next 3/7] octeontx2-vf: Virtual function driver support

[Andrew Lunn <andrew@lunn.ch>]

> > > > +static int otx2vf_process_mbox_msg_up(struct otx2_nic *vf,
> > > > +                                   struct mbox_msghdr *req)
> > > > +{
> > > > +     /* Check if valid, if not reply with a invalid msg */
> > > > +     if (req->sig != OTX2_MBOX_REQ_SIG) {
> > > > +             otx2_reply_invalid_msg(&vf->mbox.mbox_up, 0, 0, req->id);
> > > > +             return -ENODEV;
> > > > +     }
> > > > +
> > > > +     switch (req->id) {
> > > > +#define M(_name, _id, _fn_name, _req_type, _rsp_type)                        \
> > > > +     case _id: {                                                     \
> > > > +             struct _rsp_type *rsp;                                  \
> > > > +             int err;                                                \
> > > > +                                                                     \
> > > > +             rsp = (struct _rsp_type *)otx2_mbox_alloc_msg(          \
> > > > +                     &vf->mbox.mbox_up, 0,                           \
> > > > +                     sizeof(struct _rsp_type));                      \
> > > > +             if (!rsp)                                               \
> > > > +                     return -ENOMEM;                                 \
> > > > +                                                                     \
> > > > +             rsp->hdr.id = _id;                                      \
> > > > +             rsp->hdr.sig = OTX2_MBOX_RSP_SIG;                       \
> > > > +             rsp->hdr.pcifunc = 0;                                   \
> > > > +             rsp->hdr.rc = 0;                                        \
> > > > +                                                                     \
> > > > +             err = otx2_mbox_up_handler_ ## _fn_name(                \
> > > > +                     vf, (struct _req_type *)req, rsp);              \
> > > > +             return err;                                             \
> > > > +     }
> > > > +MBOX_UP_CGX_MESSAGES
> > > > +#undef M
> > >
> > > "return ..." inside macro which is called by another macro is highly
> > > discouraged by the Linux kernel coding style.
> > >
> >
> > There are many mailbox messages to handle and adding each one of them
> > to switch case would be a
> > lot of duplicate code. Hence we choose to with these macros.
> 
> The coding style section 12.1 talks exactly about that pattern.
> https://elixir.bootlin.com/linux/latest/source/Documentation/process/coding-style.rst#L752
> Somehow all other drivers succeeded to write their code without such
> macros, I'm confident that you will success too. Please try your best.

Hi Sunil

It is easy to create locking bugs with returns in macros. I've
inherited code which did this, not realized where was a return hidden
in a macro, and as a result got locks wrong, resulting in a deadlock.

     Andrew