[{"id":1760125,"web_url":"http://patchwork.ozlabs.org/comment/1760125/","msgid":"<20170830124724.GA31735@quack2.suse.cz>","list_archive_url":null,"date":"2017-08-30T12:47:24","subject":"Re: [PATCH 3/4] ext4: Add iomap support for inline data","submitter":{"id":363,"url":"http://patchwork.ozlabs.org/api/people/363/","name":"Jan Kara","email":"jack@suse.cz"},"content":"On Tue 29-08-17 16:29:41, Andreas Gruenbacher wrote:\n> Report inline data as an IOMAP_F_DATA_INLINE mapping. This allows to\n> switch to iomap_seek_hole and iomap_seek_data in ext4_llseek, and will\n> make switching to iomap_fiemap in ext4_fiemap easier as well.\n> \n> Signed-off-by: Andreas Gruenbacher \n\nJust one nit and one comment below. Feel free to add:\n\nReviewed-by: Jan Kara \n\n\n> ---\n> fs/ext4/ext4.h | 4 ++++\n> fs/ext4/inline.c | 33 +++++++++++++++++++++++++++++++++\n> fs/ext4/inode.c | 10 ++++++++--\n> 3 files changed, 45 insertions(+), 2 deletions(-)\n> \n> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h\n> index a2bb7d2870e4..017e55942a49 100644\n> --- a/fs/ext4/ext4.h\n> +++ b/fs/ext4/ext4.h\n> @@ -3048,6 +3048,10 @@ extern struct buffer_head *ext4_get_first_inline_block(struct inode *inode,\n> extern int ext4_inline_data_fiemap(struct inode *inode,\n> \t\t\t\t struct fiemap_extent_info *fieinfo,\n> \t\t\t\t int *has_inline, __u64 start, __u64 len);\n> +\n> +struct iomap;\n> +extern int ext4_inline_data_iomap(struct inode *inode, struct iomap *iomap);\n> +\n> extern int ext4_try_to_evict_inline_data(handle_t *handle,\n> \t\t\t\t\t struct inode *inode,\n> \t\t\t\t\t int needed);\n> diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c\n> index 28c5c3abddb3..21a078fef80f 100644\n> --- a/fs/ext4/inline.c\n> +++ b/fs/ext4/inline.c\n> @@ -12,6 +12,7 @@\n> * GNU General Public License for more details.\n> */\n> \n> +#include \n> #include \n> \n> #include \"ext4_jbd2.h\"\n> @@ -1827,6 +1828,38 @@ int ext4_destroy_inline_data(handle_t *handle, struct inode *inode)\n> \treturn ret;\n> }\n> \n> +int ext4_inline_data_iomap(struct inode *inode, struct iomap *iomap)\n> +{\n> +\t__u64 addr;\n> +\tint error = -ENOENT;\n> +\tstruct ext4_iloc iloc;\n> +\n> +\tdown_read(&EXT4_I(inode)->xattr_sem);\n> +\tif (!ext4_has_inline_data(inode))\n> +\t\tgoto out;\n\nHum, ENOENT looks rather arbitrary for a lost race with inode conversion.\nMaybe EAGAIN would be better? We use it in some other place as well to\nindicate that inode has been converted...\n\n> +\n> +\terror = ext4_get_inode_loc(inode, &iloc);\n> +\tif (error)\n> +\t\tgoto out;\n\nUsing ext4_get_inode_loc() just to get block + offset of the inode is a bit\noverkill since it will also allocate buffer_head, load the block with inode\nfrom disk, etc. But since this is not performance critical and the buffer\nis likely to be in cache anyway, I can live with this I guess.\n\n\t\t\t\t\t\t\t\tHonza","headers":{"Return-Path":"","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-ext4-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xj5005GLgz9sRW\n\tfor ;\n\tWed, 30 Aug 2017 22:48:16 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751456AbdH3MsN (ORCPT );\n\tWed, 30 Aug 2017 08:48:13 -0400","from mx2.suse.de ([195.135.220.15]:49862 \"EHLO mx1.suse.de\"\n\trhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP\n\tid S1751358AbdH3Mr1 (ORCPT );\n\tWed, 30 Aug 2017 08:47:27 -0400","from relay2.suse.de (charybdis-ext.suse.de [195.135.220.254])\n\tby mx1.suse.de (Postfix) with ESMTP id 262BFADED;\n\tWed, 30 Aug 2017 12:47:26 +0000 (UTC)","by quack2.suse.cz (Postfix, from userid 1000)\n\tid B0B6D1E3414; Wed, 30 Aug 2017 14:47:24 +0200 (CEST)"],"X-Virus-Scanned":"by amavisd-new at test-mx.suse.de","Date":"Wed, 30 Aug 2017 14:47:24 +0200","From":"Jan Kara ","To":"Andreas Gruenbacher ","Cc":"linux-fsdevel@vger.kernel.org, linux-ext4@vger.kernel.org,\n\tlinux-xfs@vger.kernel.org, Jan Kara ,\n\tChristoph Hellwig ","Subject":"Re: [PATCH 3/4] ext4: Add iomap support for inline data","Message-ID":"<20170830124724.GA31735@quack2.suse.cz>","References":"<20170829142942.21594-1-agruenba@redhat.com>\n\t<20170829142942.21594-4-agruenba@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170829142942.21594-4-agruenba@redhat.com>","User-Agent":"Mutt/1.5.24 (2015-08-30)","Sender":"linux-ext4-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-ext4@vger.kernel.org"}},{"id":1760136,"web_url":"http://patchwork.ozlabs.org/comment/1760136/","msgid":"<20170830125431.GB31735@quack2.suse.cz>","list_archive_url":null,"date":"2017-08-30T12:54:31","subject":"Re: [PATCH 3/4] ext4: Add iomap support for inline data","submitter":{"id":363,"url":"http://patchwork.ozlabs.org/api/people/363/","name":"Jan Kara","email":"jack@suse.cz"},"content":"On Tue 29-08-17 16:29:41, Andreas Gruenbacher wrote:\n> Report inline data as an IOMAP_F_DATA_INLINE mapping. This allows to\n> switch to iomap_seek_hole and iomap_seek_data in ext4_llseek, and will\n> make switching to iomap_fiemap in ext4_fiemap easier as well.\n> \n> Signed-off-by: Andreas Gruenbacher \n\n...\n\n> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c\n> index 429f0afde9f2..ab6ab835255e 100644\n> --- a/fs/ext4/inode.c\n> +++ b/fs/ext4/inode.c\n> @@ -3411,8 +3411,14 @@ static int ext4_iomap_begin(struct inode *inode, loff_t offset, loff_t length,\n> \tstruct ext4_map_blocks map;\n> \tint ret;\n> \n> -\tif (WARN_ON_ONCE(ext4_has_inline_data(inode)))\n> -\t\treturn -ERANGE;\n> +\tif ((flags & IOMAP_REPORT) && ext4_has_inline_data(inode)) {\n> +\t\tret = ext4_inline_data_iomap(inode, iomap);\n> +\t\tif (ret != -ENOENT) {\n> +\t\t\tif (ret == 0 && offset >= iomap->length)\n> +\t\t\t\tret = -ENOENT;\n> +\t\t\treturn ret;\n> +\t\t}\n> +\t}\n\nOne more thing: Please keep that WARN_ON and bail out for !IOMAP_REPORT\ncases. Thanks!\n\n\t\t\t\t\t\t\t\tHonza","headers":{"Return-Path":"","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-ext4-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xj5N50ZRvz9sQl\n\tfor ;\n\tWed, 30 Aug 2017 23:05:40 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751358AbdH3ND7 (ORCPT );\n\tWed, 30 Aug 2017 09:03:59 -0400","from mx2.suse.de ([195.135.220.15]:51228 \"EHLO mx1.suse.de\"\n\trhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP\n\tid S1751394AbdH3NDd (ORCPT );\n\tWed, 30 Aug 2017 09:03:33 -0400","from relay2.suse.de (charybdis-ext.suse.de [195.135.220.254])\n\tby mx1.suse.de (Postfix) with ESMTP id E9AFBAE17;\n\tWed, 30 Aug 2017 13:03:31 +0000 (UTC)","by quack2.suse.cz (Postfix, from userid 1000)\n\tid C0D951E3414; Wed, 30 Aug 2017 14:54:31 +0200 (CEST)"],"X-Virus-Scanned":"by amavisd-new at test-mx.suse.de","Date":"Wed, 30 Aug 2017 14:54:31 +0200","From":"Jan Kara ","To":"Andreas Gruenbacher ","Cc":"linux-fsdevel@vger.kernel.org, linux-ext4@vger.kernel.org,\n\tlinux-xfs@vger.kernel.org, Jan Kara ,\n\tChristoph Hellwig ","Subject":"Re: [PATCH 3/4] ext4: Add iomap support for inline data","Message-ID":"<20170830125431.GB31735@quack2.suse.cz>","References":"<20170829142942.21594-1-agruenba@redhat.com>\n\t<20170829142942.21594-4-agruenba@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170829142942.21594-4-agruenba@redhat.com>","User-Agent":"Mutt/1.5.24 (2015-08-30)","Sender":"linux-ext4-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-ext4@vger.kernel.org"}}]