linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Bob Pearson <rpearsonhpe@gmail.com>
To: Bart Van Assche <bvanassche@acm.org>,
	Leon Romanovsky <leon@kernel.org>,
	zyjzyj2000@gmail.com, jgg@ziepe.ca, linux-rdma@vger.kernel.org,
	matsuda-daisuke@fujitsu.com, shinichiro.kawasaki@wdc.com,
	linux-scsi@vger.kernel.org, Zhu Yanjun <yanjun.zhu@intel.com>
Cc: Zhu Yanjun <yanjun.zhu@linux.dev>
Subject: Re: [PATCH 1/1] Revert "RDMA/rxe: Add workqueue support for rxe tasks"
Date: Wed, 27 Sep 2023 11:36:04 -0500	[thread overview]
Message-ID: <6fe0a64c-704f-6c8b-6a24-d725e7629d3a@gmail.com> (raw)
In-Reply-To: <2d5e02d7-cf84-4170-b1a3-a65316ac84ee@acm.org>

On 9/26/23 15:24, Bart Van Assche wrote:
> On 9/26/23 11:34, Bob Pearson wrote:
>> I am working to try to reproduce the KASAN warning. Unfortunately,
>> so far I am not able to see it in Ubuntu + Linus' kernel (as you described) on metal. The config file is different but copies the CONFIG_KASAN_xxx exactly as yours. With KASAN enabled it hangs on every iteration of srp/002 but without a KASAN warning. I am now building an openSuSE VM for qemu and will see if that causes the warning.
> 
> Hi Bob,
> 
> Did you try to understand the report that I shared? 

Looking at the three stack traces from KASAN (alloc, free, and use after free)
it appears that there was an ack packet (skb) created in rxe_responder (normal) and
then passed to rxe_completer which apparently successfully processed it and then
freed the skb (also normal). Then the same skb is enqueued on the response queue
in rxe_comp_queue_pkt(). This is very strange and hard to understand. The only way
the original packet could have been 'completed' would be for it to have been first
enqueued on qp->resp_pkts by skb_queue_tail() and then dequeued after the completer
task runs by skb_dequeue(). The skb queue routines are protected by an irqsave spinlock
so they should operate atomically. In other words the completer can't get the skb until
skb_queue_tail() is finished touching the skb. So it looks like the first pass through
rxe_comp_queue_pkt() shouldn't be to blame. There is no way I can imagine that the
packet could be queued twice on the local loopback path. One strange thing in the
trace is a "? rxe_recv_mcast_pkt" which seems unlikely to be true as all the packets
are rc and hence not mcast. Not sure how to interpret this. Perhaps the stack is
corrupted from scribbles which might cause the above impossibility.

My conclusion from
> the report is that when using tasklets rxe_completer() only runs after
> rxe_requester() has finished and also that when using work queues that
> rxe_completer() may run concurrently with rxe_requester(). 

The completer task was always intended to run in parallel with the requester
and responder tasks whether they are tasklets or workqueue items. Tasklets
tend to run sequentially but there is no reason whey they can't run in
parallel. The completer task is triggered by response packets from another
process's queue pair which is asynchronous from the requester task which
generated the request packets.

For unrelated reasons I am planning to merge the requester task and completer
task into a single task because in high scale situation with lots of qps
it performs better and allows removing some of the locking between them.

This patch
> seems to fix all issues that I ran into with the rdma_rxe workqueue
> patch (I have not tried to verify the performance implications of this
> patch):
> 
> diff --git a/drivers/infiniband/sw/rxe/rxe_task.c b/drivers/infiniband/sw/rxe/rxe_task.c
> index 1501120d4f52..6cd5d5a7a316 100644
> --- a/drivers/infiniband/sw/rxe/rxe_task.c
> +++ b/drivers/infiniband/sw/rxe/rxe_task.c
> @@ -10,7 +10,7 @@ static struct workqueue_struct *rxe_wq;
> 
>  int rxe_alloc_wq(void)
>  {
> -       rxe_wq = alloc_workqueue("rxe_wq", WQ_UNBOUND, WQ_MAX_ACTIVE);
> +       rxe_wq = alloc_workqueue("rxe_wq", WQ_UNBOUND, 1);
>         if (!rxe_wq)
>                 return -ENOMEM;
> 
> Thanks,
> 
> Bart.


  parent reply	other threads:[~2023-09-27 16:36 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-22 16:32 [PATCH 1/1] Revert "RDMA/rxe: Add workqueue support for rxe tasks" Zhu Yanjun
2023-09-22 16:42 ` Bart Van Assche
2023-09-26  9:43   ` Leon Romanovsky
2023-09-26  9:43 ` Leon Romanovsky
2023-09-26 14:06   ` Leon Romanovsky
2023-09-26 17:05     ` Bart Van Assche
2023-09-26 18:34       ` Bob Pearson
2023-09-26 20:24         ` Bart Van Assche
2023-09-27  0:08           ` Rain River
2023-09-27 16:36           ` Bob Pearson [this message]
2023-09-27 16:51           ` Bob Pearson
2023-10-01  6:30             ` Leon Romanovsky
2023-10-04 17:44               ` Bart Van Assche
2023-10-04  3:41           ` Zhu Yanjun
2023-10-04 17:43             ` Bart Van Assche
2023-10-04 18:38               ` Jason Gunthorpe
2023-10-05  9:25                 ` Zhu Yanjun
2023-10-05 14:21                   ` Jason Gunthorpe
2023-10-05 14:50                     ` Bart Van Assche
2023-10-05 15:56                       ` Jason Gunthorpe
2023-10-06 15:58                         ` Bob Pearson
2023-10-07  0:35                           ` Zhu Yanjun
2023-10-08 16:01                       ` Zhu Yanjun
2023-10-08 17:09                         ` Leon Romanovsky
2023-10-10  4:53                         ` Daisuke Matsuda (Fujitsu)
2023-10-10 16:09                           ` Jason Gunthorpe
2023-10-10 21:29                             ` Bart Van Assche
2023-10-11 15:51                               ` Jason Gunthorpe
2023-10-11 20:14                                 ` Bart Van Assche
2023-10-11 23:12                                   ` Jason Gunthorpe
2023-10-12 11:49                                     ` Zhu Yanjun
2023-10-12 15:38                                       ` Bob Pearson

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=6fe0a64c-704f-6c8b-6a24-d725e7629d3a@gmail.com \
    --to=rpearsonhpe@gmail.com \
    --cc=bvanassche@acm.org \
    --cc=jgg@ziepe.ca \
    --cc=leon@kernel.org \
    --cc=linux-rdma@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=matsuda-daisuke@fujitsu.com \
    --cc=shinichiro.kawasaki@wdc.com \
    --cc=yanjun.zhu@intel.com \
    --cc=yanjun.zhu@linux.dev \
    --cc=zyjzyj2000@gmail.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).