On Tue 25-08-15 05:54:00, Chen Gang wrote: > On 8/24/15 21:57, Michal Hocko wrote: > > On Mon 24-08-15 21:34:25, Chen Gang wrote: > > [...] > > > >> It is always a little better to let the external function suppose fewer > >> callers' behalf. > > > > I am sorry but I do not understand what you are saying here. > > > > Execuse me, my English maybe be still not quite well, my meaning is: > > - For the external functions (e.g. insert_vm_struct in our case), as a > callee, it may have to supose something from the caller. > > - If we can keep callee's functional contents no touch, a little fewer > supposing will let callee a little more independent from caller. > > - If can keep functional contens no touch, the lower dependency between > caller and callee is always better. OK, I guess I understand what you mean. You are certainly right that a partial initialization for the failure case is not nice in general. I was just objecting that the callers are supposed to free the vma in the failure case so any partial initialization doesn't matter in this particular case. Your patch would be more sensible if the failure case was more likely. But this function is used for special mappings (vdso, temporary vdso stack) which are created early in the process life time so both failure paths are highly unlikely. If this was a part of a larger changes where the function would be used elsewhere I wouldn't object at all. The reason I am skeptical about such changes in general is that the effect is very marginal while it increases chances of the code conflicts. But as I've said, if others feel this is worthwhile I will not object. -- Michal Hocko SUSE Labs -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>