On Sun, Jun 4, 2023 at 5:38 AM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote:
>
> On Sat, Jun 03, 2023 at 11:34:14AM +0200, Andreas Gruenbacher wrote:
> > > * This is the same as calling block_write_full_page, but it also
> > > * writes pages outside of i_size
> > > */
> > > -static int gfs2_write_jdata_page(struct page *page,
> > > +static int gfs2_write_jdata_folio(struct folio *folio,
> > > struct writeback_control *wbc)
> > > {
> > > - struct inode * const inode = page->mapping->host;
> > > + struct inode * const inode = folio->mapping->host;
> > > loff_t i_size = i_size_read(inode);
> > > - const pgoff_t end_index = i_size >> PAGE_SHIFT;
> > > - unsigned offset;
> > >
> > > + if (folio_pos(folio) >= i_size)
> > > + return 0;
> >
> > Function gfs2_write_jdata_page was originally introduced as
> > gfs2_write_full_page in commit fd4c5748b8d3 ("gfs2: writeout truncated
> > pages") to allow writing pages even when they are beyond EOF, as the
> > function description documents.
>
> Well, that was stupid of me.
>
> > This hack was added because simply skipping journaled pages isn't
> > enough on gfs2; before a journaled page can be freed, it needs to be
> > marked as "revoked" in the journal. Journal recovery will then skip
> > the revoked blocks, which allows them to be reused for regular,
> > non-journaled data. We can end up here in contexts in which we cannot
> > "revoke" pages, so instead, we write the original pages even when they
> > are beyond EOF. This hack could be revisited, but it's pretty nasty
> > code to pick apart.
> >
> > So at least the above if needs to go for now.
>
> Understood. So we probably don't want to waste time zeroing the folio
> if it is entirely beyond i_size, right? Because at the moment we'd
> zero some essentially random part of the folio if I just take out the
> check. Should it look like this?
>
> if (folio_pos(folio) < i_size &&
> i_size < folio_pos(folio) + folio_size(folio))
> folio_zero_segment(folio, offset_in_folio(folio, i_size),
> folio_size(folio));
Yes, looking good, thanks.
If you haven't already, could you please consider my other comment as
well before you repost?
https://lore.kernel.org/linux-fsdevel/CAHc6FU6GowpTfX-MgRiqqwZZJ0r-85C9exc2pNkBkySCGUT0FA@xxxxxxxxxxxxxx/
Thanks,
Andreas
