* [PATCH] infiniband: i40iw: replace atomic_add_return()
@ 2020-11-30 8:52 Yejune Deng
2020-12-01 14:42 ` Saleem, Shiraz
2020-12-07 19:41 ` Jason Gunthorpe
0 siblings, 2 replies; 4+ messages in thread
From: Yejune Deng @ 2020-11-30 8:52 UTC (permalink / raw)
To: faisal.latif, shiraz.saleem, dledford, jgg
Cc: linux-rdma, linux-kernel, yejune.deng
atomic_inc_return() is a little neater
Signed-off-by: Yejune Deng <yejune.deng@gmail.com>
---
drivers/infiniband/hw/i40iw/i40iw_cm.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/infiniband/hw/i40iw/i40iw_cm.c b/drivers/infiniband/hw/i40iw/i40iw_cm.c
index 3053c345..26e92ae 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_cm.c
+++ b/drivers/infiniband/hw/i40iw/i40iw_cm.c
@@ -2426,7 +2426,7 @@ static void i40iw_handle_rst_pkt(struct i40iw_cm_node *cm_node,
}
break;
case I40IW_CM_STATE_MPAREQ_RCVD:
- atomic_add_return(1, &cm_node->passive_state);
+ atomic_inc_return(&cm_node->passive_state);
break;
case I40IW_CM_STATE_ESTABLISHED:
case I40IW_CM_STATE_SYN_RCVD:
@@ -3020,7 +3020,7 @@ static int i40iw_cm_reject(struct i40iw_cm_node *cm_node, const void *pdata, u8
i40iw_cleanup_retrans_entry(cm_node);
if (!loopback) {
- passive_state = atomic_add_return(1, &cm_node->passive_state);
+ passive_state = atomic_inc_return(&cm_node->passive_state);
if (passive_state == I40IW_SEND_RESET_EVENT) {
cm_node->state = I40IW_CM_STATE_CLOSED;
i40iw_rem_ref_cm_node(cm_node);
@@ -3678,7 +3678,7 @@ int i40iw_accept(struct iw_cm_id *cm_id, struct iw_cm_conn_param *conn_param)
return -EINVAL;
}
- passive_state = atomic_add_return(1, &cm_node->passive_state);
+ passive_state = atomic_inc_return(&cm_node->passive_state);
if (passive_state == I40IW_SEND_RESET_EVENT) {
i40iw_rem_ref_cm_node(cm_node);
return -ECONNRESET;
--
1.9.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* RE: [PATCH] infiniband: i40iw: replace atomic_add_return()
2020-11-30 8:52 [PATCH] infiniband: i40iw: replace atomic_add_return() Yejune Deng
@ 2020-12-01 14:42 ` Saleem, Shiraz
2020-12-01 16:14 ` Jason Gunthorpe
2020-12-07 19:41 ` Jason Gunthorpe
1 sibling, 1 reply; 4+ messages in thread
From: Saleem, Shiraz @ 2020-12-01 14:42 UTC (permalink / raw)
To: Yejune Deng, Latif, Faisal, dledford, jgg; +Cc: linux-rdma, linux-kernel
> Subject: [PATCH] infiniband: i40iw: replace atomic_add_return()
>
> atomic_inc_return() is a little neater
>
> Signed-off-by: Yejune Deng <yejune.deng@gmail.com>
> ---
> drivers/infiniband/hw/i40iw/i40iw_cm.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/infiniband/hw/i40iw/i40iw_cm.c
> b/drivers/infiniband/hw/i40iw/i40iw_cm.c
> index 3053c345..26e92ae 100644
> --- a/drivers/infiniband/hw/i40iw/i40iw_cm.c
> +++ b/drivers/infiniband/hw/i40iw/i40iw_cm.c
> @@ -2426,7 +2426,7 @@ static void i40iw_handle_rst_pkt(struct i40iw_cm_node
> *cm_node,
> }
> break;
> case I40IW_CM_STATE_MPAREQ_RCVD:
> - atomic_add_return(1, &cm_node->passive_state);
> + atomic_inc_return(&cm_node->passive_state);
Just an atomic_inc would suffice here.
> break;
> case I40IW_CM_STATE_ESTABLISHED:
> case I40IW_CM_STATE_SYN_RCVD:
> @@ -3020,7 +3020,7 @@ static int i40iw_cm_reject(struct i40iw_cm_node
> *cm_node, const void *pdata, u8
> i40iw_cleanup_retrans_entry(cm_node);
>
> if (!loopback) {
> - passive_state = atomic_add_return(1, &cm_node->passive_state);
> + passive_state = atomic_inc_return(&cm_node->passive_state);
Fine with it as its consistent across i40iw. But aren't there many more instances of this across the tree?
Isn't this a choice best left to the developer?
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] infiniband: i40iw: replace atomic_add_return()
2020-12-01 14:42 ` Saleem, Shiraz
@ 2020-12-01 16:14 ` Jason Gunthorpe
0 siblings, 0 replies; 4+ messages in thread
From: Jason Gunthorpe @ 2020-12-01 16:14 UTC (permalink / raw)
To: Saleem, Shiraz
Cc: Yejune Deng, Latif, Faisal, dledford, linux-rdma, linux-kernel
On Tue, Dec 01, 2020 at 02:42:25PM +0000, Saleem, Shiraz wrote:
> > case I40IW_CM_STATE_ESTABLISHED:
> > case I40IW_CM_STATE_SYN_RCVD:
> > @@ -3020,7 +3020,7 @@ static int i40iw_cm_reject(struct i40iw_cm_node
> > *cm_node, const void *pdata, u8
> > i40iw_cleanup_retrans_entry(cm_node);
> >
> > if (!loopback) {
> > - passive_state = atomic_add_return(1, &cm_node->passive_state);
> > + passive_state = atomic_inc_return(&cm_node->passive_state);
>
> Fine with it as its consistent across i40iw. But aren't there many
> more instances of this across the tree? Isn't this a choice best
> left to the developer?
I don't think it is a style choice, the narrowest applicable atomic
should always be used as a matter of clarity and performance.
Jason
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] infiniband: i40iw: replace atomic_add_return()
2020-11-30 8:52 [PATCH] infiniband: i40iw: replace atomic_add_return() Yejune Deng
2020-12-01 14:42 ` Saleem, Shiraz
@ 2020-12-07 19:41 ` Jason Gunthorpe
1 sibling, 0 replies; 4+ messages in thread
From: Jason Gunthorpe @ 2020-12-07 19:41 UTC (permalink / raw)
To: Yejune Deng
Cc: faisal.latif, shiraz.saleem, dledford, linux-rdma, linux-kernel
On Mon, Nov 30, 2020 at 04:52:56PM +0800, Yejune Deng wrote:
> atomic_inc_return() is a little neater
>
> Signed-off-by: Yejune Deng <yejune.deng@gmail.com>
> ---
> drivers/infiniband/hw/i40iw/i40iw_cm.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
Applied to for-next with the note from Shiraz
Thanks,
Jason
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-12-07 19:42 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-30 8:52 [PATCH] infiniband: i40iw: replace atomic_add_return() Yejune Deng
2020-12-01 14:42 ` Saleem, Shiraz
2020-12-01 16:14 ` Jason Gunthorpe
2020-12-07 19:41 ` Jason Gunthorpe
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.