From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.3 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0E749C10F14 for ; Tue, 15 Oct 2019 12:33:05 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id C714D20854 for ; Tue, 15 Oct 2019 12:33:04 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org C714D20854 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=suse.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 340628E0007; Tue, 15 Oct 2019 08:33:04 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 2CA0C8E0001; Tue, 15 Oct 2019 08:33:04 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 1B8E38E0007; Tue, 15 Oct 2019 08:33:04 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0211.hostedemail.com [216.40.44.211]) by kanga.kvack.org (Postfix) with ESMTP id E82798E0001 for ; Tue, 15 Oct 2019 08:33:03 -0400 (EDT) Received: from smtpin13.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with SMTP id 7D93D75A7 for ; Tue, 15 Oct 2019 12:33:03 +0000 (UTC) X-FDA: 76045958646.13.vest74_71f62ca86fe36 X-HE-Tag: vest74_71f62ca86fe36 X-Filterd-Recvd-Size: 3365 Received: from mx1.suse.de (mx2.suse.de [195.135.220.15]) by imf42.hostedemail.com (Postfix) with ESMTP for ; Tue, 15 Oct 2019 12:33:02 +0000 (UTC) X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 8F77FB349; Tue, 15 Oct 2019 12:33:01 +0000 (UTC) Date: Tue, 15 Oct 2019 14:33:01 +0200 From: Michal Hocko To: Gaowei Pu Cc: Andrew Morton , Vlastimil Babka , linux-mm@kvack.org Subject: Re: [PATCH v2] mm/mmap.c: use IS_ERR_VALUE to check return value of get_unmapped_area Message-ID: <20191015123301.GC24932@dhcp22.suse.cz> References: <20191012102512.28051-1-pugaowei@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20191012102512.28051-1-pugaowei@gmail.com> User-Agent: Mutt/1.10.1 (2018-07-13) X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Sat 12-10-19 18:25:12, Gaowei Pu wrote: > get_unmapped_area already cover the offset_in_page() check and returned > with error ptr. So replace offset_in_page() with IS_ERR_VALUE(). I have to say that I found offset_in_page check quite unintuitive. It is a relict from the past I suspect. A newer code seems to use IS_ERR_VALUE. If we want to make the code consistent then I am for it. Could you check other users as well? git grep suggests checking kernel/events/uprobes.c, mm/mmap.c and mm/mremap.c. I would simply use the following for the changelog " get_unmapped_area returns an address or -errno on failure. Historically we have checked for the failure by offset_in_page() which is correct but quite hard to read. Newer code started using IS_ERR_VALUE which is much easier to read. Convert remaining users of offset_in_page as well. This shouldn't introduce any functional changes. " > Signed-off-by: Gaowei Pu > > V2: fix compile warnning. > --- > mm/mmap.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/mm/mmap.c b/mm/mmap.c > index 7e8c3e8ae75f..6aebfeb6a486 100644 > --- a/mm/mmap.c > +++ b/mm/mmap.c > @@ -1430,7 +1430,7 @@ unsigned long do_mmap(struct file *file, unsigned long addr, > * that it represents a valid section of the address space. > */ > addr = get_unmapped_area(file, addr, len, pgoff, flags); > - if (offset_in_page(addr)) > + if (IS_ERR_VALUE(addr)) > return addr; > > if (flags & MAP_FIXED_NOREPLACE) { > @@ -2990,15 +2990,16 @@ static int do_brk_flags(unsigned long addr, unsigned long len, unsigned long fla > struct rb_node **rb_link, *rb_parent; > pgoff_t pgoff = addr >> PAGE_SHIFT; > int error; > + unsigned long mapped_addr; > > /* Until we need other flags, refuse anything except VM_EXEC. */ > if ((flags & (~VM_EXEC)) != 0) > return -EINVAL; > flags |= VM_DATA_DEFAULT_FLAGS | VM_ACCOUNT | mm->def_flags; > > - error = get_unmapped_area(NULL, addr, len, 0, MAP_FIXED); > - if (offset_in_page(error)) > - return error; > + mapped_addr = get_unmapped_area(NULL, addr, len, 0, MAP_FIXED); > + if (IS_ERR_VALUE(mapped_addr)) > + return mapped_addr; > > error = mlock_future_check(mm, mm->def_flags, len); > if (error) > -- > 2.23.0 > -- Michal Hocko SUSE Labs