On Fri, Oct 28, 2022 at 04:48:53PM -0700, Rohit Nair wrote: > 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. The thing is that "vendor" failed to explain internally if this debug code is useful. Like I said, extremely rare debug code shouldn't be part of main data path. Thanks > > Feel free to share your suggestions > > Thanks >
