Re: [PATCH 51/51] fs/zonefs: Fix sparse warnings in tracing code

From: Linus Torvalds <torvalds@linux-foundation.org>
To: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Cc: Bart Van Assche <bvanassche@acm.org>,
	Christoph Hellwig <hch@lst.de>, Jens Axboe <axboe@kernel.dk>,
	linux-block <linux-block@vger.kernel.org>,
	Damien Le Moal <damien.lemoal@opensource.wdc.com>,
	Naohiro Aota <naohiro.aota@wdc.com>,
	Johannes Thumshirn <jth@kernel.org>,
	Rasmus Villemoes <linux@rasmusvillemoes.dk>,
	Steven Rostedt <rostedt@goodmis.org>,
	Sparse Mailing-list <linux-sparse@vger.kernel.org>
Subject: Re: [PATCH 51/51] fs/zonefs: Fix sparse warnings in tracing code
Date: Sun, 26 Jun 2022 09:33:57 -0700	[thread overview]
Message-ID: <CAHk-=wj8gM8q04v2jS5JGjEdoE2d+B4_nm74xrFjZ77f9YRsbA@mail.gmail.com> (raw)
In-Reply-To: <20220626095814.7wtma47w4sph7dha@mail>

On Sun, Jun 26, 2022 at 2:58 AM Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
>
> What about I would add to sparse something to strip away the bitwise/
> recover the underlying type? Something like __unbitwiseof() or
> __underlying_typeof() (some better name is needed)?

Please no, we don't want to make random macros have to have sparse
logic in them when it's not actually sparse-related.

I think it would be better if sparse just recognized some of these
kinds of situation. In particular:

 (a) for the casting part, I actually suspect we should drop the
warning about castign integers to restricted types.

Note that this is actually one of the main causes of "__force" use in
the kernel, with code like

        VM_FAULT_OOM            = (__force vm_fault_t)0x000001,
        VM_FAULT_SIGBUS         = (__force vm_fault_t)0x000002,
        VM_FAULT_MAJOR          = (__force vm_fault_t)0x000004,
        VM_FAULT_WRITE          = (__force vm_fault_t)0x000008,

and I think that we could/should just say that "explicit casts of
constants are ok".

That would remove two of the four warnings right there, and probably
make bitwise types more convenient in general.

We already treat "0" as special (because for bitwise things, zero is
kind of the universal constant), and we should continue to warn about
_implicit_ casts of restricted types, but I think the use of "__force"
in the kernel does show that the explicit casts are probably a bad
idea.

 (b) I think we could also recognize "comparison of constants" to be
something that doesn't necessarily require a warning.

And here in particular the "compare with zero" and "compare with all
bits set" - which is exactly that "-1" case.

In fact, there's a very good argument that "-1" is as special as zero
is ("all bits set" vs "all bits clear"), so for that (a) case, I think
at a _minimum_ we shouldn't warn about that particular constant.

So I think we could silence this sparse warning entirely, without
really introducing any new syntax, and actually improving on how
bitwise works.

                 Linus