Re: [PATCH net-next 1/4] net/smc: Eliminate struct smc_ism_position

Tony Lu
2022-07-26T02:49:03Z

On Mon, Jul 25, 2022 at 04:09:57PM +0200, Wenjia Zhang wrote:
> From: Heiko Carstens <hca@linux.ibm.com>
>
> This struct is used in a single place only, and its usage generates
> inefficient code. Time to clean up!
>
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
> Reviewed-and-tested-by: Stefan Raspl <raspl@linux.ibm.com>
> Signed-off-by: Wenjia Zhang < wenjia@linux.ibm.com>

This patch looks good to me.

Reviewed-by: Tony Lu <tonylu@linux.alibaba.com>

> ---
> net/smc/smc_ism.c | 11 -----------
> net/smc/smc_ism.h | 20 +++++++++++---------
> net/smc/smc_tx.c | 10 +++-------
> 3 files changed, 14 insertions(+), 27 deletions(-)
>
> diff --git a/net/smc/smc_ism.c b/net/smc/smc_ism.c
> index a2084ecdb97e..c656ef25ee4b 100644
> --- a/net/smc/smc_ism.c
> +++ b/net/smc/smc_ism.c
> @@ -33,17 +33,6 @@ int smc_ism_cantalk(u64 peer_gid, unsigned short vlan_id, struct smcd_dev *smcd)
> vlan_id);
> }
>
> -int smc_ism_write(struct smcd_dev *smcd, const struct smc_ism_position *pos,
> - void *data, size_t len)
> -{
> - int rc;
> -
> - rc = smcd->ops->move_data(smcd, pos->token, pos->index, pos->signal,
> - pos->offset, data, len);
> -
> - return rc < 0 ? rc : 0;
> -}
> -
> void smc_ism_get_system_eid(u8 **eid)
> {
> if (!smc_ism_v2_capable)
> diff --git a/net/smc/smc_ism.h b/net/smc/smc_ism.h
> index 004b22a13ffa..d6b2db604fe8 100644
> --- a/net/smc/smc_ism.h
> +++ b/net/smc/smc_ism.h
> @@ -28,13 +28,6 @@ struct smc_ism_vlanid { /* VLAN id set on ISM device */
> refcount_t refcnt; /* Reference count */
> };
>
> -struct smc_ism_position { /* ISM device position to write to */
> - u64 token; /* Token of DMB */
> - u32 offset; /* Offset into DMBE */
> - u8 index; /* Index of DMBE */
> - u8 signal; /* Generate interrupt on owner side */
> -};
> -
> struct smcd_dev;
>
> int smc_ism_cantalk(u64 peer_gid, unsigned short vlan_id, struct smcd_dev *dev);
> @@ -45,12 +38,21 @@ int smc_ism_put_vlan(struct smcd_dev *dev, unsigned short vlan_id);
> int smc_ism_register_dmb(struct smc_link_group *lgr, int buf_size,
> struct smc_buf_desc *dmb_desc);
> int smc_ism_unregister_dmb(struct smcd_dev *dev, struct smc_buf_desc *dmb_desc);
> -int smc_ism_write(struct smcd_dev *dev, const struct smc_ism_position *pos,
> - void *data, size_t len);
> int smc_ism_signal_shutdown(struct smc_link_group *lgr);
> void smc_ism_get_system_eid(u8 **eid);
> u16 smc_ism_get_chid(struct smcd_dev *dev);
> bool smc_ism_is_v2_capable(void);
> void smc_ism_init(void);
> int smcd_nl_get_device(struct sk_buff *skb, struct netlink_callback *cb);
> +
> +static inline int smc_ism_write(struct smcd_dev *smcd, u64 dmb_tok,
> + unsigned int idx, bool sf, unsigned int offset,
> + void *data, size_t len)
> +{
> + int rc;
> +
> + rc = smcd->ops->move_data(smcd, dmb_tok, idx, sf, offset, data, len);
> + return rc < 0 ? rc : 0;
> +}
> +
> #endif
> diff --git a/net/smc/smc_tx.c b/net/smc/smc_tx.c
> index 4e8377657a62..64dedffe9d26 100644
> --- a/net/smc/smc_tx.c
> +++ b/net/smc/smc_tx.c
> @@ -320,15 +320,11 @@ int smc_tx_sendpage(struct smc_sock *smc, struct page *page, int offset,
> int smcd_tx_ism_write(struct smc_connection *conn, void *data, size_t len,
> u32 offset, int signal)
> {
> - struct smc_ism_position pos;
> int rc;
>
> - memset(&pos, 0, sizeof(pos));
> - pos.token = conn->peer_token;
> - pos.index = conn->peer_rmbe_idx;
> - pos.offset = conn->tx_off + offset;
> - pos.signal = signal;
> - rc = smc_ism_write(conn->lgr->smcd, &pos, data, len);
> + rc = smc_ism_write(conn->lgr->smcd, conn->peer_token,
> + conn->peer_rmbe_idx, signal, conn->tx_off + offset,
> + data, len);
> if (rc)
> conn->local_tx_ctrl.conn_state_flags.peer_conn_abort = 1;
> return rc;
> --
> 2.35.2