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 Tue, 2022-03-22 at 10:57 +0100, Marek Vasut wrote:
> 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.s
> > > > > > > > torage
> > > > > > > > bits <
> > > > > > > > +                               channels[i].scan_type.r
> > > > > > > > ealbit
> > > > > > > > s +
> > > > > > > > +                               channels[i].scan_type.s
> > > > > > > > hift);
> > > > > > > 
> > > > > > > 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 ?

Nothing I'm aware... Other than directly call 'dump_stack()'

- Nuno Sá



[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