Re: [PATCH 1/2] iomap: Add a page_prepare callback

[Andreas Gruenbacher <agruenba@redhat.com>]

On Thu, 25 Apr 2019 at 10:32, Jan Kara <jack@suse.cz> wrote:
> On Wed 24-04-19 19:18:03, Andreas Gruenbacher wrote:
> > Add a page_prepare calback that's called before a page is written to.  This
> > will be used by gfs2 to start a transaction in page_prepare and end it in
> > page_done.  Other filesystems that implement data journaling will require the
> > same kind of mechanism.
> >
> > Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
>
> Thanks for the patch. Some comments below.
>
> > diff --git a/fs/iomap.c b/fs/iomap.c
> > index 97cb9d486a7d..abd9aa76dbd1 100644
> > --- a/fs/iomap.c
> > +++ b/fs/iomap.c
> > @@ -684,6 +684,10 @@ iomap_write_begin(struct inode *inode, loff_t pos, unsigned len, unsigned flags,
> >               status = __block_write_begin_int(page, pos, len, NULL, iomap);
> >       else
> >               status = __iomap_write_begin(inode, pos, len, page, iomap);
> > +
> > +     if (likely(!status) && iomap->page_prepare)
> > +             status = iomap->page_prepare(inode, pos, len, page, iomap);
> > +
> >       if (unlikely(status)) {
> >               unlock_page(page);
> >               put_page(page);
>
> So this gets called after a page is locked. Is it OK for GFS2 to acquire
> sd_log_flush_lock under page lock? Because e.g. gfs2_write_jdata_pagevec()
> seems to acquire these locks the other way around so that could cause ABBA
> deadlocks?

Good catch, the callback indeed needs to happen earlier.

Thanks,
Andreas