public inbox for linux-rdma@vger.kernel.org
 help / color / mirror / Atom feed
From: Sagi Grimberg <sagig-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
To: Roland Dreier <roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Cc: "Nicholas A. Bellinger"
	<nab-IzHhD5pYlfBP7FQvKIMDCQ@public.gmane.org>,
	Oren Duer <oren-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>,
	"linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Subject: Re: [PATCH v5 00/10] Introduce Signature feature
Date: Sun, 09 Mar 2014 12:57:27 +0200	[thread overview]
Message-ID: <531C4917.3010604@dev.mellanox.co.il> (raw)
In-Reply-To: <CAL1RGDXG2yU5mVi=e4A2NFyFo33Um=y03u7yyXL7GZ-J-y3KRQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On 3/7/2014 9:43 PM, Roland Dreier wrote:
> So I went ahead and applied this for 3.15,

Hey Roland,

Thanks for taking the time to take a look and pick this up.

> although I suspect the
> verbs API is probably the wrong one.

I will be more than happy to here your concerns, share our approach and 
fix what is needed.

>    I understand that the mlx5
> microarchitecture requires some of this signature binding stuff to go
> through a work queue, but conceptually I don't think the
> IB_WR_REG_SIG_MR work request makes sense as the right interface.  Why
> do I need to do both a synchronous create_mr and an asynchronous work
> request for what's conceptually a single operation?

Hmm, well create_mr and REG_SIG_MR work request are not a single operation
even conceptually. create_mr is a memory key allocation while REG_SIG_MR 
is the
actual (fast) memory registration operation.
* create_mr is the equivalent of ib_alloc_fast_reg_mr and other memory 
key allocation
    routines. Note that we proposed a general routine that can 
replace/unite all memory
    allocations routines in the future.
* REG_SIG_MR is the equivalent of fastreg work request - the actual 
registration.
    It will be taken in the fast-path upon each transaction.

Note that for each data-transfer the user (iSER/SRP) will fast register 
signature MR
as this is a fast path operation and that's why it is a work request. 
REG_SIG_MR may
be seen as fastreg extension. Moreover it would be useful for the user to
pipeline WRs by using a post-list of REG_SIG_MR+RDMA and save a HW doorbell.

> Similarly the ib_check_mr_status() operation doesn't really make sense
> to me as an additional synchronous operation -- why do I not just get
> signature information as part of the completion entry for the
> transaction that generated it?

We actually thought about it more then once. But we chose this way due 
to a couple of reasons:
- Providing the data-integrity status in the completion introduces an 
asymmetric verbs behavior for the
   target and initiator. While the target is the RDMA requester posting 
RDMA operation, it may get the
   data-integrity info in the work completion of the RDMA work request. 
But the initiator can only get the
   data-integrity information in the receive completion of the SCSI 
response as this is the only indication
   that the transaction completed. As you know post recvs are not 
correlated with transactions be any means.

-  Data-integrity does not really belong to the transport completions, 
it belongs to the data itself.
    Meaning RDMA may have completed successfully but the data is 
corrupted. We aimed to keep this
    layering as most applications will handle each differently and may 
wish to explore each error in a different
    stage (for example iSER needs to construct a specific sense for 
data-integrity errors and needs to have the
    iSCSI task at reach, which is provided in a different processing stage).

So we chose to go with a simple lightweight routine that is easy to 
handle and will keep a symmetrical
implementation on all ends. In case this can be improved, I'm open to 
other ideas.

Sagi.
--
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

      parent reply	other threads:[~2014-03-09 10:57 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-02-23 12:19 [PATCH v5 00/10] Introduce Signature feature Sagi Grimberg
     [not found] ` <1393157953-24834-1-git-send-email-sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
2014-02-23 12:19   ` [PATCH v5 01/10] IB/core: Introduce protected memory regions Sagi Grimberg
2014-02-23 12:19   ` [PATCH v5 02/10] IB/core: Introduce Signature Verbs API Sagi Grimberg
2014-02-23 12:19   ` [PATCH v5 03/10] IB/mlx5, mlx5_core: Support for create_mr and destroy_mr Sagi Grimberg
2014-02-23 12:19   ` [PATCH v5 04/10] IB/mlx5: Initialize mlx5_ib_qp signature related Sagi Grimberg
     [not found]     ` <1393157953-24834-5-git-send-email-sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
2014-02-24  6:46       ` Nicholas A. Bellinger
     [not found]         ` <1393224376.22656.13.camel-XoQW25Eq2zviZyQQd+hFbcojREIfoBdhmpATvIKMPHk@public.gmane.org>
2014-02-24  8:10           ` Sagi Grimberg
2014-02-23 12:19   ` [PATCH v5 05/10] IB/mlx5: Break wqe handling to begin & finish routines Sagi Grimberg
2014-02-23 12:19   ` [PATCH v5 06/10] IB/mlx5: remove MTT access mode from umr flags helper function Sagi Grimberg
2014-02-23 12:19   ` [PATCH v5 07/10] IB/mlx5: Keep mlx5 MRs in a radix tree under device Sagi Grimberg
2014-02-23 12:19   ` [PATCH v5 08/10] IB/mlx5: Support IB_WR_REG_SIG_MR Sagi Grimberg
2014-02-23 12:19   ` [PATCH v5 09/10] IB/mlx5: Collect signature error completion Sagi Grimberg
2014-02-23 12:19   ` [PATCH v5 10/10] IB/mlx5: Publish support in signature feature Sagi Grimberg
2014-02-24  7:01   ` [PATCH v5 00/10] Introduce Signature feature Nicholas A. Bellinger
     [not found]     ` <1393225299.22656.22.camel-XoQW25Eq2zviZyQQd+hFbcojREIfoBdhmpATvIKMPHk@public.gmane.org>
2014-02-24  8:11       ` Sagi Grimberg
2014-03-07 19:43   ` Roland Dreier
     [not found]     ` <CAL1RGDXG2yU5mVi=e4A2NFyFo33Um=y03u7yyXL7GZ-J-y3KRQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-03-09 10:57       ` Sagi Grimberg [this message]

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=531C4917.3010604@dev.mellanox.co.il \
    --to=sagig-ldsdmyg8hgv8yrgs2mwiifqbs+8scbdb@public.gmane.org \
    --cc=linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=nab-IzHhD5pYlfBP7FQvKIMDCQ@public.gmane.org \
    --cc=oren-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org \
    --cc=roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org \
    /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