| Message ID | 51045F97.9070602@gmail.com |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Sat 26-01-13 14:58:31, Wang Shilong wrote: > From: Wang Shilong <wangsl-fnst@cn.fujitsu.com> > > Because 'block + count < block' always comes to false, it is useless > to have this check, just remove it. As Andreas commented, the test is actually correct. BTW any reason why you sent the patch three times? Honza > Signed-off-by: Wang Shilong <wangsl-fnst@cn.fujitsu.com> > --- > fs/ext3/balloc.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/fs/ext3/balloc.c b/fs/ext3/balloc.c > index 22548f5..c557f22 100644 > --- a/fs/ext3/balloc.c > +++ b/fs/ext3/balloc.c > @@ -507,7 +507,6 @@ void ext3_free_blocks_sb(handle_t *handle, struct super_block *sb, > sbi = EXT3_SB(sb); > es = sbi->s_es; > if (block < le32_to_cpu(es->s_first_data_block) || > - block + count < block || > block + count > le32_to_cpu(es->s_blocks_count)) { > ext3_error (sb, "ext3_free_blocks", > "Freeing blocks not in datazone - " > -- 1.7.11.7 >
于 2013-1-28 6:07, Jan Kara 写道: > On Sat 26-01-13 14:58:31, Wang Shilong wrote: >> From: Wang Shilong <wangsl-fnst@cn.fujitsu.com> >> >> Because 'block + count < block' always comes to false, it is useless >> to have this check, just remove it. > As Andreas commented, the test is actually correct. BTW any reason why > you sent the patch three times? My linux run in the virtual machine..and the clock is wrong...so i send the patch not clarified correctly in marc.info..when i find it..I correct it, but first time i correct the time as 2012....how stupid it was..:-[ so i send the patch three times...sorry to bother... BTW ， may i have a question.... As we know the block to be freed can not be superblock and GDT.. I don't see any check about it in ext2/ext3/ext4.... Thanks, Wang > > Honza > >> Signed-off-by: Wang Shilong <wangsl-fnst@cn.fujitsu.com> >> --- >> fs/ext3/balloc.c | 1 - >> 1 file changed, 1 deletion(-) >> >> diff --git a/fs/ext3/balloc.c b/fs/ext3/balloc.c >> index 22548f5..c557f22 100644 >> --- a/fs/ext3/balloc.c >> +++ b/fs/ext3/balloc.c >> @@ -507,7 +507,6 @@ void ext3_free_blocks_sb(handle_t *handle, struct super_block *sb, >> sbi = EXT3_SB(sb); >> es = sbi->s_es; >> if (block < le32_to_cpu(es->s_first_data_block) || >> - block + count < block || >> block + count > le32_to_cpu(es->s_blocks_count)) { >> ext3_error (sb, "ext3_free_blocks", >> "Freeing blocks not in datazone - " >> -- 1.7.11.7 >> -- 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 Mon 28-01-13 22:01:21, Wang Shilong wrote: > 于 2013-1-28 6:07, Jan Kara 写道: > > On Sat 26-01-13 14:58:31, Wang Shilong wrote: > >> From: Wang Shilong <wangsl-fnst@cn.fujitsu.com> > >> > >> Because 'block + count < block' always comes to false, it is useless > >> to have this check, just remove it. > > As Andreas commented, the test is actually correct. BTW any reason why > > you sent the patch three times? > My linux run in the virtual machine..and the clock is wrong...so i send the patch not clarified correctly > in marc.info..when i find it..I correct it, but first time i correct the time as 2012....how stupid it was..:-[ > so i send the patch three times...sorry to bother... > > BTW ， may i have a question.... > As we know the block to be freed can not be superblock and GDT.. > I don't see any check about it in ext2/ext3/ext4.... Yes, you are right we apparently don't check for superblock or GDT blocks. But checking for those will be a bit more complex and they are really scarce so I don't think it's worth the overhead. Honza
On Tue, Jan 29, 2013 at 03:40:14PM +0100, Jan Kara wrote: > > As we know the block to be freed can not be superblock and GDT.. > > I don't see any check about it in ext2/ext3/ext4.... > Yes, you are right we apparently don't check for superblock or GDT > blocks. But checking for those will be a bit more complex and they are > really scarce so I don't think it's worth the overhead. We actually do have code to test for that in ext4. Take a look at fs/ext4/block_validity.c. We normally only check to make sure the data block is within the file system bounds, but if you mount the file system with the block_validity, it will actually check to make sure the block being allocated or freed is not part of the superblock, GDT, allocation bitmaps, or inode table. I use this mount option when running my xfstests, just to add an additional level of checking. It's not enabled by default, since it does increase CPU usage. I suspect it would only be visible for benchmarks such as TPC-C, but it's not something we've actually measured to see if we could afford to enable by default. The other major short coming is that we don't update the system zone after an online resize, which means we don't protect the newly metadata blocks until the next time the file system is mounted. If we added updating after a online resize, we'd also have to add some kind of locking to protect the rbtree while it is being changed, and this would increase its overhead if people wanted to use it in production. We might be able to use RCU to avoid doing a real hard lock, but it's not something that I've considered high priority. If someone wanted to look at this, it would actually be a pretty good starter project for someone who wanted to get started with doing ext4 hacking. Cheers, - 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/ext3/balloc.c b/fs/ext3/balloc.c index 22548f5..c557f22 100644 --- a/fs/ext3/balloc.c +++ b/fs/ext3/balloc.c @@ -507,7 +507,6 @@ void ext3_free_blocks_sb(handle_t *handle, struct super_block *sb, sbi = EXT3_SB(sb); es = sbi->s_es; if (block < le32_to_cpu(es->s_first_data_block) || - block + count < block || block + count > le32_to_cpu(es->s_blocks_count)) { ext3_error (sb, "ext3_free_blocks", "Freeing blocks not in datazone - "