Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220920014036.2295853-1-daeho43@gmail.com> <f4ce9486-f104-b0e2-25ed-f6de96316b76@kernel.org>
 <CACOAw_z=9H6jEQNd8C99c6xO55PJXWJOW7Q=78qtppgysebN2A@mail.gmail.com>
 <4aca0d00-d3b7-975f-6b72-ccd6f07d22e5@kernel.org> <CACOAw_wVU1gmH1gyWHYNqCpgy4KGKB+EZK6pbGL-h_1ToDV=vg@mail.gmail.com>
 <53df1b78-c611-6a22-88b1-74cc83a9e148@kernel.org> <CACOAw_w_09sz8PxnSGzNmJwh1-y_3JoKbBu80VjgZgV1uxo7KA@mail.gmail.com>
In-Reply-To: <CACOAw_w_09sz8PxnSGzNmJwh1-y_3JoKbBu80VjgZgV1uxo7KA@mail.gmail.com>
From:   Daeho Jeong <daeho43@gmail.com>
Date:   Fri, 30 Sep 2022 13:01:37 -0700
Message-ID: <CACOAw_wjcFr1fg3QRYPUyOGyo-G9D9U6=qprkPUP8PzoxPQkMQ@mail.gmail.com>
Subject: Re: [f2fs-dev] [PATCH v2] f2fs: introduce F2FS_IOC_START_ATOMIC_REPLACE
To:     Chao Yu <chao@kernel.org>
Cc:     linux-kernel@vger.kernel.org,
        linux-f2fs-devel@lists.sourceforge.net, kernel-team@android.com,
        Daeho Jeong <daehojeong@google.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Fri, Sep 30, 2022 at 9:04 AM Daeho Jeong <daeho43@gmail.com> wrote:
>
> > >>>>
> > >>>> Hi Daeho,
> > >>>>
> > >>>> isize should be updated after tagging inode as atomic_write one?
> > >>>> otherwise f2fs_mark_inode_dirty_sync() may update isize to inode page,
> > >>>> latter checkpoint may persist that change? IIUC...
> > >>>>
> > >>>> Thanks,
> > >>>
> > >>> Hi Chao,
> > >>>
> > >>> The first patch of this patchset prevents the inode page from being
> > >>> updated as dirty for atomic file cases.
> > >>> Is there any other chances for the inode page to be marked as dirty?
> > >>
> > >> I mean:
> > >>
> > >> Thread A                                Thread B
> > >> - f2fs_ioc_start_atomic_write
> > >>    - f2fs_i_size_write(inode, 0)
> > >>     - f2fs_mark_inode_dirty_sync
> > >>                                          - checkpoint
> > >>                                           - persist inode with incorrect zero isize
> > >>
> > >>    - set_inode_flag(inode, FI_ATOMIC_FILE)
> > >>
> > >> Am I missing something?
> > >>
> > >
> > > So, f2fs_mark_inode_dirty_sync() will not work for atomic files
> > > anymore, which means it doesn't make the inode dirty.
> > > Plz, refer to the first patch of this patchset. Or I might be confused
> > > with something. :(
> >
> > I mean FI_ATOMIC_FILE was set after f2fs_i_size_write(), so inode will be set
> > as dirty.
> >
> > Thanks,
> >
>
> Oh, I was confused that f2fs_update_inode() is called in
> f2fs_mark_inode_dirty_sync().
> That is called in f2fs_write_inode(). Let me fix this.

Hmm, I think the issue was already there before my patch.
So, how about making the inode flushed and clean if the inode is
already dirty when starting atomic write?

>
> Thanks,