Messages in this thread | | | Date | Thu, 10 Nov 2011 04:06:47 +0100 | From | Andrea Arcangeli <> | Subject | Re: [patch 1/5]thp: improve the error code path |
| |
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.
| |