Message ID | 1404602162-16730-1-git-send-email-tytso@mit.edu |
---|---|
State | Accepted, archived |
Headers | show |
On Sat, 5 Jul 2014, Theodore Ts'o wrote: > Date: Sat, 5 Jul 2014 19:16:02 -0400 > From: Theodore Ts'o <tytso@mit.edu> > To: Ext4 Developers List <linux-ext4@vger.kernel.org> > Cc: Theodore Ts'o <tytso@mit.edu>, stable@vger.kernel.org > Subject: [PATCH] ext4: clarify ext4_error message in > ext4_mb_generate_buddy_error() > > We are spending a lot of time explaining to users what this error > means. Let's try to improve the message to avoid this problem. It is a bit better, even though strictly speaking it's not right, because it is not block bitmap alone, but rather aggregation of block bitmap and preallocations. But for the user this is really an implementation detail they do not need to worry about I guess. Reviewed-by: Lukas Czerner <lczerner@redhat.com> Thanks! -Lukas > > Signed-off-by: Theodore Ts'o <tytso@mit.edu> > Cc: stable@vger.kernel.org > --- > fs/ext4/mballoc.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c > index 7f72f50..2dcb936 100644 > --- a/fs/ext4/mballoc.c > +++ b/fs/ext4/mballoc.c > @@ -752,8 +752,8 @@ void ext4_mb_generate_buddy(struct super_block *sb, > > if (free != grp->bb_free) { > ext4_grp_locked_error(sb, group, 0, 0, > - "%u clusters in bitmap, %u in gd; " > - "block bitmap corrupt.", > + "block bitmap and bg descriptor " > + "inconsistent: %u vs %u free clusters", > free, grp->bb_free); > /* > * If we intend to continue, we consider group descriptor > -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jul 08, 2014 at 09:03:48AM +0200, Lukáš Czerner wrote: > > It is a bit better, even though strictly speaking it's not > right, because it is not block bitmap alone, but rather aggregation > of block bitmap and preallocations. But for the user this is really > an implementation detail they do not need to worry about I guess. Actually, no, because the preallocations aren't reflected in the in-block bitmap. And, oh sh*t, I wonder if that's the cause of the ext4_mb_generate_buddy(). We don't need the buddy bitmaps to allocate out of the preallocations, and it's not needed by ext4_mb_mark_diskspace_used(). If the buddy bitmaps have been pushed out of the page cache between when the blocks were originally preallocated and when we try to use some preallocated blocks, and then we have a race between ext4_mb_mark_diskspace_used() and ext4_mb_buddy_generate(), that could explain the discrepancy between the block group descriptors and the loaded buddy bitmap. If this is what's going on, it doesn't explain why we hacen't been seeing this until post 3.15, though.... - Ted -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 8 Jul 2014, Theodore Ts'o wrote: > Date: Tue, 8 Jul 2014 08:54:54 -0400 > From: Theodore Ts'o <tytso@mit.edu> > To: Lukáš Czerner <lczerner@redhat.com> > Cc: Ext4 Developers List <linux-ext4@vger.kernel.org>, stable@vger.kernel.org > Subject: Re: [PATCH] ext4: clarify ext4_error message in > ext4_mb_generate_buddy_error() > > On Tue, Jul 08, 2014 at 09:03:48AM +0200, Lukáš Czerner wrote: > > > > It is a bit better, even though strictly speaking it's not > > right, because it is not block bitmap alone, but rather aggregation > > of block bitmap and preallocations. But for the user this is really > > an implementation detail they do not need to worry about I guess. > > Actually, no, because the preallocations aren't reflected in the > in-block bitmap. But the "bitmap" argument we're getting in ext4_mb_generate_buddy() is not just the original on disk bitmap, is it ? It's aggregation of the on on disk block bitmap and preallocation (this is done in ext4_mb_init_cache()). Or am I missing something ? > > And, oh sh*t, I wonder if that's the cause of the > ext4_mb_generate_buddy(). We don't need the buddy bitmaps to allocate > out of the preallocations, and it's not needed by > ext4_mb_mark_diskspace_used(). If the buddy bitmaps have been pushed > out of the page cache between when the blocks were originally > preallocated and when we try to use some preallocated blocks, and then > we have a race between ext4_mb_mark_diskspace_used() and > ext4_mb_buddy_generate(), that could explain the discrepancy between > the block group descriptors and the loaded buddy bitmap. ext4_mb_generate_buddy() is run under the group lock and the actual ext4_set_bits() in ext4_mb_mark_diskspace_used as well. And again in ext4_mb_generate_buddy() we do take into account preallocations which will be marked as used in the buddy bitmap. > > If this is what's going on, it doesn't explain why we hacen't been > seeing this until post 3.15, though.... > > - Ted > -- > To unsubscribe from this list: send the line "unsubscribe linux-ext4" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c index 7f72f50..2dcb936 100644 --- a/fs/ext4/mballoc.c +++ b/fs/ext4/mballoc.c @@ -752,8 +752,8 @@ void ext4_mb_generate_buddy(struct super_block *sb, if (free != grp->bb_free) { ext4_grp_locked_error(sb, group, 0, 0, - "%u clusters in bitmap, %u in gd; " - "block bitmap corrupt.", + "block bitmap and bg descriptor " + "inconsistent: %u vs %u free clusters", free, grp->bb_free); /* * If we intend to continue, we consider group descriptor
We are spending a lot of time explaining to users what this error means. Let's try to improve the message to avoid this problem. Signed-off-by: Theodore Ts'o <tytso@mit.edu> Cc: stable@vger.kernel.org --- fs/ext4/mballoc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)