Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751987AbdC0FgJ (ORCPT ); Mon, 27 Mar 2017 01:36:09 -0400 Received: from condef-08.nifty.com ([202.248.20.73]:64865 "EHLO condef-08.nifty.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751925AbdC0FgB (ORCPT ); Mon, 27 Mar 2017 01:36:01 -0400 DKIM-Filter: OpenDKIM Filter v2.10.3 conssluserg-05.nifty.com v2R5QqXC011783 X-Nifty-SrcIP: [209.85.161.173] MIME-Version: 1.0 In-Reply-To: References: <1489496093-15315-1-git-send-email-nicolas.dichtel@6wind.com> <365519b5-d69e-34b7-fa2f-f11b8fabe69f@6wind.com> From: Masahiro Yamada Date: Mon, 27 Mar 2017 14:26:51 +0900 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v10 00/11] uapi: export all headers under uapi directories To: Nicolas Dichtel Cc: Arnd Bergmann , Michal Marek , Linux Kbuild mailing list , Linux Kernel Mailing List , linux-arch@vger.kernel.org, David Airlie , "David S. Miller" , Russell King , bp@alien8.de, slash.tmp@free.fr, Daniel Vetter , rmk+kernel@armlinux.org.uk, msalter@redhat.com, jengelh@inai.de, hch@infradead.org, Tobias Klauser , mpe@ellerman.id.au, Ingo Molnar , Thomas Gleixner , "Dmitry V. Levin" , Herbert Xu , linux-rdma@vger.kernel.org Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by mail.home.local id v2R5bFXw001228 Content-Length: 3927 Lines: 137 Hi Nocolas, 2017-03-24 18:03 GMT+09:00 Nicolas Dichtel : > Le 24/03/2017 à 09:42, Masahiro Yamada a écrit : >> Hi Nicolas, >> >> >> 2017-03-24 17:32 GMT+09:00 Nicolas Dichtel : >>> Le 14/03/2017 à 13:54, Nicolas Dichtel a écrit : >>>> Patches #1 and #2 are just cleanup: some exported headers were still under >>>> a non-uapi directory. Patch #3 is a fix to avoid exporting a file that was >>>> not under an uapi directory. >>>> After these three patches, all exported headers are under an uapi directory: >>>> path #4 stops searching files in non uapi directories. >>>> The patch #5 was spotted by code review: there is no in-tree user of this >>>> functionality. >>>> Patch #6 fixes some warnings/errors reported by 0-day tests. >>>> Patch #7 to #9 fix some errors when the corresponding files are included by >>>> userland. >>>> Patches #10 and #11 remove the need to list explicitly headers. Now all files >>>> under an uapi directory are exported. >>>> >>>> This series has been tested with a 'make headers_install' on x86 and a >>>> 'make headers_install_all'. I've checked the result of both commands. >>>> >>>> This patch is built on top of masahiroy/linux-kbuild.git#for-next (v4.11-rc1). >>>> I didn't find any conflict with v4.11-rc2. >>> Masahiro, is this series under review or do you expect something else on my side? >>> >> >> Under review. >> Please give me time to take a closer look. >> Sorry for the delay. > No problem, take your time. I just wanted to be sure to not miss something ;-) > > As a whole, this series is amazing. Thanks for your great work! I added some comments, but they are trivial. I wanted to leave comments/questions on 10/11, but I could not find 10/11 in my mailbox. I do not know why. I am leaving comments on the cover-letter, the following are related to 10/11. [1] >mandatory-y += $(foreach hdr,$(opt-header), \ > $(if \ > $(wildcard \ > $(srctree)/arch/$(SRCARCH)/include/uapi/asm/$(hdr) \ > $(srctree)/arch/$(SRCARCH)/include/asm/$(hdr) \ > ), \ > $(hdr) \ > )) What is this actually checking? If ARCH has its own (uapi/)asm/{kvm.h,kvm_para.h,a.out.h}, they are added to mandatory-y, then they are checked if they exist. But, we know they exist. This check reminds us only when we added asm/*.h but forgot to add uapi/asm/*.h $(srctree)/arch/$(SRCARCH)/include/uapi/asm/$(hdr) seems unneeded at least. (perhaps, the whole hunk might be unneeded.) [2] >ifeq ($(wildcard $(srctree)/arch/$(SRCARCH)/include/uapi/asm/a.out.h \ > $(srctree)/arch/$(SRCARCH)/include/asm/a.out.h),) >header-n += a.out.h >endif > >ifeq ($(wildcard $(srctree)/arch/$(SRCARCH)/include/uapi/asm/kvm.h \ > $(srctree)/arch/$(SRCARCH)/include/asm/kvm.h),) >header-n += kvm.h >endif > >ifeq ($(wildcard $(srctree)/arch/$(SRCARCH)/include/uapi/asm/kvm_para.h \ > $(srctree)/arch/$(SRCARCH)/include/asm/kvm_para.h),) >header-n += kvm_para.h >endif This series intends all headers are exported from uapi/, correct? Do we still need to check $(srctree)/arch/$(SRCARCH)/include/asm/*.h ? (related to [1]) [3] >--- 7.1 header-n > >header-n is essentially used by include/uapi/linux/Kbuild to avoid >exporting specific headers (e.g. kvm.h) on architectures that do not >support it. It should be avoided as much as possible. Going forward, header-y will be never used because uapi/ is exported by default. So, I wonder if we could rename this into something clearer. Kbuild supports "no-clean-files". (Please see ./Kbuild for its usage) I guess this notation seems clearer when we want to negate the default behavior. Can you consider "no-export", "no-export-files", "no-export-headers" or whatever you like? Thanks! -- Best Regards Masahiro Yamada