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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 653EEC433EF for ; Tue, 15 Feb 2022 07:11:07 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 9E3DE6B0078; Tue, 15 Feb 2022 02:11:06 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 9934C6B007B; Tue, 15 Feb 2022 02:11:06 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 881F26B007D; Tue, 15 Feb 2022 02:11:06 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0022.hostedemail.com [216.40.44.22]) by kanga.kvack.org (Postfix) with ESMTP id 764F76B0078 for ; Tue, 15 Feb 2022 02:11:06 -0500 (EST) Received: from smtpin24.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id 2D253180AD837 for ; Tue, 15 Feb 2022 07:11:06 +0000 (UTC) X-FDA: 79144142532.24.180CD28 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf21.hostedemail.com (Postfix) with ESMTP id 99A501C0002 for ; Tue, 15 Feb 2022 07:11:05 +0000 (UTC) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id B4E436158C; Tue, 15 Feb 2022 07:11:04 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id B9BB7C340EC; Tue, 15 Feb 2022 07:10:56 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1644909064; bh=loxfJNPJZCU7xFx+mlZ0BgFDRoU9Xb7lz1Nf2gofob4=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=W+14GcqnJPUq2wfMoDt80IZWh46UZJ1BScWaCynkF9GrXmpRwbdJLstkoFB6V8goZ nim1a1iRTWP6M+BmkpAQXalD0apKYgwqGl5Gv83xEhYgj2J6d5curnMQ7U1ADVB3Fl 0yuplf1b12BgRgG58UEkbvDz9B7xfvYXLtAjEJniuy4DUkGYRp8IAf+wcQAJ3CWRQc WYy5y2T53S7KcVaTvMERdo/Tdfrvgm4FhwSPtrbjZ5/akfYgv+tZxm0DiJs/YJdlsL b0MFRHea8Ceih/3Q6HI6rk8Iw9NxnXrZNUL8RpeBnVq3+sWx+elkdebPzYaVJjZMKP GHUkatxLlvmKQ== Date: Tue, 15 Feb 2022 09:10:51 +0200 From: Mike Rapoport To: Martin Fernandez Cc: Kees Cook , linux-kernel@vger.kernel.org, linux-efi@vger.kernel.org, platform-driver-x86@vger.kernel.org, linux-mm@kvack.org, tglx@linutronix.de, mingo@redhat.com, bp@alien8.de, dave.hansen@linux.intel.com, x86@kernel.org, hpa@zytor.com, ardb@kernel.org, dvhart@infradead.org, andy@infradead.org, gregkh@linuxfoundation.org, rafael@kernel.org, akpm@linux-foundation.org, daniel.gutson@eclypsium.com, hughsient@gmail.com, alex.bazhaniuk@eclypsium.com, alison.schofield@intel.com Subject: Re: [PATCH v6 3/6] x86/e820: Refactor range_update and range_remove Message-ID: References: <20220203164328.203629-1-martin.fernandez@eclypsium.com> <20220203164328.203629-4-martin.fernandez@eclypsium.com> <202202071325.F8450B3B2D@keescook> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspam-User: X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 99A501C0002 X-Stat-Signature: z8u7aqg6w9af1ymwyodadyndgutxjqw3 Authentication-Results: imf21.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=W+14Gcqn; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf21.hostedemail.com: domain of rppt@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=rppt@kernel.org X-HE-Tag: 1644909065-504270 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: Hi Martin, On Tue, Feb 08, 2022 at 06:01:21PM -0300, Martin Fernandez wrote: > On 2/8/22, Mike Rapoport wrote: > > On Mon, Feb 07, 2022 at 01:45:40PM -0800, Kees Cook wrote: > >> On Thu, Feb 03, 2022 at 01:43:25PM -0300, Martin Fernandez wrote: > >> > __e820__range_update and e820__range_remove had a very similar > >> > implementation with a few lines different from each other, the lines > >> > that actually perform the modification over the e820_table. The > >> > similiraties were found in the checks for the different cases on how > >> > each entry intersects with the given range (if it does at all). These > >> > checks were very presice and error prone so it was not a good idea to > >> > have them in both places. > >> > >> Yay removing copy/paste code! :) > > > > Removing copy/paste is nice but diffstat of > > > > arch/x86/kernel/e820.c | 383 ++++++++++++++++++++++++++++++----------- > > 1 file changed, 283 insertions(+), 100 deletions(-) > > > > does not look nice even accounting for lots of comments :( > > > > I didn't look closely, but diffstat clues that the refactoring making > > things much more complex. > > > > Yes, that diffstat surprised me as well. > > I have to mention that 110 of those lines are kerneldocs and blank > lines, which is quite a lot. Also you have to take into account that I > expanded most of the function definitions for better formatting, which > also took some space. At last I had time to look more closely and I think that using a set of callbacks is over-complicated. I think this can be done way simpler, e.g like this (untested) draft: https://git.kernel.org/rppt/h/x86/e820-update-range > And as I was able to focus the "hard" part of the problem into a > single function, testing can be done easily as Kees suggested and I'm > planning to do so in the next patch. -- Sincerely yours, Mike.