On Mon, Sep 28, 2020 at 01:34:31PM -0700, Kees Cook wrote: > On Sun, Sep 27, 2020 at 07:35:26PM -0400, Joel Fernandes wrote: > > On Fri, Sep 25, 2020 at 05:47:14PM -0600, Shuah Khan wrote: > > > This patch series is a result of discussion at the refcount_t BOF > > > the Linux Plumbers Conference. In this discussion, we identified > > > a need for looking closely and investigating atomic_t usages in > > > the kernel when it is used strictly as a counter without it > > > controlling object lifetimes and state changes. > > > > > > There are a number of atomic_t usages in the kernel where atomic_t api > > > is used strictly for counting and not for managing object lifetime. In > > > some cases, atomic_t might not even be needed. > > > > > > The purpose of these counters is twofold: 1. clearly differentiate > > > atomic_t counters from atomic_t usages that guard object lifetimes, > > > hence prone to overflow and underflow errors. It allows tools that scan > > > for underflow and overflow on atomic_t usages to detect overflow and > > > underflows to scan just the cases that are prone to errors. 2. provides > > > non-atomic counters for cases where atomic isn't necessary. > > > > Nice series :) > > > > It appears there is no user of counter_simple in this series other than the > > selftest. Would you be planning to add any conversions in the series itself, > > for illustration of use? Sorry if I missed a usage. > > > > Also how do we guard against atomicity of counter_simple RMW operations? Is > > the implication that it should be guarded using other synchronization to > > prevent lost-update problem? > > > > Some more comments: > > > > 1. atomic RMW operations that have a return value are fully ordered. Would > > you be adding support to counter_simple for such ordering as well, for > > consistency? > > No -- there is no atomicity guarantee for counter_simple. I would prefer > counter_simple not exist at all, specifically for this reason. Yeah I am ok with it not existing, especially also as there are no examples of its conversion/usage in the series. > > 2. I felt counter_atomic and counter_atomic64 would be nice equivalents to > > the atomic and atomic64 naming currently used (i.e. dropping the '32'). > > However that is just my opinion and I am ok with either naming. > > I had asked that they be size-named to avoid any confusion (i.e. we're > making a new API). Works for me. Cheers, - Joel
