Re: [patch 1/5]thp: improve the error code path

[Andrea Arcangeli]

On Wed, Nov 09, 2011 at 06:43:58PM -0800, David Rientjes wrote:
> You're right, but I agree that the #ifdef's just make the function error 
> handling much too complex.  Would you mind adding sysfs_*_out labels at 
> the end of the function to handle these errors instead?  And I think we 
> should be doing khugepaged_slab_init() and mm_slots_hash_init() before 
> initializing sysfs.
> 
> Something like
> 
> 	out:
> 		khugepaged_slab_free();
> 		mm_slots_hash_free();	<-- after you remove it from #if 0
> 		return err;
> 
> 	#ifdef CONFIG_SYSFS
> 	sysfs_khugepaged_out:
> 		sysfs_remove_group(hugepage_kobj, &khugepaged_attr_group);
> 	sysfs_hugepage_out:
> 		sysfs_remove_group(hugepage_kobj, &hugepage_attr_group);
> 		...
> 		goto out;
> 	#endif

Before after won't matter much I guess... If you really want to clean
the code, I wonder what is exactly the point of those dummy functions
if we can't call those outside of #ifdefs. I mean a cleanup that adds
more #ifdefs when there are explicit dummy functions which I assume
are meant to be used outside of #ifdef CONFIG_SYSFS doesn't sound so
clean in the first place. I understand you need to refactor the code
above to call those outside of #ifdefs but hey if you're happy with
#ifdef I'm happy too :). It just looks fishy to read sysfs.h dummy
functions and #ifdefs. When I wrote the code I hardly could have
wondered about the sysfs #ifdefs but at this point it's only cleanups
I'm seeing so I actually noticed that.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/