[Christoph's email ended up in my spam folder; I hope that was a one-time-only occurrence.] On Wed, Jan 18, 2023 at 5:00 PM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote: > On Wed, Jan 18, 2023 at 10:43:27AM +0100, Christoph Hellwig wrote: > > filemap_get_folio can return NULL, so exit early for that case. > > I'm not sure it can return NULL in this specific case. As I understand > this code, we're scanning the journal looking for the log head. We've > just submitted the bio to read this page. I suppose memory pressure > could theoretically push the page out, but if it does, we're doing the > wrong thing by just returning here; we need to retry reading the page. > > Assuming we're not willing to do the work to add that case, I think I'd > rather see the crash in folio_wait_locked() than get data corruption > from failing to find the head of the log. > > > Signed-off-by: Christoph Hellwig <hch@xxxxxx> > > --- > > fs/gfs2/lops.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/fs/gfs2/lops.c b/fs/gfs2/lops.c > > index 1902413d5d123e..51d4b610127cdb 100644 > > --- a/fs/gfs2/lops.c > > +++ b/fs/gfs2/lops.c > > @@ -472,6 +472,8 @@ static void gfs2_jhead_process_page(struct gfs2_jdesc *jd, unsigned long index, > > struct folio *folio; > > > > folio = filemap_get_folio(jd->jd_inode->i_mapping, index); > > + if (!folio) > > + return; We're actually still holding a reference to the folio from the find_or_create_page() in gfs2_find_jhead() here, so we know that memory pressure can't push the page out and filemap_get_folio() won't return NULL. > > > > folio_wait_locked(folio); > > if (folio_test_error(folio)) > > -- > > 2.39.0 > > > Thanks, Andreas
