netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "dust.li" <dust.li@linux.alibaba.com>
To: Karsten Graul <kgraul@linux.ibm.com>,
	"David S . Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>
Cc: linux-s390@vger.kernel.org, netdev@vger.kernel.org,
	Wen Gu <guwen@linux.alibaba.com>,
	Tony Lu <tonylu@linux.alibaba.com>
Subject: Re: [PATCH net 1/2] net/smc: don't send CDC/LLC message if link not ready
Date: Thu, 30 Dec 2021 11:02:26 +0800	[thread overview]
Message-ID: <20211230030226.GA55356@linux.alibaba.com> (raw)
In-Reply-To: <2b3dd919-029c-cd44-b39c-5467bb723c0f@linux.ibm.com>

On Wed, Dec 29, 2021 at 01:36:06PM +0100, Karsten Graul wrote:
>On 28/12/2021 10:03, Dust Li wrote:
>> We found smc_llc_send_link_delete_all() sometimes wait
>> for 2s timeout when testing with RDMA link up/down.
>> It is possible when a smc_link is in ACTIVATING state,
>> the underlaying QP is still in RESET or RTR state, which
>> cannot send any messages out.
>
>I see your point, but why do you needed to introduce a new wrapper instead of
>extending the existing smc_link_usable() wrapper?
>With that and without any comments the reader of the code does not know why there are
>2 different functions and what is the reason for having two of them.

Sorry for that, I should add some comments for those wrappers to
better explain that.

The reason we need two wrappers here is because the QP state for the
client side is not turned into RTS directly, but seperated into two
stages. In the middle on RTR to RTS, we still need smc_link_usable().

For example:
For the client side in first contact, the QP is still in RTR before it
receives the LLC_CONFIRM message. So for functions like smc_llc_wait()
or smc_llc_event_handler(), we can't use smc_link_sendable(), or we
can't even receive LLC_CONFIRM message anymore.

So the meaning for those 2 wrappers should be:
smc_link_usable():   the link is ready to receive RDMA messages
smc_link_sendable(): the link is ready to send RDMA messages

For those places that need to send any CDC or LLC message,
should go for smc_link_sendable(), otherwise, use smc_link_usable()
instead.

I saw David has already applied this to net, should I send another
patch to add some comments ?


  reply	other threads:[~2021-12-30  3:02 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-28  9:03 [PATCH net 0/2] net/smc: fix kernel panic caused by race of smc_sock Dust Li
2021-12-28  9:03 ` [PATCH net 1/2] net/smc: don't send CDC/LLC message if link not ready Dust Li
2021-12-29 12:36   ` Karsten Graul
2021-12-30  3:02     ` dust.li [this message]
2021-12-30 18:55       ` Karsten Graul
2021-12-31  3:15         ` dust.li
2022-01-03 10:40           ` Karsten Graul
2021-12-31  6:08   ` [PATCH net] net/smc: add comments for smc_link_{usable|sendable} Dust Li
2022-01-02 16:20     ` patchwork-bot+netdevbpf
2021-12-28  9:03 ` [PATCH net 2/2] net/smc: fix kernel panic caused by race of smc_sock Dust Li
2021-12-29 12:33   ` Karsten Graul
2021-12-30  3:46     ` dust.li
2021-12-28 12:50 ` [PATCH net 0/2] " patchwork-bot+netdevbpf

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=20211230030226.GA55356@linux.alibaba.com \
    --to=dust.li@linux.alibaba.com \
    --cc=davem@davemloft.net \
    --cc=guwen@linux.alibaba.com \
    --cc=kgraul@linux.ibm.com \
    --cc=kuba@kernel.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=tonylu@linux.alibaba.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).