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=-2.5 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_PASS,USER_AGENT_MUTT 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 5575EC43387 for ; Mon, 14 Jan 2019 16:56:20 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 2BFB4206B7 for ; Mon, 14 Jan 2019 16:56:20 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726729AbfANQ4T (ORCPT ); Mon, 14 Jan 2019 11:56:19 -0500 Received: from verein.lst.de ([213.95.11.211]:47761 "EHLO newverein.lst.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726673AbfANQ4T (ORCPT ); Mon, 14 Jan 2019 11:56:19 -0500 Received: by newverein.lst.de (Postfix, from userid 2407) id 8610868D93; Mon, 14 Jan 2019 17:56:17 +0100 (CET) Date: Mon, 14 Jan 2019 17:56:17 +0100 From: Christoph Hellwig To: Carlos Maiolino Cc: linux-fsdevel@vger.kernel.org, hch@lst.de, adilger@dilger.ca, sandeen@redhat.com, david@fromorbit.com Subject: Re: [PATCH 09/10 V2] Use FIEMAP for FIBMAP calls Message-ID: <20190114165617.GG7187@lst.de> References: <20181205091728.29903-1-cmaiolino@redhat.com> <20181205091728.29903-10-cmaiolino@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20181205091728.29903-10-cmaiolino@redhat.com> User-Agent: Mutt/1.5.17 (2007-11-01) Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org > +int fiemap_fill_kernel_extent(struct fiemap_extent_info *fieinfo, u64 logical, > + u64 phys, u64 len, u32 flags) Any reason this function isn't in inode.c next to the caller and marked static? Otherwise looks fine except for the additional sanity checking pointed out by Darrick. > + /* only count the extents */ > + if (fieinfo->fi_extents_max == 0) { > + fieinfo->fi_extents_mapped++; > + return (flags & FIEMAP_EXTENT_LAST) ? 1 : 0; Maybe do a 'goto out' here? > + return (flags & FIEMAP_EXTENT_LAST) ? 1 : 0; And reuse this return. Bonus points for using a good old if here: if (flags & FIEMAP_EXTENT_LAST) return 1; return 0;