On 10/27/22 5:23 AM, Leon Romanovsky wrote:
On Tue, Oct 25, 2022 at 10:44:12AM -0700, Rohit Nair wrote:
Hey Leon,
Please find my replies to your comments here below:
<...>
This patch does not introduce any significant performance degradations
and has been tested using qperf.
What does it mean? You made changes in kernel verbs flow, they are not
executed through qperf.
We also conducted several extensive performance tests using our test-suite
which utilizes rds-stress and also saw no significant performance
degrdations in those results.
What does it mean "also"? Your change is applicable ONLY for kernel path.
Anyway, I'm not keen adding rare debug code to performance critical path.
Thanks
rds-stress exercises the codepath we are modifying here. rds-stress
didn't show much of performance degrade when we ran internally. We also
requested our DB team for performance regression testing and this change
passed their test suite. This motivated us to submit this to upstream.
If there is any other test that is better suited for this change, I am
willing to test it. Please let me know if you have something in mind. We
can revisit this patch after such a test may be.
I agree that, this was a rare debug scenario, but it took lot more than
needed to narrow down[engaged vendor on live sessions]. We are adding
this in the hope to finding the cause at the earliest or at least point
us which direction to look at. We also requested the vendor[mlx] to
include some diagnostics[HW counter], which can help us narrow it faster
next time. This is our attempt to add kernel side of diagnostics.
Feel free to share your suggestions
Thanks
