From mboxrd@z Thu Jan 1 00:00:00 1970 From: Luis Chamberlain Subject: Re: [RFC v3 08/19] arch: um: add shim to trap to allow installing a fault catcher for tests Date: Mon, 3 Dec 2018 15:46:28 -0800 Message-ID: <20181203234628.GR28501@garbanzo.do-not-panic.com> References: <20181128193636.254378-1-brendanhiggins@google.com> <20181128193636.254378-9-brendanhiggins@google.com> <20181130033429.GK18410@garbanzo.do-not-panic.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: linux-nvdimm-bounces-hn68Rpc1hR1g9hUCZPvPmw@public.gmane.org Sender: "Linux-nvdimm" To: Brendan Higgins Cc: brakmo-b10kYP2dOMg@public.gmane.org, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org, linux-kselftest-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, shuah-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, Rob Herring , Frank Rowand , linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw@public.gmane.org, richard-/L3Ra7n9ekc@public.gmane.org, Knut Omang , kieran.bingham-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org, Joel Stanley , jdike-OPE4K8JWMJJBDgjK7y7TUQ@public.gmane.org, Tim.Bird-7U/KSKJipcs@public.gmane.org, Kees Cook , linux-um-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, rostedt-nx8X9YLhiw1AfugRpC6u6w@public.gmane.org, Julia Lawall , kunit-dev-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org, Greg KH , Linux Kernel Mailing List , Daniel Vetter , mpe-Gsx/Oe8HsFggBc27wqDAHg@public.gmane.org, joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org, khilman-rdvid1DuHRBWk0Htik3J/w@public.gmane.org List-Id: linux-nvdimm@lists.01.org On Mon, Dec 03, 2018 at 03:34:57PM -0800, Brendan Higgins wrote: > On Thu, Nov 29, 2018 at 7:34 PM Luis Chamberlain wrote: > > > > On Wed, Nov 28, 2018 at 11:36:25AM -0800, Brendan Higgins wrote: > > > diff --git a/arch/um/kernel/trap.c b/arch/um/kernel/trap.c > > > index cced829460427..bf90e678b3d71 100644 > > > --- a/arch/um/kernel/trap.c > > > +++ b/arch/um/kernel/trap.c > > > @@ -201,6 +201,12 @@ void segv_handler(int sig, struct siginfo *unused_si, struct uml_pt_regs *regs) > > > segv(*fi, UPT_IP(regs), UPT_IS_USER(regs), regs); > > > } > > > > > > +static void segv_run_catcher(jmp_buf *catcher, void *fault_addr) > > > +{ > > > + current->thread.fault_addr = fault_addr; > > > + UML_LONGJMP(catcher, 1); > > > +} > > > + > > > /* > > > * We give a *copy* of the faultinfo in the regs to segv. > > > * This must be done, since nesting SEGVs could overwrite > > > @@ -219,7 +225,10 @@ unsigned long segv(struct faultinfo fi, unsigned long ip, int is_user, > > > if (!is_user && regs) > > > current->thread.segv_regs = container_of(regs, struct pt_regs, regs); > > > > > > - if (!is_user && (address >= start_vm) && (address < end_vm)) { > > > + catcher = current->thread.fault_catcher; > > > > This and.. > > > > > + if (catcher && current->thread.is_running_test) > > > + segv_run_catcher(catcher, (void *) address); > > > + else if (!is_user && (address >= start_vm) && (address < end_vm)) { > > > flush_tlb_kernel_vm(); > > > goto out; > > > } > > > > *not this* > > I don't understand. Are you saying the previous block of code is good > and this one is bad? No, I was saying that the above block of code is a functional change, but I was also pointing out other areas which were not and could be folded into a separate atomic patch where no functionality changes. > > > @@ -246,12 +255,10 @@ unsigned long segv(struct faultinfo fi, unsigned long ip, int is_user, > > > address = 0; > > > } > > > > > > - catcher = current->thread.fault_catcher; > > > if (!err) > > > goto out; > > > else if (catcher != NULL) { > > > - current->thread.fault_addr = (void *) address; > > > - UML_LONGJMP(catcher, 1); > > > + segv_run_catcher(catcher, (void *) address); > > > } > > > else if (current->thread.fault_addr != NULL) > > > panic("fault_addr set but no fault catcher"); > > > > But with this seems one atomic change which should be submitted > > separately, its just a helper. Think it would make the actual > > change needed easier to review, ie, your needed changes would > > be smaller and clearer for what you need. > > Are you suggesting that I pull out the bits needed to implement abort > in the next patch and squash it into this one? No, I'm suggesting you can probably split this patch in 2, one which wraps things with no functional changes, and another which adds your changes. Luis From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.5 required=3.0 tests=INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_PASS,USER_AGENT_MUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 85A34C04EB9 for ; Mon, 3 Dec 2018 23:46:37 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 4AC6220850 for ; Mon, 3 Dec 2018 23:46:37 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 4AC6220850 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726038AbeLCXqf (ORCPT ); Mon, 3 Dec 2018 18:46:35 -0500 Received: from mail-pl1-f195.google.com ([209.85.214.195]:37064 "EHLO mail-pl1-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725903AbeLCXqe (ORCPT ); Mon, 3 Dec 2018 18:46:34 -0500 Received: by mail-pl1-f195.google.com with SMTP id b5so7289542plr.4; Mon, 03 Dec 2018 15:46:34 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=tNc78mhpc6WKfwdLM4Nks85v2+hkCSmRaIJzKiGfIcI=; b=ZXNc33Hahf4fq0o/xWXpqDQeZyNi4DSD/5fFxXUnmaE7kdElVVIofnMW7QWzcs5BV5 G2XwjEk9XgsOyZmDqiYVjuYXIfl4PEWRpc60zEu0M3xdQixPC88R0LK+EHwymLGYznHQ 7aJ9ShJiPMKblQm+KlQ+7nlEEjMh0VIQNDxcBvM0hYnZb9B9/RPUY1o+1dtu/jNAkT+5 wLKLe1STa4A/5GnpuEbQTf0yQq5YwqOsu3Rt/rHvur3N66uSwT6neucwbXL+jTmTMUo1 Q0W/SeyGOdu9M57gqU3pFll0Vtr8LIoJpMFRr9wT4cQLsJgtU6NmWyhGklk01BNPVGRo 9Kjg== X-Gm-Message-State: AA+aEWbnJrJo5KpB+QuDnIlTlQ4bUPzoQ1S8GOrp6RSYYXfwlFy390jn Wk6nagUg7QwIFE6h4y0DQsg= X-Google-Smtp-Source: AFSGD/UZ6G0AErnQoHzUlXf6WmINBQZAfhplFN6H+iOT8dv44xKiKAdw1j5dgGOu+YGL/bkYO6KDhw== X-Received: by 2002:a17:902:2aaa:: with SMTP id j39mr18333934plb.335.1543880793764; Mon, 03 Dec 2018 15:46:33 -0800 (PST) Received: from garbanzo.do-not-panic.com (c-73-71-40-85.hsd1.ca.comcast.net. [73.71.40.85]) by smtp.gmail.com with ESMTPSA id y184sm18229297pgd.71.2018.12.03.15.46.30 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 03 Dec 2018 15:46:32 -0800 (PST) Received: by garbanzo.do-not-panic.com (sSMTP sendmail emulation); Mon, 03 Dec 2018 15:46:28 -0800 Date: Mon, 3 Dec 2018 15:46:28 -0800 From: Luis Chamberlain To: Brendan Higgins Cc: Greg KH , Kees Cook , shuah@kernel.org, Joel Stanley , mpe@ellerman.id.au, joe@perches.com, brakmo@fb.com, rostedt@goodmis.org, Tim.Bird@sony.com, khilman@baylibre.com, Julia Lawall , linux-kselftest@vger.kernel.org, kunit-dev@googlegroups.com, Linux Kernel Mailing List , jdike@addtoit.com, richard@nod.at, linux-um@lists.infradead.org, Daniel Vetter , dri-devel@lists.freedesktop.org, Rob Herring , dan.j.williams@intel.com, linux-nvdimm@lists.01.org, kieran.bingham@ideasonboard.com, Frank Rowand , Knut Omang Subject: Re: [RFC v3 08/19] arch: um: add shim to trap to allow installing a fault catcher for tests Message-ID: <20181203234628.GR28501@garbanzo.do-not-panic.com> References: <20181128193636.254378-1-brendanhiggins@google.com> <20181128193636.254378-9-brendanhiggins@google.com> <20181130033429.GK18410@garbanzo.do-not-panic.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Dec 03, 2018 at 03:34:57PM -0800, Brendan Higgins wrote: > On Thu, Nov 29, 2018 at 7:34 PM Luis Chamberlain wrote: > > > > On Wed, Nov 28, 2018 at 11:36:25AM -0800, Brendan Higgins wrote: > > > diff --git a/arch/um/kernel/trap.c b/arch/um/kernel/trap.c > > > index cced829460427..bf90e678b3d71 100644 > > > --- a/arch/um/kernel/trap.c > > > +++ b/arch/um/kernel/trap.c > > > @@ -201,6 +201,12 @@ void segv_handler(int sig, struct siginfo *unused_si, struct uml_pt_regs *regs) > > > segv(*fi, UPT_IP(regs), UPT_IS_USER(regs), regs); > > > } > > > > > > +static void segv_run_catcher(jmp_buf *catcher, void *fault_addr) > > > +{ > > > + current->thread.fault_addr = fault_addr; > > > + UML_LONGJMP(catcher, 1); > > > +} > > > + > > > /* > > > * We give a *copy* of the faultinfo in the regs to segv. > > > * This must be done, since nesting SEGVs could overwrite > > > @@ -219,7 +225,10 @@ unsigned long segv(struct faultinfo fi, unsigned long ip, int is_user, > > > if (!is_user && regs) > > > current->thread.segv_regs = container_of(regs, struct pt_regs, regs); > > > > > > - if (!is_user && (address >= start_vm) && (address < end_vm)) { > > > + catcher = current->thread.fault_catcher; > > > > This and.. > > > > > + if (catcher && current->thread.is_running_test) > > > + segv_run_catcher(catcher, (void *) address); > > > + else if (!is_user && (address >= start_vm) && (address < end_vm)) { > > > flush_tlb_kernel_vm(); > > > goto out; > > > } > > > > *not this* > > I don't understand. Are you saying the previous block of code is good > and this one is bad? No, I was saying that the above block of code is a functional change, but I was also pointing out other areas which were not and could be folded into a separate atomic patch where no functionality changes. > > > @@ -246,12 +255,10 @@ unsigned long segv(struct faultinfo fi, unsigned long ip, int is_user, > > > address = 0; > > > } > > > > > > - catcher = current->thread.fault_catcher; > > > if (!err) > > > goto out; > > > else if (catcher != NULL) { > > > - current->thread.fault_addr = (void *) address; > > > - UML_LONGJMP(catcher, 1); > > > + segv_run_catcher(catcher, (void *) address); > > > } > > > else if (current->thread.fault_addr != NULL) > > > panic("fault_addr set but no fault catcher"); > > > > But with this seems one atomic change which should be submitted > > separately, its just a helper. Think it would make the actual > > change needed easier to review, ie, your needed changes would > > be smaller and clearer for what you need. > > Are you suggesting that I pull out the bits needed to implement abort > in the next patch and squash it into this one? No, I'm suggesting you can probably split this patch in 2, one which wraps things with no functional changes, and another which adds your changes. Luis From mboxrd@z Thu Jan 1 00:00:00 1970 From: mcgrof at kernel.org (Luis Chamberlain) Date: Mon, 3 Dec 2018 15:46:28 -0800 Subject: [RFC v3 08/19] arch: um: add shim to trap to allow installing a fault catcher for tests In-Reply-To: References: <20181128193636.254378-1-brendanhiggins@google.com> <20181128193636.254378-9-brendanhiggins@google.com> <20181130033429.GK18410@garbanzo.do-not-panic.com> Message-ID: <20181203234628.GR28501@garbanzo.do-not-panic.com> On Mon, Dec 03, 2018 at 03:34:57PM -0800, Brendan Higgins wrote: > On Thu, Nov 29, 2018 at 7:34 PM Luis Chamberlain wrote: > > > > On Wed, Nov 28, 2018 at 11:36:25AM -0800, Brendan Higgins wrote: > > > diff --git a/arch/um/kernel/trap.c b/arch/um/kernel/trap.c > > > index cced829460427..bf90e678b3d71 100644 > > > --- a/arch/um/kernel/trap.c > > > +++ b/arch/um/kernel/trap.c > > > @@ -201,6 +201,12 @@ void segv_handler(int sig, struct siginfo *unused_si, struct uml_pt_regs *regs) > > > segv(*fi, UPT_IP(regs), UPT_IS_USER(regs), regs); > > > } > > > > > > +static void segv_run_catcher(jmp_buf *catcher, void *fault_addr) > > > +{ > > > + current->thread.fault_addr = fault_addr; > > > + UML_LONGJMP(catcher, 1); > > > +} > > > + > > > /* > > > * We give a *copy* of the faultinfo in the regs to segv. > > > * This must be done, since nesting SEGVs could overwrite > > > @@ -219,7 +225,10 @@ unsigned long segv(struct faultinfo fi, unsigned long ip, int is_user, > > > if (!is_user && regs) > > > current->thread.segv_regs = container_of(regs, struct pt_regs, regs); > > > > > > - if (!is_user && (address >= start_vm) && (address < end_vm)) { > > > + catcher = current->thread.fault_catcher; > > > > This and.. > > > > > + if (catcher && current->thread.is_running_test) > > > + segv_run_catcher(catcher, (void *) address); > > > + else if (!is_user && (address >= start_vm) && (address < end_vm)) { > > > flush_tlb_kernel_vm(); > > > goto out; > > > } > > > > *not this* > > I don't understand. Are you saying the previous block of code is good > and this one is bad? No, I was saying that the above block of code is a functional change, but I was also pointing out other areas which were not and could be folded into a separate atomic patch where no functionality changes. > > > @@ -246,12 +255,10 @@ unsigned long segv(struct faultinfo fi, unsigned long ip, int is_user, > > > address = 0; > > > } > > > > > > - catcher = current->thread.fault_catcher; > > > if (!err) > > > goto out; > > > else if (catcher != NULL) { > > > - current->thread.fault_addr = (void *) address; > > > - UML_LONGJMP(catcher, 1); > > > + segv_run_catcher(catcher, (void *) address); > > > } > > > else if (current->thread.fault_addr != NULL) > > > panic("fault_addr set but no fault catcher"); > > > > But with this seems one atomic change which should be submitted > > separately, its just a helper. Think it would make the actual > > change needed easier to review, ie, your needed changes would > > be smaller and clearer for what you need. > > Are you suggesting that I pull out the bits needed to implement abort > in the next patch and squash it into this one? No, I'm suggesting you can probably split this patch in 2, one which wraps things with no functional changes, and another which adds your changes. Luis From mboxrd@z Thu Jan 1 00:00:00 1970 From: mcgrof@kernel.org (Luis Chamberlain) Date: Mon, 3 Dec 2018 15:46:28 -0800 Subject: [RFC v3 08/19] arch: um: add shim to trap to allow installing a fault catcher for tests In-Reply-To: References: <20181128193636.254378-1-brendanhiggins@google.com> <20181128193636.254378-9-brendanhiggins@google.com> <20181130033429.GK18410@garbanzo.do-not-panic.com> Message-ID: <20181203234628.GR28501@garbanzo.do-not-panic.com> Content-Type: text/plain; charset="UTF-8" Message-ID: <20181203234628.ENsouRb8y4b8qh_JFhpbnF1PRpTtBJPfEBAKdZyzJUs@z> On Mon, Dec 03, 2018@03:34:57PM -0800, Brendan Higgins wrote: > On Thu, Nov 29, 2018@7:34 PM Luis Chamberlain wrote: > > > > On Wed, Nov 28, 2018@11:36:25AM -0800, Brendan Higgins wrote: > > > diff --git a/arch/um/kernel/trap.c b/arch/um/kernel/trap.c > > > index cced829460427..bf90e678b3d71 100644 > > > --- a/arch/um/kernel/trap.c > > > +++ b/arch/um/kernel/trap.c > > > @@ -201,6 +201,12 @@ void segv_handler(int sig, struct siginfo *unused_si, struct uml_pt_regs *regs) > > > segv(*fi, UPT_IP(regs), UPT_IS_USER(regs), regs); > > > } > > > > > > +static void segv_run_catcher(jmp_buf *catcher, void *fault_addr) > > > +{ > > > + current->thread.fault_addr = fault_addr; > > > + UML_LONGJMP(catcher, 1); > > > +} > > > + > > > /* > > > * We give a *copy* of the faultinfo in the regs to segv. > > > * This must be done, since nesting SEGVs could overwrite > > > @@ -219,7 +225,10 @@ unsigned long segv(struct faultinfo fi, unsigned long ip, int is_user, > > > if (!is_user && regs) > > > current->thread.segv_regs = container_of(regs, struct pt_regs, regs); > > > > > > - if (!is_user && (address >= start_vm) && (address < end_vm)) { > > > + catcher = current->thread.fault_catcher; > > > > This and.. > > > > > + if (catcher && current->thread.is_running_test) > > > + segv_run_catcher(catcher, (void *) address); > > > + else if (!is_user && (address >= start_vm) && (address < end_vm)) { > > > flush_tlb_kernel_vm(); > > > goto out; > > > } > > > > *not this* > > I don't understand. Are you saying the previous block of code is good > and this one is bad? No, I was saying that the above block of code is a functional change, but I was also pointing out other areas which were not and could be folded into a separate atomic patch where no functionality changes. > > > @@ -246,12 +255,10 @@ unsigned long segv(struct faultinfo fi, unsigned long ip, int is_user, > > > address = 0; > > > } > > > > > > - catcher = current->thread.fault_catcher; > > > if (!err) > > > goto out; > > > else if (catcher != NULL) { > > > - current->thread.fault_addr = (void *) address; > > > - UML_LONGJMP(catcher, 1); > > > + segv_run_catcher(catcher, (void *) address); > > > } > > > else if (current->thread.fault_addr != NULL) > > > panic("fault_addr set but no fault catcher"); > > > > But with this seems one atomic change which should be submitted > > separately, its just a helper. Think it would make the actual > > change needed easier to review, ie, your needed changes would > > be smaller and clearer for what you need. > > Are you suggesting that I pull out the bits needed to implement abort > in the next patch and squash it into this one? No, I'm suggesting you can probably split this patch in 2, one which wraps things with no functional changes, and another which adds your changes. Luis From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pl1-f196.google.com ([209.85.214.196]) by bombadil.infradead.org with esmtps (Exim 4.90_1 #2 (Red Hat Linux)) id 1gTxvI-0005Nv-LL for linux-um@lists.infradead.org; Mon, 03 Dec 2018 23:46:46 +0000 Received: by mail-pl1-f196.google.com with SMTP id 101so7281985pld.6 for ; Mon, 03 Dec 2018 15:46:34 -0800 (PST) Date: Mon, 3 Dec 2018 15:46:28 -0800 From: Luis Chamberlain Subject: Re: [RFC v3 08/19] arch: um: add shim to trap to allow installing a fault catcher for tests Message-ID: <20181203234628.GR28501@garbanzo.do-not-panic.com> References: <20181128193636.254378-1-brendanhiggins@google.com> <20181128193636.254378-9-brendanhiggins@google.com> <20181130033429.GK18410@garbanzo.do-not-panic.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-um" Errors-To: linux-um-bounces+geert=linux-m68k.org@lists.infradead.org To: Brendan Higgins Cc: brakmo@fb.com, dri-devel@lists.freedesktop.org, linux-kselftest@vger.kernel.org, shuah@kernel.org, Rob Herring , Frank Rowand , linux-nvdimm@lists.01.org, richard@nod.at, Knut Omang , kieran.bingham@ideasonboard.com, Joel Stanley , jdike@addtoit.com, Tim.Bird@sony.com, Kees Cook , linux-um@lists.infradead.org, rostedt@goodmis.org, Julia Lawall , dan.j.williams@intel.com, kunit-dev@googlegroups.com, Greg KH , Linux Kernel Mailing List , Daniel Vetter , mpe@ellerman.id.au, joe@perches.com, khilman@baylibre.com On Mon, Dec 03, 2018 at 03:34:57PM -0800, Brendan Higgins wrote: > On Thu, Nov 29, 2018 at 7:34 PM Luis Chamberlain wrote: > > > > On Wed, Nov 28, 2018 at 11:36:25AM -0800, Brendan Higgins wrote: > > > diff --git a/arch/um/kernel/trap.c b/arch/um/kernel/trap.c > > > index cced829460427..bf90e678b3d71 100644 > > > --- a/arch/um/kernel/trap.c > > > +++ b/arch/um/kernel/trap.c > > > @@ -201,6 +201,12 @@ void segv_handler(int sig, struct siginfo *unused_si, struct uml_pt_regs *regs) > > > segv(*fi, UPT_IP(regs), UPT_IS_USER(regs), regs); > > > } > > > > > > +static void segv_run_catcher(jmp_buf *catcher, void *fault_addr) > > > +{ > > > + current->thread.fault_addr = fault_addr; > > > + UML_LONGJMP(catcher, 1); > > > +} > > > + > > > /* > > > * We give a *copy* of the faultinfo in the regs to segv. > > > * This must be done, since nesting SEGVs could overwrite > > > @@ -219,7 +225,10 @@ unsigned long segv(struct faultinfo fi, unsigned long ip, int is_user, > > > if (!is_user && regs) > > > current->thread.segv_regs = container_of(regs, struct pt_regs, regs); > > > > > > - if (!is_user && (address >= start_vm) && (address < end_vm)) { > > > + catcher = current->thread.fault_catcher; > > > > This and.. > > > > > + if (catcher && current->thread.is_running_test) > > > + segv_run_catcher(catcher, (void *) address); > > > + else if (!is_user && (address >= start_vm) && (address < end_vm)) { > > > flush_tlb_kernel_vm(); > > > goto out; > > > } > > > > *not this* > > I don't understand. Are you saying the previous block of code is good > and this one is bad? No, I was saying that the above block of code is a functional change, but I was also pointing out other areas which were not and could be folded into a separate atomic patch where no functionality changes. > > > @@ -246,12 +255,10 @@ unsigned long segv(struct faultinfo fi, unsigned long ip, int is_user, > > > address = 0; > > > } > > > > > > - catcher = current->thread.fault_catcher; > > > if (!err) > > > goto out; > > > else if (catcher != NULL) { > > > - current->thread.fault_addr = (void *) address; > > > - UML_LONGJMP(catcher, 1); > > > + segv_run_catcher(catcher, (void *) address); > > > } > > > else if (current->thread.fault_addr != NULL) > > > panic("fault_addr set but no fault catcher"); > > > > But with this seems one atomic change which should be submitted > > separately, its just a helper. Think it would make the actual > > change needed easier to review, ie, your needed changes would > > be smaller and clearer for what you need. > > Are you suggesting that I pull out the bits needed to implement abort > in the next patch and squash it into this one? No, I'm suggesting you can probably split this patch in 2, one which wraps things with no functional changes, and another which adds your changes. Luis _______________________________________________ linux-um mailing list linux-um@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-um