Re: [PATCH] iio: core: WARN in case sample bits do not fit storage bits

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

 



On 3/22/22 10:49, Andy Shevchenko wrote:
On Tue, Mar 22, 2022 at 08:43:10AM +0100, Nuno Sá wrote:
On Mon, 2022-03-21 at 20:46 +0100, Marek Vasut wrote:
On 3/21/22 17:10, Andy Shevchenko wrote:
On Mon, Mar 21, 2022 at 03:46:51PM +0100, Marek Vasut wrote:
On 3/21/22 11:40, Andy Shevchenko wrote:
On Sun, Mar 20, 2022 at 07:15:42PM +0100, Marek Vasut wrote:
Add runtime check to verify whether storagebits are at least
as big
as shifted realbits. This should help spot broken drivers
which may
set realbits + shift above storagebits.

Thanks!

...

+                       /* Verify that sample bits fit into
storage */
+                       WARN_ON(channels[i].scan_type.storage
bits <
+                               channels[i].scan_type.realbit
s +
+                               channels[i].scan_type.shift);

Not sure WARN is a good level (it might be fatal on some setups
and we won't that),
besides the fact that we may use dev_WARN(). Perhaps dev_warn()
would suffice?

I was actually thinking about BUG(), but that might crash
existing systems.
I think we want a strong indicator that something wrong is going
on which
must be fixed and the splat produced by WARN_ON() is a good
indicator of
that. It also does not crash existing systems,

It does crash _some_ of them, unfortunately.

Details please ?

WARN_ON() shouldn't cause crash outright, or do I miss something ?

Arghh, completely forgot about this... Andy is right, maybe there are
other cases (in which case, it would be nice to share :D), but this one
is definitely one of them:

https://elixir.bootlin.com/linux/latest/source/kernel/panic.c#L579

You can have a cmdline parameter to panic on _WARN() and some systems
may have it.

Yes, I meant panic on warning.

And I can't imagine that this driver can be system critical to the extent
that we have to crash the system.

Is there something which does trigger a backtrace, but without panic()ing the system ?



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux