Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Thu, 21 Nov 2019 17:58:24 +0100
From:   Ingo Molnar <mingo@kernel.org>
To:     Thomas Gleixner <tglx@linutronix.de>
Cc:     Davidlohr Bueso <dave@stgolabs.net>, peterz@infradead.org,
        bp@alien8.de, x86@kernel.org, linux-kernel@vger.kernel.org,
        Davidlohr Bueso <dbueso@suse.de>
Subject: [PATCH] x86/mm/pat: Simplify the free_memtype() control flow
Message-ID: <20191121165824.GA12042@gmail.com>
References: <20191021231924.25373-1-dave@stgolabs.net>
 <20191021231924.25373-2-dave@stgolabs.net>
 <alpine.DEB.2.21.1911201901270.29534@nanos.tec.linutronix.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <alpine.DEB.2.21.1911201901270.29534@nanos.tec.linutronix.de>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


* Thomas Gleixner <tglx@linutronix.de> wrote:

> On Mon, 21 Oct 2019, Davidlohr Bueso wrote:
> >  int rbt_memtype_check_insert(struct memtype *new,
> >  			     enum page_cache_mode *ret_type)
> >  {
> >  	int err = 0;
> >  
> >  	err = memtype_rb_check_conflict(&memtype_rbroot, new->start, new->end,
> > -						new->type, ret_type);
> > -
> > -	if (!err) {
> > -		if (ret_type)
> > -			new->type = *ret_type;
> > -
> > -		new->subtree_max_end = new->end;
> > -		memtype_rb_insert(&memtype_rbroot, new);
> > -	}
> > +					new->type, ret_type);
> > +	if (err)
> > +		goto done;
> 
> Please return err here. That goto is pointless.
> 
> > +
> > +	if (ret_type)
> > +		new->type = *ret_type;
> > +	memtype_interval_insert(new, &memtype_rbroot);
> > +done:
> >  	return err;
> >  }
> 
> Other than that.
> 
> Reviewed-by: Thomas Gleixner <tglx@linutronix.de>

Thanks - I rebased the v2 version in x86/mm with this cleanup included.

Two days ago I noticed a similarly quirky control flow in free_memtype() 
as well, and fixed it via the patch below.

	Ingo

==============>
From: Ingo Molnar <mingo@kernel.org>
Date: Tue, 19 Nov 2019 10:18:56 +0100
Subject: [PATCH] x86/mm/pat: Simplify the free_memtype() control flow

Simplify/streamline the quirky handling of the pat_pagerange_is_ram() logic,
and get rid of the 'err' local variable.

Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/mm/pat.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/arch/x86/mm/pat.c b/arch/x86/mm/pat.c
index ea7da7e62e17..af049920e59a 100644
--- a/arch/x86/mm/pat.c
+++ b/arch/x86/mm/pat.c
@@ -656,7 +656,6 @@ int reserve_memtype(u64 start, u64 end, enum page_cache_mode req_type,
 
 int free_memtype(u64 start, u64 end)
 {
-	int err = -EINVAL;
 	int is_range_ram;
 	struct memtype *entry;
 
@@ -671,14 +670,10 @@ int free_memtype(u64 start, u64 end)
 		return 0;
 
 	is_range_ram = pat_pagerange_is_ram(start, end);
-	if (is_range_ram == 1) {
-
-		err = free_ram_pages_type(start, end);
-
-		return err;
-	} else if (is_range_ram < 0) {
+	if (is_range_ram == 1)
+		return free_ram_pages_type(start, end);
+	if (is_range_ram < 0)
 		return -EINVAL;
-	}
 
 	spin_lock(&memtype_lock);
 	entry = memtype_erase(start, end);