On Thu, Apr 22, 2021 at 5:57 PM Bob Peterson <[email protected]> wrote:
> ----- Original Message -----
> > OK, thanks for explanation! I missed that GFS2 glocks are task local. But
> > then I have another question. We have the following:
> >
> > gfs2_file_read_iter()
> > grabs inode glock in shared mode
> > generic_file_read_iter()
> > filemap_read()
> > copy_page_to_iter()
> > <page fault>
> > grabs mmap_sem
> > gfs2_fault()
> > - tries to grab inode glock again -> possible recursive locking
> >
> > And even if the locking isn't recursive, you have glock->mmap_sem and
> > mmap_sem->glock lock orderings so ABBA deadlocks are possible AFAICT.
> >
> > And there's a similar problem with the write path as well, just the lock is
> > grabbed exclusively.
> >
> > Honza
> > --
> > Jan Kara <[email protected]>
> > SUSE Labs, CR
>
> Hi Jan,
>
> IIRC, gfs2 tries to prevent the page fault by having gfs2_file_read_iter
> call into generic_file_read_iter twice:
>
> The first time without the glock held, but specifying IOCB_NOIO, which
> brings the pages in and returns -EAGAIN. Then it acquires the glock,
> then calls into generic_file_read_iter a second time.
The IOCB_NOIO generic_file_read_iter call is there for lockless reads,
not there for faulting pages in at all. See commit 20f829999c38
("gfs2: Rework read and page fault locking").
Andreas
