| Message ID | 20210827164926.1726765-16-agruenba@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | gfs2: Fix mmap + page fault deadlocks | expand |
On Fri, Aug 27, 2021 at 06:49:22PM +0200, Andreas Gruenbacher wrote: > In iomap_dio_rw, when iomap_apply returns an -EFAULT error and the > IOMAP_DIO_PARTIAL flag is set, complete the request synchronously and > return a partial result. This allows the caller to deal with the page > fault and retry the remainder of the request. > > Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com> Pretty straightforward. Reviewed-by: Darrick J. Wong <djwong@kernel.org> --D > --- > fs/iomap/direct-io.c | 6 ++++++ > include/linux/iomap.h | 7 +++++++ > 2 files changed, 13 insertions(+) > > diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c > index 8054f5d6c273..ba88fe51b77a 100644 > --- a/fs/iomap/direct-io.c > +++ b/fs/iomap/direct-io.c > @@ -561,6 +561,12 @@ __iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter, > ret = iomap_apply(inode, pos, count, iomap_flags, ops, dio, > iomap_dio_actor); > if (ret <= 0) { > + if (ret == -EFAULT && dio->size && > + (dio_flags & IOMAP_DIO_PARTIAL)) { > + wait_for_completion = true; > + ret = 0; > + } > + > /* magic error code to fall back to buffered I/O */ > if (ret == -ENOTBLK) { > wait_for_completion = true; > diff --git a/include/linux/iomap.h b/include/linux/iomap.h > index 479c1da3e221..bcae4814b8e3 100644 > --- a/include/linux/iomap.h > +++ b/include/linux/iomap.h > @@ -267,6 +267,13 @@ struct iomap_dio_ops { > */ > #define IOMAP_DIO_OVERWRITE_ONLY (1 << 1) > > +/* > + * When a page fault occurs, return a partial synchronous result and allow > + * the caller to retry the rest of the operation after dealing with the page > + * fault. > + */ > +#define IOMAP_DIO_PARTIAL (1 << 2) > + > ssize_t iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter, > const struct iomap_ops *ops, const struct iomap_dio_ops *dops, > unsigned int dio_flags); > -- > 2.26.3 >
On Fri, Aug 27, 2021 at 06:49:22PM +0200, Andreas Gruenbacher wrote: > In iomap_dio_rw, when iomap_apply returns an -EFAULT error and the > IOMAP_DIO_PARTIAL flag is set, complete the request synchronously and > return a partial result. This allows the caller to deal with the page > fault and retry the remainder of the request. > > Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com> > --- > fs/iomap/direct-io.c | 6 ++++++ > include/linux/iomap.h | 7 +++++++ > 2 files changed, 13 insertions(+) > > diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c > index 8054f5d6c273..ba88fe51b77a 100644 > --- a/fs/iomap/direct-io.c > +++ b/fs/iomap/direct-io.c > @@ -561,6 +561,12 @@ __iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter, > ret = iomap_apply(inode, pos, count, iomap_flags, ops, dio, > iomap_dio_actor); > if (ret <= 0) { > + if (ret == -EFAULT && dio->size && > + (dio_flags & IOMAP_DIO_PARTIAL)) { > + wait_for_completion = true; > + ret = 0; Do we need a NOWAIT check here to skip the wait_for_completion for that case?
On Thu, Sep 9, 2021 at 1:22 PM Christoph Hellwig <hch@infradead.org> wrote: > On Fri, Aug 27, 2021 at 06:49:22PM +0200, Andreas Gruenbacher wrote: > > In iomap_dio_rw, when iomap_apply returns an -EFAULT error and the > > IOMAP_DIO_PARTIAL flag is set, complete the request synchronously and > > return a partial result. This allows the caller to deal with the page > > fault and retry the remainder of the request. > > > > Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com> > > --- > > fs/iomap/direct-io.c | 6 ++++++ > > include/linux/iomap.h | 7 +++++++ > > 2 files changed, 13 insertions(+) > > > > diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c > > index 8054f5d6c273..ba88fe51b77a 100644 > > --- a/fs/iomap/direct-io.c > > +++ b/fs/iomap/direct-io.c > > @@ -561,6 +561,12 @@ __iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter, > > ret = iomap_apply(inode, pos, count, iomap_flags, ops, dio, > > iomap_dio_actor); > > if (ret <= 0) { > > + if (ret == -EFAULT && dio->size && > > + (dio_flags & IOMAP_DIO_PARTIAL)) { > > + wait_for_completion = true; > > + ret = 0; > > Do we need a NOWAIT check here to skip the wait_for_completion > for that case? Hmm, you're probably right, yes. I'll add that. Thanks, Andreas
diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c index 8054f5d6c273..ba88fe51b77a 100644 --- a/fs/iomap/direct-io.c +++ b/fs/iomap/direct-io.c @@ -561,6 +561,12 @@ __iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter, ret = iomap_apply(inode, pos, count, iomap_flags, ops, dio, iomap_dio_actor); if (ret <= 0) { + if (ret == -EFAULT && dio->size && + (dio_flags & IOMAP_DIO_PARTIAL)) { + wait_for_completion = true; + ret = 0; + } + /* magic error code to fall back to buffered I/O */ if (ret == -ENOTBLK) { wait_for_completion = true; diff --git a/include/linux/iomap.h b/include/linux/iomap.h index 479c1da3e221..bcae4814b8e3 100644 --- a/include/linux/iomap.h +++ b/include/linux/iomap.h @@ -267,6 +267,13 @@ struct iomap_dio_ops { */ #define IOMAP_DIO_OVERWRITE_ONLY (1 << 1) +/* + * When a page fault occurs, return a partial synchronous result and allow + * the caller to retry the rest of the operation after dealing with the page + * fault. + */ +#define IOMAP_DIO_PARTIAL (1 << 2) + ssize_t iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter, const struct iomap_ops *ops, const struct iomap_dio_ops *dops, unsigned int dio_flags);
In iomap_dio_rw, when iomap_apply returns an -EFAULT error and the IOMAP_DIO_PARTIAL flag is set, complete the request synchronously and return a partial result. This allows the caller to deal with the page fault and retry the remainder of the request. Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com> --- fs/iomap/direct-io.c | 6 ++++++ include/linux/iomap.h | 7 +++++++ 2 files changed, 13 insertions(+)