From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.linuxfoundation.org ([140.211.169.12]:49228 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753106AbcLII01 (ORCPT ); Fri, 9 Dec 2016 03:26:27 -0500 Date: Fri, 9 Dec 2016 09:26:31 +0100 From: Greg KH To: Henrik Austad Cc: Henrik Austad , linux-kernel@vger.kernel.org, Ingo Molnar , Peter Zijlstra , Steven Rostedt , stable@vger.kernel.org Subject: Re: [PATCH] tracing: (backport) Replace kmap with copy_from_user() in trace_marker Message-ID: <20161209082631.GA26643@kroah.com> References: <20161208134422.1ca7455a@gandalf.local.home> <1481265244-13986-1-git-send-email-haustad@cisco.com> <20161209072205.GC29639@kroah.com> <20161209080551.GA7533@clotho.rd.cisco.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161209080551.GA7533@clotho.rd.cisco.com> Sender: stable-owner@vger.kernel.org List-ID: On Fri, Dec 09, 2016 at 09:05:51AM +0100, Henrik Austad wrote: > On Fri, Dec 09, 2016 at 08:22:05AM +0100, Greg KH wrote: > > On Fri, Dec 09, 2016 at 07:34:04AM +0100, Henrik Austad wrote: > > > Instead of using get_user_pages_fast() and kmap_atomic() when writing > > > to the trace_marker file, just allocate enough space on the ring buffer > > > directly, and write into it via copy_from_user(). > > > > > > Writing into the trace_marker file use to allocate a temporary buffer > > > to perform the copy_from_user(), as we didn't want to write into the > > > ring buffer if the copy failed. But as a trace_marker write is suppose > > > to be extremely fast, and allocating memory causes other tracepoints to > > > trigger, Peter Zijlstra suggested using get_user_pages_fast() and > > > kmap_atomic() to keep the user space pages in memory and reading it > > > directly. > > > > > > Instead, just allocate the space in the ring buffer and use > > > copy_from_user() directly. If it faults, return -EFAULT and write > > > "" into the ring buffer. > > > > > > On architectures without a arch-specific get_user_pages_fast(), this > > > will end up in the generic get_user_pages_fast() and this grabs > > > mm->mmap_sem. Once you do this, then suddenly writing to the > > > trace_marker can cause priority-inversions. > > > > > > This is a backport of Steven Rostedts patch [1] and applied to 3.10.x so the > > > signed-off-chain by is somewhat uncertain at this stage. > > > > > > The patch compiles, boots and does not immediately explode on impact. By > > > definition [2] it must therefore be perfect > > > > > > 2) https://www.spinics.net/lists/kernel/msg2400769.html > > > 2) http://lkml.iu.edu/hypermail/linux/kernel/9804.1/0149.html > > > > > > Cc: Ingo Molnar > > > Cc: Henrik Austad > > > Cc: Peter Zijlstra > > > Cc: Steven Rostedt > > > Cc: stable@vger.kernel.org > > > > > > Suggested-by: Thomas Gleixner > > > Used-to-be-signed-off-by: Steven Rostedt > > > Backported-by: Henrik Austad > > > Tested-by: Henrik Austad > > > Signed-off-by: Henrik Austad > > > --- > > > kernel/trace/trace.c | 78 +++++++++++++++------------------------------------- > > > 1 file changed, 22 insertions(+), 56 deletions(-) > > > > What is the git commit id of this patch in Linus's tree? And what > > stable trees do you feel it should be applied to? > > Ah, perhaps I jumped the gun here. I don't think Linus has picked this one > up yet, Steven sent out the patch yesterday. > > Since then, I've backported it to 3.10 and ran the first set of tests > over night and it looks good. So ideally this would find its way into > 3.10(.104). > > Do you want med to resubmit when Stevens patch is merged upstream? Yes please, we can't do anything until it is in Linus's tree, please see Documentation/stable_kernel_rules.txt for how this all works. thanks, greg k-h