Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <cover.1587555962.git.riteshh@linux.ibm.com> <20200424101153.GC456@infradead.org>
 <20200424232024.A39974C046@d06av22.portsmouth.uk.ibm.com>
In-Reply-To: <20200424232024.A39974C046@d06av22.portsmouth.uk.ibm.com>
From:   Amir Goldstein <amir73il@gmail.com>
Date:   Sat, 25 Apr 2020 12:11:59 +0300
Message-ID: <CAOQ4uxgiome-BnHDvDC=vHfidf4Ru3jqzOki0Z_YUkinEeYCRQ@mail.gmail.com>
Subject: Re: [PATCH 0/5] ext4/overlayfs: fiemap related fixes
To:     Ritesh Harjani <riteshh@linux.ibm.com>
Cc:     Christoph Hellwig <hch@infradead.org>,
        Ext4 <linux-ext4@vger.kernel.org>, Jan Kara <jack@suse.cz>,
        Theodore Tso <tytso@mit.edu>,
        Andreas Dilger <adilger@dilger.ca>,
        "Darrick J. Wong" <darrick.wong@oracle.com>,
        Alexander Viro <viro@zeniv.linux.org.uk>,
        Dan Carpenter <dan.carpenter@oracle.com>,
        "Aneesh Kumar K . V" <aneesh.kumar@linux.ibm.com>,
        Murphy Zhou <jencce.kernel@gmail.com>,
        Miklos Szeredi <miklos@szeredi.hu>,
        linux-fsdevel <linux-fsdevel@vger.kernel.org>,
        overlayfs <linux-unionfs@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-ext4-owner@vger.kernel.org
Precedence: bulk

On Sat, Apr 25, 2020 at 2:20 AM Ritesh Harjani <riteshh@linux.ibm.com> wrote:
>
> Hello Christoph,
>
> Thanks for your review comments.
>
> On 4/24/20 3:41 PM, Christoph Hellwig wrote:
> > I think the right fix is to move fiemap_check_ranges into all the ->fiemap
>
> I do welcome your suggestion here. But I am not sure of what you are
> suggesting should be done as a 1st round of changes for the immediate
> reported problem.
> So currently these patches take the same approach on overlayfs on how
> VFS does it. So as a fix to the overlayfs over ext4 reported problems in
> thread [1] & [2]. I think these patches are doing the right thing.
>
> Also maybe I am biased in some way because as I see these are the right
> fixes with minimal changes only at places which does have a direct
> problem.
>

FWIW, I agree with you.
And seems like Jan does as well, since he ACKed all your patches.
Current patches would be easier to backport to stable kernels.

Plus, if we are going to cleanup the fiemap interface, need to look into
FIEMAP_FLAG_SYNC handling.
Does it makes sense to handle this flag in vfs ioctl code and other flags
by filesystem code?
See, iomap_fiemap() takes care of FIEMAP_FLAG_SYNC in addition
to ioctl_fiemap(), so I would think that FIEMAP_FLAG_SYNC should
probably be removed from ioctl_fiemap() and handled by
generic_block_fiemap() and other filesystem specific implementation.

Thanks,
Amir.