Re: [PATCH 1/7] Btrfs: replace BUG() with WARN_ONCE in raid56

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

 



On Sun, May 15, 2016 at 04:19:28PM +0200, Holger Hoffstätte wrote:
> On 05/14/16 02:06, Liu Bo wrote:
> > This BUG() has been triggered by a fuzz testing image, but in fact
> > btrfs can handle this gracefully by returning -EIO.
> > 
> > Thus, use WARN_ONCE for warning purpose and don't leave a possible
> > kernel panic.
> > 
> > Signed-off-by: Liu Bo <bo.li.liu@xxxxxxxxxx>
> > ---
> >  fs/btrfs/raid56.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/fs/btrfs/raid56.c b/fs/btrfs/raid56.c
> > index 0b7792e..863f7fe 100644
> > --- a/fs/btrfs/raid56.c
> > +++ b/fs/btrfs/raid56.c
> > @@ -2139,7 +2139,7 @@ int raid56_parity_recover(struct btrfs_root *root, struct bio *bio,
> >  
> >  	rbio->faila = find_logical_bio_stripe(rbio, bio);
> >  	if (rbio->faila == -1) {
> > -		BUG();
> > +		WARN_ONCE(1, KERN_WARNING "rbio->faila is -1\n");
> 
> I'm generally in favor of not BUGing out for no good reason, but what
> is e.g. an admin (or user) supposed to do when he sees this message?
> Same for the other rather cryptic WARNs - they contain no actionable
> information, and are most likely going to be ignored as "debug spam".
> IMHO things that can be ignored can be deleted.

Agreed, the way this patchset repalces BUG on is very confusing.
WARN_ONCE is a global state, the message does not even print on which
filesystem the error happened. The only way to reset the state is to
unload the module.

This should be handled as a corruption, no matter if it's fuzzed or not,
report more details about what is corrupted or what was expected.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Filesystem Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux