From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dfw.source.kernel.org (dfw.source.kernel.org [IPv6:2604:1380:4641:c500::1]) by sourceware.org (Postfix) with ESMTPS id 3AE903858D38 for ; Tue, 11 Apr 2023 06:34:12 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 3AE903858D38 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=linuxfoundation.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linuxfoundation.org Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id C78FF60BC9; Tue, 11 Apr 2023 06:34:11 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id B4ADFC433D2; Tue, 11 Apr 2023 06:34:10 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1681194851; bh=x+oH/HMKu73BCpFd8IM0DhA30uUo8CLngL7hAosyFdY=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=MlONgH5GrXXlf5h6VLeHlLslNecKJZXb24RI+5PjWyd8gctDAHUuci4CEziLuXGtq ewaqggQL3HcYvIxtsGPMLFguVDX4T6QoYzWaySXUAFwW6oGUV4NpGCU5wDooAyoTpZ V6UB4TZttKhqX7K1BUZ8K+mivIMIoQO9yaYvO4Rk= Date: Tue, 11 Apr 2023 08:34:08 +0200 From: Greg Kroah-Hartman To: John Moon Cc: Masahiro Yamada , Nathan Chancellor , Nick Desaulniers , Nicolas Schier , linux-kbuild@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-arm-msm@vger.kernel.org, Randy Dunlap , Arnd Bergmann , Bjorn Andersson , Todd Kjos , Matthias Maennich , Giuliano Procida , kernel-team@android.com, libabigail@sourceware.org, Jordan Crouse , Trilok Soni , Satya Durga Srinivasu Prabhala , Elliot Berman , Guru Das Srinagesh Subject: Re: [PATCH v5 1/2] check-uapi: Introduce check-uapi.sh Message-ID: <2023041136-donator-faceplate-5f91@gregkh> References: <20230407203456.27141-1-quic_johmoo@quicinc.com> <20230407203456.27141-2-quic_johmoo@quicinc.com> <2023041015-lunar-dandelion-1b4e@gregkh> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Spam-Status: No, score=-3.6 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Mon, Apr 10, 2023 at 04:32:49PM -0700, John Moon wrote: > > > According to this tool, it looks like we broke a lot of UAPI > > > headers in the previous MW (between v6.2 and v6.3-rc1). > > > > That's not ok, and needs to be fixed, otherwise this is useless as no > > one can rely on it at all. > > > > Right, there are several classes of false positives that we've documented > and when examining thousands of commits at time, it'll flag many things. > > For some comparison, if you run checkpatch on the same changeset > (v6.2..v6.3-rc1), you get 995 errors and 7,313 warnings. Still, checkpatch > is helpful for spot-checks. checkpatch.pl does not matter, it is a "hint", and many patches explicitly ignore it (think about patches in the staging tree, you could fix up one checkpatch issue for a line, but ignore another one as you are not supposed to mix them up.) Also for some subsystems, checkpatch does not matter because their codebase is old and follows different rules. And in some places, checkpatch is just wrong, because it's a perl script and can not really parse code. So NEVER use that as a comparison to the user/kernel abi please. It's a false comparison. > "./scripts/check-uapi.sh -b v6.3-rc1 -p v6.2" flags 36 out of the 911 files > checked. Of those 36, 19 fell into the currently documented false positive > categories: > > Enum expansion: 17 > Expanding into padded/reserved fields: 2 > > Beyond those, the tool appears to be flagging legitimate breakages. > > Some fit into the definition of "intentional breakages" where support is > being dropped or something is being refactored: > > File removals: > - include/uapi/drm/i810_drm.h > - include/uapi/drm/mga_drm.h > - include/uapi/drm/r128_drm.h > - include/uapi/drm/savage_drm.h > - include/uapi/drm/sis_drm.h > - include/uapi/drm/via_drm.h > - include/uapi/linux/meye.h > > File moves: > - include/uapi/misc/habanalabs.h > > Removal of struct: > - include/uapi/linux/uuid.h (5e6a51787fef) > - include/uapi/linux/mei.h (failed due to uuid.h) > - include/uapi/linux/ublk_cmd.h (failed due to uuid.h) > > Others do not seem to be intentional: > > Addition/use of flex arrays: > - include/uapi/linux/rseq.h (f7b01bb0b57f) > - include/uapi/scsi/scsi_bsg_mpi3mr.h (c6f2e6b6eaaf) That is not a breakage, that's a tool problem. > Type change: > - include/uapi/scsi/scsi_bsg_ufs.h (3f5145a615238) Again, not a real breakage, size is still the same. > Additions into existing struct: > - include/uapi/drm/amdgpu_drm.h (b299221faf9b) > - include/uapi/linux/perf_event.h (09519ec3b19e) > - include/uapi/linux/virtio_blk.h (95bfec41bd3d) Adding data to the end of a structure is a well-known way to extend the api, in SOME instances if it is used properly. So again, not a break. > Is there something I'm missing that makes these changes false positives? If > so, I'd be happy to add on to the documentation and work towards a way to > filter them out. > > In the mean time, we will start a thread on the libabigail mailing list to > see if there's a way to add flags such as --ignore-enum-expansion, > --ignore-expansion-into-reserved-fields, etc. Enum expansion seems to be > making up the largest portion of false positives, so would be the best thing > to filter out. Increasing enums is in no way an abi break unless the size of the structure changes. Using reserved fields too is not a breakage. So yes, it looks like the tooling needs some work in order for us to be able to use this properly, digging through false positives like this is going to make it not used at all. thanks, greg k-h