On Thu, Feb 16, 2023 at 02:00:20PM +0800, Herbert Xu wrote: > > > + if (IS_ENABLED(CONFIG_CRYPTO_STATS)) > > > + memset(istat, 0, sizeof(*istat)); > > > + > > > > Above is another place that can just do 'if (istat)'. > > As I mentioned before, this would create an unnecessary branch > when STATS are enabled (which would presumably be the common case). > I was hoping the compiler would know the pointer is non-NULL, since it's created through an expression like &foo->bar where bar is at nonzero offset, and foo is also dereferenced. Unfortunately it does seem that's not the case, though, probably because of some of the compiler flags the kernel is compiled with (-fno-strict-aliasing and -fno-delete-null-pointer-checks, maybe?). Anyway, if CONFIG_CRYPTO_STATS=y is the common case, that's unfortunate. Surely hardly anyone actually uses the feature, and all this stats collection for every crypto operation is for nothing? Here's a thread where someone claimed that disabling CONFIG_CRYPTO_STATS significantly improves performance: https://lists.ceph.io/hyperkitty/list/ceph-users@xxxxxxx/thread/44GMO5UGOXDZKFSOQMCPPHYTREUEA3ZI/ IMO this feature should never have been accepted. But could we at least put the stats collection behind a static branch that defaults to off? If someone really wants to collect stats, they can set a sysctl that turns on the static branch. - Eric
