| Message ID | 1508792849-3115-9-git-send-email-paulmck@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | [01/19] dm integrity: Kill off ACCESS_ONCE() | expand |
On Mon, 2017-10-23 at 21:07 +0000, Paul E. McKenney wrote: > From: Mark Rutland <mark.rutland@arm.com> > > For several reasons, it is desirable to use {READ,WRITE}_ONCE() in > preference to ACCESS_ONCE(), and new code is expected to use one of the > former. So far, there's been no reason to change most existing uses of > ACCESS_ONCE(), as these aren't currently harmful. > > However, for some features it is necessary to instrument reads and > writes separately, which is not possible with ACCESS_ONCE(). This > distinction is critical to correct operation. > > It's possible to transform the bulk of kernel code using the Coccinelle > script below. However, this doesn't pick up some uses, including those > in <linux/average.h>. As a preparatory step, this patch converts the > file to use {READ,WRITE}_ONCE() consistently. > > At the same time, this patch addds missing includes necessary for > {READ,WRITE}_ONCE(), *BUG_ON*(), and ilog2(). > > ---- > virtual patch > > @ depends on patch @ > expression E1, E2; > @@ > > - ACCESS_ONCE(E1) = E2 > + WRITE_ONCE(E1, E2) > > @ depends on patch @ > expression E; > @@ > > - ACCESS_ONCE(E) > + READ_ONCE(E) > ---- > > Signed-off-by: Mark Rutland <mark.rutland@arm.com> > Cc: Johannes Berg <johannes.berg@intel.com> Reviewed-by: Johannes Berg <johannes@sipsolutions.net> Let me know if you want me to apply this, since I seem to be the average.h maintainer :-) johannes
* Johannes Berg <johannes@sipsolutions.net> wrote: > On Mon, 2017-10-23 at 21:07 +0000, Paul E. McKenney wrote: > > From: Mark Rutland <mark.rutland@arm.com> > > > > For several reasons, it is desirable to use {READ,WRITE}_ONCE() in > > preference to ACCESS_ONCE(), and new code is expected to use one of the > > former. So far, there's been no reason to change most existing uses of > > ACCESS_ONCE(), as these aren't currently harmful. > > > > However, for some features it is necessary to instrument reads and > > writes separately, which is not possible with ACCESS_ONCE(). This > > distinction is critical to correct operation. > > > > It's possible to transform the bulk of kernel code using the Coccinelle > > script below. However, this doesn't pick up some uses, including those > > in <linux/average.h>. As a preparatory step, this patch converts the > > file to use {READ,WRITE}_ONCE() consistently. > > > > At the same time, this patch addds missing includes necessary for > > {READ,WRITE}_ONCE(), *BUG_ON*(), and ilog2(). > > > > ---- > > virtual patch > > > > @ depends on patch @ > > expression E1, E2; > > @@ > > > > - ACCESS_ONCE(E1) = E2 > > + WRITE_ONCE(E1, E2) > > > > @ depends on patch @ > > expression E; > > @@ > > > > - ACCESS_ONCE(E) > > + READ_ONCE(E) > > ---- > > > > Signed-off-by: Mark Rutland <mark.rutland@arm.com> > > Cc: Johannes Berg <johannes.berg@intel.com> > > Reviewed-by: Johannes Berg <johannes@sipsolutions.net> Thanks! > Let me know if you want me to apply this, since I seem to be the > average.h maintainer :-) Would be nice to keep these patches together if possible, so that we can remove ACCESS_ONCE() - or at least add a build warning. Thanks, Ingo
On Tue, Oct 24, 2017 at 11:34:13AM +0200, Ingo Molnar wrote: > > * Johannes Berg <johannes@sipsolutions.net> wrote: > > > On Mon, 2017-10-23 at 21:07 +0000, Paul E. McKenney wrote: > > > From: Mark Rutland <mark.rutland@arm.com> > > > > > > For several reasons, it is desirable to use {READ,WRITE}_ONCE() in > > > preference to ACCESS_ONCE(), and new code is expected to use one of the > > > former. So far, there's been no reason to change most existing uses of > > > ACCESS_ONCE(), as these aren't currently harmful. > > > > > > However, for some features it is necessary to instrument reads and > > > writes separately, which is not possible with ACCESS_ONCE(). This > > > distinction is critical to correct operation. > > > > > > It's possible to transform the bulk of kernel code using the Coccinelle > > > script below. However, this doesn't pick up some uses, including those > > > in <linux/average.h>. As a preparatory step, this patch converts the > > > file to use {READ,WRITE}_ONCE() consistently. > > > > > > At the same time, this patch addds missing includes necessary for > > > {READ,WRITE}_ONCE(), *BUG_ON*(), and ilog2(). > > > > > > ---- > > > virtual patch > > > > > > @ depends on patch @ > > > expression E1, E2; > > > @@ > > > > > > - ACCESS_ONCE(E1) = E2 > > > + WRITE_ONCE(E1, E2) > > > > > > @ depends on patch @ > > > expression E; > > > @@ > > > > > > - ACCESS_ONCE(E) > > > + READ_ONCE(E) > > > ---- > > > > > > Signed-off-by: Mark Rutland <mark.rutland@arm.com> > > > Cc: Johannes Berg <johannes.berg@intel.com> > > > > Reviewed-by: Johannes Berg <johannes@sipsolutions.net> > > Thanks! > > > Let me know if you want me to apply this, since I seem to be the > > average.h maintainer :-) > > Would be nice to keep these patches together if possible, so that we can remove > ACCESS_ONCE() - or at least add a build warning. My goal is to remove it. After all, I am the guy who added it. ;-) Thanx, Paul
diff --git a/include/linux/average.h b/include/linux/average.h index 7ddaf340d2ac..3f462292269c 100644 --- a/include/linux/average.h +++ b/include/linux/average.h @@ -1,6 +1,10 @@ #ifndef _LINUX_AVERAGE_H #define _LINUX_AVERAGE_H +#include <linux/bug.h> +#include <linux/compiler.h> +#include <linux/log2.h> + /* * Exponentially weighted moving average (EWMA) * @@ -48,7 +52,7 @@ static inline void ewma_##name##_add(struct ewma_##name *e, \ unsigned long val) \ { \ - unsigned long internal = ACCESS_ONCE(e->internal); \ + unsigned long internal = READ_ONCE(e->internal); \ unsigned long weight_rcp = ilog2(_weight_rcp); \ unsigned long precision = _precision; \ \ @@ -57,10 +61,10 @@ BUILD_BUG_ON((_precision) > 30); \ BUILD_BUG_ON_NOT_POWER_OF_2(_weight_rcp); \ \ - ACCESS_ONCE(e->internal) = internal ? \ + WRITE_ONCE(e->internal, internal ? \ (((internal << weight_rcp) - internal) + \ (val << precision)) >> weight_rcp : \ - (val << precision); \ + (val << precision)); \ } #endif /* _LINUX_AVERAGE_H */