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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id CB2A4C433FE for ; Mon, 21 Nov 2022 08:13:46 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229678AbiKUINn (ORCPT ); Mon, 21 Nov 2022 03:13:43 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44012 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229476AbiKUINl (ORCPT ); Mon, 21 Nov 2022 03:13:41 -0500 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C78C8DF79 for ; Mon, 21 Nov 2022 00:12:46 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1669018365; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=hMypv4aKfhWhASDI7Pu73Edum8Uj+KeekUIMkw/Gd5o=; b=BMx09v6Jk+a0/pFdWNoy3Yvig6YHBGIdFcJfSAzxKXN2v6SsnOW8U0QDCtghU/VnmnYo+w SCMO7BGoP99WZpdfyS7uwLWOB6acEFqmcZDGEiEOITYEU8YPucRYb8K9QdWh8U1d7rpaZp KUwgn+41Lz/i2Rz7qoBoXHQDKSs3u+E= Received: from mail-ed1-f70.google.com (mail-ed1-f70.google.com [209.85.208.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-556-PGAMsevvMOuSwJdOVDg8zA-1; Mon, 21 Nov 2022 03:12:44 -0500 X-MC-Unique: PGAMsevvMOuSwJdOVDg8zA-1 Received: by mail-ed1-f70.google.com with SMTP id e15-20020a056402190f00b00461b0576620so6226511edz.2 for ; Mon, 21 Nov 2022 00:12:44 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=hMypv4aKfhWhASDI7Pu73Edum8Uj+KeekUIMkw/Gd5o=; b=qFwNxuvG0MziKWq2Qhh3nu/bUy20R2Q3ypEJdkUGM4A3wHZYyquqDWFypdNnjjdghl d2zN2e82htZRRijK0xrTKujaKwg2CTO2wFHTzMh/BaXN5HByn1szKgFk7yhYaWf5jH44 AfpH8g5xg6OT+KPmrfZmIN7vZI2th9N4GKKKyOYNqGwk5BR7MEqKVEwyqSICeDhs+hQ+ M86lmSIrLUyrePN/KaW9wRcy9xLI1MLSKEyFdzNvPpL7xaGqzWIyvoXr6ZHH6mej3mZ4 86wK/s43vTK8ymjRDdvN488jWDtQgxqN+U4js9qEGjom6XTYSEnaz0Nncb4JD1huU58Q jWeQ== X-Gm-Message-State: ANoB5pn4O8O1bYCOBf5bskSJqtM2xgaAfce1T8IWNAa7OPgymrN3MTIN UqkNiEOe1qaFj+5ils3Kux3QVCU0GmoMH8X30T6zR1KEk0MHZv2KIF5vjUCW8uHcWmtMx+n0PQ1 uKyYF9JBKORsgw/8Mvr9iH2Ea X-Received: by 2002:aa7:d88c:0:b0:468:ffca:6982 with SMTP id u12-20020aa7d88c000000b00468ffca6982mr12715939edq.282.1669018363389; Mon, 21 Nov 2022 00:12:43 -0800 (PST) X-Google-Smtp-Source: AA0mqf6ybpIG8u2hcigGGwEqFA04vd1rDRh+RTTrzfenOkLjnnicFchPhqeipAMyMzvy7IvlmIpLhQ== X-Received: by 2002:aa7:d88c:0:b0:468:ffca:6982 with SMTP id u12-20020aa7d88c000000b00468ffca6982mr12715917edq.282.1669018363172; Mon, 21 Nov 2022 00:12:43 -0800 (PST) Received: from [10.40.98.142] ([78.108.130.194]) by smtp.gmail.com with ESMTPSA id s19-20020aa7cb13000000b0045cf4f72b04sm4845464edt.94.2022.11.21.00.12.41 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 21 Nov 2022 00:12:42 -0800 (PST) Message-ID: <87852fc9-0757-7e58-35a2-90cccf970f5c@redhat.com> Date: Mon, 21 Nov 2022 09:12:41 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.3.1 Subject: Re: [PATCH 11/18] platform/x86: int3472: fix object shared between several modules Content-Language: en-US To: Masahiro Yamada Cc: Andy Shevchenko , Alexander Lobakin , linux-kbuild@vger.kernel.org, Nicolas Schier , Jens Axboe , Boris Brezillon , Borislav Petkov , Tony Luck , Miquel Raynal , Vladimir Oltean , Alexandre Belloni , Derek Chickles , Ioana Ciornei , Salil Mehta , Sunil Goutham , Grygorii Strashko , Daniel Scally , Mark Brown , NXP Linux Team , netdev@vger.kernel.org, linux-kernel@vger.kernel.org References: <20221119225650.1044591-1-alobakin@pm.me> <20221119225650.1044591-12-alobakin@pm.me> <961a7d7e-c917-86a8-097b-5961428e9ddc@redhat.com> From: Hans de Goede In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kbuild@vger.kernel.org Hi, On 11/21/22 00:45, Masahiro Yamada wrote: > On Mon, Nov 21, 2022 at 5:55 AM Hans de Goede wrote: >> >> Hi, >> >> On 11/20/22 14:55, Andy Shevchenko wrote: >>> On Sat, Nov 19, 2022 at 11:08:17PM +0000, Alexander Lobakin wrote: >>>> common.o is linked to both intel_skl_int3472_{discrete,tps68470}: >>>> >>>>> scripts/Makefile.build:252: ./drivers/platform/x86/intel/int3472/Makefile: >>>>> common.o is added to multiple modules: intel_skl_int3472_discrete >>>>> intel_skl_int3472_tps68470 >>>> >>>> Although both drivers share one Kconfig option >>>> (CONFIG_INTEL_SKL_INT3472), it's better to not link one object file >>>> into several modules (and/or vmlinux). >>>> Under certain circumstances, such can lead to the situation fixed by >>>> commit 637a642f5ca5 ("zstd: Fixing mixed module-builtin objects"). >>>> >>>> Introduce the new module, intel_skl_int3472_common, to provide the >>>> functions from common.o to both discrete and tps68470 drivers. This >>>> adds only 3 exports and doesn't provide any changes to the actual >>>> code. >> >> Replying to Andy's reply here since I never saw the original submission >> which was not Cc-ed to platform-driver-x86@vger.kernel.org . >> >> As you mention already in the commit msg, the issue from: >> >> commit 637a642f5ca5 ("zstd: Fixing mixed module-builtin objects") >> >> is not an issue here since both modules sharing the .o file are >> behind the same Kconfig option. >> >> So there is not really an issue here and common.o is tiny, so >> small chances are it does not ever increase the .ko size >> when looking a the .ko size rounded up to a multiple of >> the filesystem size. >> >> At the same time adding an extra module does come with significant >> costs, it will eat up at least 1 possibly more then 1 fs blocks >> (I don't know what the module header size overhead is). >> >> And it needs to be loaded separately and module loading is slow; >> and it will grow the /lib/modules//modules.* sizes. >> >> So nack from me for this patch, since I really don't see >> it adding any value. > > > > > This does have a value. > > This clarifies the ownership of the common.o, > in other words, makes KBUILD_MODNAME deterministic. > > > If an object belongs to a module, > KBUILD_MODNAME is defined as the module name. > > If an object is always built-in, > KBUILD_MODNAME is defined as the basename of the object. > > > > Here is a question: > if common.o is shared by two modules intel_skl_int3472_discrete > and intel_skl_int3472_tps68470, what should KBUILD_MODNAME be? > > > I see some patch submissions relying on the assumption that > KBUILD_MODNAME is unique. > We cannot determine KBUILD_MODNAME correctly if an object is shared > by multiple modules. > > > > > > > BTW, this patch is not the way I suggested. > The Suggested-by should not have been there > (or at least Reported-by) > > > You argued "common.o is tiny", so I would vote for > making them inline functions, like > > > https://lore.kernel.org/linux-kbuild/20221119225650.1044591-2-alobakin@pm.me/T/#u Yes just moving the contents of common.c to static inline helpers in common.h would be much better. If someone creates such a patch, please do not forget to Cc platform-driver-x86@vger.kernel.org Regards, Hans > > > > > > > > >> Regards, >> >> Hans >> >> >> >> >> >>> >>> ... >>> >>>> +MODULE_IMPORT_NS(INTEL_SKL_INT3472); >>>> + >>> >>> Redundant blank line. You may put it to be last MODULE_*() in the file, if you >>> think it would be more visible. >>> >>>> MODULE_DESCRIPTION("Intel SkyLake INT3472 ACPI Discrete Device Driver"); >>>> MODULE_AUTHOR("Daniel Scally "); >>>> MODULE_LICENSE("GPL v2"); >>> >>> ... >>> >>>> +MODULE_IMPORT_NS(INTEL_SKL_INT3472); >>>> + >>>> MODULE_DESCRIPTION("Intel SkyLake INT3472 ACPI TPS68470 Device Driver"); >>>> MODULE_AUTHOR("Daniel Scally "); >>>> MODULE_LICENSE("GPL v2"); >>> >>> Ditto. And the same to all your patches. >>> >> > >