Messages in this thread | | | Date | Mon, 24 Aug 2015 15:57:16 +0200 | From | Michal Hocko <> | Subject | Re: [PATCH] mm: mmap: Check all failures before set values |
| |
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, gang.chen.5i5j@qq.com wrote: > >>> From: Chen Gang <gang.chen.5i5j@gmail.com> > >>> > >>> 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
| |