Re: [PATCH v2 0/3] nvmet-rdma: SRQ per completion vector

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 11/18/2017 4:40 PM, Leon Romanovsky wrote:
On Sat, Nov 18, 2017 at 03:57:15PM +0200, Max Gurtovoy wrote:


On 11/18/2017 2:52 PM, Leon Romanovsky wrote:
On Fri, Nov 17, 2017 at 09:32:42PM +0200, Max Gurtovoy wrote:


On 11/16/2017 8:36 PM, Sagi Grimberg wrote:

Since there is an active discussion regarding the CQ pool
architecture, I decided to push
this feature (maybe it can be pushed before CQ pool).

This is a new feature for NVMEoF RDMA target,

Any chance having this for the rest? isert, srpt, svcrdma?


We can implement it for isert, but I think it's better to see how the CQ
pool will be defined first.
It can bring a big benefit and improvement for ib_srpt (similar to NVMEoF
target) but I'm not sure if I can commit for that one soon..

Too bad, but I don't see inclusion of generic SRQ pool code in RDMA
subsystem without actual conversion of existing ULP clients.

Thanks


This patchset adds this feature to NVMEoF target so actually there are ULPs
that use it. Same issue we have with mr_pool that only RDMA rw.c use it (Now
we're adding it to NVMEoF initiators too - in review).

The difference between your code and mr_pool is that mr_pool is part of
RDMA/core and in use by RDMA/core (rw.c), which in use by all ULPs.

However if you insist, we can remove EXPORT_SYMBOL from mr_pool
implementation, because of being part of RDMA/core and it blows
symbols map without need. Should I?

No, we'll use it in NVMEoF host as I mentioned earlier.

In your case, you are proposing generic interface, which supposed to be
good fit for all ULPs but without those ULPs.

I can add srq_pool to iSER target code but I don't want to re-write it again
in few weeks, when the CQ pool will be added.

So, please finalize interface in RFC stage and once you are ready, proceed to
the actual patches.

Regarding other ULPs, we don't have a testing environment for them so I
prefer not to commit on their implementation in the near future.

You are not expected to have all testing environment, it is their (ULPs
maintainers) responsibility to test your conversion, because you are
doing conversion to generic interface.


I don't know why we can't add this feature "as is".
Other ULPs maintainers might use it once it will be pushed.

Sorry, but it is not how kernel development process works.
"You propose -> you do" and not "You propose -> they do".

I'm not changing an interface here. So all the other ULPs that use SRQ (ipoib and srpt) currently will cuntinue using it. I don't know why this patchset brought up the idea to add SRQ pools to isert/svcrdma/etc.., but knowing that there are patches (under discussions) that will have a big enfluance on these drivers (at least isert), it doesn't make sence to implement *new* feature (SRQ usage) and chage it a week afterwards.

I will send V3 in a few days with some fixes that I got, so it would be nice to have a more comments on the code (I don't see a problem with the kernel development process in this patchset).



Thanks

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux