linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andreas Gruenbacher <agruenba@redhat.com>
To: Christoph Hellwig <hch@lst.de>
Cc: Bob Peterson <rpeterso@redhat.com>,
	linux-fsdevel@vger.kernel.org, cluster-devel@redhat.com
Subject: Re: [PATCH] Revert "gfs2: stop using generic_writepages in gfs2_ail1_start_one"
Date: Sun, 22 Jan 2023 09:48:38 +0100	[thread overview]
Message-ID: <CAHc6FU5udwDCBaH8Cm1EMDh8P1_7WwRVU2SXgF-SZmh5pE2-8Q@mail.gmail.com> (raw)
In-Reply-To: <20230121142927.GB6786@lst.de>

On Sat, Jan 21, 2023 at 3:29 PM Christoph Hellwig <hch@lst.de> wrote:
>
> > +     struct address_space *mapping = data;
> > +     int ret = mapping->a_ops->writepage(page, wbc);
> > +     mapping_set_error(mapping, ret);
> > +     return ret;
>
> I guess beggars can't be choosers, but is there a chance to directly
> call the relevant gfs2 writepage methods here instead of the
> ->writepage call?

Yes, we could wrap struct address_space_operations and move the
writepage method into its wrapper structure relatively easily, but
that would still leave things in a messy state. So I'd really like to
reassess the validity of commit 5ac048bb7ea6 ("GFS2: Use
filemap_fdatawrite() to write back the AIL") before deciding to go
that way.

Also, we're really trying to iterate the list of inodes that are part
of the transaction here, not the list of blocks, and if we stick with
that, an actual list of inodes would help. That would be the
complement of our list of ordered inodes in a sense.

Until then, I'd like to stick with the simplest possible solution
though, which seems to be this.

> Otherwise this looks good:
>
> Acked-by: Christoph Hellwig <hch@lst.de>

Thanks a lot,
Andreas


      reply	other threads:[~2023-01-22  8:49 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-20 14:11 [PATCH] Revert "gfs2: stop using generic_writepages in gfs2_ail1_start_one" Andreas Gruenbacher
2023-01-21 14:29 ` Christoph Hellwig
2023-01-22  8:48   ` Andreas Gruenbacher [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAHc6FU5udwDCBaH8Cm1EMDh8P1_7WwRVU2SXgF-SZmh5pE2-8Q@mail.gmail.com \
    --to=agruenba@redhat.com \
    --cc=cluster-devel@redhat.com \
    --cc=hch@lst.de \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=rpeterso@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).