Skip to content

Enhance force down link feature.#37

Merged
wesleywesley merged 1 commit intodevelfrom
enhance_force_linkdown
Nov 12, 2018
Merged

Enhance force down link feature.#37
wesleywesley merged 1 commit intodevelfrom
enhance_force_linkdown

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Sep 28, 2018

When a host is not a surviving host, but it receives the link force down massage, it should skip the link force down massage. To reinitialize the shared memory window, the driver just need to reinitialize the reserved LUT window.

@ghost ghost requested review from kelvin-cao, lsgunth and wesleywesley September 28, 2018 07:38
@ghost
Copy link
Copy Markdown
Author

ghost commented Sep 28, 2018

To use the force down link feature, we should add following patch on ntb_transport.c .

diff --git a/ntb_transport.c b/ntb_transport.c
index 9878c48..8a8638d 100644
--- a/ntb_transport.c
+++ b/ntb_transport.c
@@ -700,10 +700,6 @@ static int ntb_set_mw(struct ntb_transport_ctx *nt, int num_mw,
xlat_size = round_up(size, xlat_align_size);
buff_size = round_up(size, xlat_align);

- /* No need to re-setup */
- if (mw->xlat_size == xlat_size)
- return 0;
-
if (mw->buff_size)
ntb_free_mw(nt, num_mw);

Thanks
Joey

@ghost ghost changed the title Enhance force link down feature. Enhance force down link feature. Sep 28, 2018
@wesleywesley
Copy link
Copy Markdown
Contributor

wesleywesley commented Sep 28, 2018

it is better to rework the commit message
https://www.kernel.org/doc/html/v4.16/process/submitting-patches.html#describe-your-changes
a good commit message not only make people easily understood the patch, but also smoothing the process of upstream.

Comment thread ntb_hw_switchtec.c Outdated
schedule_work(&sndev->link_reinit_work);

if (sndev->link_is_up) {
schedule_work(&sndev->link_reinit_work);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think I like this change. If a peer dies and comes back when the link is down, then we won't reinitialize the shared memory window as we should....

Otherwise this looks good.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Logan,
Thanks for the comment. I have canceled this change.

In our test, we used a shell script to load the NTB and Switchtec drivers. After using the force down link, we need to add a delay between loading ntb_hw_switchtec and ntb_transport module, otherwise the NT I/O test would be fail. is it ok?

Function switchtec_ntb_reinit_peer() is used to reinitialize the shared NTB
memory window. it doesn't need to free and reallocate the memory. We can
implement it by reconfiguring the reseved LUT window.

Signed-off-by: Joey Zhang <joey.zhang@microchip.com>
@ghost ghost force-pushed the enhance_force_linkdown branch from d151c8e to e6537c7 Compare October 18, 2018 04:04
@ghost
Copy link
Copy Markdown
Author

ghost commented Oct 18, 2018

@wesleywesley I have updated the commit message. Please help to review it.

@wesleywesley
Copy link
Copy Markdown
Contributor

@JoeyZhang-Microsemi

  1. reconfigring --> reconfiguring

  2. change to microchip account

  3. . pls run checkpatch.pl for this patch and fix the error and warnings if any, before pull request for others review

test@server1:~/switchtec-kernel$ ~/kernel/linux-4.17.6/scripts/checkpatch.pl 0001-ntb_hw_switchtec-Optimize-function-switchtec_ntb_rei.patch
WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#7:
Function switchtec_ntb_reinit_peer() is used to reinitialize the shared NTB memory window.

WARNING: Missing a blank line after declarations
#44: FILE: ntb_hw_switchtec.c:1481:

  •   int rc;
    
  •   dev_info(&sndev->stdev->dev, "reinitialize shared memory window\n");
    

ERROR: Missing Signed-off-by: line(s)

total: 1 errors, 2 warnings, 32 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.

0001-ntb_hw_switchtec-Optimize-function-switchtec_ntb_rei.patch has style problems, please review.

NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.

@ghost ghost force-pushed the enhance_force_linkdown branch from e6537c7 to 68cee85 Compare October 18, 2018 06:24
@ghost
Copy link
Copy Markdown
Author

ghost commented Oct 18, 2018

@wesleywesley Thanks for the review. I have fixed issues.

Copy link
Copy Markdown
Collaborator

@lsgunth lsgunth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@wesleywesley wesleywesley merged commit 8164c56 into devel Nov 12, 2018
@wesleywesley wesleywesley deleted the enhance_force_linkdown branch December 5, 2018 02:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants