(2011/04/21 21:18), David Sterba wrote: > On Wed, Apr 20, 2011 at 09:51:30AM +0900, Tsutomu Itoh wrote: >>> 2030 delayed = kmalloc(sizeof(*delayed), GFP_NOFS | __GFP_NOFAIL); >> >> I think that it doesn't fail ordinary when __GFP_NOFAIL is specified... > > yes I agree with you, my oversight. However, from linux/gfp.h > > * __GFP_NOFAIL: The VM implementation _must_ retry infinitely: the > * caller cannot handle allocation failures. This modifier is > * deprecated and no new users should be added. > > in long-term, redoing without NOFAIL would be probably wise. Currently > the btrfs_add_delayed_iput is called at places which do not seem to > like failure, I'm not sure whether possibly blocking indefinetely is > better than occasional failure with chance to do recovery ... > >> >>> >>> and in extent-tree.c:relocate_one_extent() >>> >>> 7992 new_extents = kmalloc(sizeof(*new_extents), >>> 7993 GFP_NOFS); >>> >>> the value is checked later, new_extents is passed to get_new_locations >>> and there it's checked, but no other callers pass potential NULL and the >>> check fits here and can be dropped from get_new_locations; > >>> there's a >>> little chance that get_new_locations will be able to succesfully >>> allocate the same data a jiffy later. >> >> Yes, therefore I did not check 'new_extents'. > > heh reading it again after myself, it sounds quite the opposite than I > wanted: I'd rather see the kmalloc checked right at the callsite, easier > to read and understand, than diving into get_new_locations and there > seeing checking extents for NULL and doing own alloc/free. OK, I understand. But, reading code again, I noticed nobody use relocate_one_extent() now. ;-) However, because there is a possibility to be going to be used in the future, I'll add the check of 'new_extents' for the readability. Thanks, Tsutomu > > > david > > -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
