From: Logan Gunthorpe <logang@deltatee.com>
To: Serge Semin <fancer.lancer@gmail.com>, Jon Mason <jdmason@kudzu.us>
Cc: Dave Jiang <dave.jiang@intel.com>,
"Hubbe, Allen" <Allen.Hubbe@emc.com>,
"S-k, Shyam-sundar" <Shyam-sundar.S-k@amd.com>,
"Yu, Xiangliang" <Xiangliang.Yu@amd.com>,
Gary R Hook <gary.hook@amd.com>,
Sergey.Semin@t-platforms.ru,
linux-ntb <linux-ntb@googlegroups.com>,
linux-kernel <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 01/15] NTB: Rename NTB messaging API methods
Date: Tue, 5 Dec 2017 12:11:48 -0700 [thread overview]
Message-ID: <c86120cc-6144-f648-27b9-5865970449e4@deltatee.com> (raw)
In-Reply-To: <20171205173154.GB1701@mobilestation>
On 05/12/17 10:31 AM, Serge Semin wrote:
>>> -static int idt_ntb_msg_read(struct ntb_dev *ntb, int midx, int *pidx, u32 *msg)
>>> +static u32 idt_ntb_msg_read(struct ntb_dev *ntb, int *pidx, int midx)
>>> {
>>> struct idt_ntb_dev *ndev = to_ndev_ntb(ntb);
>>>
>>> if (midx < 0 || IDT_MSG_CNT <= midx)
>>> - return -EINVAL;
>>> + return (u32)-1;
>>
>> Please don't do this. If this is an error return standard error
>> number. And why are we casting to an unsigned int now?
>>
>
> We discussed these changes on the v1 series. Additionally I asked similar
> question sometime ago even before the patchset was introduced.
> This patch is made to provide the Message interface similar to the Scratchpad
> one. I didn't introduce anything new or unjustified. As you can see the
> spad_read/peer_spad_read methods return u32 type too. As well as the
> Intel/AMD callbacks. The functions are intentionally made to return FFs
> in case if some of the passed arguments get out from the allowed limits.
> In such circumstances the return value emulates a situation like if user would
> reference an invalid PCIe MMIO address. Since the 32-bits register can in general
> have any value including -errno ones, then returning an error within the NTB API
> would be incorrect. I remember Allen described it this way.
> Nobody argued about it last time. If you think it's incorrect, then it should be
> changed in both Scratchpad and Message register interfaces.
I agree with Jon to not make this change. The original interface is
better. Making the interface similar to spad_read() has no value
especially seeing it makes it less correct. As you mention, *msg can be
any value (even -1) so this restricts the values possible message values
(making future potential hidden bugs) and removes error information
(making debugging more difficult).
I haven't really looked into it but, if anything, it would make sense to
make the spad_read function more like this one.
Logan
next prev parent reply other threads:[~2017-12-05 19:12 UTC|newest]
Thread overview: 88+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-12-03 19:17 [PATCH v2 00/15] NTB: Add full multi-port API support to the test drivers Serge Semin
2017-12-03 19:17 ` [PATCH v2 01/15] NTB: Rename NTB messaging API methods Serge Semin
2017-12-05 16:49 ` Jon Mason
2017-12-05 17:31 ` Serge Semin
2017-12-05 18:07 ` Jon Mason
2017-12-05 20:54 ` Serge Semin
2017-12-05 21:48 ` Logan Gunthorpe
2017-12-05 19:11 ` Logan Gunthorpe [this message]
2017-12-03 19:17 ` [PATCH v2 02/15] NTB: Set dma mask and dma coherent mask to NTB devices Serge Semin
2017-12-05 16:51 ` Jon Mason
2017-12-05 17:21 ` Logan Gunthorpe
2017-12-05 17:48 ` Serge Semin
2017-12-03 19:17 ` [PATCH v2 03/15] NTB: Fix UB/bug in ntb_mw_get_align() Serge Semin
2017-12-05 16:52 ` Jon Mason
2017-12-05 17:01 ` Logan Gunthorpe
2017-12-05 17:08 ` Jon Mason
2017-12-05 17:56 ` Serge Semin
2017-12-05 18:08 ` Jon Mason
2017-12-05 20:57 ` Serge Semin
2017-12-03 19:17 ` [PATCH v2 04/15] NTB: ntb_pp: Add full multi-port NTB API support Serge Semin
2017-12-05 17:02 ` Jon Mason
2017-12-05 19:53 ` Serge Semin
2017-12-05 21:50 ` Logan Gunthorpe
2017-12-03 19:17 ` [PATCH v2 05/15] NTB: ntb_tool: " Serge Semin
2017-12-05 18:03 ` Jon Mason
2017-12-05 19:16 ` Logan Gunthorpe
2017-12-05 20:01 ` Serge Semin
2017-12-03 19:17 ` [PATCH v2 06/15] NTB: ntb_perf: " Serge Semin
2017-12-05 18:21 ` Jon Mason
2017-12-05 20:29 ` Serge Semin
2017-12-06 1:17 ` kbuild test robot
2017-12-03 19:17 ` [PATCH v2 07/15] NTB: ntb_test: Safely use paths with whitespace Serge Semin
2017-12-05 18:24 ` Jon Mason
2017-12-03 19:17 ` [PATCH v2 08/15] NTB: ntb_test: Add ntb_tool port tests Serge Semin
2017-12-03 19:17 ` [PATCH v2 09/15] NTB: ntb_test: Update ntb_tool link tests Serge Semin
2017-12-05 18:26 ` Jon Mason
2017-12-05 20:37 ` Serge Semin
2017-12-03 19:17 ` [PATCH v2 10/15] NTB: ntb_test: Update ntb_tool DB tests Serge Semin
2017-12-05 18:27 ` Jon Mason
2017-12-05 19:38 ` Logan Gunthorpe
2017-12-05 20:42 ` Serge Semin
2017-12-03 19:17 ` [PATCH v2 11/15] NTB: ntb_test: Update ntb_tool Scratchpad tests Serge Semin
2017-12-03 19:17 ` [PATCH v2 12/15] NTB: ntb_test: Add ntb_tool Message tests Serge Semin
2017-12-03 19:17 ` [PATCH v2 13/15] NTB: ntb_test: Update ntb_tool MW tests Serge Semin
2017-12-03 19:17 ` [PATCH v2 14/15] NTB: ntb_test: Update ntb_perf tests Serge Semin
2017-12-03 19:17 ` [PATCH v2 15/15] NTB: ntb_hw_idt: Set NTB_TOPO_SWITCH topology Serge Semin
2017-12-04 4:54 ` [PATCH v2 00/15] NTB: Add full multi-port API support to the test drivers Logan Gunthorpe
2017-12-04 6:12 ` Serge Semin
2017-12-04 20:10 ` Allen Hubbe
2017-12-05 15:54 ` Serge Semin
2017-12-05 16:57 ` Logan Gunthorpe
2017-12-05 22:39 ` [PATCH v3 " Serge Semin
2017-12-05 22:39 ` [PATCH v3 01/15] NTB: Rename NTB messaging API methods Serge Semin
2017-12-05 23:36 ` Logan Gunthorpe
2017-12-05 22:39 ` [PATCH v3 02/15] NTB: Set dma mask and dma coherent mask to NTB devices Serge Semin
2017-12-05 23:39 ` Logan Gunthorpe
2017-12-05 22:39 ` [PATCH v3 03/15] NTB: Fix UB/bug in ntb_mw_get_align() Serge Semin
2017-12-05 23:38 ` Logan Gunthorpe
2017-12-05 22:39 ` [PATCH v3 04/15] NTB: ntb_pp: Add full multi-port NTB API support Serge Semin
2017-12-05 22:39 ` [PATCH v3 05/15] NTB: ntb_tool: " Serge Semin
2017-12-05 22:39 ` [PATCH v3 06/15] NTB: ntb_perf: " Serge Semin
2017-12-05 22:39 ` [PATCH v3 07/15] NTB: ntb_test: Safely use paths with whitespace Serge Semin
2017-12-05 22:39 ` [PATCH v3 08/15] NTB: ntb_test: Add ntb_tool port tests Serge Semin
2017-12-05 22:39 ` [PATCH v3 09/15] NTB: ntb_test: Update ntb_tool link tests Serge Semin
2017-12-05 22:39 ` [PATCH v3 10/15] NTB: ntb_test: Update ntb_tool DB tests Serge Semin
2017-12-05 22:39 ` [PATCH v3 11/15] NTB: ntb_test: Update ntb_tool Scratchpad tests Serge Semin
2017-12-05 22:39 ` [PATCH v3 12/15] NTB: ntb_test: Add ntb_tool Message tests Serge Semin
2017-12-05 22:39 ` [PATCH v3 13/15] NTB: ntb_test: Update ntb_tool MW tests Serge Semin
2017-12-05 22:39 ` [PATCH v3 14/15] NTB: ntb_test: Update ntb_perf tests Serge Semin
2017-12-05 22:39 ` [PATCH v3 15/15] NTB: ntb_hw_idt: Set NTB_TOPO_SWITCH topology Serge Semin
2017-12-05 23:36 ` [PATCH v3 00/15] NTB: Add full multi-port API support to the test drivers Logan Gunthorpe
2017-12-06 14:31 ` [PATCH v4 " Serge Semin
2017-12-06 14:31 ` [PATCH v4 01/15] NTB: Rename NTB messaging API methods Serge Semin
2017-12-06 14:31 ` [PATCH v4 02/15] NTB: Set dma mask and dma coherent mask to NTB devices Serge Semin
2017-12-06 14:31 ` [PATCH v4 03/15] NTB: Fix UB/bug in ntb_mw_get_align() Serge Semin
2017-12-06 14:31 ` [PATCH v4 04/15] NTB: ntb_pp: Add full multi-port NTB API support Serge Semin
2017-12-06 14:31 ` [PATCH v4 05/15] NTB: ntb_tool: " Serge Semin
2017-12-06 14:31 ` [PATCH v4 06/15] NTB: ntb_perf: " Serge Semin
2017-12-06 14:31 ` [PATCH v4 07/15] NTB: ntb_test: Safely use paths with whitespace Serge Semin
2017-12-06 14:31 ` [PATCH v4 08/15] NTB: ntb_test: Add ntb_tool port tests Serge Semin
2017-12-06 14:32 ` [PATCH v4 09/15] NTB: ntb_test: Update ntb_tool link tests Serge Semin
2017-12-06 14:32 ` [PATCH v4 10/15] NTB: ntb_test: Update ntb_tool DB tests Serge Semin
2017-12-06 14:32 ` [PATCH v4 11/15] NTB: ntb_test: Update ntb_tool Scratchpad tests Serge Semin
2017-12-06 14:32 ` [PATCH v4 12/15] NTB: ntb_test: Add ntb_tool Message tests Serge Semin
2017-12-06 14:32 ` [PATCH v4 13/15] NTB: ntb_test: Update ntb_tool MW tests Serge Semin
2017-12-06 14:32 ` [PATCH v4 14/15] NTB: ntb_test: Update ntb_perf tests Serge Semin
2017-12-06 14:32 ` [PATCH v4 15/15] NTB: ntb_hw_idt: Set NTB_TOPO_SWITCH topology Serge Semin
2018-01-18 22:28 ` [PATCH v4 00/15] NTB: Add full multi-port API support to the test drivers Jon Mason
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=c86120cc-6144-f648-27b9-5865970449e4@deltatee.com \
--to=logang@deltatee.com \
--cc=Allen.Hubbe@emc.com \
--cc=Sergey.Semin@t-platforms.ru \
--cc=Shyam-sundar.S-k@amd.com \
--cc=Xiangliang.Yu@amd.com \
--cc=dave.jiang@intel.com \
--cc=fancer.lancer@gmail.com \
--cc=gary.hook@amd.com \
--cc=jdmason@kudzu.us \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-ntb@googlegroups.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).