All of lore.kernel.org
 help / color / mirror / Atom feed
From: Allison Henderson <achender@kernel.org>
To: Sunny Tiwari <sunnytiwari@google.com>,
	Santosh Shilimkar <santosh.shilimkar@oracle.com>,
	netdev@vger.kernel.org,  linux-rdma@vger.kernel.org
Cc: rds-devel@oss.oracle.com,
	"David S . Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	 Simon Horman <horms@kernel.org>
Subject: Re: [PATCH net-next v2] net/rds: replace tasklets with workqueue
Date: Thu, 24 Sep 2026 20:38:04 -0700	[thread overview]
Message-ID: <0a141f59b37b9055764c97dddfdf9506fc4d7fa0.camel@kernel.org> (raw)
In-Reply-To: <20cbfc32678019daadd3c67fd75262b8264aa180.1790178801.git.sunnytiwari@google.com>

On Thu, 2026-09-24 at 06:26 +0000, Sunny Tiwari wrote:
> Convert the deprecated send and receive completion tasklets in
> net/rds/ib to BH work items queued on system_bh_wq.
> 
> In rds_ib_conn_path_shutdown(), replace tasklet_kill() with
> disable_work_sync(), and call enable_work() after destroying the CQs so
> the work items can be used again when the connection reconnects.
> 
> Assisted-by: Gemini
> Signed-off-by: Sunny Tiwari <sunnytiwari@google.com>

Hi Sunny,

Thanks for the patch.  This looks like a straight forward conversion to me.
I've also run it through the rds selftests on my end, and everything came
up clean.

Reviewed-by: Allison Henderson <achender@kernel.org>
Tested-by: Allison Henderson <achender@kernel.org>

Thanks!
Allison
 
> ---
> Changes in v2:
> - Use system_bh_wq instead of allocating a custom rds_ib_wq workqueue,
>   dropping the net/rds/ib.c changes.
> - Use from_work() and disable_work_sync() / enable_work() instead of
>   cancel_work_sync().
> - Link to v1: https://lore.kernel.org/linux-rdma/6b07af04b3ba66a09009e1a0a7b98ec5ec85d3a0.1790109248.git.sunnytiwari@google.com/
> 
>  net/rds/ib.h    |  4 ++--
>  net/rds/ib_cm.c | 29 +++++++++++++++--------------
>  2 files changed, 17 insertions(+), 16 deletions(-)
> 
> diff --git a/net/rds/ib.h b/net/rds/ib.h
> index 5ff346a1e..190122636 100644
> --- a/net/rds/ib.h
> +++ b/net/rds/ib.h
> @@ -159,8 +159,8 @@ struct rds_ib_connection {
>  	atomic_t		i_fastreg_inuse_count;
>  
>  	/* interrupt handling */
> -	struct tasklet_struct	i_send_tasklet;
> -	struct tasklet_struct	i_recv_tasklet;
> +	struct work_struct	i_send_work;
> +	struct work_struct	i_recv_work;
>  
>  	/* tx */
>  	struct rds_ib_work_ring	i_send_ring;
> diff --git a/net/rds/ib_cm.c b/net/rds/ib_cm.c
> index 6e3110a04..3ebe13d00 100644
> --- a/net/rds/ib_cm.c
> +++ b/net/rds/ib_cm.c
> @@ -253,7 +253,7 @@ static void rds_ib_cq_comp_handler_recv(struct ib_cq *cq, void *context)
>  
>  	rds_ib_stats_inc(s_ib_evt_handler_call);
>  
> -	tasklet_schedule(&ic->i_recv_tasklet);
> +	queue_work(system_bh_wq, &ic->i_recv_work);
>  }
>  
>  static void poll_scq(struct rds_ib_connection *ic, struct ib_cq *cq,
> @@ -279,9 +279,9 @@ static void poll_scq(struct rds_ib_connection *ic, struct ib_cq *cq,
>  	}
>  }
>  
> -static void rds_ib_tasklet_fn_send(unsigned long data)
> +static void rds_ib_send_worker(struct work_struct *work)
>  {
> -	struct rds_ib_connection *ic = (struct rds_ib_connection *)data;
> +	struct rds_ib_connection *ic = from_work(ic, work, i_send_work);
>  	struct rds_connection *conn = ic->conn;
>  
>  	rds_ib_stats_inc(s_ib_tasklet_call);
> @@ -319,9 +319,9 @@ static void poll_rcq(struct rds_ib_connection *ic, struct ib_cq *cq,
>  	}
>  }
>  
> -static void rds_ib_tasklet_fn_recv(unsigned long data)
> +static void rds_ib_recv_worker(struct work_struct *work)
>  {
> -	struct rds_ib_connection *ic = (struct rds_ib_connection *)data;
> +	struct rds_ib_connection *ic = from_work(ic, work, i_recv_work);
>  	struct rds_connection *conn = ic->conn;
>  	struct rds_ib_device *rds_ibdev = ic->rds_ibdev;
>  	struct rds_ib_ack_state state;
> @@ -381,7 +381,7 @@ static void rds_ib_cq_comp_handler_send(struct ib_cq *cq, void *context)
>  
>  	rds_ib_stats_inc(s_ib_evt_handler_call);
>  
> -	tasklet_schedule(&ic->i_send_tasklet);
> +	queue_work(system_bh_wq, &ic->i_send_work);
>  }
>  
>  static inline int ibdev_get_unused_vector(struct rds_ib_device *rds_ibdev)
> @@ -1099,12 +1099,12 @@ void rds_ib_conn_path_shutdown(struct rds_conn_path *cp)
>  		while (!wait_event_timeout(rds_ib_ring_empty_wait,
>  					   rds_ib_conn_path_shutdown_check_wait(cp) == 0,
>  					   msecs_to_jiffies(1000))) {
> -			tasklet_schedule(&ic->i_send_tasklet);
> -			tasklet_schedule(&ic->i_recv_tasklet);
> +			queue_work(system_bh_wq, &ic->i_send_work);
> +			queue_work(system_bh_wq, &ic->i_recv_work);
>  		}
>  
> -		tasklet_kill(&ic->i_send_tasklet);
> -		tasklet_kill(&ic->i_recv_tasklet);
> +		disable_work_sync(&ic->i_send_work);
> +		disable_work_sync(&ic->i_recv_work);
>  
>  		atomic_set(&ic->i_cq_quiesce, 1);
>  
> @@ -1123,6 +1123,9 @@ void rds_ib_conn_path_shutdown(struct rds_conn_path *cp)
>  			ib_destroy_cq(ic->i_recv_cq);
>  		}
>  
> +		enable_work(&ic->i_send_work);
> +		enable_work(&ic->i_recv_work);
> +
>  		if (ic->rds_ibdev) {
>  			/* then free the resources that ib callbacks use */
>  			if (ic->i_send_hdrs) {
> @@ -1234,10 +1237,8 @@ int rds_ib_conn_alloc(struct rds_connection *conn, gfp_t gfp)
>  	}
>  
>  	INIT_LIST_HEAD(&ic->ib_node);
> -	tasklet_init(&ic->i_send_tasklet, rds_ib_tasklet_fn_send,
> -		     (unsigned long)ic);
> -	tasklet_init(&ic->i_recv_tasklet, rds_ib_tasklet_fn_recv,
> -		     (unsigned long)ic);
> +	INIT_WORK(&ic->i_send_work, rds_ib_send_worker);
> +	INIT_WORK(&ic->i_recv_work, rds_ib_recv_worker);
>  	mutex_init(&ic->i_recv_mutex);
>  #ifndef KERNEL_HAS_ATOMIC64
>  	spin_lock_init(&ic->i_ack_lock);


  parent reply	other threads:[~2026-09-25  3:38 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-24  6:26 [PATCH net-next v2] net/rds: replace tasklets with workqueue Sunny Tiwari
2026-09-24  6:32 ` sashiko-bot
2026-09-25  3:38 ` Allison Henderson [this message]
2026-09-26  1:20 ` patchwork-bot+netdevbpf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0a141f59b37b9055764c97dddfdf9506fc4d7fa0.camel@kernel.org \
    --to=achender@kernel.org \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-rdma@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=rds-devel@oss.oracle.com \
    --cc=santosh.shilimkar@oracle.com \
    --cc=sunnytiwari@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.