| Message ID | 20200703095325.1491832-2-agruenba@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix gfs2 readahead deadlocks | expand |
On Fri, Jul 03, 2020 at 11:53:24AM +0200, Andreas Gruenbacher wrote: > Add an IOCB_NOIO flag that indicates to generic_file_read_iter that it > shouldn't trigger any filesystem I/O for the actual request or for > readahead. This allows to do tentative reads out of the page cache as > some filesystems allow, and to take the appropriate locks and retry the > reads only if the requested pages are not cached. > > Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com> Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org> > @@ -2249,9 +2253,18 @@ EXPORT_SYMBOL_GPL(generic_file_buffered_read); > * > * This is the "read_iter()" routine for all filesystems > * 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 -EAGAIN shall be > + * returned when no data can be read without issuing new I/O requests, > + * and 0 shall be returned when readhead would have been triggered. s/shall/may/ -- if we read a previous page then hit a readahead page, we'll return a positive value. If the first page we hit is a readahead page, then yes, we'll return zero. Again, I'm happy for the patch to go in as-is without this nitpick.
On Fri, Jul 3, 2020 at 1:41 PM Matthew Wilcox <willy@infradead.org> wrote: > > On Fri, Jul 03, 2020 at 11:53:24AM +0200, Andreas Gruenbacher wrote: > > Add an IOCB_NOIO flag that indicates to generic_file_read_iter that it > > shouldn't trigger any filesystem I/O for the actual request or for > > readahead. This allows to do tentative reads out of the page cache as > > some filesystems allow, and to take the appropriate locks and retry the > > reads only if the requested pages are not cached. > > > > Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com> > > Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org> > > > @@ -2249,9 +2253,18 @@ EXPORT_SYMBOL_GPL(generic_file_buffered_read); > > * > > * This is the "read_iter()" routine for all filesystems > > * 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 -EAGAIN shall be > > + * returned when no data can be read without issuing new I/O requests, > > + * and 0 shall be returned when readhead would have been triggered. > > s/shall/may/ -- if we read a previous page then hit a readahead page, > we'll return a positive value. If the first page we hit is a readahead > page, then yes, we'll return zero. How about this? * 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 short read (possibly of length 0) shall be returned. > Again, I'm happy for the patch to go in as-is without this nitpick. Thanks, Andreas
diff --git a/include/linux/fs.h b/include/linux/fs.h index 3f881a892ea7..1ab2ea19e883 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -315,6 +315,7 @@ enum rw_hint { #define IOCB_SYNC (1 << 5) #define IOCB_WRITE (1 << 6) #define IOCB_NOWAIT (1 << 7) +#define IOCB_NOIO (1 << 8) struct kiocb { struct file *ki_filp; diff --git a/mm/filemap.c b/mm/filemap.c index f0ae9a6308cb..22f7ff2d369e 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -2028,7 +2028,7 @@ ssize_t generic_file_buffered_read(struct kiocb *iocb, page = find_get_page(mapping, index); if (!page) { - if (iocb->ki_flags & IOCB_NOWAIT) + if (iocb->ki_flags & (IOCB_NOWAIT | IOCB_NOIO)) goto would_block; page_cache_sync_readahead(mapping, ra, filp, @@ -2038,6 +2038,10 @@ ssize_t generic_file_buffered_read(struct kiocb *iocb, goto no_cached_page; } if (PageReadahead(page)) { + if (iocb->ki_flags & IOCB_NOIO) { + put_page(page); + goto out; + } page_cache_async_readahead(mapping, ra, filp, page, index, last_index - index); @@ -2249,9 +2253,18 @@ EXPORT_SYMBOL_GPL(generic_file_buffered_read); * * This is the "read_iter()" routine for all filesystems * 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 -EAGAIN shall be + * returned when no data can be read without issuing new I/O requests, + * and 0 shall be returned when readhead would have been triggered. + * * Return: * * 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 generic_file_read_iter(struct kiocb *iocb, struct iov_iter *iter)
Add an IOCB_NOIO flag that indicates to generic_file_read_iter that it shouldn't trigger any filesystem I/O for the actual request or for readahead. This allows to do tentative reads out of the page cache as some filesystems allow, and to take the appropriate locks and retry the reads only if the requested pages are not cached. Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com> --- include/linux/fs.h | 1 + mm/filemap.c | 17 +++++++++++++++-- 2 files changed, 16 insertions(+), 2 deletions(-)