Re: [PATCH 00/23] Update driver to 0.9-294

[Moni Shoua]

> Understood, however, unlike SoftRoCE, qib and hfi currently share a lot of 
> code
> to drive the hardware.
>
> The underlying reason for the TODO item "Remove software processing of IB
> protocol..." is because we have a large amount of duplicated code between 
> these
> drivers.  _Some_ of which, at a high level, is sharable with SoftRoCE.
>
After studying the qib driver (briefly I must admit) I found that it
has a lot in common with SoftRoCE driver and both can be redesigned to
share a fair amount of code.
> These patches (and more to follow), further differentiate the 2 drivers along
> hardware lines.  Accepting these patches will help us make sure that we don't
> create some common code between qib and hfi which, because of our testing we
> now know, needs to be separated out.
>
I'm sorry but I'm not sure that I understand the plan. What we had in
mind is that we start following the idea which was presented by Deny,
build a detailed design plan and implement it. All that can be done
without fixing bugs, adding functionality and improving performance
but I understand that you also need to move forward.
Us, in Mellanox are willing to contribute to the effort and take
responsibility over the Software Verbs Transport.
> This is a separate issue from the higher level code sharing which will be done
> with SoftRoCE.
I'm not sure I agree. Can you explain the plan to remove code sharing
between qib and hfi that is not part of the Software Verbs Transport
module?
>
> Ira
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 00/23] Update driver to 0.9-294

[Moni Shoua]

>
> Perhaps I did not chose my words carefully enough.
>
> The largest issue on the TODO list is the refactoring of the code to be 
> shared between the hfi1 and qib driver.  While the hardware between hfi1 and 
> qib is similar and thus the initial code looked similar, our performance 
> tuning on hfi1 has revealed that some changes will be required to the hfi1 
> code to fully utilize the hardware.  We would like to get these updates in to 
> make sure that the refactoring work is done properly for the 2 hardware types.
Please keep in mind that there are 3 HW types (our SoftRoCE driver)
that needs to be part of the framework.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 00/23] Update driver to 0.9-294

[ira.weiny]

On Tue, Oct 20, 2015 at 03:45:47PM +0300, Moni Shoua wrote:
> >
> > Perhaps I did not chose my words carefully enough.
> >
> > The largest issue on the TODO list is the refactoring of the code to be
> > shared between the hfi1 and qib driver.  While the hardware between hfi1
> > and qib is similar and thus the initial code looked similar, our
> > performance tuning on hfi1 has revealed that some changes will be required
> > to the hfi1 code to fully utilize the hardware.  We would like to get these
> > updates in to make sure that the refactoring work is done properly for the
> > 2 hardware types.
>
> Please keep in mind that there are 3 HW types (our SoftRoCE driver)
> that needs to be part of the framework.

Understood, however, unlike SoftRoCE, qib and hfi currently share a lot of code
to drive the hardware.

The underlying reason for the TODO item "Remove software processing of IB
protocol..." is because we have a large amount of duplicated code between these
drivers.  _Some_ of which, at a high level, is sharable with SoftRoCE.

These patches (and more to follow), further differentiate the 2 drivers along
hardware lines.  Accepting these patches will help us make sure that we don't
create some common code between qib and hfi which, because of our testing we
now know, needs to be separated out.

This is a separate issue from the higher level code sharing which will be done
with SoftRoCE.

Ira

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 00/23] Update driver to 0.9-294

[Weiny, Ira]

> 
> On Mon, Oct 19, 2015 at 12:43:24PM -0400, ira.we...@intel.com wrote:
> > From: Ira Weiny 
> >
> > The following series has bug fixes and updates to the staging hfi1 driver.
> 
> Why are you adding new functionality to this driver before it is moved out of
> drivers/staging/ ?  I _REALLY_ don't like to see that, because it implies 
> that you
> are spending time and energy on new stuff, not fixing up the known issues
> instead.

Perhaps I did not chose my words carefully enough.

The largest issue on the TODO list is the refactoring of the code to be shared 
between the hfi1 and qib driver.  While the hardware between hfi1 and qib is 
similar and thus the initial code looked similar, our performance tuning on 
hfi1 has revealed that some changes will be required to the hfi1 code to fully 
utilize the hardware.  We would like to get these updates in to make sure that 
the refactoring work is done properly for the 2 hardware types.

> 
> Please redo this patch series and don't add new stuff.
> 

I can do this but it will cause us to do our refactoring work 2 times in order 
for it to be done correctly (properly support both hardware types).  Of the 23 
patches I sent only one has significant updates which don't conflict with this 
refactoring work.

[PATCH 15/23] staging/rdma/hfi1: Implement Expected Receive TID caching

However, without this patch users performance will be impacted.

In the interest of full disclosure we still have at least 2 more series, 
possibly 3 which are similar to this series, bug fixes and performance 
improvements.

Given the above explanation, would you find it acceptable to take these 
"updates"?  I can certainly rework patch 15 to separate out the code clean up 
as per your other email.

Ira

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 00/23] Update driver to 0.9-294

[Greg KH]

On Mon, Oct 19, 2015 at 06:16:55PM +, Weiny, Ira wrote:
> > 
> > On Mon, Oct 19, 2015 at 12:43:24PM -0400, ira.we...@intel.com wrote:
> > > From: Ira Weiny 
> > >
> > > The following series has bug fixes and updates to the staging hfi1 driver.
> > 
> > Why are you adding new functionality to this driver before it is moved out 
> > of
> > drivers/staging/ ?  I _REALLY_ don't like to see that, because it implies 
> > that you
> > are spending time and energy on new stuff, not fixing up the known issues
> > instead.
> 
> Perhaps I did not chose my words carefully enough.
> 
> The largest issue on the TODO list is the refactoring of the code to
> be shared between the hfi1 and qib driver.  While the hardware between
> hfi1 and qib is similar and thus the initial code looked similar, our
> performance tuning on hfi1 has revealed that some changes will be
> required to the hfi1 code to fully utilize the hardware.  We would
> like to get these updates in to make sure that the refactoring work is
> done properly for the 2 hardware types.

Then you need to say that very carefully :)

> > Please redo this patch series and don't add new stuff.
> > 
> 
> I can do this but it will cause us to do our refactoring work 2 times in 
> order for it to be done correctly (properly support both hardware types).  Of 
> the 23 patches I sent only one has significant updates which don't conflict 
> with this refactoring work.

Please wrap your email lines...

If you are just "refactoring to make things better for X" then great,
that's fine, but that's not what you said here.

> [PATCH 15/23] staging/rdma/hfi1: Implement Expected Receive TID caching
> 
> However, without this patch users performance will be impacted.

You mean it would be just as bad as it is today, which is fine :)

See my comments on that patch for what I object to it specifically.

> In the interest of full disclosure we still have at least 2 more series, 
> possibly 3 which are similar to this series, bug fixes and performance 
> improvements.

Why are you keeping these patch series from being submitted?  The longer
you wait, the more work it will take on your part.  If you have
performance fixes, those are just that, 'fixes', but please don't go
adding new functionality, take the time to do the work to get this out
of the staging tree and then you can optimize it as much as you want.

> Given the above explanation, would you find it acceptable to take these 
> "updates"?  I can certainly rework patch 15 to separate out the code clean up 
> as per your other email.

Please rework the series.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 00/23] Update driver to 0.9-294

[ira . weiny]

From: Ira Weiny 

The following series has bug fixes and updates to the staging hfi1 driver.

Arthur Kepner (1):
  staging/rdma/hfi1: optionally prescan rx queue for {B,F}ECNs - UC, RC

Caz Yokoyama (1):
  staging/rdma/hfi1: Reset firmware instead of reloading Sbus

Dean Luick (4):
  staging/rdma/hfi1: Extend the offline timeout
  staging/rdma/hfi1: Add a schedule in send thread
  staging/rdma/hfi1: Add irqsaves in the packet processing path
  staging/rdma/hfi1: Thread the receive interrupt.

Easwar Hariharan (4):
  staging/rdma/hfi1: Reset ASIC CSRs on FLR, and once per ASIC
  staging/rdma/hfi1: Remove QSFP_ENABLED from HFI capability mask
  staging/rdma/hfi1: Fix port bounce issues with 0.22 DC firmware
  staging/rdma/hfi1: Load SBus firmware once per ASIC

Jareer Abdel-Qader (1):
  staging/rdma/hfi1: close shared context security hole

Jubin John (2):
  staging/rdma/hfi1: Add unit # to verbs txreq cache name
  staging/rdma/hfi1: Update driver version string to 0.9-294

Mike Marciniszyn (3):
  staging/rdma/hfi1: inline clear_ahg()
  staging/rdma/hfi: modify workqueue for parallelism
  staging/rdma/hfi1: add additional rc traces

Mitko Haralanov (3):
  staging/rdma/hfi1: Prevent silent data corruption with user SDMA
  staging/rdma/hfi1: Implement Expected Receive TID caching
  staging/rdma/hfi1: Allow tuning of SDMA interrupt rate

Niranjana Vishwanathapura (2):
  staging/rdma/hfi1: Add coalescing support for SDMA TX descriptors
  staging/rdma/hfi1: Fix sparse error in sdma.h file

Vennila Megavannan (2):
  staging/rdma/hfi1: Implement time-out for send context halt recovery
  staging/rdma/hfi1: Method to toggle "fast ECN" detection

 drivers/staging/rdma/hfi1/Kconfig|   14 +-
 drivers/staging/rdma/hfi1/Makefile   |2 +-
 drivers/staging/rdma/hfi1/chip.c |  157 +++-
 drivers/staging/rdma/hfi1/chip.h |2 +
 drivers/staging/rdma/hfi1/common.h   |   19 +-
 drivers/staging/rdma/hfi1/driver.c   |  178 ++---
 drivers/staging/rdma/hfi1/file_ops.c |  495 ++---
 drivers/staging/rdma/hfi1/firmware.c |   37 +-
 drivers/staging/rdma/hfi1/hfi.h  |   98 +--
 drivers/staging/rdma/hfi1/init.c |   23 +-
 drivers/staging/rdma/hfi1/iowait.h   |6 +-
 drivers/staging/rdma/hfi1/mad.c  |4 +-
 drivers/staging/rdma/hfi1/pcie.c |   15 +-
 drivers/staging/rdma/hfi1/pio.c  |   14 +-
 drivers/staging/rdma/hfi1/qp.c   |   47 +-
 drivers/staging/rdma/hfi1/qp.h   |   51 ++
 drivers/staging/rdma/hfi1/qsfp.c |   13 +-
 drivers/staging/rdma/hfi1/rc.c   |   33 +-
 drivers/staging/rdma/hfi1/ruc.c  |   55 +-
 drivers/staging/rdma/hfi1/sdma.c |  160 +++-
 drivers/staging/rdma/hfi1/sdma.h |   82 ++-
 drivers/staging/rdma/hfi1/trace.c|4 +-
 drivers/staging/rdma/hfi1/trace.h|  180 +++--
 drivers/staging/rdma/hfi1/uc.c   |   15 +-
 drivers/staging/rdma/hfi1/ud.c   |1 +
 drivers/staging/rdma/hfi1/user_exp_rcv.c | 1171 ++
 drivers/staging/rdma/hfi1/user_exp_rcv.h |   82 +++
 drivers/staging/rdma/hfi1/user_pages.c   |  110 +--
 drivers/staging/rdma/hfi1/user_sdma.c|  103 ++-
 drivers/staging/rdma/hfi1/user_sdma.h|   10 +-
 drivers/staging/rdma/hfi1/verbs.c|   91 ++-
 drivers/staging/rdma/hfi1/verbs.h|9 +-
 include/uapi/rdma/hfi/hfi1_user.h|   46 +-
 33 files changed, 2279 insertions(+), 1048 deletions(-)
 create mode 100644 drivers/staging/rdma/hfi1/user_exp_rcv.c
 create mode 100644 drivers/staging/rdma/hfi1/user_exp_rcv.h

-- 
1.8.2

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 00/23] Update driver to 0.9-294

[Greg KH]

On Mon, Oct 19, 2015 at 12:43:24PM -0400, ira.we...@intel.com wrote:
> From: Ira Weiny 
> 
> The following series has bug fixes and updates to the staging hfi1 driver.

Why are you adding new functionality to this driver before it is moved
out of drivers/staging/ ?  I _REALLY_ don't like to see that, because it
implies that you are spending time and energy on new stuff, not fixing
up the known issues instead.

Please redo this patch series and don't add new stuff.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html