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

Re: [dm-devel] [PATCH 2/6] dm: introduce dm_accept_partial_bio




On Mon, 17 Mar 2014, Mike Snitzer wrote:

> > The user will not hit a BUG with REQ_FLUSH. Regarding the developer - it 
> > is expected that the kernel developer is able to decode the oops message, 
> > compare it with disassembled code and find out which variable was NULL. 
> > The function is small, so it's an easy task.
> 
> By that logic then no BUG_ON() should ever be needed.  You need to know
> to quit while you're ahead ;)

The difference is this - if you ignore BUG_ON on bad arithmetics in bio 
processing, you may introduce subtle data corruption. So, the developer 
may spend hours or days trying to reproduce the data corruption. When he 
finds a reproducible case, he has to spend additional time trying to find 
the reason for the corruption, in worst case it may involve recording the 
content of all incoming and outcoming bios and searching for a 
discrepancy.

BUG_ON saves all this work.

> > We don't really need
> > BUG_ON(!pointer);
> > variable = *pointer;
> 
> Right, we don't.. but it helps resolve issues quicker if thoughtful
> BUG_ON()s hit before more obscure crashes do.  Whereby obviating the
> need to reverse engineer _why_ the NULL pointer occurred.

On the other hand, if the developer calls dm_accept_partial_bio on a flush 
request, he gets a NULL pointer dereference, the crash is perfectly 
reproducible, there is no subltle data corruption. You can add 
BUG_ON(bio->bi_rw & REQ_FLUSH), I'm not strongly opposed to that, I just 
think it is not needed.

Mikulas


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