Re: [PATCH] mm: mmap: Check all failures before set values

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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>



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]