linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Daisuke Matsuda (Fujitsu)" <matsuda-daisuke@fujitsu.com>
To: 'Zhu Yanjun' <yanjun.zhu@linux.dev>,
	Bart Van Assche <bvanassche@acm.org>,
	Jason Gunthorpe <jgg@ziepe.ca>
Cc: Bob Pearson <rpearsonhpe@gmail.com>,
	Leon Romanovsky <leon@kernel.org>,
	"zyjzyj2000@gmail.com" <zyjzyj2000@gmail.com>,
	"linux-rdma@vger.kernel.org" <linux-rdma@vger.kernel.org>,
	"shinichiro.kawasaki@wdc.com" <shinichiro.kawasaki@wdc.com>,
	"linux-scsi@vger.kernel.org" <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: Tue, 10 Oct 2023 04:53:55 +0000	[thread overview]
Message-ID: <OS3PR01MB9865F9BEB1A90DDCAEEBFC8BE5CDA@OS3PR01MB9865.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <a8453889-3f5f-49ff-89f2-ec0ef929d915@linux.dev>

On Mon, Oct 9, 2023 1:02 AM Zhu Yanjun wrote:
> 在 2023/10/5 22:50, Bart Van Assche 写道:
> > 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?
> 
> Hi, Jason && Leon
> 
> I spent a lot of time on this problem. It seems that it is a very
> difficult problem.
> 
> So I agree with Bart. Can we revert patch "RDMA/rxe: Add workqueue
> support for rxe tasks" rather than trying to fix it? Then Bob can apply
> his new patch to a stable RXE?

Cf. https://lore.kernel.org/lkml/f15b06b934aa0ace8b28dc046022e5507458eb99.1694153251.git.matsuda-daisuke@fujitsu.com/
I have ODP patches that is fully dependent on "RDMA/rxe: Add workqueue
support for rxe tasks". So I personally prefer preserving workqueue to reverting
the workqueue patch.

Each developer here has different motive and interest. I think the rxe driver should
take in new specs and new features actively so that it can be used by developers
without access to HCAs. I believe workqueue is better suited for this purpose.
Additionally, the disadvantages of tasklet are documented as follows:
https://lwn.net/Articles/830964/
However, stability is very important, so I will not insist on my opinion.

I agree it is very difficult to find the root cause of the locking problem. It cannot
be helped that we will somehow hide the issue for now so that it will not bother
actual users of the driver. Perhaps, there are three choices to do this.

Solution 1: Reverting "RDMA/rxe: Add workqueue support for rxe tasks"
I see this is supported by Zhu, Bart and approved by Leon.

Solution 2: Serializing execution of work items
> -       rxe_wq = alloc_workqueue("rxe_wq", WQ_UNBOUND, WQ_MAX_ACTIVE);
> +       rxe_wq = alloc_workqueue("rxe_wq", WQ_HIGHPRI | WQ_UNBOUND, 1);

Solution 3: Merging requester and completer (not yet submitted/tested)
https://lore.kernel.org/all/93c8ad67-f008-4352-8887-099723c2f4ec@gmail.com/
Not clear to me if we should call this a new feature or a fix.
If it can eliminate the hang issue, it could be an ultimate solution.

It is understandable some people do not want to wait for solution 3 to be submitted and verified.
Is there any problem if we adopt solution 2?
If so, then I agree to going with solution 1.
If not, solution 2 is better to me.

Thanks,
Daisuke Matsuda


  parent reply	other threads:[~2023-10-10  4:55 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
2023-10-08 16:01                       ` Zhu Yanjun
2023-10-08 17:09                         ` Leon Romanovsky
2023-10-10  4:53                         ` Daisuke Matsuda (Fujitsu) [this message]
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=OS3PR01MB9865F9BEB1A90DDCAEEBFC8BE5CDA@OS3PR01MB9865.jpnprd01.prod.outlook.com \
    --to=matsuda-daisuke@fujitsu.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=rpearsonhpe@gmail.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).