* [PATCH 0/2] Graceful failures for XFS on an unconfigured loop device
@ 2010-07-13 7:50 Dave Chinner
2010-07-13 7:50 ` [PATCH 1/2] xfs: don't block on buffer read errors Dave Chinner
2010-07-13 7:50 ` [PATCH 2/2] blkdev: check for valid request queue before issuing flush Dave Chinner
0 siblings, 2 replies; 6+ messages in thread
From: Dave Chinner @ 2010-07-13 7:50 UTC (permalink / raw)
To: xfs; +Cc: linux-kernel, axboe
Run:
# mount -t xfs /dev/loop0 /mnt
on a freshly booted system and it won't like you anymore. This series fixes
the broken XFS error handling for dispatch errors on synchronous reads that
causes a hang, and then patches the panic it uncovers. With these two patches,
the mount fails gracefully.
The modification to blkdev_issue_flush is probably not the right place to check
for a valid q->make_request_fn. This just patches around the problem in the
simplest way possible. There's probably a better way to fix it - personally I'd
prefer that we don't even get to mounting a filesystem on an unconfigured loop
device...
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] xfs: don't block on buffer read errors
2010-07-13 7:50 [PATCH 0/2] Graceful failures for XFS on an unconfigured loop device Dave Chinner
@ 2010-07-13 7:50 ` Dave Chinner
2010-07-14 18:28 ` Christoph Hellwig
2010-07-13 7:50 ` [PATCH 2/2] blkdev: check for valid request queue before issuing flush Dave Chinner
1 sibling, 1 reply; 6+ messages in thread
From: Dave Chinner @ 2010-07-13 7:50 UTC (permalink / raw)
To: xfs; +Cc: linux-kernel, axboe
From: Dave Chinner <dchinner@redhat.com>
xfs_buf_read() fails to detect dispatch errors before attempting to
wait on sychronous IO. If there was an error, it will get stuck
forever, waiting for an I/O that was never started. Make sure the
error is detected correctly.
Further, such a failure can leave locked pages in the page cache
which will cause a later operation to hang on the page. Ensure that
we correctly process pages in the buffers when we get a dispatch
error.
Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
fs/xfs/linux-2.6/xfs_buf.c | 15 +++++++++++++--
1 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/fs/xfs/linux-2.6/xfs_buf.c b/fs/xfs/linux-2.6/xfs_buf.c
index 76bb13c..ec79634 100644
--- a/fs/xfs/linux-2.6/xfs_buf.c
+++ b/fs/xfs/linux-2.6/xfs_buf.c
@@ -578,7 +578,7 @@ _xfs_buf_read(
XBF_READ_AHEAD | _XBF_RUN_QUEUES);
status = xfs_buf_iorequest(bp);
- if (!status && !(flags & XBF_ASYNC))
+ if (!(status || XFS_BUF_ISERROR(bp) || (flags & XBF_ASYNC)))
status = xfs_buf_iowait(bp);
return status;
}
@@ -1280,8 +1280,19 @@ submit_io:
if (size)
goto next_chunk;
} else {
- bio_put(bio);
+ /*
+ * if we get here, no pages were added to the bio. However,
+ * we can't just error out here - if the pages are locked then
+ * we have to unlock them otherwise we can hang on a later
+ * access to the page.
+ */
xfs_buf_ioerror(bp, EIO);
+ if (bp->b_flags & _XBF_PAGE_LOCKED) {
+ int i;
+ for (i = 0; i < bp->b_page_count; i++)
+ unlock_page(bp->b_pages[i]);
+ }
+ bio_put(bio);
}
}
--
1.7.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] blkdev: check for valid request queue before issuing flush
2010-07-13 7:50 [PATCH 0/2] Graceful failures for XFS on an unconfigured loop device Dave Chinner
2010-07-13 7:50 ` [PATCH 1/2] xfs: don't block on buffer read errors Dave Chinner
@ 2010-07-13 7:50 ` Dave Chinner
2010-07-13 12:55 ` Jens Axboe
1 sibling, 1 reply; 6+ messages in thread
From: Dave Chinner @ 2010-07-13 7:50 UTC (permalink / raw)
To: xfs; +Cc: linux-kernel, axboe
From: Dave Chinner <dchinner@redhat.com>
Issuing a blkdev_issue_flush() on an unconfigured loop device causes a panic as
q->make_request_fn is not configured. This can occur when trying to mount the
unconfigured loop device as an XFS filesystem. There are no guards that catch
the bio before the request function is called because we don't add a payload to
the bio. Instead, manually check this case as soon as we have a pointer to the
queue to flush.
Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
block/blk-barrier.c | 9 +++++++++
1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/block/blk-barrier.c b/block/blk-barrier.c
index 0d710c9..0fd766e 100644
--- a/block/blk-barrier.c
+++ b/block/blk-barrier.c
@@ -319,6 +319,15 @@ int blkdev_issue_flush(struct block_device *bdev, gfp_t gfp_mask,
if (!q)
return -ENXIO;
+ /*
+ * some block devices may not have their queue correctly set up here
+ * (e.g. loop device without a backing file) and so issuing a flush
+ * here will panic. Ensure there is a request function before issuing
+ * the barrier.
+ */
+ if (!q->make_request_fn)
+ return -ENXIO;
+
bio = bio_alloc(gfp_mask, 0);
bio->bi_end_io = bio_end_empty_barrier;
bio->bi_bdev = bdev;
--
1.7.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] blkdev: check for valid request queue before issuing flush
2010-07-13 7:50 ` [PATCH 2/2] blkdev: check for valid request queue before issuing flush Dave Chinner
@ 2010-07-13 12:55 ` Jens Axboe
0 siblings, 0 replies; 6+ messages in thread
From: Jens Axboe @ 2010-07-13 12:55 UTC (permalink / raw)
To: Dave Chinner; +Cc: xfs, linux-kernel
On 2010-07-13 09:50, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> Issuing a blkdev_issue_flush() on an unconfigured loop device causes a panic as
> q->make_request_fn is not configured. This can occur when trying to mount the
> unconfigured loop device as an XFS filesystem. There are no guards that catch
> the bio before the request function is called because we don't add a payload to
> the bio. Instead, manually check this case as soon as we have a pointer to the
> queue to flush.
>
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---
> block/blk-barrier.c | 9 +++++++++
> 1 files changed, 9 insertions(+), 0 deletions(-)
>
> diff --git a/block/blk-barrier.c b/block/blk-barrier.c
> index 0d710c9..0fd766e 100644
> --- a/block/blk-barrier.c
> +++ b/block/blk-barrier.c
> @@ -319,6 +319,15 @@ int blkdev_issue_flush(struct block_device *bdev, gfp_t gfp_mask,
> if (!q)
> return -ENXIO;
>
> + /*
> + * some block devices may not have their queue correctly set up here
> + * (e.g. loop device without a backing file) and so issuing a flush
> + * here will panic. Ensure there is a request function before issuing
> + * the barrier.
> + */
> + if (!q->make_request_fn)
> + return -ENXIO;
> +
> bio = bio_alloc(gfp_mask, 0);
> bio->bi_end_io = bio_end_empty_barrier;
> bio->bi_bdev = bdev;
This may appear ugly, but I think the patch is fine since there's not
much we can do about the loop crap (outside of changing how you do
setup/configure of it).
I'll apply this, thanks.
--
Jens Axboe
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] xfs: don't block on buffer read errors
2010-07-13 7:50 ` [PATCH 1/2] xfs: don't block on buffer read errors Dave Chinner
@ 2010-07-14 18:28 ` Christoph Hellwig
2010-07-15 0:05 ` Dave Chinner
0 siblings, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2010-07-14 18:28 UTC (permalink / raw)
To: Dave Chinner; +Cc: xfs, axboe, linux-kernel
Looks good, except that I'd rework the snipplet below
> status = xfs_buf_iorequest(bp);
> - if (!status && !(flags & XBF_ASYNC))
> + if (!(status || XFS_BUF_ISERROR(bp) || (flags & XBF_ASYNC)))
> status = xfs_buf_iowait(bp);
> return status;
as:
if (status || XFS_BUF_ISERROR(bp) || (flags & XBF_ASYNC))
return status;
return xfs_buf_iowait(bp);
to make it a bit more clear.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] xfs: don't block on buffer read errors
2010-07-14 18:28 ` Christoph Hellwig
@ 2010-07-15 0:05 ` Dave Chinner
0 siblings, 0 replies; 6+ messages in thread
From: Dave Chinner @ 2010-07-15 0:05 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs, axboe, linux-kernel
On Wed, Jul 14, 2010 at 02:28:36PM -0400, Christoph Hellwig wrote:
> Looks good, except that I'd rework the snipplet below
>
> > status = xfs_buf_iorequest(bp);
> > - if (!status && !(flags & XBF_ASYNC))
> > + if (!(status || XFS_BUF_ISERROR(bp) || (flags & XBF_ASYNC)))
> > status = xfs_buf_iowait(bp);
> > return status;
>
> as:
>
> if (status || XFS_BUF_ISERROR(bp) || (flags & XBF_ASYNC))
> return status;
> return xfs_buf_iowait(bp);
>
> to make it a bit more clear.
Make sense. Fixed it up.
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-07-15 0:05 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-07-13 7:50 [PATCH 0/2] Graceful failures for XFS on an unconfigured loop device Dave Chinner
2010-07-13 7:50 ` [PATCH 1/2] xfs: don't block on buffer read errors Dave Chinner
2010-07-14 18:28 ` Christoph Hellwig
2010-07-15 0:05 ` Dave Chinner
2010-07-13 7:50 ` [PATCH 2/2] blkdev: check for valid request queue before issuing flush Dave Chinner
2010-07-13 12:55 ` Jens Axboe
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).