linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Zhu Yanjun <yanjun.zhu@linux.dev>
To: Bob Pearson <rpearsonhpe@gmail.com>,
	Jason Gunthorpe <jgg@ziepe.ca>,
	Bart Van Assche <bvanassche@acm.org>
Cc: 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: Sat, 7 Oct 2023 08:35:10 +0800	[thread overview]
Message-ID: <ee0b7977-d063-fe58-f185-4f94a1d7a3be@linux.dev> (raw)
In-Reply-To: <93c8ad67-f008-4352-8887-099723c2f4ec@gmail.com>


在 2023/10/6 23:58, Bob Pearson 写道:
> 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.

Hi, Bob

The blktest hang problem is not fixed now. I do not think that it is a 
good time

to introduce this new feature.

IMHO, we should fix this blktest hang problem. And we also need some time to

wait for the test results from the users. If this problem proves fixed. 
Then we have a

stable RXE version for the new feature from you.

In short, we need a stable RXE version for use. Then based on the stable 
RXE version,

new features are welcomed.

Please let me know your advice.

Please Bart comment on this.

Please Jason && Leon comment on this.

Warm Regards,

Zhu Yanjun

>
> Bob

  reply	other threads:[~2023-10-07  0:35 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
2023-10-07  0:35                           ` Zhu Yanjun [this message]
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=ee0b7977-d063-fe58-f185-4f94a1d7a3be@linux.dev \
    --to=yanjun.zhu@linux.dev \
    --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=rpearsonhpe@gmail.com \
    --cc=shinichiro.kawasaki@wdc.com \
    --cc=yanjun.zhu@intel.com \
    --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).