Messages in this thread Patch in this message | | | Date | Tue, 19 Sep 2023 12:01:15 +0800 | Subject | Re: [PATCH for-next v3 2/2] RDMA/rxe: Call rxe_set_mtu after rxe_register_device | From | Zhu Yanjun <> |
| |
在 2023/9/19 11:25, Zhijian Li (Fujitsu) 写道: > > On 19/09/2023 09:11, Zhu Yanjun wrote: >> On Tue, Sep 19, 2023 at 8:57 AM Zhijian Li (Fujitsu) >> <lizhijian@fujitsu.com> wrote: >>> >>> >>> On 18/09/2023 20:37, Leon Romanovsky wrote: >>>> On Mon, Sep 18, 2023 at 10:05:43AM +0800, Li Zhijian wrote: >>>>> rxe_set_mtu() will call rxe_info_dev() to print message, and >>>>> rxe_info_dev() expects dev_name(rxe->ib_dev->dev) has been assigned. >>>>> >>>>> Previously since dev_name() is not set, when a new rxe link is being >>>>> added, 'null' will be used as the dev_name like: >>>>> >>>>> "(null): rxe_set_mtu: Set mtu to 1024" >>>>> >>>>> Move rxe_register_device() earlier to assign the correct dev_name >>>>> so that it can be read by rxe_set_mtu() later. >>>> I would expect removal of that print line instead of moving >>>> rxe_register_device(). >>> >>> I also struggled with this point. The last option is keep it as it is. >>> Once rxe is registered, this print will work fine. >> I delved into the source code. About moving rxe_register_device, I >> could not find any harm to the driver. > The point i'm struggling was that, it's strange/opaque to move rxe_register_device(). > There is no doubt that the original order was more clear.
Please check the source code. It is very clear.
> In terms of the message content, is it valuable to print(pr_info) this message, i noticed > that there is a duplicate pr_dbg() in rxe_notify().
rxe_notify is for netdevice event, not for rdma event.
> > rxe's mtu is always same with the NIC, isn't it ?
Please check the following source code.
void rxe_set_mtu(struct rxe_dev *rxe, unsigned int ndev_mtu)
{ struct rxe_port *port = &rxe->port; enum ib_mtu mtu;
mtu = eth_mtu_int_to_enum(ndev_mtu);
/* Make sure that new MTU in range */ mtu = mtu ? min_t(enum ib_mtu, mtu, IB_MTU_4096) : IB_MTU_256;
port->attr.active_mtu = mtu; port->mtu_cap = ib_mtu_enum_to_int(mtu);
rxe_info_dev(rxe, "Set mtu to %d", port->mtu_cap); }
Zhu Yanjun
> > Thanks > Zhijian > > > >> So I think this is also a solution to this problem. >> >> Zhu Yanjun >> >>> Thanks >>> Zhijian >>> >>> >>>> Thanks >>>> >>>>> And it's safe to do such change since mtu will not be used during the >>>>> rxe_register_device() >>>>> >>>>> After this change, the message becomes: >>>>> "rxe_eth0: rxe_set_mtu: Set mtu to 4096" >>>>> >>>>> Fixes: 9ac01f434a1e ("RDMA/rxe: Extend dbg log messages to err and info") >>>>> Reviewed-by: Daisuke Matsuda <matsuda-daisuke@fujitsu.com> >>>>> Reviewed-by: Zhu Yanjun <yanjun.zhu@linux.dev> >>>>> Signed-off-by: Li Zhijian <lizhijian@fujitsu.com> >>>>> --- >>>>> drivers/infiniband/sw/rxe/rxe.c | 5 ++++- >>>>> 1 file changed, 4 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/drivers/infiniband/sw/rxe/rxe.c b/drivers/infiniband/sw/rxe/rxe.c >>>>> index a086d588e159..8a43c0c4f8d8 100644 >>>>> --- a/drivers/infiniband/sw/rxe/rxe.c >>>>> +++ b/drivers/infiniband/sw/rxe/rxe.c >>>>> @@ -169,10 +169,13 @@ void rxe_set_mtu(struct rxe_dev *rxe, unsigned int ndev_mtu) >>>>> */ >>>>> int rxe_add(struct rxe_dev *rxe, unsigned int mtu, const char *ibdev_name) >>>>> { >>>>> + int ret; >>>>> + >>>>> rxe_init(rxe); >>>>> + ret = rxe_register_device(rxe, ibdev_name); >>>>> rxe_set_mtu(rxe, mtu); >>>>> >>>>> - return rxe_register_device(rxe, ibdev_name); >>>>> + return ret; >>>>> } >>>>> >>>>> static int rxe_newlink(const char *ibdev_name, struct net_device *ndev) >>>>> -- >>>>> 2.29.2 >>>> >
| |