On Sun, May 6, 2018 at 12:22 PM Frederic Weisbecker <frede...@kernel.org> wrote:
> arch_validate_hwbkpt_settings() mixes up attribute check and commit into > a single code entity. Therefore the validation may return an error due to > incorrect atributes while still leaving halfway modified architecture > breakpoint struct. > Now that we have split its logic on all archs, we can remove this > misdesigned function and call directly the arch check and commit > functions instead. This allows us to later avoid commiting > a breakpoint to architecture when its slot couldn't be allocated. [...] > diff --git a/kernel/events/hw_breakpoint.c b/kernel/events/hw_breakpoint.c > index 6e28d28..6896ceeb 100644 > --- a/kernel/events/hw_breakpoint.c > +++ b/kernel/events/hw_breakpoint.c > @@ -402,11 +402,12 @@ int dbg_release_bp_slot(struct perf_event *bp) > static int validate_hw_breakpoint(struct perf_event *bp) > { > - int ret; > + int err; > - ret = arch_validate_hwbkpt_settings(bp); > - if (ret) > - return ret; > + err = hw_breakpoint_arch_check(bp, &bp->attr); > + if (err) > + return err; > + hw_breakpoint_arch_commit(bp); minor nit: To preserve bisectability, shouldn't this be the following in this and earlier patches? err = hw_breakpoint_arch_check(bp, &bp->attr); hw_breakpoint_arch_commit(bp); if (err) return err; And then in patch 9/9 you can fix it the right way? thanks, - Joel