From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751685AbdG0Gbd (ORCPT ); Thu, 27 Jul 2017 02:31:33 -0400 Received: from mx2.suse.de ([195.135.220.15]:39029 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751663AbdG0Gbc (ORCPT ); Thu, 27 Jul 2017 02:31:32 -0400 Date: Thu, 27 Jul 2017 08:31:28 +0200 From: Jean Delvare To: LKML Cc: Jani Nikula , Daniel Vetter , "Rafael J. Wysocki" , Jeff Garzik , Hans de Goede Subject: [PATCH] firmware: dmi: Optimize dmi_matches Message-ID: <20170727083128.7ab14de9@endymion> Organization: SUSE Linux X-Mailer: Claws Mail 3.13.2 (GTK+ 2.24.31; x86_64-suse-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Function dmi_matches can me made a bit faster: * The documented purpose of dmi_initialized is to catch too early calls to dmi_check_system(). I'm not fully convinced it justifies slowing down the initialization of all systems out there, but at least the check should not have been moved from dmi_check_system() to dmi_matches(). dmi_matches() is being called for every entry of the table passed to dmi_check_system(), causing the same redundant check to be performed again and again. So move it back to dmi_check_system(), reverting this specific portion of commit d7b1956fed33 ("DMI: Introduce dmi_first_match to make the interface more flexible"). * Don't check for the exact_match flag again when we already know its value. Signed-off-by: Jean Delvare Cc: Jani Nikula Cc: Daniel Vetter Cc: Rafael J. Wysocki Cc: Jeff Garzik --- Regarding dmi_initialized, I don't think it makes sense to check for a possible bad initialization order at run time on every system when it is all decided at build time. If a developer introduces a new call to dmi_check_system() and it is too early in the initialization sequence, I believe he/she would notice upon first testing, and a comment to his/her intention in the source code would serve the same purpose without the worldwide performance penalty. Would anyone object to such a change? drivers/firmware/dmi_scan.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) --- linux-4.12.orig/drivers/firmware/dmi_scan.c 2017-07-27 08:15:38.283519194 +0200 +++ linux-4.12/drivers/firmware/dmi_scan.c 2017-07-27 08:26:23.013053058 +0200 @@ -784,19 +784,20 @@ static bool dmi_matches(const struct dmi { int i; - WARN(!dmi_initialized, KERN_ERR "dmi check: not initialized yet.\n"); - for (i = 0; i < ARRAY_SIZE(dmi->matches); i++) { int s = dmi->matches[i].slot; if (s == DMI_NONE) break; if (dmi_ident[s]) { - if (!dmi->matches[i].exact_match && - strstr(dmi_ident[s], dmi->matches[i].substr)) - continue; - else if (dmi->matches[i].exact_match && - !strcmp(dmi_ident[s], dmi->matches[i].substr)) - continue; + if (dmi->matches[i].exact_match) { + if (!strcmp(dmi_ident[s], + dmi->matches[i].substr)) + continue; + } else { + if (strstr(dmi_ident[s], + dmi->matches[i].substr)) + continue; + } } /* No match */ @@ -832,6 +833,8 @@ int dmi_check_system(const struct dmi_sy int count = 0; const struct dmi_system_id *d; + WARN(!dmi_initialized, KERN_ERR "dmi check: not initialized yet.\n"); + for (d = list; !dmi_is_end_of_table(d); d++) if (dmi_matches(d)) { count++; -- Jean Delvare SUSE L3 Support