From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jiri Olsa Subject: Re: [PATCH] perf/core: fast breakpoint modification via _IOC_MODIFY_BREAKPOINT Date: Wed, 8 Nov 2017 16:12:18 +0100 Message-ID: <20171108151218.GA11018@krava> References: <20171106092305.GA16382@krava> <20171108141522.GA6320@krava> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: Sender: linux-man-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Milind Chabbi Cc: Peter Zijlstra , Ingo Molnar , Arnaldo Carvalho de Melo , Alexander Shishkin , Namhyung Kim , linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Michael Kerrisk-manpages , linux-man-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Michael Ellerman , Andi Kleen , Kan Liang , Hari Bathini , Sukadev Bhattiprolu , Jin Yao List-Id: linux-man@vger.kernel.org On Wed, Nov 08, 2017 at 07:02:22AM -0800, Milind Chabbi wrote: > On Wed, Nov 8, 2017 at 6:15 AM, Jiri Olsa wrote: > > On Mon, Nov 06, 2017 at 07:04:40AM -0800, Milind Chabbi wrote: > >> Hi Jirka, > >> > >> I see the tabs in my sent email, do you have suggestions on how best to > >> send this patch so that the tabs are preserved by the email client? > >> Can anybody else also check if they received with/without tabs? > >> > >> release_bp_slot/reserve_bp_slot majic is not necessary since > >> _IOC_MODIFY_BREAKPOINT ioctl modifies an already registered breakpoint > >> without affecting the count of breakpoints active. > > > > but AFAICS you allow to change the breakpoint type (bp_type) > > and slot counts are based on the breakpoint type > > > > jirka > > Jirka, > I am not able to fully understand your concern. > Can you point to a code file and line related to your observation? > The patch is modeled after the existing modify_user_hw_breakpoint() function > present in events/hw_breakpoint.c; don't you see this problem in that code? the reserve_bp_slot/release_bp_slot functions manage counts for current breakpoints based on its type those counts are cumulated in here: static DEFINE_PER_CPU(struct bp_cpuinfo, bp_cpuinfo[TYPE_MAX]); you allow to change the breakpoint type, so I'd expect to see some code that release slot count for old type and take new one (if it's available) jirka -- To unsubscribe from this list: send the line "unsubscribe linux-man" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html