Messages in this thread | | | Date | Thu, 06 Sep 2012 22:12:12 +0800 | From | Wang Sheng-Hui <> | Subject | Re: [PATCH 1/3] btrfs: remove unnecessary -ENOMEM BUG_ON check in extent-tree.c/exclude_super_stripes |
| |
On 2012年09月06日 18:09, David Sterba wrote: > On Thu, Sep 06, 2012 at 02:40:41PM +0800, Wang Sheng-Hui wrote: >> The memory allocation failure is BUG_ON in add_excluded_extent (following >> the code path) and btrfs_rmap_block. No need to BUG_ON -ENOMEM inside >> exclude_super_stripes itself. > > No please. > >> Its return value is always 0, and useless for its callers. Set it as void >> instead 0-returned. > > btrfs_rmap_block itself contains a BUG_ON: > > 3980 int btrfs_rmap_block(struct btrfs_mapping_tree *map_tree, > 3981 u64 chunk_start, u64 physical, u64 devid, > 3982 u64 **logical, int *naddrs, int *stripe_len) > 3983 { > 3984 struct extent_map_tree *em_tree = &map_tree->map_tree; > 3985 struct extent_map *em; > 3986 struct map_lookup *map; > 3987 u64 *buf; > 3988 u64 bytenr; > 3989 u64 length; > 3990 u64 stripe_nr; > 3991 int i, j, nr = 0; > 3992 > 3993 read_lock(&em_tree->lock); > 3994 em = lookup_extent_mapping(em_tree, chunk_start, 1); > 3995 read_unlock(&em_tree->lock); > 3996 > 3997 BUG_ON(!em || em->start != chunk_start); > > And this should be turned into an 'return error', thus giving a non-zero return > code that should be handled in the callers. > > Eg. this patch attempts to do that > http://www.mail-archive.com/linux-btrfs@vger.kernel.org/msg15470.html > > but has not been merged due to incorrect fix inside exclude_super_stripes > (introduced in the patch). > > The same objection for return code cleanups will hold for any function that > returns 0 but is full of BUG_ONs. > > > david
Got it. Thanks, David!
Regards, Sheng-Hui -- 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/
| |