Re: [Nouveau] Problem with kernel 5.10-rc5
Hi Mark, Presumably this is the same thing as the other thread you started, which seemed to conclude with a working patch, and this email was just stuck in the moderation queue? Cheers, -ilia On Sun, Nov 29, 2020 at 6:02 PM Mark Hounschell wrote: > > I am not subscribed to this mailing list so please CC me directly for > any response. > > I'm running an older "NVIDIA Corporation G70 [GeForce 7800 GT] (rev a1)" > card in a newer BIOSTAR B550GTA MB. Kernel 5.10-rc5 does not work. I get > no virtual consoles or X screen. > > I have not tried any other RC versions. Only rc5. Kernel 5.9.10 works > fine. I am running a SuSE Leap-15.2 system. > > I have another machine with an NVIDIA Corporation GT218 [GeForce 210] > (rev a2) that works just fine with 5.10-rc5. > > > On the broken machine the screen goes blank, the leds on the front turn > orange as if the PC was turned off. > > My dmesg seems to show the cause: > > [5.825702] fb0: switching to nouveaufb from VESA VGA > [6.242991] nouveau :14:00.0: vgaarb: deactivate vga console > [6.243066] nouveau :14:00.0: NVIDIA G70 (047200a1) > [6.443671] nouveau :14:00.0: bios: version 05.70.02.13.7b > [6.443948] nouveau :14:00.0: fb: 256 MiB GDDR3 > [6.498039] nouveau :14:00.0: DRM: VRAM: 250 MiB > [6.498041] nouveau :14:00.0: DRM: GART: 512 MiB > [6.498044] nouveau :14:00.0: DRM: TMDS table version 1.1 > [6.498046] nouveau :14:00.0: DRM: DCB version 3.0 > [6.498049] nouveau :14:00.0: DRM: DCB outp 00: 01000300 0028 > [6.498051] nouveau :14:00.0: DRM: DCB outp 01: 03000302 > [6.498053] nouveau :14:00.0: DRM: DCB outp 02: 04011310 0028 > [6.498055] nouveau :14:00.0: DRM: DCB outp 03: 04011312 > [6.498057] nouveau :14:00.0: DRM: DCB outp 04: 020223f1 0040c080 > [6.498060] nouveau :14:00.0: DRM: DCB conn 00: 1030 > [6.498062] nouveau :14:00.0: DRM: DCB conn 01: 2130 > [6.498064] nouveau :14:00.0: DRM: DCB conn 02: 0210 > [6.498065] nouveau :14:00.0: DRM: DCB conn 03: 0211 > [6.498067] nouveau :14:00.0: DRM: DCB conn 04: 0213 > [6.502949] nouveau :14:00.0: DRM: Setting dpms mode 3 on TV > encoder (output 4) > [6.585845] nouveau :14:00.0: DRM: failed to map fb: -28 > [6.585949] [drm] Initialized nouveau 1.3.1 20120801 for :14:00.0 > on minor 0 > > If I can do anything else please let me know. > > Regards > Mark > > ___ > Nouveau mailing list > Nouveau@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/nouveau ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [Intel-wired-lan] [PATCH 000/141] Fix fall-through warnings for Clang
On Wed, 25 Nov 2020, Nick Desaulniers wrote: > So developers and distributions using Clang can't have > -Wimplicit-fallthrough enabled because GCC is less strict (which has > been shown in this thread to lead to bugs)? We'd like to have nice > things too, you know. > Apparently the GCC developers don't want you to have "nice things" either. Do you think that the kernel should drop gcc in favour of clang? Or do you think that a codebase can somehow satisfy multiple checkers and their divergent interpretations of the language spec? > This is not a shiny new warning; it's already on for GCC and has existed > in both compilers for multiple releases. > Perhaps you're referring to the compiler feature that lead to the ill-fated, tree-wide /* fallthrough */ patch series. When the ink dries on the C23 language spec and the implementations figure out how to interpret it then sure, enforce the warning for new code -- the cost/benefit analysis is straight forward. However, the case for patching existing mature code is another story. ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH 000/141] Fix fall-through warnings for Clang
On Thu, Nov 26, 2020 at 4:28 PM Geert Uytterhoeven wrote: > > The maintainer is not necessarily the owner/author of the code, and > thus may not know the intent of the code. Agreed, I was not blaming maintainers -- just trying to point out that the problem is there :-) In those cases, it is still very useful: we add the `fallthrough` and a comment saying `FIXME: fallthrough intended? Figure this out...`. Thus a previous unknown unknown is now a known unknown. And no new unknown unknowns will be introduced since we enabled the warning globally. > BTW, you cannot mindlessly fix the latter, as you cannot know if > "(a == b)" or "((a = b))" was intended, without understanding the code > (and the (possibly unavailable) data sheet, and the hardware, ...). That's right, I was referring to the cases where the compiler saves someone time from a typo they just made. Cheers, Miguel ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH 000/141] Fix fall-through warnings for Clang
On Mon, Nov 23, 2020 at 05:32:51PM -0800, Nick Desaulniers wrote:
> On Sun, Nov 22, 2020 at 8:17 AM Kees Cook wrote:
> >
> > On Fri, Nov 20, 2020 at 11:51:42AM -0800, Jakub Kicinski wrote:
> > > If none of the 140 patches here fix a real bug, and there is no change
> > > to machine code then it sounds to me like a W=2 kind of a warning.
> >
> > FWIW, this series has found at least one bug so far:
> > https://lore.kernel.org/lkml/CAFCwf11izHF=g1mGry1fE5kvFFFrxzhPSM6qKAO8gxSp=kr...@mail.gmail.com/
>
> So looks like the bulk of these are:
> switch (x) {
> case 0:
> ++x;
> default:
> break;
> }
>
> I have a patch that fixes those up for clang:
> https://reviews.llvm.org/D91895
I still think this isn't right -- it's a case statement that runs off
the end without an explicit flow control determination. I think Clang is
right to warn for these, and GCC should also warn.
--
Kees Cook
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [Intel-wired-lan] [PATCH 000/141] Fix fall-through warnings for Clang
On Wed, Nov 25, 2020 at 5:24 PM Jakub Kicinski wrote: > > And just to spell it out, > > case ENUM_VALUE1: > bla(); > break; > case ENUM_VALUE2: > bla(); > default: > break; > > is a fairly idiomatic way of indicating that not all values of the enum > are expected to be handled by the switch statement. It looks like a benign typo to me -- `ENUM_VALUE2` does not follow the same pattern like `ENUM_VALUE1`. To me, the presence of the `default` is what indicates (explicitly) that not everything is handled. > Applying a real patch set and then getting a few follow ups the next day > for trivial coding things like fallthrough missing or static missing, > just because I didn't have the full range of compilers to check with > before applying makes me feel pretty shitty, like I'm not doing a good > job. YMMV. The number of compilers, checkers, static analyzers, tests, etc. we use keeps going up. That, indeed, means maintainers will miss more things (unless maintainers do more work than before). But catching bugs before they happen is *not* a bad thing. Perhaps we could encourage more rebasing in -next (while still giving credit to bots and testers) to avoid having many fixing commits afterwards, but that is orthogonal. I really don't think we should encourage the feeling that a maintainer is doing a bad job if they don't catch everything on their reviews. Any review is worth it. Maintainers, in the end, are just the "guaranteed" reviewers that decide when the code looks reasonable enough. They should definitely not feel pressured to be perfect. Cheers, Miguel ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [Intel-wired-lan] [PATCH 000/141] Fix fall-through warnings for Clang
On Tue, Nov 24, 2020 at 11:24 PM Finn Thain wrote: > > These statements are not "missing" unless you presume that code written > before the latest de facto language spec was written should somehow be > held to that spec. There is no "language spec" the kernel adheres to. Even if it did, kernel code is not frozen. If an improvement is found, it should be applied. > If the 'fallthrough' statement is not part of the latest draft spec then > we should ask why not before we embrace it. Being that the kernel still > prefers -std=gnu89 you might want to consider what has prevented > -std=gnu99 or -std=gnu2x etc. The C standard has nothing to do with this. We use compiler extensions of several kinds, for many years. Even discounting those extensions, the kernel is not even conforming to C due to e.g. strict aliasing. I am not sure what you are trying to argue here. But, since you insist: yes, the `fallthrough` attribute is in the current C2x draft. Cheers, Miguel ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [Intel-wired-lan] [PATCH 000/141] Fix fall-through warnings for Clang
On Wed, 25 Nov 2020 04:24:27 -0800 Nick Desaulniers wrote: > I even agree that most of the churn comes from > > case 0: > ++x; > default: > break; And just to spell it out, case ENUM_VALUE1: bla(); break; case ENUM_VALUE2: bla(); default: break; is a fairly idiomatic way of indicating that not all values of the enum are expected to be handled by the switch statement. I really hope the Clang folks are reasonable and merge your patch. > If trivial patches are adding too much to your workload, consider > training a co-maintainer or asking for help from one of your reviewers > whom you trust. I don't doubt it's hard to find maintainers, but > existing maintainers should go out of their way to entrust > co-maintainers especially when they find their workload becomes too > high. And reviewing/picking up trivial patches is probably a great > way to get started. If we allow too much knowledge of any one > subsystem to collect with one maintainer, what happens when that > maintainer leaves the community (which, given a finite lifespan, is an > inevitability)? The burn out point is about enjoying your work and feeling that it matters. It really doesn't make much difference if you're doing something you don't like for 12 hours every day or only in shifts with another maintainer. You'll dislike it either way. Applying a real patch set and then getting a few follow ups the next day for trivial coding things like fallthrough missing or static missing, just because I didn't have the full range of compilers to check with before applying makes me feel pretty shitty, like I'm not doing a good job. YMMV. ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH 000/141] Fix fall-through warnings for Clang
On Mon, Nov 23, 2020 at 10:39 PM James Bottomley wrote: > On Mon, 2020-11-23 at 19:56 +0100, Miguel Ojeda wrote: > > On Mon, Nov 23, 2020 at 4:58 PM James Bottomley > > wrote: ... > > But if we do the math, for an author, at even 1 minute per line > > change and assuming nothing can be automated at all, it would take 1 > > month of work. For maintainers, a couple of trivial lines is noise > > compared to many other patches. > > So you think a one line patch should take one minute to produce ... I > really don't think that's grounded in reality. I suppose a one line > patch only takes a minute to merge with b4 if no-one reviews or tests > it, but that's not really desirable. In my practice most of the one line patches were either to fix or to introduce quite interesting issues. 1 minute is 2-3 orders less than usually needed for such patches. That's why I don't like churn produced by people who often even didn't compile their useful contributions. -- With Best Regards, Andy Shevchenko ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [Intel-wired-lan] [PATCH 000/141] Fix fall-through warnings for Clang
On Wed, 25 Nov 2020, Nick Desaulniers wrote: > On Wed, Nov 25, 2020 at 1:33 PM Finn Thain > wrote: > > > > Or do you think that a codebase can somehow satisfy multiple checkers > > and their divergent interpretations of the language spec? > > Have we found any cases yet that are divergent? I don't think so. There are many implementations, so I think you are guaranteed to find more divergence if you look. That's because the spec is full of language like this: "implementations are encouraged not to emit a diagnostic" and "implementations are encouraged to issue a diagnostic". Some implementations will decide to not emit (under the premise that vast amounts of existing code would have to get patched until the compiler goes quiet) whereas other implementations will decide to emit (under the premise that the author is doing the checking and not the janitor or the packager). > It sounds to me like GCC's cases it warns for is a subset of Clang's. > Having additional coverage with Clang then should ensure coverage for > both. > If that claim were true, the solution would be simple. (It's not.) For the benefit of projects that enable -Werror and projects that nominated gcc as their preferred compiler, clang would simply need a flag to enable conformance with gcc by suppressing those additional warnings that clang would normally produce. This simple solution is, of course, completely unworkable, since it would force clang to copy some portion of gcc's logic (rewritten under LLVM's unique license) and then to track future changes to that portion of gcc indefinitely. ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH 07/15] drm/i915: Remove references to struct drm_device.pdev
Quoting Thomas Zimmermann (2020-11-24 13:38:16)
> Using struct drm_device.pdev is deprecated. Convert i915 to struct
> drm_device.dev. No functional changes.
>
> Signed-off-by: Thomas Zimmermann
> Cc: Jani Nikula
> Cc: Joonas Lahtinen
> Cc: Rodrigo Vivi
Any chance of sharing used a cocci script(s)? think this will
hit many in-flight series, so life would made easier :)
Or is this done manually? I notice a few places hoist the pdev
variable and others repeat the call. Regardless, using the cocci
script as baseline would make review bit more comforting.
The gvt changes would go in through the gvt tree, and we also
probably need to split between drm-intel-next/drm-intel-gt-next,
too.
Jani or Rodrigo, any thoughts?
Regards, Joonas
> ---
> drivers/gpu/drm/i915/display/intel_bios.c | 2 +-
> drivers/gpu/drm/i915/display/intel_cdclk.c| 14 ++---
> drivers/gpu/drm/i915/display/intel_csr.c | 2 +-
> drivers/gpu/drm/i915/display/intel_dsi_vbt.c | 2 +-
> drivers/gpu/drm/i915/display/intel_fbdev.c| 2 +-
> drivers/gpu/drm/i915/display/intel_gmbus.c| 2 +-
> .../gpu/drm/i915/display/intel_lpe_audio.c| 5 +++--
> drivers/gpu/drm/i915/display/intel_opregion.c | 6 +++---
> drivers/gpu/drm/i915/display/intel_overlay.c | 2 +-
> drivers/gpu/drm/i915/display/intel_panel.c| 4 ++--
> drivers/gpu/drm/i915/display/intel_quirks.c | 2 +-
> drivers/gpu/drm/i915/display/intel_sdvo.c | 2 +-
> drivers/gpu/drm/i915/display/intel_vga.c | 8
> drivers/gpu/drm/i915/gem/i915_gem_phys.c | 6 +++---
> drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 2 +-
> drivers/gpu/drm/i915/gt/intel_engine_cs.c | 2 +-
> drivers/gpu/drm/i915/gt/intel_ggtt.c | 10 +-
> drivers/gpu/drm/i915/gt/intel_ppgtt.c | 2 +-
> drivers/gpu/drm/i915/gt/intel_rc6.c | 4 ++--
> drivers/gpu/drm/i915/gt/intel_reset.c | 6 +++---
> drivers/gpu/drm/i915/gvt/cfg_space.c | 5 +++--
> drivers/gpu/drm/i915/gvt/firmware.c | 10 +-
> drivers/gpu/drm/i915/gvt/gtt.c| 12 +--
> drivers/gpu/drm/i915/gvt/gvt.c| 6 +++---
> drivers/gpu/drm/i915/gvt/kvmgt.c | 4 ++--
> drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
> drivers/gpu/drm/i915/i915_drv.c | 20 +--
> drivers/gpu/drm/i915/i915_drv.h | 2 +-
> drivers/gpu/drm/i915/i915_gem_gtt.c | 4 ++--
> drivers/gpu/drm/i915/i915_getparam.c | 5 +++--
> drivers/gpu/drm/i915/i915_gpu_error.c | 2 +-
> drivers/gpu/drm/i915/i915_irq.c | 6 +++---
> drivers/gpu/drm/i915/i915_pmu.c | 5 +++--
> drivers/gpu/drm/i915/i915_suspend.c | 4 ++--
> drivers/gpu/drm/i915/i915_switcheroo.c| 4 ++--
> drivers/gpu/drm/i915/i915_vgpu.c | 2 +-
> drivers/gpu/drm/i915/intel_device_info.c | 2 +-
> drivers/gpu/drm/i915/intel_region_lmem.c | 8
> drivers/gpu/drm/i915/intel_runtime_pm.c | 2 +-
> drivers/gpu/drm/i915/intel_uncore.c | 4 ++--
> .../gpu/drm/i915/selftests/mock_gem_device.c | 1 -
> drivers/gpu/drm/i915/selftests/mock_gtt.c | 2 +-
> 42 files changed, 99 insertions(+), 98 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c
> b/drivers/gpu/drm/i915/display/intel_bios.c
> index 4cc949b228f2..8879676372a3 100644
> --- a/drivers/gpu/drm/i915/display/intel_bios.c
> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> @@ -2088,7 +2088,7 @@ bool intel_bios_is_valid_vbt(const void *buf, size_t
> size)
>
> static struct vbt_header *oprom_get_vbt(struct drm_i915_private *dev_priv)
> {
> - struct pci_dev *pdev = dev_priv->drm.pdev;
> + struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev);
> void __iomem *p = NULL, *oprom;
> struct vbt_header *vbt;
> u16 vbt_size;
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c
> b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index c449d28d0560..a6e13208dc50 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -96,7 +96,7 @@ static void fixed_450mhz_get_cdclk(struct drm_i915_private
> *dev_priv,
> static void i85x_get_cdclk(struct drm_i915_private *dev_priv,
>struct intel_cdclk_config *cdclk_config)
> {
> - struct pci_dev *pdev = dev_priv->drm.pdev;
> + struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev);
> u16 hpllcc = 0;
>
> /*
> @@ -138,7 +138,7 @@ static void i85x_get_cdclk(struct drm_i915_private
> *dev_priv,
> static void i915gm_get_cdclk(struct drm_i915_private *dev_priv,
> struct intel_cdclk_config *cdclk_config)
> {
> - struct pci_dev *pdev = dev_priv->drm.pdev;
> + struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev);
> u16 gcfgc = 0;
>
>
Re: [Nouveau] [PATCH 000/141] Fix fall-through warnings for Clang
On 24/11/2020 21:25, Kees Cook wrote: > I still think this isn't right -- it's a case statement that runs off > the end without an explicit flow control determination. Proves too much — for instance case foo: case bar: thing; break; doesn't require a fallthrough; after case foo:, and afaik no-one is suggesting it should. Yet it, too, is "a case statement that runs off the end without an explicit flow control determination". -ed ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH 000/141] Fix fall-through warnings for Clang
On Mon, Nov 23, 2020 at 07:58:06AM -0800, James Bottomley wrote: > On Mon, 2020-11-23 at 15:19 +0100, Miguel Ojeda wrote: > > On Sun, Nov 22, 2020 at 11:36 PM James Bottomley > > wrote: > > > It's not about the risk of the changes it's about the cost of > > > implementing them. Even if you discount the producer time (which > > > someone gets to pay for, and if I were the engineering manager, I'd > > > be unhappy about), the review/merge/rework time is pretty > > > significant in exchange for six minor bug fixes. Fine, when a new > > > compiler warning comes along it's certainly reasonable to see if we > > > can benefit from it and the fact that the compiler people think > > > it's worthwhile is enough evidence to assume this initially. But > > > at some point you have to ask whether that assumption is supported > > > by the evidence we've accumulated over the time we've been using > > > it. And if the evidence doesn't support it perhaps it is time to > > > stop the experiment. > > > > Maintainers routinely review 1-line trivial patches, not to mention > > internal API changes, etc. > > We're also complaining about the inability to recruit maintainers: > > https://www.theregister.com/2020/06/30/hard_to_find_linux_maintainers_says_torvalds/ > > And burn out: > > http://antirez.com/news/129 > > The whole crux of your argument seems to be maintainers' time isn't > important so we should accept all trivial patches ... I'm pushing back > on that assumption in two places, firstly the valulessness of the time > and secondly that all trivial patches are valuable. You're assuming burn out or recruitment problems is due to patch workload or too many "trivial" patches. In my experience, "other maintainers" is by far the biggest cause of burn out for my kernel maintenance work. Certainly arguing with a maintainer about some obviously-correct patch series must be a good example of this. Sean ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [Intel-wired-lan] [PATCH 000/141] Fix fall-through warnings for Clang
On Wed, 25 Nov 2020, Nick Desaulniers wrote: > On Wed, Nov 25, 2020 at 1:33 PM Finn Thain wrote: > > > > Or do you think that a codebase can somehow satisfy multiple checkers > > and their divergent interpretations of the language spec? > > Have we found any cases yet that are divergent? I don't think so. You mean, aside from -Wimplicit-fallthrough? I'm glad you asked. How about -Wincompatible-pointer-types and -Wframe-larger-than? All of the following files have been affected by divergent diagnostics produced by clang and gcc. arch/arm64/include/asm/neon-intrinsics.h arch/powerpc/xmon/Makefile drivers/gpu/drm/i915/Makefile drivers/gpu/drm/i915/i915_utils.h drivers/staging/media/atomisp/pci/atomisp_subdev.c fs/ext4/super.c include/trace/events/qla.h net/mac80211/rate.c tools/lib/string.c tools/perf/util/setup.py tools/scripts/Makefile.include And if I searched for 'smatch' or 'coverity' instead of 'clang' I'd probably find more divergence. Here are some of the relevant commits. 0738c8b5915c7eaf1e6007b441008e8f3b460443 9c87156cce5a63735d1218f0096a65c50a7a32aa babaab2f473817f173a2d08e410c25abf5ed0f6b 065e5e559555e2f100bc95792a8ef1b609bbe130 93f56de259376d7e4fff2b2d104082e1fa66e237 6c4798d3f08b81c2c52936b10e0fa872590c96ae b7a313d84e853049062011d78cb04b6decd12f5c 093b75ef5995ea35d7f6bdb6c7b32a42a1999813 And before you object, "but -Wconstant-logical-operand is a clang-only warning! it can't be divergent with gcc!", consider that the special cases added to deal with clang-only warnings have to be removed when gcc catches up, which is more churn. Now multiply that by the number of checkers you care about. ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH 000/141] Fix fall-through warnings for Clang
On Wed, Nov 25, 2020 at 11:44 PM Edward Cree wrote: > > To make the intent clear, you have to first be certain that you > understand the intent; otherwise by adding either a break or a > fallthrough to suppress the warning you are just destroying the > information that "the intent of this code is unknown". If you don't know what the intent of your own code is, then you *already* have a problem in your hands. > Figuring out the intent of a piece of unfamiliar code takes more > than 1 minute; just because > case foo: > thing; > case bar: > break; > produces identical code to > case foo: > thing; > break; > case bar: > break; > doesn't mean that *either* is correct — maybe the author meant What takes 1 minute is adding it *mechanically* by the author, i.e. so that you later compare whether codegen is the same. > to write > case foo: > return thing; > case bar: > break; > and by inserting that break you've destroyed the marker that > would direct someone who knew what the code was about to look > at that point in the code and spot the problem. Then it means you already have a bug. This patchset gives the maintainer a chance to notice it, which is a good thing. The "you've destroyed the market" claim is bogus, because: 1. you were not looking into it 2. you didn't notice the bug so far 3. is implicit -- harder to spot 4. is only useful if you explicitly take a look at this kind of bug. So why don't you do it now? > Thus, you *always* have to look at more than just the immediate > mechanical context of the code, to make a proper judgement that > yes, this was the intent. I find that is the responsibility of the maintainers and reviewers for tree-wide patches like this, assuming they want. They can also keep the behavior (and the bugs) without spending time. Their choice. > If you think that that sort of thing > can be done in an *average* time of one minute, then I hope you > stay away from code I'm responsible for! Please don't accuse others of recklessness or incompetence, especially if you didn't understand what they said. > A warning is only useful because it makes you *think* about the > code. If you suppress the warning without doing that thinking, > then you made the warning useless; and if the warning made you > think about code that didn't *need* it, then the warning was > useless from the start. We are not suppressing the warning. Quite the opposite, in fact. > So make your mind up: does Clang's stricter -Wimplicit-fallthrough > flag up code that needs thought (in which case the fixes take > effort both to author and to review) As I said several times already, it does take time to review if the maintainer wants to take the chance to see if they had a bug to begin with, but it does not require thought for the author if they just go for equivalent codegen. > or does it flag up code > that can be mindlessly "fixed" (in which case the warning is > worthless)? Proponents in this thread seem to be trying to > have it both ways. A warning is not worthless just because you can mindlessly fix it. There are many counterexamples, e.g. many checkpatch/lint/lang-format/indentation warnings, functional ones like the `if (a = b)` warning... Cheers, Miguel ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [Intel-wired-lan] [PATCH 000/141] Fix fall-through warnings for Clang
On Wed, Nov 25, 2020 at 12:53 AM Finn Thain wrote: > > I'm saying that supporting the official language spec makes more sense > than attempting to support a multitude of divergent interpretations of the > spec (i.e. gcc, clang, coverity etc.) Making the kernel strictly conforming is a ship that sailed long ago, for several reasons. Anyway, supporting several compilers and other tools, regardless of extensions, is valuable. > I'm also saying that the reason why we use -std=gnu89 is that existing > code was written in that language, not in ad hoc languages comprised of > collections of extensions that change with every release. No, we aren't particularly tied to `gnu89` or anything like that. We could actually go for `gnu11` already, since the minimum GCC and Clang support it. Even if a bit of code needs fixing, that shouldn't be a problem if someone puts the work. In other words, the kernel code is not frozen, nor are the features it uses from compilers. They do, in fact, change from time to time. > Thank you for checking. I found a free version that's only 6 weeks old: You're welcome! There are quite a few new attributes coming, mostly following C++ ones. > It will be interesting to see whether 6.7.11.5 changes once the various > implementations reach agreement. Not sure what you mean. The standard does not evolve through implementations' agreement (although standardizing existing practice is one of the best arguments to back a change). Cheers, Miguel ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH 000/141] Fix fall-through warnings for Clang
On Mon, Nov 23, 2020 at 9:38 PM James Bottomley wrote: > > So you think a one line patch should take one minute to produce ... I > really don't think that's grounded in reality. No, I have not said that. Please don't put words in my mouth (again). I have said *authoring* lines of *this* kind takes a minute per line. Specifically: lines fixing the fallthrough warning mechanically and repeatedly where the compiler tells you to, and doing so full-time for a month. For instance, take the following one from Gustavo. Are you really saying it takes 12 minutes (your number) to write that `break;`? diff --git a/drivers/gpu/drm/via/via_irq.c b/drivers/gpu/drm/via/via_irq.c index 24cc445169e2..a3e0fb5b8671 100644 --- a/drivers/gpu/drm/via/via_irq.c +++ b/drivers/gpu/drm/via/via_irq.c @@ -364,6 +364,7 @@ int via_wait_irq(struct drm_device *dev, void *data, struct drm_file *file_priv) irqwait->request.sequence += atomic_read(_irq->irq_received); irqwait->request.type &= ~_DRM_VBLANK_RELATIVE; + break; case VIA_IRQ_ABSOLUTE: break; default: > I suppose a one line > patch only takes a minute to merge with b4 if no-one reviews or tests > it, but that's not really desirable. I have not said that either. I said reviewing and merging those are noise compared to any complex patch. Testing should be done by the author comparing codegen. > Part of what I'm trying to measure is the "and useful" bit because > that's not a given. It is useful since it makes intent clear. It also catches actual bugs, which is even more valuable. > Well, you know, subsystems are very different in terms of the amount of > patches a maintainer has to process per release cycle of the kernel. > If a maintainer is close to capacity, additional patches, however > trivial, become a problem. If a maintainer has spare cycles, trivial > patches may look easy. First of all, voluntary maintainers choose their own workload. Furthermore, we already measure capacity in the `MAINTAINERS` file: maintainers can state they can only handle a few patches. Finally, if someone does not have time for a trivial patch, they are very unlikely to have any time to review big ones. > You seem to be saying that because you find it easy to merge trivial > patches, everyone should. Again, I have not said anything of the sort. Cheers, Miguel ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH 000/141] Fix fall-through warnings for Clang
Hi Miguel,
On Thu, Nov 26, 2020 at 3:54 PM Miguel Ojeda
wrote:
> On Wed, Nov 25, 2020 at 11:44 PM Edward Cree wrote:
> > To make the intent clear, you have to first be certain that you
> > understand the intent; otherwise by adding either a break or a
> > fallthrough to suppress the warning you are just destroying the
> > information that "the intent of this code is unknown".
>
> If you don't know what the intent of your own code is, then you
> *already* have a problem in your hands.
The maintainer is not necessarily the owner/author of the code, and
thus may not know the intent of the code.
> > or does it flag up code
> > that can be mindlessly "fixed" (in which case the warning is
> > worthless)? Proponents in this thread seem to be trying to
> > have it both ways.
>
> A warning is not worthless just because you can mindlessly fix it.
> There are many counterexamples, e.g. many
> checkpatch/lint/lang-format/indentation warnings, functional ones like
> the `if (a = b)` warning...
BTW, you cannot mindlessly fix the latter, as you cannot know if
"(a == b)" or "((a = b))" was intended, without understanding the code
(and the (possibly unavailable) data sheet, and the hardware, ...).
P.S. So far I've stayed out of this thread, as I like it if the compiler
flags possible mistakes. After all I was the one fixing new
"may be used uninitialized" warnings thrown up by gcc-4.1, until
(a bit later than) support for that compiler was removed...
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH 000/141] Fix fall-through warnings for Clang
On Thu, Nov 26, 2020 at 4:28 PM Geert Uytterhoeven wrote:
>
> Hi Miguel,
>
> On Thu, Nov 26, 2020 at 3:54 PM Miguel Ojeda
> wrote:
> > On Wed, Nov 25, 2020 at 11:44 PM Edward Cree wrote:
> > > To make the intent clear, you have to first be certain that you
> > > understand the intent; otherwise by adding either a break or a
> > > fallthrough to suppress the warning you are just destroying the
> > > information that "the intent of this code is unknown".
> >
> > If you don't know what the intent of your own code is, then you
> > *already* have a problem in your hands.
>
> The maintainer is not necessarily the owner/author of the code, and
> thus may not know the intent of the code.
>
> > > or does it flag up code
> > > that can be mindlessly "fixed" (in which case the warning is
> > > worthless)? Proponents in this thread seem to be trying to
> > > have it both ways.
> >
> > A warning is not worthless just because you can mindlessly fix it.
> > There are many counterexamples, e.g. many
> > checkpatch/lint/lang-format/indentation warnings, functional ones like
> > the `if (a = b)` warning...
>
> BTW, you cannot mindlessly fix the latter, as you cannot know if
> "(a == b)" or "((a = b))" was intended, without understanding the code
> (and the (possibly unavailable) data sheet, and the hardware, ...).
>
to allow assignments in if statements was clearly a mistake and if you
need outside information to understand the code, your code is the
issue already.
> P.S. So far I've stayed out of this thread, as I like it if the compiler
> flags possible mistakes. After all I was the one fixing new
> "may be used uninitialized" warnings thrown up by gcc-4.1, until
> (a bit later than) support for that compiler was removed...
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 --
> ge...@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like
> that.
> -- Linus Torvalds
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [Intel-wired-lan] [PATCH 000/141] Fix fall-through warnings for Clang
On Tue, 2020-11-24 at 13:32 -0800, Kees Cook wrote: > On Mon, Nov 23, 2020 at 08:31:30AM -0800, James Bottomley wrote: > > Really, no ... something which produces no improvement has no value > > at all ... we really shouldn't be wasting maintainer time with it > > because it has a cost to merge. I'm not sure we understand where > > the balance lies in value vs cost to merge but I am confident in > > the zero value case. > > What? We can't measure how many future bugs aren't introduced because > the kernel requires explicit case flow-control statements for all new > code. No but we can measure how vulnerable our current coding habits are to the mistake this warning would potentially prevent. I don't think it's wrong to extrapolate that if we had no instances at all of prior coding problems we likely wouldn't have any in future either making adopting the changes needed to enable the warning valueless ... that's the zero value case I was referring to above. Now, what we have seems to be about 6 cases (at least what's been shown in this thread) where a missing break would cause potentially user visible issues. That means the value of this isn't zero, but it's not a no-brainer massive win either. That's why I think asking what we've invested vs the return isn't a useless exercise. > We already enable -Wimplicit-fallthrough globally, so that's not the > discussion. The issue is that Clang is (correctly) even more strict > than GCC for this, so these are the remaining ones to fix for full > Clang coverage too. > > People have spent more time debating this already than it would have > taken to apply the patches. :) You mean we've already spent 90% of the effort to come this far so we might as well go the remaining 10% because then at least we get some return? It's certainly a clinching argument in defence procurement ... > This is about robustness and language wrangling. It's a big code- > base, and this is the price of our managing technical debt for > permanent robustness improvements. (The numbers I ran from Gustavo's > earlier patches were that about 10% of the places adjusted were > identified as legitimate bugs being fixed. This final series may be > lower, but there are still bugs being found from it -- we need to > finish this and shut the door on it for good.) I got my six patches by analyzing the lwn.net report of the fixes that was cited which had 21 of which 50% didn't actually change the emitted code, and 25% didn't have a user visible effect. But the broader point I'm making is just because the compiler people come up with a shiny new warning doesn't necessarily mean the problem it's detecting is one that causes us actual problems in the code base. I'd really be happier if we had a theory about what classes of CVE or bug we could eliminate before we embrace the next new warning. James ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [Intel-wired-lan] [PATCH 000/141] Fix fall-through warnings for Clang
On Mon, Nov 23, 2020 at 08:31:30AM -0800, James Bottomley wrote: > Really, no ... something which produces no improvement has no value at > all ... we really shouldn't be wasting maintainer time with it because > it has a cost to merge. I'm not sure we understand where the balance > lies in value vs cost to merge but I am confident in the zero value > case. What? We can't measure how many future bugs aren't introduced because the kernel requires explicit case flow-control statements for all new code. We already enable -Wimplicit-fallthrough globally, so that's not the discussion. The issue is that Clang is (correctly) even more strict than GCC for this, so these are the remaining ones to fix for full Clang coverage too. People have spent more time debating this already than it would have taken to apply the patches. :) This is about robustness and language wrangling. It's a big code-base, and this is the price of our managing technical debt for permanent robustness improvements. (The numbers I ran from Gustavo's earlier patches were that about 10% of the places adjusted were identified as legitimate bugs being fixed. This final series may be lower, but there are still bugs being found from it -- we need to finish this and shut the door on it for good.) -- Kees Cook ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH 000/141] Fix fall-through warnings for Clang
On 25/11/2020 00:32, Miguel Ojeda wrote: > I have said *authoring* lines of *this* kind takes a minute per line. > Specifically: lines fixing the fallthrough warning mechanically and > repeatedly where the compiler tells you to, and doing so full-time for > a month. > It is useful since it makes intent clear. To make the intent clear, you have to first be certain that you understand the intent; otherwise by adding either a break or a fallthrough to suppress the warning you are just destroying the information that "the intent of this code is unknown". Figuring out the intent of a piece of unfamiliar code takes more than 1 minute; just because case foo: thing; case bar: break; produces identical code to case foo: thing; break; case bar: break; doesn't mean that *either* is correct — maybe the author meant to write case foo: return thing; case bar: break; and by inserting that break you've destroyed the marker that would direct someone who knew what the code was about to look at that point in the code and spot the problem. Thus, you *always* have to look at more than just the immediate mechanical context of the code, to make a proper judgement that yes, this was the intent. If you think that that sort of thing can be done in an *average* time of one minute, then I hope you stay away from code I'm responsible for! One minute would be an optimistic target for code that, as the maintainer, one is already somewhat familiar with. For code that you're seeing for the first time, as is usually the case with the people doing these mechanical fix-a-warning patches, it's completely unrealistic. A warning is only useful because it makes you *think* about the code. If you suppress the warning without doing that thinking, then you made the warning useless; and if the warning made you think about code that didn't *need* it, then the warning was useless from the start. So make your mind up: does Clang's stricter -Wimplicit-fallthrough flag up code that needs thought (in which case the fixes take effort both to author and to review) or does it flag up code that can be mindlessly "fixed" (in which case the warning is worthless)? Proponents in this thread seem to be trying to have it both ways. -ed ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [Intel-wired-lan] [PATCH 000/141] Fix fall-through warnings for Clang
On Tue, 24 Nov 2020, Kees Cook wrote: > On Mon, Nov 23, 2020 at 08:31:30AM -0800, James Bottomley wrote: > > Really, no ... something which produces no improvement has no value at > > all ... we really shouldn't be wasting maintainer time with it because > > it has a cost to merge. I'm not sure we understand where the balance > > lies in value vs cost to merge but I am confident in the zero value > > case. > > What? We can't measure how many future bugs aren't introduced because > the kernel requires explicit case flow-control statements for all new > code. > These statements are not "missing" unless you presume that code written before the latest de facto language spec was written should somehow be held to that spec. If the 'fallthrough' statement is not part of the latest draft spec then we should ask why not before we embrace it. Being that the kernel still prefers -std=gnu89 you might want to consider what has prevented -std=gnu99 or -std=gnu2x etc. > We already enable -Wimplicit-fallthrough globally, so that's not the > discussion. The issue is that Clang is (correctly) even more strict than > GCC for this, so these are the remaining ones to fix for full Clang > coverage too. > Seems to me you should be patching the compiler. When you have consensus among the language lawyers you'll have more credibility with those being subjected to enforcement. ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [Intel-wired-lan] [PATCH 000/141] Fix fall-through warnings for Clang
On Wed, 25 Nov 2020, Miguel Ojeda wrote: > > The C standard has nothing to do with this. We use compiler extensions > of several kinds, for many years. Even discounting those extensions, the > kernel is not even conforming to C due to e.g. strict aliasing. I am not > sure what you are trying to argue here. > I'm saying that supporting the official language spec makes more sense than attempting to support a multitude of divergent interpretations of the spec (i.e. gcc, clang, coverity etc.) I'm also saying that the reason why we use -std=gnu89 is that existing code was written in that language, not in ad hoc languages comprised of collections of extensions that change with every release. > But, since you insist: yes, the `fallthrough` attribute is in the > current C2x draft. > Thank you for checking. I found a free version that's only 6 weeks old: http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2583.pdf It will be interesting to see whether 6.7.11.5 changes once the various implementations reach agreement. ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [Intel-wired-lan] [PATCH 000/141] Fix fall-through warnings for Clang
On Tue, Nov 24, 2020 at 11:05:35PM -0800, James Bottomley wrote: > Now, what we have seems to be about 6 cases (at least what's been shown > in this thread) where a missing break would cause potentially user > visible issues. That means the value of this isn't zero, but it's not > a no-brainer massive win either. That's why I think asking what we've > invested vs the return isn't a useless exercise. The number is much higher[1]. If it were 6 in the entire history of the kernel, I would agree with you. :) Some were fixed _before_ Gustavo's effort too, which I also count towards the idea of "this is a dangerous weakness in C, and now we have stopped it forever." > But the broader point I'm making is just because the compiler people > come up with a shiny new warning doesn't necessarily mean the problem > it's detecting is one that causes us actual problems in the code base. > I'd really be happier if we had a theory about what classes of CVE or > bug we could eliminate before we embrace the next new warning. But we did! It was long ago justified and documented[2], and even links to the CWE[3] for it. This wasn't random joy over discovering a new warning we could turn on, this was turning on a warning that the compiler folks finally gave us to handle an entire class of flaws. If we need to update the code-base to address it not a useful debate -- that was settled already, even if you're only discovering it now. :P. This last patch set is about finishing that work for Clang, which is correctly even more strict than GCC. -Kees [1] https://outflux.net/slides/2019/lss/kspp.pdf calls out specific numbers (about 6.5% of the patches fixed missing breaks): v4.19: 3 of 129 v4.20: 2 of 59 v5.0: 3 of 56 v5.1: 10 of 100 v5.2: 6 of 71 v5.3: 7 of 69 And in the history of the kernel, it's been an ongoing source of flaws: $ l --no-merges | grep -i 'missing break' | wc -l 185 The frequency of such errors being "naturally" found was pretty steady until the static checkers started warning, and then it was on the rise, but the full effort flushed the rest out, and now it's dropped to almost zero: 1 v2.6.12 3 v2.6.16.28 1 v2.6.17 1 v2.6.19 2 v2.6.21 1 v2.6.22 3 v2.6.24 3 v2.6.29 1 v2.6.32 1 v2.6.33 1 v2.6.35 4 v2.6.36 3 v2.6.38 2 v2.6.39 7 v3.0 2 v3.1 2 v3.2 2 v3.3 3 v3.4 1 v3.5 8 v3.6 7 v3.7 3 v3.8 6 v3.9 3 v3.10 2 v3.11 5 v3.12 5 v3.13 2 v3.14 4 v3.15 2 v3.16 3 v3.17 2 v3.18 2 v3.19 1 v4.0 2 v4.1 5 v4.2 4 v4.5 5 v4.7 6 v4.8 1 v4.9 3 v4.10 2 v4.11 6 v4.12 3 v4.13 2 v4.14 5 v4.15 2 v4.16 7 v4.18 2 v4.19 6 v4.20 3 v5.0 12 v5.1 3 v5.2 4 v5.3 2 v5.4 1 v5.8 And the reason it's fully zero, is because we still have the cases we're cleaning up right now. Even this last one from v5.8 is specifically of the same type this series addresses: case 4: color_index = TrueCModeIndex; + break; default: return; } [2] https://www.kernel.org/doc/html/latest/process/deprecated.html#implicit-switch-case-fall-through All switch/case blocks must end in one of: break; fallthrough; continue; goto ; return [expression]; [3] https://cwe.mitre.org/data/definitions/484.html -- Kees Cook ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] Problem with kernel 5.10-rc5
I am not subscribed to this mailing list so please CC me directly for any response. I'm running an older "NVIDIA Corporation G70 [GeForce 7800 GT] (rev a1)" card in a newer BIOSTAR B550GTA MB. Kernel 5.10-rc5 does not work. I get no virtual consoles or X screen. I have not tried any other RC versions. Only rc5. Kernel 5.9.10 works fine. I am running a SuSE Leap-15.2 system. I have another machine with an NVIDIA Corporation GT218 [GeForce 210] (rev a2) that works just fine with 5.10-rc5. On the broken machine the screen goes blank, the leds on the front turn orange as if the PC was turned off. My dmesg seems to show the cause: [5.825702] fb0: switching to nouveaufb from VESA VGA [6.242991] nouveau :14:00.0: vgaarb: deactivate vga console [6.243066] nouveau :14:00.0: NVIDIA G70 (047200a1) [6.443671] nouveau :14:00.0: bios: version 05.70.02.13.7b [6.443948] nouveau :14:00.0: fb: 256 MiB GDDR3 [6.498039] nouveau :14:00.0: DRM: VRAM: 250 MiB [6.498041] nouveau :14:00.0: DRM: GART: 512 MiB [6.498044] nouveau :14:00.0: DRM: TMDS table version 1.1 [6.498046] nouveau :14:00.0: DRM: DCB version 3.0 [6.498049] nouveau :14:00.0: DRM: DCB outp 00: 01000300 0028 [6.498051] nouveau :14:00.0: DRM: DCB outp 01: 03000302 [6.498053] nouveau :14:00.0: DRM: DCB outp 02: 04011310 0028 [6.498055] nouveau :14:00.0: DRM: DCB outp 03: 04011312 [6.498057] nouveau :14:00.0: DRM: DCB outp 04: 020223f1 0040c080 [6.498060] nouveau :14:00.0: DRM: DCB conn 00: 1030 [6.498062] nouveau :14:00.0: DRM: DCB conn 01: 2130 [6.498064] nouveau :14:00.0: DRM: DCB conn 02: 0210 [6.498065] nouveau :14:00.0: DRM: DCB conn 03: 0211 [6.498067] nouveau :14:00.0: DRM: DCB conn 04: 0213 [6.502949] nouveau :14:00.0: DRM: Setting dpms mode 3 on TV encoder (output 4) [6.585845] nouveau :14:00.0: DRM: failed to map fb: -28 [6.585949] [drm] Initialized nouveau 1.3.1 20120801 for :14:00.0 on minor 0 If I can do anything else please let me know. Regards Mark ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH 000/141] Fix fall-through warnings for Clang
On Tue, Nov 24, 2020 at 1:58 AM Finn Thain wrote:
>
> What I meant was that you've used pessimism as if it was fact.
"future mistakes that it might prevent" is neither pessimism nor states a fact.
> For example, "There is no way to guess what the effect would be if the
> compiler trained programmers to add a knee-jerk 'break' statement to avoid
> a warning".
It is only knee-jerk if you think you are infallible.
> Moreover, what I meant was that preventing programmer mistakes is a
> problem to be solved by development tools
This warning comes from a development tool -- the compiler.
> The idea that retro-fitting new
> language constructs onto mature code is somehow necessary to "prevent
> future mistakes" is entirely questionable.
The kernel is not a frozen codebase.
Further, "mature code vs. risk of change" arguments don't apply here
because the semantics of the program and binary output isn't changing.
> Sure. And if you put -Wimplicit-fallthrough into the Makefile and if that
> leads to well-intentioned patches that cause regressions, it is partly on
> you.
Again: adding a `fallthrough` does not change the program semantics.
If you are a maintainer and want to cross-check, compare the codegen.
> Have you ever considered the overall cost of the countless
> -Wpresume-incompetence flags?
Yeah: negative. On the other hand, the overall cost of the countless
-fI-am-infallible flags is very noticeable.
> Perhaps you pay the power bill for a build farm that produces logs that
> no-one reads? Perhaps you've run git bisect, knowing that the compiler
> messages are not interesting? Or compiled software in using a language
> that generates impenetrable messages? If so, here's a tip:
>
> # grep CFLAGS /etc/portage/make.conf
> CFLAGS="... -Wno-all -Wno-extra ..."
> CXXFLAGS="${CFLAGS}"
>
> Now allow me some pessimism: the hardware upgrades, gigawatt hours and
> wait time attributable to obligatory static analyses are a net loss.
If you really believe compiler warnings and static analysis are
useless and costly, I think there is not much point in continuing the
discussion.
> No, it's not for me to prove that such patches don't affect code
> generation. That's for the patch author and (unfortunately) for reviewers.
I was not asking you to prove it. I am stating that proving it is very easy.
Cheers,
Miguel
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] Could Fedora-5.9.9/X-server problem be a Nouveau-problem?
When in doubt, blame nouveau. I believe there has been some recent work around adjusting the logic which checks that modes have enough bandwidth. Adding Lyude, who worked on these changes. Not sure when they landed. Klaus -- please supply a full Xorg log. Cheers, -ilia On Sun, Nov 29, 2020 at 4:28 PM Klaus Ebbe Grue wrote: > > Hi nouveau@lists.freedesktop.org > > My X-server gives a blank screen on Fedora kernel 5.9.9. > > I am in doubt whether or not I should blame Nouveau for that. > > I know the nvidea drivers have problems with Fedora kernel 5.9.9. > > I can file a bug against Nouveau and provide logs if you think there is a > remote chance that Nouveau could be the cause. > > Here are some details: > > Fedora 5.8.18 / X-server works fine. > > I can boot into Fedora 5.9.9 runlevel 3 (no X-server). > > If I boot into Fedora 5.9.9 runlevel 5 (with X-server), the screen powers off > instead of giving a login prompt, but apart from that, Fedora 5.9.9 works > fine and produces log files. > > If I then power off and boot into Fedora 5.8.18 and read Xorg.0.log.old I can > see that the X-server under Fedora 5.9.9 did not get the screen resolutions > right. > > Here is what monitor-edid says under both Fedora 5.9.9 and 5.8.18: > > > monitor-edid > Name: PHL BDM4350 > EISA ID: PHL08fa > EDID version: 1.4 > EDID extension blocks: 1 > Screen size: 95.3 cm x 54.3 cm (43.18 inches, aspect ratio 16/9 = 1.76) > Gamma: 2.2 > Digital signal > Max video bandwidth: 600 MHz > HorizSync 30-160 > VertRefresh 23-80 > # Monitor preferred modeline (60.0 Hz vsync, 133.3 kHz hsync, ratio > 16/9, 102 dpi) (bad ratio) > ModeLine "3840x2160" 533.25 3840 3888 3920 4000 2160 2163 2168 > -hsync +vsync > ... > > But under Fedora 5.9.9, the X-server is unaware of the 3840x2160 resolution > according to Xorg.0.log.old. It also gets the screen physical size wrong. > > My first cry of help is here: > https://ask.fedoraproject.org/t/no-login-screen-after-upgrade/10618 > That is where I leaned that the nvidea drivers have problems with Fedora 5.9.9 > > Cheers, > Klaus > ___ > Nouveau mailing list > Nouveau@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/nouveau ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] Could Fedora-5.9.9/X-server problem be a Nouveau-problem?
Hi nouveau@lists.freedesktop.org My X-server gives a blank screen on Fedora kernel 5.9.9. I am in doubt whether or not I should blame Nouveau for that. I know the nvidea drivers have problems with Fedora kernel 5.9.9. I can file a bug against Nouveau and provide logs if you think there is a remote chance that Nouveau could be the cause. Here are some details: Fedora 5.8.18 / X-server works fine. I can boot into Fedora 5.9.9 runlevel 3 (no X-server). If I boot into Fedora 5.9.9 runlevel 5 (with X-server), the screen powers off instead of giving a login prompt, but apart from that, Fedora 5.9.9 works fine and produces log files. If I then power off and boot into Fedora 5.8.18 and read Xorg.0.log.old I can see that the X-server under Fedora 5.9.9 did not get the screen resolutions right. Here is what monitor-edid says under both Fedora 5.9.9 and 5.8.18: > monitor-edid Name: PHL BDM4350 EISA ID: PHL08fa EDID version: 1.4 EDID extension blocks: 1 Screen size: 95.3 cm x 54.3 cm (43.18 inches, aspect ratio 16/9 = 1.76) Gamma: 2.2 Digital signal Max video bandwidth: 600 MHz HorizSync 30-160 VertRefresh 23-80 # Monitor preferred modeline (60.0 Hz vsync, 133.3 kHz hsync, ratio 16/9, 102 dpi) (bad ratio) ModeLine "3840x2160" 533.25 3840 3888 3920 4000 2160 2163 2168 -hsync +vsync ... But under Fedora 5.9.9, the X-server is unaware of the 3840x2160 resolution according to Xorg.0.log.old. It also gets the screen physical size wrong. My first cry of help is here: https://ask.fedoraproject.org/t/no-login-screen-after-upgrade/10618 That is where I leaned that the nvidea drivers have problems with Fedora 5.9.9 Cheers, Klaus ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
