Messages in this thread | | | Date | Tue, 05 Sep 2006 14:51:41 -0700 | From | Mingming Cao <> | Subject | Re: BUG: warning at fs/ext3/inode.c:1016/ext3_getblk() |
| |
Dave Kleikamp wrote: > On Tue, 2006-09-05 at 13:14 -0700, Badari Pulavarty wrote: > >>Dave Kleikamp wrote: > > >>>I'm having a hard time figuring out exactly what ext3_get_blocks_handle >>>is trying to return, but it looks to me like if it is allocating one >>>data block, and needs to allocate an indirect block as well, then it >>>will return 2 rather than 1. Is this expected, or am I just confused? >>> >>> >> >>It would return "1" in that case.. (data block) >> >> > 0 means get_block() suceeded and indicates the number of blocks mapped >>= 0 lookup failed >>< 0 mean error case > > > Okay, I got confused looking through the code. I still don't see how > ext3_get_blocks_handle() should be returning a number greater than > maxblocks. >
yes ext3_get_blocks_handle() will return the number of data blocks allocated(not including the indirect/double indirecto blocks), and that number should not than maxblocks. In this case, it should return 1 instead.
The ext3_get_blocks_handle() behavior was changed when multiple blocks map/allocation was added to ext3 via this function. Previously, the behavior of ext3_get_blokc_handle() returns 0 for success case, and returns non-zero(nagive) for error case. While with new behavior, the success case is the thre returned value should > 0.
How many blocks is being mapped in this case? > 1? or 0? If it failed to map the block (ext3_get_blocks_handle() returns 0), ext3_getblk() will also WARN_ON().
> >>>>I did search for callers of ext3_get_blocks_handle() and found that >>>>ext3_readdir() seems to do wrong thing all the time with error check :( >>>>Need to take a closer look.. >>>> >>>> err = ext3_get_blocks_handle(NULL, inode, blk, 1, >>>> &map_bh, 0, 0); >>>> if (err > 0) { <<<< BAD >>>> page_cache_readahead(sb->s_bdev->bd_inode->i_mapping, >>>> &filp->f_ra, >>>> filp, >>>> map_bh.b_blocknr >> >>>> (PAGE_CACHE_SHIFT - inode->i_blkbits), >>>> 1); >>>> bh = ext3_bread(NULL, inode, blk, 0, &err); >>>> } >>>> >>> >>>Bad to do what it's doing, or bad to call name the variable "err"? >>>I think if it looked like this: >>> >>> count = ext3_get_blocks_handle(NULL, inode, blk, 1, >>> &map_bh, 0, 0); >>> if (count > 0) { >>> >>>it would be a lot less confusing. >>> >> >>I am sorry !! it is doing the right thing :( > > > Not your fault. The variable is very badly named.
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |