Messages in this thread | | | Date | Sat, 23 Jun 2018 00:47:12 +0200 (CEST) | From | Thomas Gleixner <> | Subject | Re: [PATCH] arch/x86/entry/vsyscall/vsyscall_gtod.c: remove __read_mostly from vclocks_used |
| |
Michael,
On Mon, 4 Jun 2018, Michael Rodin wrote:
> The variable "vclocks_used" doesn't appear to be "read mostly". > Measurements of the access frequency with perf stat [1] and > perf report show, that approximately half of the accesses to > this variable are write accesses and happen in update_vsyscall() > in the file arch/x86/entry/vsyscall/vsyscall_gtod.c. > The measurements were done with the kernel 4.13.0-43-generic used by > ubuntu as well as with the stable kernel 4.16.7 with a custom config. > I've used "perf bench sched" and iperf3 as workloads. > > This was discovered during my master thesis in the CADOS project [2].
Nice find, but ...
The point is that the content of that variable changes once in a blue moon, so the intent of marking it read_mostly is almost correct. Almost, because the unconditional write even with the ever repeating same value is not free. To the best of my knowledge there is no CPU implementation which supports optimizing silent stores, but I might be just not up to date or wrong as usual.
So there is a cost to the write and there are two things which have to be looked at before just doing the obvious knee jerk reaction 's/read_mostly//'.
1) Does it make sense to avoid the write if the value hasn't changed? That might be pretty much a free lunch because the variable has to be read in the first place in order to OR the supplied value to it before writing it back.
2) The variable is in a totally separate cache line than what the following stores target, i.e. vsyscall_gtod_data
So it might make sense just to do #1 but as vsyscall_gtod_data is anyway going to be dirtied it probably makes even more sense to move that vclocks_used variable into struct vsyscall_gtod_data right away to reduce the amount of cache lines touched by update_vsyscall().
Care to have a look at that?
Thanks,
tglx
| |