Re: [PATCH rdma-next] Revert "IB/core: Add flow control to the portmapper netlink calls"

Leon Romanovsky
2017-06-05T06:03:18Z

[-- Attachment #1: Type: text/plain, Size: 4118 bytes --]

On Sun, Jun 04, 2017 at 11:50:43PM -0500, Chien Tin Tung wrote:
> You jump around in this thread so much it is hard for a sane person to follow
> so I will attempt to summarize what has taken place.
>
> You try to revert a patch that fixed a real problem in portmapper, claiming:
>
> 1) Code in question impacts the whole RDMA subsystem.
> which is false by my multiple replies on this.
> 2) There is a deadlock.
> Which is false. I'm still asking for proof.
> 3) you want netlink receive to be non-blocking and asynchronous
> what does that have to do with the non-existing deadlock?
> Asnwer is you can't when there isn't one.
> If you want it, create a patch for it instead of creating a
> regression with a lazy revert.
>
> I will continue this discussion if you answer directly to any of those
> points. If you choose to dance around the subject and claim falsehood, you
> will only damage your own creditibility on the list. I hope you take that
> to heart.

OK, I got your point. It is worthless discussion.

FYI, ibnl_unicast holds global lock for whole NETLINK_RDMA
static void ibnl_rcv(struct sk_buff *skb)
{
mutex_lock(&ibnl_mutex);
ibnl_rcv_reply_skb(skb);
netlink_rcv_skb(skb, &ibnl_rcv_msg);
mutex_unlock(&ibnl_mutex);
}

I'll wait for a maintainer's decision on the proposed patch.

Thanks

>
> Chien
>
> On Sun, Jun 04, 2017 at 11:20:07PM -0500, Chien Tin Tung wrote:
> > Mon, Jun 05, 2017 at 07:00:30AM +0300, Leon Romanovsky wrote:
> > > On Sun, Jun 04, 2017 at 09:23:13PM -0500, Chien Tin Tung wrote:
> > > > Sun, Jun 04, 2017 at 08:36:35AM +0300, Leon Romanovsky wrote:
> > > > > On Fri, Jun 02, 2017 at 11:28:49AM -0500, Shiraz Saleem wrote:
> > > > > > On Wed, May 31, 2017 at 02:10:31PM -0600, Bart Van Assche wrote:
> > > > > > > On Wed, 2017-05-31 at 12:42 -0500, Shiraz Saleem wrote:
> > > > > > > > > 5. I proposed a solution -> go and fix your user space program.
> > > > > > > >
> > > > > > > > This is a kernel patch you are trying to revert, you are breaking existing
> > > > > > > > kernel functionality. Nothing to do with user space.
> > > > > > > >
> > > > > > > > Bottom line, come up with a solution that will address both port mapper
> > > > > > > > functionality and your issue.
> > > > > > >
> > > > > > > Hello Shiraz,
> > > > > > >
> > > > > > > Sorry that this means additional work for you, but I agree with Leon that
> > > > > > > user space software should not assume that netlink sockets are a reliable
> > > > > > > communication mechanism.
> > > > > >
> > > > > > Hi Bart - Thank you for your response.
> > > > > >
> > > > > > The original problem was that ibnl_unicast, which is used to send nl messages from
> > > > > > portmapper kernel space to user-space, would occasionally and momentarily fail under stress.
> > > > > > We could have retried the call for a certain amount of time, but since netlink_unicast has a
> > > > > > nonblock/block parameter, we chose to use the blocking option with a timeout. So we thought we
> > > > > > did account for deadlocks with this timeout.
> > > > >
> > > > > Not really, you just reduced the chances. In very large scale, you will
> > > > > have a very large chances of such deadlocks.
> > > >
> > > > Please stop using the word deadlock until you can prove that the deadlock exists with the timeout
> > > > in place.
> > >
> > > Can you please post the whole list of forbidden words? It will be great to
> > > have it accompanied with technical response to my and Bart's claims, and
> > > to summarize it, it is very simple: "netlink receive should be
> > > non-blocking and asynchronous".
> >
> > Non-blocking and asynchronous is not deadlock, please get it right. Again, provide proof of
> > deadlock, until then refrain from using that word in support of your argument. Which you still
> > have not proved there is a problem.
> >
> > Chien
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]