Graceful failures for XFS on an unconfigured loop device
Subsystems (2)
cover Graceful failures for XFS on an unconfigured loop device
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...
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
PATCH 1/2 xfs: don't block on buffer read errors
From: Dave Chinner <[email protected]>
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.
--- fs/xfs/linux-2.6/xfs_buf.c | 15 +++++++++++++-- 1 files changed, 13 insertions(+), 2 deletions(-)
fs/xfs/linux-2.6/xfs_buf.c +13 -2
@@ -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); } }
Signature · 6 lines
1.7.1 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Re: [PATCH 1/2] xfs: don't block on buffer read errors
Looks good, except that I'd rework the snipplet below
fs/xfs/linux-2.6/xfs_buf.c _xfs_buf_read( · line 583
Quoted 5 lines
fs/xfs/linux-2.6/xfs_buf.c +1
-1status =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.
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Re: [PATCH 1/2] xfs: don't block on buffer read errors
Quoted 10 lines · Christoph Hellwig
Looks good, except that I'd rework the snipplet below
fs/xfs/linux-2.6/xfs_buf.c _xfs_buf_read( · line 583
Quoted 5 lines
fs/xfs/linux-2.6/xfs_buf.c +1
-1status =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:
Quoted 5 lines
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.
Signature · 7 lines
Dave Chinner [email protected] _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
PATCH 2/2 blkdev: check for valid request queue before issuing flush
In reply to Dave ChinnerFrom: Dave Chinner <[email protected]>
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.
--- block/blk-barrier.c | 9 +++++++++ 1 files changed, 9 insertions(+), 0 deletions(-)
block/blk-barrier.c +9 -0
@@ -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;
Signature · 6 lines
1.7.1 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Jens Axboe
maintainerRe: [PATCH 2/2] blkdev: check for valid request queue before issuing flush
Quoted 24 lines · Dave Chinner
From: Dave Chinner <[email protected]>
Issuing a
blkdev_issue_flush()on an unconfigured loop device causes a panic asq->make_request_fnis 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 <[email protected]>--- block/blk-barrier.c | 9 +++++++++ 1 files changed, 9 insertions(+), 0 deletions(-)block/blk-barrier.c +2
-0@@ -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
block/blk-barrier.c int blkdev_issue_flush(struct block_device *bdev, gfp_t gfp_mask, · line 333
Quoted 10 lines
block/blk-barrier.c +7
-0+* (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.
Signature · 6 lines
Jens Axboe _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs