Fix gfs2 readahead deadlocks

-----BEGIN PGP SIGNATURE-----
 
 iQJIBAABCAAyFiEEJZs3krPW0xkhLMTc1b+f6wMTZToFAl8IgKwUHGFncnVlbmJh
 QHJlZGhhdC5jb20ACgkQ1b+f6wMTZTpm2Q//bj3ZDIjep9a4d7mRVGeX3OeslLzk
 NDB2Vu03B0oZKQFYbQNdpblxy2Cfyz4m8xkNCdsD8EQ2d1zaPWhywJ6vxc1VO5Dw
 wRODwRMgVe0hd9dLR8b8GzUO0+4ncpjqmyEyrCRjwPRkghcX8uuSTifXtY+yeDEv
 X2BHlSGMjqCFBfq+RTa8Fi3wWFy9QhGy74QVoidMM0ulFLJbWSu0EnCXZ+hZQ4vR
 sJokd2SDSP60LE964CwMxuMNUNwSMwL3VrlUm74qx1WVCK8lyYtm231E5CAHRbAw
 C/f6sIKoyzyfJbv2HqgvMXvh72hO4MaJgIb8Pbht8a9GZdfk6i2JbcNmHXXk5OMN
 GkYLLhkDrj4X/MChNuk20Zsylaij1+CCLb6C4UsQeXF0e/QA6iYIGRmpApGN2gNP
 IA8rTz4Ibmd5ZpVMJNPOGSbq3fpPEboEoxVn+fWVvhDTopATxYS85tKqU5Bfvdr5
 QcBqqeAL9yludQa520C1lIbGDBOJ57LisybMBVufklx8ZtFNNbHyB/b1YnfUBvRF
 8WXVpYkh1ckB4VvVj7qnKY2/JJT0VVhQmTogqwqZy9m+Nb8I4l0pemUsJnypS0qs
 KmoBvZmhWhE3tnqmCVzSvuHzO/eYGSfN91AavGBaddFzsqLLe8Hkm8kzlS5bZxGn
 OVWGWVvuoSu72s8=
 =dfnJ
 -----END PGP SIGNATURE-----

Merge tag 'gfs2-v5.8-rc4.fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2

Pull gfs2 fixes from Andreas Gruenbacher:
 "Fix gfs2 readahead deadlocks by adding a IOCB_NOIO flag that allows
  gfs2 to use the generic fiel read iterator functions without having to
  worry about being called back while holding locks".

* tag 'gfs2-v5.8-rc4.fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2:
  gfs2: Rework read and page fault locking
  fs: Add IOCB_NOIO flag for generic_file_read_iter
This commit is contained in:
Linus Torvalds 2020-07-10 08:53:21 -07:00
commit d02b0478c1
4 changed files with 73 additions and 48 deletions

View File

@ -468,21 +468,10 @@ static int stuffed_readpage(struct gfs2_inode *ip, struct page *page)
} }
/**
* __gfs2_readpage - readpage
* @file: The file to read a page for
* @page: The page to read
*
* This is the core of gfs2's readpage. It's used by the internal file
* reading code as in that case we already hold the glock. Also it's
* called by gfs2_readpage() once the required lock has been granted.
*/
static int __gfs2_readpage(void *file, struct page *page) static int __gfs2_readpage(void *file, struct page *page)
{ {
struct gfs2_inode *ip = GFS2_I(page->mapping->host); struct gfs2_inode *ip = GFS2_I(page->mapping->host);
struct gfs2_sbd *sdp = GFS2_SB(page->mapping->host); struct gfs2_sbd *sdp = GFS2_SB(page->mapping->host);
int error; int error;
if (i_blocksize(page->mapping->host) == PAGE_SIZE && if (i_blocksize(page->mapping->host) == PAGE_SIZE &&
@ -505,36 +494,11 @@ static int __gfs2_readpage(void *file, struct page *page)
* gfs2_readpage - read a page of a file * gfs2_readpage - read a page of a file
* @file: The file to read * @file: The file to read
* @page: The page of the file * @page: The page of the file
*
* This deals with the locking required. We have to unlock and
* relock the page in order to get the locking in the right
* order.
*/ */
static int gfs2_readpage(struct file *file, struct page *page) static int gfs2_readpage(struct file *file, struct page *page)
{ {
struct address_space *mapping = page->mapping; return __gfs2_readpage(file, page);
struct gfs2_inode *ip = GFS2_I(mapping->host);
struct gfs2_holder gh;
int error;
unlock_page(page);
gfs2_holder_init(ip->i_gl, LM_ST_SHARED, 0, &gh);
error = gfs2_glock_nq(&gh);
if (unlikely(error))
goto out;
error = AOP_TRUNCATED_PAGE;
lock_page(page);
if (page->mapping == mapping && !PageUptodate(page))
error = __gfs2_readpage(file, page);
else
unlock_page(page);
gfs2_glock_dq(&gh);
out:
gfs2_holder_uninit(&gh);
if (error && error != AOP_TRUNCATED_PAGE)
lock_page(page);
return error;
} }
/** /**
@ -598,16 +562,9 @@ static void gfs2_readahead(struct readahead_control *rac)
{ {
struct inode *inode = rac->mapping->host; struct inode *inode = rac->mapping->host;
struct gfs2_inode *ip = GFS2_I(inode); struct gfs2_inode *ip = GFS2_I(inode);
struct gfs2_holder gh;
gfs2_holder_init(ip->i_gl, LM_ST_SHARED, 0, &gh);
if (gfs2_glock_nq(&gh))
goto out_uninit;
if (!gfs2_is_stuffed(ip)) if (!gfs2_is_stuffed(ip))
mpage_readahead(rac, gfs2_block_map); mpage_readahead(rac, gfs2_block_map);
gfs2_glock_dq(&gh);
out_uninit:
gfs2_holder_uninit(&gh);
} }
/** /**

View File

@ -558,8 +558,29 @@ out_uninit:
return block_page_mkwrite_return(ret); return block_page_mkwrite_return(ret);
} }
static vm_fault_t gfs2_fault(struct vm_fault *vmf)
{
struct inode *inode = file_inode(vmf->vma->vm_file);
struct gfs2_inode *ip = GFS2_I(inode);
struct gfs2_holder gh;
vm_fault_t ret;
int err;
gfs2_holder_init(ip->i_gl, LM_ST_SHARED, 0, &gh);
err = gfs2_glock_nq(&gh);
if (err) {
ret = block_page_mkwrite_return(err);
goto out_uninit;
}
ret = filemap_fault(vmf);
gfs2_glock_dq(&gh);
out_uninit:
gfs2_holder_uninit(&gh);
return ret;
}
static const struct vm_operations_struct gfs2_vm_ops = { static const struct vm_operations_struct gfs2_vm_ops = {
.fault = filemap_fault, .fault = gfs2_fault,
.map_pages = filemap_map_pages, .map_pages = filemap_map_pages,
.page_mkwrite = gfs2_page_mkwrite, .page_mkwrite = gfs2_page_mkwrite,
}; };
@ -824,6 +845,9 @@ out_uninit:
static ssize_t gfs2_file_read_iter(struct kiocb *iocb, struct iov_iter *to) static ssize_t gfs2_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
{ {
struct gfs2_inode *ip;
struct gfs2_holder gh;
size_t written = 0;
ssize_t ret; ssize_t ret;
if (iocb->ki_flags & IOCB_DIRECT) { if (iocb->ki_flags & IOCB_DIRECT) {
@ -832,7 +856,31 @@ static ssize_t gfs2_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
return ret; return ret;
iocb->ki_flags &= ~IOCB_DIRECT; iocb->ki_flags &= ~IOCB_DIRECT;
} }
return generic_file_read_iter(iocb, to); iocb->ki_flags |= IOCB_NOIO;
ret = generic_file_read_iter(iocb, to);
iocb->ki_flags &= ~IOCB_NOIO;
if (ret >= 0) {
if (!iov_iter_count(to))
return ret;
written = ret;
} else {
if (ret != -EAGAIN)
return ret;
if (iocb->ki_flags & IOCB_NOWAIT)
return ret;
}
ip = GFS2_I(iocb->ki_filp->f_mapping->host);
gfs2_holder_init(ip->i_gl, LM_ST_SHARED, 0, &gh);
ret = gfs2_glock_nq(&gh);
if (ret)
goto out_uninit;
ret = generic_file_read_iter(iocb, to);
if (ret > 0)
written += ret;
gfs2_glock_dq(&gh);
out_uninit:
gfs2_holder_uninit(&gh);
return written ? written : ret;
} }
/** /**

View File

@ -315,6 +315,7 @@ enum rw_hint {
#define IOCB_SYNC (1 << 5) #define IOCB_SYNC (1 << 5)
#define IOCB_WRITE (1 << 6) #define IOCB_WRITE (1 << 6)
#define IOCB_NOWAIT (1 << 7) #define IOCB_NOWAIT (1 << 7)
#define IOCB_NOIO (1 << 9)
struct kiocb { struct kiocb {
struct file *ki_filp; struct file *ki_filp;

View File

@ -2028,7 +2028,7 @@ find_page:
page = find_get_page(mapping, index); page = find_get_page(mapping, index);
if (!page) { if (!page) {
if (iocb->ki_flags & IOCB_NOWAIT) if (iocb->ki_flags & (IOCB_NOWAIT | IOCB_NOIO))
goto would_block; goto would_block;
page_cache_sync_readahead(mapping, page_cache_sync_readahead(mapping,
ra, filp, ra, filp,
@ -2038,6 +2038,10 @@ find_page:
goto no_cached_page; goto no_cached_page;
} }
if (PageReadahead(page)) { if (PageReadahead(page)) {
if (iocb->ki_flags & IOCB_NOIO) {
put_page(page);
goto out;
}
page_cache_async_readahead(mapping, page_cache_async_readahead(mapping,
ra, filp, page, ra, filp, page,
index, last_index - index); index, last_index - index);
@ -2160,6 +2164,11 @@ page_not_up_to_date_locked:
} }
readpage: readpage:
if (iocb->ki_flags & IOCB_NOIO) {
unlock_page(page);
put_page(page);
goto would_block;
}
/* /*
* A previous I/O error may have been due to temporary * A previous I/O error may have been due to temporary
* failures, eg. multipath errors. * failures, eg. multipath errors.
@ -2249,9 +2258,19 @@ EXPORT_SYMBOL_GPL(generic_file_buffered_read);
* *
* This is the "read_iter()" routine for all filesystems * This is the "read_iter()" routine for all filesystems
* that can use the page cache directly. * that can use the page cache directly.
*
* The IOCB_NOWAIT flag in iocb->ki_flags indicates that -EAGAIN shall
* be returned when no data can be read without waiting for I/O requests
* to complete; it doesn't prevent readahead.
*
* The IOCB_NOIO flag in iocb->ki_flags indicates that no new I/O
* requests shall be made for the read or for readahead. When no data
* can be read, -EAGAIN shall be returned. When readahead would be
* triggered, a partial, possibly empty read shall be returned.
*
* Return: * Return:
* * number of bytes copied, even for partial reads * * number of bytes copied, even for partial reads
* * negative error code if nothing was read * * negative error code (or 0 if IOCB_NOIO) if nothing was read
*/ */
ssize_t ssize_t
generic_file_read_iter(struct kiocb *iocb, struct iov_iter *iter) generic_file_read_iter(struct kiocb *iocb, struct iov_iter *iter)