[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [RFC] docs/misra: List files in Xen originated from external sources


  • To: Michal Orzel <michal.orzel@xxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Thu, 17 Nov 2022 12:00:23 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=jasQHcIxN6EHMhwqWyfdrKJiQeUrsfV8YGhcHfUEZzc=; b=jQPkXT/9l7uAkrDi/g7+D5HWchXSF9k55UgAk9T/BJ1y0mDo/KDijvi/BsgrG5HmOh1wYxjH8ZJ1I0SR6MJxYkulVCdK6gE9yH74Z8eupFjal7cnWuKymrhB7tVxuuYSZjSxfPbCc8TOgcbCEWDs/qxNSzwoE3Zug+/9YMasdu/HLXVC8XZ6V4VRyVVqJC5HhSZ6tlX7XNXjNwY7WhQjgbFDyQbgXSP/2awdF0wwADUSVuX4fndU3H7CQZezEA6QtxsIsw21CGKpIGe36j+bMQmaNQqQ2MivHsVihD0rfAW9qov6hlKZNtC7K3hkEf3rseIBZ6pNCqDHNIFBLVnYbg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Z9yhTOWOm0bVfgWe5GH3eyM4Zj7xnTvROdy2+FNMdeVwdt3gRgZzm7CWu3KCceHvx1lMDr5+aXW/zFKxXK1vHMrReuvjzMKX5GSZYl+NZ50CZ1RxMgSih4IgynhHGCzDyN2mnjG95/fjq9G2/8m1D6DTgqJ1MXw2bO/oTWdllDb3X6ROTHDYWsPKZ9oqiLDEtLJ9mTjrbohj/vtgS7r6L5iIxK55XVsEflEQSzFzEXXgBYkBMu3btRv9mc6YB9Lym3kKUuFS3Idl9ox8NodHBZ2lYS3x+xwJrGtXsb2m0Xh7Y8kf0dXvKywx8hyShLj3rs9e69aZ9pgCf72J3E49wA==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • Delivery-date: Thu, 17 Nov 2022 11:00:38 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 17.11.2022 11:39, Michal Orzel wrote:
> On 17/11/2022 11:03, Jan Beulich wrote:
>> On 16.11.2022 10:20, Michal Orzel wrote:
>>> --- /dev/null
>>> +++ b/docs/misra/external-files.txt
>>> @@ -0,0 +1,70 @@
>>> +External files in Xen
>>> +=====================
>>> +
>>> +The following table lists files present in the Xen codebase that originated
>>> +from external sources e.g. Linux kernel. The purpose of this table is to:
>>> + - keep track of the external files in the Xen codebase,
>>> + - help with the review process (e.g. changes done to the files that did 
>>> not
>>> +   diverge, shall be first submitted to the external projects and then
>>> +   backported to Xen),
>>> + - act as a base for listing files to exclude from MISRA checkers.
>>> +
>>> +NOTES:
>>> +1) The files shall be listed in alphabetical order.
>>
>> But then you don't?
> True, it is alphabetical order with directories having a precedence.

Which is kind of surprising and, at least for me, confusing.

>>> +2) The origin of the files derived from the projects other than Linux, 
>>> shall
>>> +   be specified within the () placed next to the path.
>>
>> Might it be more generally useful to have another column, then not only
>> stating the project but also the path it lives at there (or lived at the
>> time of cloning)?
> We though about it. Would be a good idea but can be quite challenging for 
> files
> that appeared in Xen before switching to git (difficult to establish the time 
> of cloning in such cases).
> 
>>
>>> +3) The table shall be updated when importing new files from external 
>>> projects.
>>> +4) At the moment, only the source files are listed in the table.
>>> +
>>> +| Relative path to xen/                     | Diverged from | Subject to   
>>>     |
>>> +|                                           | origin? [Y/N] | backports? 
>>> [Y/N] |
>>> ++-------------------------------------------+---------------+------------------+
>>> +| arch/arm/arm64/cpufeature.c               | N             | Y            
>>>     |
>>> +| arch/arm/arm64/insn.c                     | N             | Y            
>>>     |
>>> +| arch/arm/arm64/lib/find_next_bit.c        | N             | Y            
>>>     |
>>> +| arch/x86/acpi/boot.c                      | Y             | ?            
>>>     |
>>> +| arch/x86/acpi/lib.c                       | ?             | ?            
>>>     |
>>
>> This was simply split off from boot.c, which I'd be inclined to take to
>> mean Y in the "diverged" column. In the backports column I'd prefer to
>> keep ? for both, or any other indicator taken to mean "maybe / uncertain".
>>
>> What about arch/x86/acpi/cpufreq/* and other stuff in arch/x86/acpi/?
>>
>>> +| arch/x86/cpu/mcheck/mce-apei.c            | N             | Y            
>>>     |
>>> +| arch/x86/cpu/mcheck/non-fatal.c           | ?             | Y            
>>>     |
>>
>> Even before disappearing in 2.6.32 the file was different from Linux'es,
>> simply because we don't have anything like schedule_delayed_work(). So
>> it's pretty clearly Y for "diverged".
>>
>>> +| arch/x86/cpu/mtrr/*                       | Y             | N            
>>>     |
>>> +| arch/x86/cpu/amd.c                        | Y             | N            
>>>     |
>>> +| arch/x86/cpu/centaur.c                    | Y             | N            
>>>     |
>>> +| arch/x86/cpu/common.c                     | Y             | N            
>>>     |
>>> +| arch/x86/cpu/hygon.c                      | Y             | N            
>>>     |
>>> +| arch/x86/cpu/intel_cacheinfo.c            | Y             | Y            
>>>     |
>>> +| arch/x86/cpu/intel.c                      | Y             | N            
>>>     |
>>> +| arch/x86/cpu/mwait-idle.c                 | Y             | Y            
>>>     |
>>> +| arch/x86/genapic/*                        | Y             | N            
>>>     |
>>> +| arch/x86/x86_64/mmconf-fam10h.c           | N             | Y            
>>>     |
>>> +| arch/x86/dmi_scan.c                       | Y             | ?            
>>>     |
>>> +| arch/x86/mpparse.c                        | Y             | ?            
>>>     |
>>
>> Like above I'd like to keep ? (or alike) here, as neither Y nor N are
>> fully accurate.
>>
>>> +| arch/x86/srat.c                           | Y             | N            
>>>     |
>>
>> What about common/cpu.c?
>>
>>> +| common/libfdt/* (libfdt)                  | N             | Y            
>>>     |
>>> +| common/lz4/decompress.c                   | N             | Y            
>>>     |
>>> +| common/ubsan/ubsan.c                      | Y             | Y            
>>>     |
>>> +| common/xz/*                               | N             | Y            
>>>     |
>>> +| common/zstd/*                             | N             | Y            
>>>     |
>>> +| common/bitmap.c                           | N             | Y            
>>>     |
>>> +| common/bunzip2.c                          | N             | Y            
>>>     |
>>> +| common/earlycpio.c                        | N             | Y            
>>>     |
>>> +| common/inflate.c                          | N             | Y            
>>>     |
>>
>> What about common/notifier.c?
>>
>>> +| common/radix-tree.c                       | N             | Y            
>>>     |
>>
>> What about common/rcupdate.c? (Stopping at this in this regard:
>> It's unclear by what criteria you have gone. Even as simple an
>> indicator as "Copyright (C) ... Linus Torvalds" was apparently not
> Please see [1]
> 
>> used. Similarly mentioning criteria for considering a file
>> "diverged" would be very helpful to spell out, even if there's
>> likely some fuzziness involved there.)
> 
> We would need to pre-define some criteria to avoid having a long 
> justifications.
> Any ideas?

Well, changing just #include-s to fit Xen's model shouldn't count as
divergence. But coding style conversion already may. I'm afraid
criteria here depend very much on the purpose, and hence I don't
feel qualified to suggest any.

>>> +| common/un*.c                              | N             | Y            
>>>     |
>>> +| crypto/rijndael.c (OpenBSD)               | N             | Y            
>>>     |
>>> +| crypto/vmac.c (public domain)             | N             | Y            
>>>     |
>>> +| drivers/acpi/apei/*                       | N             | Y            
>>>     |
>>
>> I'm not sure of the N here.
>>
>>> +| drivers/acpi/tables/*                     | N             | Y            
>>>     |
>>> +| drivers/acpi/utilities/*                  | N             | Y            
>>>     |
>>> +| drivers/acpi/hwregs.c                     | N             | Y            
>>>     |
>>> +| drivers/acpi/numa.c                       | ?             | Y            
>>>     |
>>
>> Y
>>
>>> +| drivers/acpi/osl.c                        | Y             | Y            
>>>     |
>>> +| drivers/acpi/reboot.c                     | N             | Y            
>>>     |
>>> +| drivers/acpi/tables.c                     | ?             | Y            
>>>     |
>>
>> Y
>>
>> What about drivers/cpufreq/*, drivers/char/ehci-dbgp.c,
>> drivers/char/xhci-dbc.c, and drivers/video/font*? What about some of
>> the stuff under tools/, especially tools/kconfig/?
> 
> [1]
> For the first shot, the criteria was to list files using different coding 
> style than Xen,
> especially the ones using tabs instead of spaces. As I indicated before, the 
> list may not be
> completed, hence a gentle ask to list the missing ones. Some of the files you 
> mentioned
> use Xen coding style + there is no information in the git history that they 
> originated from
> external sources. This is why, the maintainers who are the addressee of this 
> RFC should have
> a better knowledge of the origin of such files.

Hmm. Please forgive me being blunt, but to me this then looks like
offloading work to people who shouldn't be required to invest
meaningful amounts of time. But maybe that's just me viewing it this
way ... Yet this is particularly relevant if ...

> As for the files under tools/, FWICS they are being filtered-out from MISRA 
> checks, hence I
> did not list them.

... the goal here then indeed is use for MISRA alone. I did e.g. ask
whether it wouldn't be worthwhile to more precisely describe the
origin of files because at some point in the past it was also
proposed to arrange for some more automatic monitoring of changes
being applied at their origins for files we have cloned. Which
obviously first of all requires establishing an association between
our files and their origins.

Jan



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.