Quoting Rob Clark (2022-03-03 13:47:14) > On Thu, Mar 3, 2022 at 1:17 PM Rob Clark <robdclark@xxxxxxxxx> wrote: > > > > On Thu, Mar 3, 2022 at 12:47 PM Stephen Boyd <swboyd@xxxxxxxxxxxx> wrote: > > > > > > Quoting Rob Clark (2022-03-03 11:46:47) > > > > + > > > > + /* then apply new value: */ > > > > > > It would be safer to swap this. Otherwise a set when the values are at > > > "1" would drop to "zero" here and potentially trigger some glitch, > > > whereas incrementing one more time and then dropping the previous state > > > would avoid that short blip. > > > > > > > + switch (sysprof) { > > > > + default: > > > > + return -EINVAL; > > > > > > This will become more complicated though. > > > > Right, that is why I took the "unwind first and then re-apply" > > approach.. in practice I expect userspace to set the value before it > > starts sampling counter values, so I wasn't too concerned about this > > racing with a submit and clearing the counters. (Plus any glitch if > > userspace did decide to change it dynamically would just be transient > > and not really a big deal.) > > Actually I could just swap the two switch's.. the result would be that > an EINVAL would not change the state instead of dropping the state to > zero. Maybe that is better anyways > Yeah it isn't clear to me what should happen if the new state is invalid. Outright rejection is probably better than replacing the previous state with an invalid state.