Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755647AbbHYLf1 (ORCPT ); Tue, 25 Aug 2015 07:35:27 -0400 Received: from mail-wi0-f172.google.com ([209.85.212.172]:36198 "EHLO mail-wi0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755603AbbHYLfY (ORCPT ); Tue, 25 Aug 2015 07:35:24 -0400 Date: Tue, 25 Aug 2015 13:35:21 +0200 From: Michal Hocko To: Chen Gang Cc: Chen Gang , Andrew Morton , "kirill.shutemov@linux.intel.com" , "riel@redhat.com" , "sasha.levin@oracle.com" , "gang.chen.5i5j@gmail.com" , Linux Memory , kernel mailing list Subject: Re: [PATCH] mm: mmap: Check all failures before set values Message-ID: <20150825113521.GA6285@dhcp22.suse.cz> References: <1440349179-18304-1-git-send-email-gang.chen.5i5j@qq.com> <20150824113212.GL17078@dhcp22.suse.cz> <55DB1D94.3050404@hotmail.com> <20150824135716.GO17078@dhcp22.suse.cz> <55DB9278.2020603@qq.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <55DB9278.2020603@qq.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1933 Lines: 51 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 from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/