[{"id":1768547,"web_url":"http://patchwork.ozlabs.org/comment/1768547/","msgid":"<20170914120229.GA22886@quack2.suse.cz>","list_archive_url":null,"date":"2017-09-14T12:02:29","subject":"Re: [PATCH v2 4/4] ext4: Switch to iomap for SEEK_HOLE / SEEK_DATA","submitter":{"id":363,"url":"http://patchwork.ozlabs.org/api/people/363/","name":"Jan Kara","email":"jack@suse.cz"},"content":"On Thu 14-09-17 11:50:47, Andreas Gruenbacher wrote:\n> @@ -3422,9 +3422,38 @@ static int ext4_iomap_begin(struct inode *inode, loff_t offset, loff_t length,\n> \tmap.m_lblk = first_block;\n> \tmap.m_len = last_block - first_block + 1;\n> \n> -\tif (!(flags & IOMAP_WRITE)) {\n> +\tif (flags & IOMAP_REPORT) {\n> \t\tret = ext4_map_blocks(NULL, inode, &map, 0);\n> -\t} else {\n> +\t\tif (ret < 0)\n> +\t\t\treturn ret;\n> +\n> +\t\tif (ret == 0) {\n> +\t\t\text4_lblk_t end = map.m_lblk + map.m_len - 1;\n> +\t\t\tstruct extent_status es;\n> +\n> +\t\t\text4_es_find_delayed_extent_range(inode, map.m_lblk, end, &es);\n> +\n> +\t\t\tif (!es.es_len || es.es_lblk > end) {\n> +\t\t\t\t/* entire range is a hole */\n> +\t\t\t} else if (es.es_lblk > map.m_lblk) {\n> +\t\t\t\t/* range starts with a hole */\n> +\t\t\t\tmap.m_len = es.es_lblk - map.m_lblk;\n> +\t\t\t} else {\n> +\t\t\t\text4_lblk_t offs = 0;\n> +\n> +\t\t\t\tif (es.es_lblk < map.m_lblk)\n> +\t\t\t\t\toffs = map.m_lblk - es.es_lblk;\n> +\t\t\t\tmap.m_lblk = es.es_lblk + offs;\n> +\t\t\t\tmap.m_pblk = ext4_es_pblock(&es) + offs;\n> +\t\t\t\tmap.m_len = es.es_len - offs;\n> +\t\t\t\tif (ext4_es_is_unwritten(&es))\n> +\t\t\t\t\tmap.m_flags |= EXT4_MAP_UNWRITTEN;\n\nThis is not possible. If there is unwritten extent at map->m_lblk, then\next4_map_blocks() could not have possibly returned 0. So just delete this\ncondition and set map.m_pblk to 0 since delalloc extent has no physical\npossition.\n\n> +\t\t\t\tif (ext4_es_is_delayed(&es))\n> +\t\t\t\t\tdelalloc = true;\n\nAlso ext4_es_is_delayed(&es) is guaranteed to be true since you've looked\nit up by ext4_es_find_delayed_extent_range(). So just set delalloc = true\nunconditionally.\n\nOtherwise the patch looks good so after fixing up these small issues feel\nfree to add:\n\nReviewed-by: Jan Kara \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 3xtHGK6gCnz9sBZ\n\tfor ;\n\tThu, 14 Sep 2017 22:02:33 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751621AbdINMCc (ORCPT );\n\tThu, 14 Sep 2017 08:02:32 -0400","from mx2.suse.de ([195.135.220.15]:55179 \"EHLO mx1.suse.de\"\n\trhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP\n\tid S1751413AbdINMCb (ORCPT );\n\tThu, 14 Sep 2017 08:02:31 -0400","from relay2.suse.de (charybdis-ext.suse.de [195.135.220.254])\n\tby mx1.suse.de (Postfix) with ESMTP id C4AC7AC5E;\n\tThu, 14 Sep 2017 12:02:29 +0000 (UTC)","by quack2.suse.cz (Postfix, from userid 1000)\n\tid 6B3271E343C; Thu, 14 Sep 2017 14:02:29 +0200 (CEST)"],"X-Virus-Scanned":"by amavisd-new at test-mx.suse.de","Date":"Thu, 14 Sep 2017 14:02:29 +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 v2 4/4] ext4: Switch to iomap for SEEK_HOLE / SEEK_DATA","Message-ID":"<20170914120229.GA22886@quack2.suse.cz>","References":"<20170914095047.23935-1-agruenba@redhat.com>\n\t<20170914095047.23935-5-agruenba@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170914095047.23935-5-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":1768565,"web_url":"http://patchwork.ozlabs.org/comment/1768565/","msgid":"","list_archive_url":null,"date":"2017-09-14T12:47:28","subject":"Re: [PATCH v2 4/4] ext4: Switch to iomap for SEEK_HOLE / SEEK_DATA","submitter":{"id":66251,"url":"http://patchwork.ozlabs.org/api/people/66251/","name":"Andreas Grünbacher","email":"andreas.gruenbacher@gmail.com"},"content":"2017-09-14 14:02 GMT+02:00 Jan Kara :\n> On Thu 14-09-17 11:50:47, Andreas Gruenbacher wrote:\n>> @@ -3422,9 +3422,38 @@ static int ext4_iomap_begin(struct inode *inode, loff_t offset, loff_t length,\n>> map.m_lblk = first_block;\n>> map.m_len = last_block - first_block + 1;\n>>\n>> - if (!(flags & IOMAP_WRITE)) {\n>> + if (flags & IOMAP_REPORT) {\n>> ret = ext4_map_blocks(NULL, inode, &map, 0);\n>> - } else {\n>> + if (ret < 0)\n>> + return ret;\n>> +\n>> + if (ret == 0) {\n>> + ext4_lblk_t end = map.m_lblk + map.m_len - 1;\n>> + struct extent_status es;\n>> +\n>> + ext4_es_find_delayed_extent_range(inode, map.m_lblk, end, &es);\n>> +\n>> + if (!es.es_len || es.es_lblk > end) {\n>> + /* entire range is a hole */\n>> + } else if (es.es_lblk > map.m_lblk) {\n>> + /* range starts with a hole */\n>> + map.m_len = es.es_lblk - map.m_lblk;\n>> + } else {\n>> + ext4_lblk_t offs = 0;\n>> +\n>> + if (es.es_lblk < map.m_lblk)\n>> + offs = map.m_lblk - es.es_lblk;\n>> + map.m_lblk = es.es_lblk + offs;\n>> + map.m_pblk = ext4_es_pblock(&es) + offs;\n>> + map.m_len = es.es_len - offs;\n>> + if (ext4_es_is_unwritten(&es))\n>> + map.m_flags |= EXT4_MAP_UNWRITTEN;\n>\n> This is not possible. If there is unwritten extent at map->m_lblk, then\n> ext4_map_blocks() could not have possibly returned 0. So just delete this\n> condition and set map.m_pblk to 0 since delalloc extent has no physical\n> position.\n>\n>> + if (ext4_es_is_delayed(&es))\n>> + delalloc = true;\n>\n> Also ext4_es_is_delayed(&es) is guaranteed to be true since you've looked\n> it up by ext4_es_find_delayed_extent_range(). So just set delalloc = true\n> unconditionally.\n\nSince map.m_pblk is never used in the delalloc case, we can leave it\nundefined above as well.\n\n> Otherwise the patch looks good so after fixing up these small issues feel\n> free to add:\n>\n> Reviewed-by: Jan Kara \n\nThanks,\nAndreas","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=)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"g6uNUmTa\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xtJGH05CTz9sRm\n\tfor ;\n\tThu, 14 Sep 2017 22:47:35 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751400AbdINMrb (ORCPT );\n\tThu, 14 Sep 2017 08:47:31 -0400","from mail-it0-f66.google.com ([209.85.214.66]:33897 \"EHLO\n\tmail-it0-f66.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751265AbdINMra (ORCPT\n\t); Thu, 14 Sep 2017 08:47:30 -0400","by mail-it0-f66.google.com with SMTP id o200so15243itg.1;\n\tThu, 14 Sep 2017 05:47:29 -0700 (PDT)","by 10.157.56.36 with HTTP; Thu, 14 Sep 2017 05:47:28 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=gmail.com; s=20161025;\n\th=mime-version:in-reply-to:references:from:date:message-id:subject:to\n\t:cc; bh=iiHygoejZVCoJ5BOuu5aqMyNRtUPim9vUUmsGfslzl8=;\n\tb=g6uNUmTarvt8deKb+9tWHkLM3RAo6PWVUAOEqd5CUzkskKDBeaaoLIwscHgiz7wR/b\n\t4Mt2LMwEyEUSD0Ekxs3S1Qwc1EZ6sUtjifXXNeydrdtOHgknS+WIXgummnYv3qZOs+Bq\n\tKMbfWe5n4t9bmjCKX/kAIqROCk/SxxUckPUEa8UiJWJDkNo2pXtFqm0yj+jCj8wnB/Ki\n\t6owW45pk9TAbT4fTrEqj+smU0VkG0j6LZt8jnex6zvzM89XXebo26Ksnyw4s3WPfsBu2\n\tUbu1gOv+tTZo0e8OQm9yT1q2ob5KqI1lEWBHXXdDTUwF8AAnkVNUOJWEWjPmbTfNBGOU\n\tEs3A==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:in-reply-to:references:from:date\n\t:message-id:subject:to:cc;\n\tbh=iiHygoejZVCoJ5BOuu5aqMyNRtUPim9vUUmsGfslzl8=;\n\tb=SDL41LQyNQMgh/hJzV3fhbKzrjLSid/9IBm33EN805b12nPUXtvnvMbEuVSSmNx+rA\n\tg7OLzI4UwskojyMCyuWr24KIV7WMZXjSLh5kcnoVBhLk/W4gipEMsh/UWUNBHJu+jsuc\n\tJBW49h2kXqyu9oAu0fW86yx8s51oq9XdvMC4HiWffiRWI1BY2LKOMSnFFtPFZvU7MT0U\n\tw7+cCZkteSELSzjcy/bv5UwAO+OsM+Ubf99JFpg61pzrAN8tWTQlwYHMUl2kdM4DdaBc\n\tG5dTFp13sOmvGS7h0rumhKC3I9zpLgSd/W6v3eNS9I7PuF+t51/42UWYYPJKGnNHXjKH\n\txaNw==","X-Gm-Message-State":"AHPjjUgIP3+75s00SKrcv/LLctJCfKxktLFwu9c9LCfqPwOHp7j43NuK\n\txXraoj1+qCNlWXQ9t3RoAkOe1RQtmsufaKKCFLE=","X-Google-Smtp-Source":"AOwi7QD0k8sGNgBAFNBfIk8Yc5Z01pgo3FNZtmevUGDh5t+mGCaNAKYO6Gad6iZ6LYuFwzqogWaBVkN+CI7uFImSE1Q=","X-Received":"by 10.202.198.131 with SMTP id\n\tw125mr23137150oif.120.1505393249240; \n\tThu, 14 Sep 2017 05:47:29 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"<20170914120229.GA22886@quack2.suse.cz>","References":"<20170914095047.23935-1-agruenba@redhat.com>\n\t<20170914095047.23935-5-agruenba@redhat.com>\n\t<20170914120229.GA22886@quack2.suse.cz>","From":"=?utf-8?q?Andreas_Gr=C3=BCnbacher?= ","Date":"Thu, 14 Sep 2017 14:47:28 +0200","Message-ID":"","Subject":"Re: [PATCH v2 4/4] ext4: Switch to iomap for SEEK_HOLE / SEEK_DATA","To":"Jan Kara ","Cc":"Andreas Gruenbacher ,\n\tLinux FS-devel Mailing List ,\n\tlinux-ext4@vger.kernel.org, linux-xfs@vger.kernel.org,\n\tChristoph Hellwig ","Content-Type":"text/plain; charset=\"UTF-8\"","Sender":"linux-ext4-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-ext4@vger.kernel.org"}},{"id":1768581,"web_url":"http://patchwork.ozlabs.org/comment/1768581/","msgid":"<20170914131431.GA26021@quack2.suse.cz>","list_archive_url":null,"date":"2017-09-14T13:14:31","subject":"Re: [PATCH v2 4/4] ext4: Switch to iomap for SEEK_HOLE / SEEK_DATA","submitter":{"id":363,"url":"http://patchwork.ozlabs.org/api/people/363/","name":"Jan Kara","email":"jack@suse.cz"},"content":"On Thu 14-09-17 14:47:28, Andreas Grünbacher wrote:\n> 2017-09-14 14:02 GMT+02:00 Jan Kara :\n> > On Thu 14-09-17 11:50:47, Andreas Gruenbacher wrote:\n> >> @@ -3422,9 +3422,38 @@ static int ext4_iomap_begin(struct inode *inode, loff_t offset, loff_t length,\n> >> map.m_lblk = first_block;\n> >> map.m_len = last_block - first_block + 1;\n> >>\n> >> - if (!(flags & IOMAP_WRITE)) {\n> >> + if (flags & IOMAP_REPORT) {\n> >> ret = ext4_map_blocks(NULL, inode, &map, 0);\n> >> - } else {\n> >> + if (ret < 0)\n> >> + return ret;\n> >> +\n> >> + if (ret == 0) {\n> >> + ext4_lblk_t end = map.m_lblk + map.m_len - 1;\n> >> + struct extent_status es;\n> >> +\n> >> + ext4_es_find_delayed_extent_range(inode, map.m_lblk, end, &es);\n> >> +\n> >> + if (!es.es_len || es.es_lblk > end) {\n> >> + /* entire range is a hole */\n> >> + } else if (es.es_lblk > map.m_lblk) {\n> >> + /* range starts with a hole */\n> >> + map.m_len = es.es_lblk - map.m_lblk;\n> >> + } else {\n> >> + ext4_lblk_t offs = 0;\n> >> +\n> >> + if (es.es_lblk < map.m_lblk)\n> >> + offs = map.m_lblk - es.es_lblk;\n> >> + map.m_lblk = es.es_lblk + offs;\n> >> + map.m_pblk = ext4_es_pblock(&es) + offs;\n> >> + map.m_len = es.es_len - offs;\n> >> + if (ext4_es_is_unwritten(&es))\n> >> + map.m_flags |= EXT4_MAP_UNWRITTEN;\n> >\n> > This is not possible. If there is unwritten extent at map->m_lblk, then\n> > ext4_map_blocks() could not have possibly returned 0. So just delete this\n> > condition and set map.m_pblk to 0 since delalloc extent has no physical\n> > position.\n> >\n> >> + if (ext4_es_is_delayed(&es))\n> >> + delalloc = true;\n> >\n> > Also ext4_es_is_delayed(&es) is guaranteed to be true since you've looked\n> > it up by ext4_es_find_delayed_extent_range(). So just set delalloc = true\n> > unconditionally.\n> \n> Since map.m_pblk is never used in the delalloc case, we can leave it\n> undefined above as well.\n\nYeah, that's fine by me as well.\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 3xtJsV5w4dz9sPs\n\tfor ;\n\tThu, 14 Sep 2017 23:14:38 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751335AbdINNOe (ORCPT );\n\tThu, 14 Sep 2017 09:14:34 -0400","from mx2.suse.de ([195.135.220.15]:33498 \"EHLO mx1.suse.de\"\n\trhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP\n\tid S1751131AbdINNOd (ORCPT );\n\tThu, 14 Sep 2017 09:14:33 -0400","from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254])\n\tby mx1.suse.de (Postfix) with ESMTP id 83E1EACB0;\n\tThu, 14 Sep 2017 13:14:32 +0000 (UTC)","by quack2.suse.cz (Postfix, from userid 1000)\n\tid DDF041E343C; Thu, 14 Sep 2017 15:14:31 +0200 (CEST)"],"X-Virus-Scanned":"by amavisd-new at test-mx.suse.de","Date":"Thu, 14 Sep 2017 15:14:31 +0200","From":"Jan Kara ","To":"Andreas =?iso-8859-1?q?Gr=FCnbacher?=\n\t","Cc":"Jan Kara , Andreas Gruenbacher ,\n\tLinux FS-devel Mailing List ,\n\tlinux-ext4@vger.kernel.org, linux-xfs@vger.kernel.org,\n\tChristoph Hellwig ","Subject":"Re: [PATCH v2 4/4] ext4: Switch to iomap for SEEK_HOLE / SEEK_DATA","Message-ID":"<20170914131431.GA26021@quack2.suse.cz>","References":"<20170914095047.23935-1-agruenba@redhat.com>\n\t<20170914095047.23935-5-agruenba@redhat.com>\n\t<20170914120229.GA22886@quack2.suse.cz>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","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"}}]