On Mon 24-08-15 21:34:25, Chen Gang wrote: > On 8/24/15 19:32, Michal Hocko wrote: > > On Mon 24-08-15 00:59:39, [email protected] wrote: > >>> From: Chen Gang <[email protected]> > >>> > >>> When failure occurs and return, vma->vm_pgoff is already set, which is > >>> not a good idea. > > Why? The vma is not inserted anywhere and the failure path is supposed > > to simply free the vma. > > > > It can save several insns when failure occurs.
The failure is quite unlikely, though. > 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. > It can save the code readers' (especially new readers') time resource > to avoid to analyze why set 'vma->vm_pgoff' before checking '-ENOMEM' > (may it cause issue? or is 'vm_pgoff' related with the next checking?). Then your changelog should be specific about these reasons. "not a good idea" is definitely not a good justification for a patch. I am not saying the patch is incorrect I just do not sure it is worth it. The code is marginally better. But others might think otherwise. The changelog needs some more work for sure. -- Michal Hocko SUSE Labs -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
