linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Bob Pearson <rpearsonhpe@gmail.com>
To: Jason Gunthorpe <jgg@ziepe.ca>, Bart Van Assche <bvanassche@acm.org>
Cc: Zhu Yanjun <yanjun.zhu@linux.dev>,
	Leon Romanovsky <leon@kernel.org>,
	zyjzyj2000@gmail.com, linux-rdma@vger.kernel.org,
	matsuda-daisuke@fujitsu.com, shinichiro.kawasaki@wdc.com,
	linux-scsi@vger.kernel.org, Zhu Yanjun <yanjun.zhu@intel.com>
Subject: Re: [PATCH 1/1] Revert "RDMA/rxe: Add workqueue support for rxe tasks"
Date: Fri, 6 Oct 2023 10:58:26 -0500	[thread overview]
Message-ID: <93c8ad67-f008-4352-8887-099723c2f4ec@gmail.com> (raw)
In-Reply-To: <20231005155616.GR13795@ziepe.ca>

On 10/5/23 10:56, Jason Gunthorpe wrote:
> On Thu, Oct 05, 2023 at 07:50:28AM -0700, Bart Van Assche wrote:
>> On 10/5/23 07:21, Jason Gunthorpe wrote:
>>> Which is why it shows there are locking problems in this code.
>>
>> Hi Jason,
>>
>> Since the locking problems have not yet been root-caused, do you
>> agree that it is safer to revert patch "RDMA/rxe: Add workqueue
>> support for rxe tasks" rather than trying to fix it?
> 
> I don't think that makes the locking problems go away any more that
> using a single threaded work queue?
> 
> Jason

This is slightly off topic but may still be relevant.
If there are locking bugs they are between the two send side tasks
rxe_completer and rxe_requester which share the send queue and other
state. Bart attempts to fix this by setting max_active to 1 which
limits the ability of these two work queue tasks from interfering.
For completely different reasons we have looked at merging these
two tasks into a single task which it turns out improves performance,
especially in high scale situations where it reduces the number of
cpu cores needed to complete work. But even at low scale (1-2 QPs)
it helps because of improved caching. It turns out that if the work
is mostly sends and writes that there isn't much for the completer
to do while if it is mostly reads there isn't much for the requester
to do. So combining them doesn't hurt performance by having fewer cores
to do the work. But this also prevents the two tasks for a given QP
to run at the same time which should eliminate locking issues.
If no one hates the idea I can send in our patch that does this.

Bob

  reply	other threads:[~2023-10-06 15:58 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
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 [this message]
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=93c8ad67-f008-4352-8887-099723c2f4ec@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).