On Mon, May 16, 2016 at 10:32:48AM +0200, David Sterba wrote:
> 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....@oracle.com>
> > > ---
> > >  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.

Looking again at the patch, it compares an inode property (a range to
cow) against a global filesystem size, stored in superblock. This does
not IMO belong here, either we'd have to do such check everywhere (and
expect that it could really happen) or it should be removed completely.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to