From: Carlos Maiolino <cmaiolino@redhat.com> To: Christoph Hellwig <hch@lst.de> Cc: linux-fsdevel@vger.kernel.org, adilger@dilger.ca, sandeen@redhat.com, david@fromorbit.com Subject: Re: [PATCH 09/10 V2] Use FIEMAP for FIBMAP calls Date: Tue, 5 Feb 2019 10:56:01 +0100 Message-ID: <20190205095601.drsskdfzbpul2zh2@hades.usersys.redhat.com> (raw) In-Reply-To: <20190114165617.GG7187@lst.de> Hi Christoph. On Mon, Jan 14, 2019 at 05:56:17PM +0100, Christoph Hellwig wrote: > > +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? > No reason other than to keep it close to its peer fiemap_fill_user_extent(), I honestly do prefer to keep both together than in separated files. But, I'm up to move it to fs/inode.c if required. > Otherwise looks fine except for the additional sanity checking pointed > out by Darrick. Working on that. > > > + /* 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; Ok, will be in the new version, thanks for the review :) -- Carlos
next prev parent reply index Thread overview: 53+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-12-05 9:17 [PATCH 00/10 V2] New ->fiemap infrastructure and ->bmap removal Carlos Maiolino 2018-12-05 9:17 ` [PATCH 01/10] fs: Enable bmap() function to properly return errors Carlos Maiolino 2018-12-05 9:17 ` [PATCH 02/10] cachefiles: drop direct usage of ->bmap method Carlos Maiolino 2018-12-05 9:17 ` [PATCH 03/10] ecryptfs: drop direct calls to ->bmap Carlos Maiolino 2018-12-05 9:17 ` [PATCH 04/10 V2] fibmap: Use bmap instead of ->bmap method in ioctl_fibmap Carlos Maiolino 2019-01-14 16:49 ` Christoph Hellwig 2019-02-04 11:34 ` Carlos Maiolino 2018-12-05 9:17 ` [PATCH 05/10] fs: Move start and length fiemap fields into fiemap_extent_info Carlos Maiolino 2019-01-14 16:50 ` Christoph Hellwig 2018-12-05 9:17 ` [PATCH 06/10] iomap: Remove length and start fields from iomap_fiemap Carlos Maiolino 2019-01-14 16:51 ` Christoph Hellwig 2018-12-05 9:17 ` [PATCH 07/10] fs: Use a void pointer to store fiemap_extent Carlos Maiolino 2019-01-14 16:53 ` Christoph Hellwig 2018-12-05 9:17 ` [PATCH 08/10 V2] fiemap: Use a callback to fill fiemap extents Carlos Maiolino 2019-01-14 16:53 ` Christoph Hellwig 2018-12-05 9:17 ` [PATCH 09/10 V2] Use FIEMAP for FIBMAP calls Carlos Maiolino 2018-12-05 17:36 ` Darrick J. Wong 2018-12-07 9:09 ` Carlos Maiolino 2018-12-07 20:14 ` Andreas Dilger 2019-02-04 15:11 ` Carlos Maiolino 2019-02-04 18:27 ` Darrick J. Wong 2019-02-06 13:37 ` Carlos Maiolino 2019-02-06 20:44 ` Darrick J. Wong 2019-02-06 21:13 ` Andreas Dilger 2019-02-07 9:52 ` Carlos Maiolino 2019-02-08 8:43 ` Christoph Hellwig 2019-02-11 12:57 ` Christoph Hellwig 2019-02-11 16:21 ` Carlos Maiolino 2019-02-11 16:48 ` Christoph Hellwig 2019-02-07 11:59 ` Carlos Maiolino 2019-02-07 17:02 ` Darrick J. Wong 2019-02-07 21:25 ` Andreas Dilger 2019-02-08 8:46 ` Christoph Hellwig 2019-02-08 10:36 ` Carlos Maiolino 2019-02-08 21:03 ` Andreas Dilger 2019-02-08 9:08 ` Carlos Maiolino 2019-02-08 9:03 ` Carlos Maiolino 2019-02-07 12:36 ` Carlos Maiolino 2019-02-07 18:16 ` Darrick J. Wong 2019-02-08 8:58 ` Carlos Maiolino 2019-02-06 21:04 ` Andreas Dilger 2019-01-14 16:56 ` Christoph Hellwig 2019-02-05 9:56 ` Carlos Maiolino [this message] 2019-02-05 18:25 ` Christoph Hellwig 2019-02-06 9:50 ` Carlos Maiolino 2018-12-05 9:17 ` [PATCH 10/10] xfs: Get rid of ->bmap Carlos Maiolino 2018-12-05 17:37 ` Darrick J. Wong 2018-12-06 13:06 ` Carlos Maiolino 2018-12-06 18:56 ` [PATCH 00/10 V2] New ->fiemap infrastructure and ->bmap removal Andreas Grünbacher 2018-12-07 9:34 ` Carlos Maiolino 2019-01-14 16:50 ` Christoph Hellwig 2019-01-14 17:56 ` Andreas Grünbacher 2019-01-14 17:58 ` Christoph Hellwig
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20190205095601.drsskdfzbpul2zh2@hades.usersys.redhat.com \ --to=cmaiolino@redhat.com \ --cc=adilger@dilger.ca \ --cc=david@fromorbit.com \ --cc=hch@lst.de \ --cc=linux-fsdevel@vger.kernel.org \ --cc=sandeen@redhat.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
Linux-Fsdevel Archive on lore.kernel.org Archives are clonable: git clone --mirror https://lore.kernel.org/linux-fsdevel/0 linux-fsdevel/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 linux-fsdevel linux-fsdevel/ https://lore.kernel.org/linux-fsdevel \ linux-fsdevel@vger.kernel.org public-inbox-index linux-fsdevel Example config snippet for mirrors Newsgroup available over NNTP: nntp://nntp.lore.kernel.org/org.kernel.vger.linux-fsdevel AGPL code for this site: git clone https://public-inbox.org/public-inbox.git