netdev - RE: [PATCH net-next 3/3] mlx4_en: Saving mem access on data path

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 24 Feb 2012 19:57:43 +0000
From:	Yevgeny Petrilin <yevgenyp@...lanox.com>
To:	David Miller <davem@...emloft.net>
CC:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: [PATCH net-next 3/3] mlx4_en: Saving mem access on data path

> >
> > Localized the pdev->dev, and using dma_map instead of pci_map
> > There are multiple map/unmap operations on data path,
> > optimizing those by saving redundant pointer access.
> >
> > Signed-off-by: Yevgeny Petrilin <yevgenyp@...lanox.co.il>
> 
> I doubt you can measure any improvement from this at all, all of the
> real cost is going to be in programming the hardware (both the
> networking chip and potentially any IOMMU hardware to setup the DMA
> mappings).  A pointer deref in of your datastructures will be unnoticable.

We actually did measure improvements with this changes.
Those places were found as hot spots when we did kernel profiling on some benchmarks.
Making those changes actually helped us reduce cpu utilization in several %, and in some cases was the
difference between being CPU bounded or reaching wire speed.
One scenario that we had improvement in when making this change is measuring Packet Rate for
small (64 Byte) packets.

> 
> When you make changes like this I want you to justify them with real
> data and facts.

I agree, the explanation should have been part of the patch description.

> I think this entire patch set was in very poor taste and judgment.

Dave,
Thank you for the comprehensive review, please allow me to kindly disagree with the last statement.

The patch set is a result of performance work we did, and those changes helped us to achieve
better results without hurting us in other areas.
I do understand you concern regarding patch 2/3, although we did test it in various scenarios without
running into the issues you described, and there are other vendors doing the same.

I still think that patches 1 and 3 are good for our device,
I'll be happy to make changes in them if you find it necessary, and resubmit.

Thanks,
Yevgeny 
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html