Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1609458

Re: [PATCH v10 00/11] uapi: export all headers under uapi directories

Show key headers only | View raw

Hi Nocolas,


2017-03-24 18:03 GMT+09:00 Nicolas Dichtel <nicolas.dichtel@6wind.com>:
> Le 24/03/2017 à 09:42, Masahiro Yamada a écrit :
>> Hi Nicolas,
>>
>>
>> 2017-03-24 17:32 GMT+09:00 Nicolas Dichtel <nicolas.dichtel@6wind.com>:
>>> 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

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH v10 00/11] uapi: export all headers under uapi directories Nicolas Dichtel <nicolas.dichtel@6wind.com> - 2017-03-14 14:00 +0100
  [PATCH v10 04/11] Makefile.headersinst: cleanup input files Nicolas Dichtel <nicolas.dichtel@6wind.com> - 2017-03-14 14:00 +0100
  [PATCH v10 08/11] cryptouser.h: fix include from userland Nicolas Dichtel <nicolas.dichtel@6wind.com> - 2017-03-14 14:00 +0100
  [PATCH v10 01/11] h8300: put bitsperlong.h in uapi Nicolas Dichtel <nicolas.dichtel@6wind.com> - 2017-03-14 14:00 +0100
    Re: [PATCH v10 01/11] h8300: put bitsperlong.h in uapi Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-03-27 07:40 +0200
      Re: [PATCH v10 01/11] h8300: put bitsperlong.h in uapi Nicolas Dichtel <nicolas.dichtel@6wind.com> - 2017-03-27 11:50 +0200
  [PATCH v10 03/11] x86: stop exporting msr-index.h to userland Nicolas Dichtel <nicolas.dichtel@6wind.com> - 2017-03-14 14:00 +0100
  Re: [PATCH v10 00/11] uapi: export all headers under uapi directories Nicolas Dichtel <nicolas.dichtel@6wind.com> - 2017-03-24 09:40 +0100
    Re: [PATCH v10 00/11] uapi: export all headers under uapi directories Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-03-24 09:50 +0100
      Re: [PATCH v10 00/11] uapi: export all headers under uapi directories Nicolas Dichtel <nicolas.dichtel@6wind.com> - 2017-03-24 10:10 +0100
        Re: [PATCH v10 00/11] uapi: export all headers under uapi directories Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-03-27 07:40 +0200
          Re: [PATCH v10 00/11] uapi: export all headers under uapi directories Nicolas Dichtel <nicolas.dichtel@6wind.com> - 2017-03-27 11:50 +0200
            Re: [PATCH v10 00/11] uapi: export all headers under uapi directories Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-03-27 17:40 +0200
      [PATCH v11 11/12] uapi: export all arch specifics directories Nicolas Dichtel <nicolas.dichtel@6wind.com> - 2017-03-27 14:30 +0200
      [PATCH v11 08/12] cryptouser.h: fix include from userland Nicolas Dichtel <nicolas.dichtel@6wind.com> - 2017-03-27 14:30 +0200
      [PATCH v11 02/12] nios2: put setup.h in uapi Nicolas Dichtel <nicolas.dichtel@6wind.com> - 2017-03-27 14:30 +0200
      [PATCH v11 05/12] Makefile.headersinst: remove destination-y option Nicolas Dichtel <nicolas.dichtel@6wind.com> - 2017-03-27 14:30 +0200
      [PATCH v11 04/12] Makefile.headersinst: cleanup input files Nicolas Dichtel <nicolas.dichtel@6wind.com> - 2017-03-27 14:30 +0200
      [PATCH v11 03/12] x86: stop exporting msr-index.h to userland Nicolas Dichtel <nicolas.dichtel@6wind.com> - 2017-03-27 14:30 +0200
      [PATCH v11 07/12] btrfs_tree.h: fix include from userland Nicolas Dichtel <nicolas.dichtel@6wind.com> - 2017-03-27 14:30 +0200
      [PATCH v11 00/12] uapi: export all headers under uapi directories Nicolas Dichtel <nicolas.dichtel@6wind.com> - 2017-03-27 14:30 +0200
        [PATCH v11 09/12] smc_diag.h: fix include from userland Nicolas Dichtel <nicolas.dichtel@6wind.com> - 2017-03-27 14:30 +0200
        [PATCH v11 01/12] h8300: put bitsperlong.h in uapi Nicolas Dichtel <nicolas.dichtel@6wind.com> - 2017-03-27 14:30 +0200
        [PATCH v11 06/12] uapi: includes linux/types.h before exporting files Nicolas Dichtel <nicolas.dichtel@6wind.com> - 2017-03-27 14:30 +0200
        [PATCH v11 12/12] arch/include: remove empty Kbuild files Nicolas Dichtel <nicolas.dichtel@6wind.com> - 2017-03-27 14:40 +0200
        Re: [PATCH v11 00/12] uapi: export all headers under uapi directories Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-03-27 17:40 +0200

csiph-web