From b744c2ac4bbc040794efb33207d6ebc14f88ea2e Mon Sep 17 00:00:00 2001 From: Robert Elliott Date: Tue, 21 Oct 2014 13:55:09 -0600 Subject: fs: merge I/O error prints into one line buffer.c uses two printk calls to print these messages: [67353.422338] Buffer I/O error on device sdr, logical block 212868488 [67353.422338] lost page write due to I/O error on sdr In a busy system, they may be interleaved with other prints, losing the context for the second message. Merge them into one line with one printk call so the prints are atomic. Also, differentiate between async page writes, sync page writes, and async page reads. Also, shorten "device" to "dev" to match the block layer prints: [67353.467906] blk_update_request: critical target error, dev sdr, sector 1707107328 Also, use %llu rather than %Lu. Resulting prints look like: [ 1356.437006] blk_update_request: critical target error, dev sdr, sector 1719693992 [ 1361.383522] quiet_error: 659876 callbacks suppressed [ 1361.385816] Buffer I/O error on dev sdr, logical block 256902912, lost async page write [ 1361.385819] Buffer I/O error on dev sdr, logical block 256903644, lost async page write Signed-off-by: Robert Elliott Reviewed-by: Webb Scales Signed-off-by: Jens Axboe --- fs/buffer.c | 27 ++++++++------------------- 1 file changed, 8 insertions(+), 19 deletions(-) diff --git a/fs/buffer.c b/fs/buffer.c index 6c48f20e..9d1da1d 100644 --- a/fs/buffer.c +++ b/fs/buffer.c @@ -137,12 +137,12 @@ static int quiet_error(struct buffer_head *bh) } -static void buffer_io_error(struct buffer_head *bh) +static void buffer_io_error(struct buffer_head *bh, char *msg) { char b[BDEVNAME_SIZE]; - printk(KERN_ERR "Buffer I/O error on device %s, logical block %Lu\n", + printk(KERN_ERR "Buffer I/O error on dev %s, logical block %llu%s\n", bdevname(bh->b_bdev, b), - (unsigned long long)bh->b_blocknr); + (unsigned long long)bh->b_blocknr, msg); } /* @@ -177,17 +177,11 @@ EXPORT_SYMBOL(end_buffer_read_sync); void end_buffer_write_sync(struct buffer_head *bh, int uptodate) { - char b[BDEVNAME_SIZE]; - if (uptodate) { set_buffer_uptodate(bh); } else { - if (!quiet_error(bh)) { - buffer_io_error(bh); - printk(KERN_WARNING "lost page write due to " - "I/O error on %s\n", - bdevname(bh->b_bdev, b)); - } + if (!quiet_error(bh)) + buffer_io_error(bh, ", lost sync page write"); set_buffer_write_io_error(bh); clear_buffer_uptodate(bh); } @@ -305,7 +299,7 @@ static void end_buffer_async_read(struct buffer_head *bh, int uptodate) } else { clear_buffer_uptodate(bh); if (!quiet_error(bh)) - buffer_io_error(bh); + buffer_io_error(bh, ", async page read"); SetPageError(page); } @@ -353,7 +347,6 @@ still_busy: */ void end_buffer_async_write(struct buffer_head *bh, int uptodate) { - char b[BDEVNAME_SIZE]; unsigned long flags; struct buffer_head *first; struct buffer_head *tmp; @@ -365,12 +358,8 @@ void end_buffer_async_write(struct buffer_head *bh, int uptodate) if (uptodate) { set_buffer_uptodate(bh); } else { - if (!quiet_error(bh)) { - buffer_io_error(bh); - printk(KERN_WARNING "lost page write due to " - "I/O error on %s\n", - bdevname(bh->b_bdev, b)); - } + if (!quiet_error(bh)) + buffer_io_error(bh, ", lost async page write"); set_bit(AS_EIO, &page->mapping->flags); set_buffer_write_io_error(bh); clear_buffer_uptodate(bh); -- cgit v1.1 From 432f16e64f50fd4999a476543d04dd52f7a2d753 Mon Sep 17 00:00:00 2001 From: Robert Elliott Date: Tue, 21 Oct 2014 13:55:11 -0600 Subject: fs: clarify rate limit suppressed buffer I/O errors When quiet_error applies rate limiting to buffer_io_error calls, what the they apply to is unclear because the name is so generic, particularly if the messages are interleaved with others: [ 1936.063572] quiet_error: 664293 callbacks suppressed [ 1936.065297] Buffer I/O error on dev sdr, logical block 257429952, lost async page write [ 1936.067814] Buffer I/O error on dev sdr, logical block 257429953, lost async page write Also, the function uses printk_ratelimit(), although printk.h includes a comment advising "Please don't use... Instead use printk_ratelimited()." Change buffer_io_error to check the BH_Quiet bit itself, drop the printk_ratelimit call, and print using printk_ratelimited. This makes the messages look like: [ 387.208839] buffer_io_error: 676394 callbacks suppressed [ 387.210693] Buffer I/O error on dev sdr, logical block 211291776, lost async page write [ 387.213432] Buffer I/O error on dev sdr, logical block 211291777, lost async page write Signed-off-by: Robert Elliott Reviewed-by: Webb Scales Signed-off-by: Jens Axboe --- fs/buffer.c | 23 +++++++---------------- 1 file changed, 7 insertions(+), 16 deletions(-) diff --git a/fs/buffer.c b/fs/buffer.c index 9d1da1d..20805db 100644 --- a/fs/buffer.c +++ b/fs/buffer.c @@ -128,19 +128,13 @@ __clear_page_buffers(struct page *page) page_cache_release(page); } - -static int quiet_error(struct buffer_head *bh) -{ - if (!test_bit(BH_Quiet, &bh->b_state) && printk_ratelimit()) - return 0; - return 1; -} - - static void buffer_io_error(struct buffer_head *bh, char *msg) { char b[BDEVNAME_SIZE]; - printk(KERN_ERR "Buffer I/O error on dev %s, logical block %llu%s\n", + + if (!test_bit(BH_Quiet, &bh->b_state)) + printk_ratelimited(KERN_ERR + "Buffer I/O error on dev %s, logical block %llu%s\n", bdevname(bh->b_bdev, b), (unsigned long long)bh->b_blocknr, msg); } @@ -180,8 +174,7 @@ void end_buffer_write_sync(struct buffer_head *bh, int uptodate) if (uptodate) { set_buffer_uptodate(bh); } else { - if (!quiet_error(bh)) - buffer_io_error(bh, ", lost sync page write"); + buffer_io_error(bh, ", lost sync page write"); set_buffer_write_io_error(bh); clear_buffer_uptodate(bh); } @@ -298,8 +291,7 @@ static void end_buffer_async_read(struct buffer_head *bh, int uptodate) set_buffer_uptodate(bh); } else { clear_buffer_uptodate(bh); - if (!quiet_error(bh)) - buffer_io_error(bh, ", async page read"); + buffer_io_error(bh, ", async page read"); SetPageError(page); } @@ -358,8 +350,7 @@ void end_buffer_async_write(struct buffer_head *bh, int uptodate) if (uptodate) { set_buffer_uptodate(bh); } else { - if (!quiet_error(bh)) - buffer_io_error(bh, ", lost async page write"); + buffer_io_error(bh, ", lost async page write"); set_bit(AS_EIO, &page->mapping->flags); set_buffer_write_io_error(bh); clear_buffer_uptodate(bh); -- cgit v1.1 From 76d8137a31139f0d69ecc4177497ad6b8d4f016c Mon Sep 17 00:00:00 2001 From: Ming Lei Date: Wed, 22 Oct 2014 08:30:30 +0800 Subject: blk-merge: recaculate segment if it isn't less than max segments The problem is introduced by commit 764f612c6c3c231b(blk-merge: don't compute bi_phys_segments from bi_vcnt for cloned bio), and merge is needed if number of current segment isn't less than max segments. Strictly speaking, bio->bi_vcnt shouldn't be used here since it may not be accurate in cases of both cloned bio or bio cloned from, but bio_segments() is a bit expensive, and bi_vcnt is still the biggest number, so the approach should work. Signed-off-by: Ming Lei Signed-off-by: Jens Axboe --- block/blk-merge.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/block/blk-merge.c b/block/blk-merge.c index ba99351..b3ac40a 100644 --- a/block/blk-merge.c +++ b/block/blk-merge.c @@ -99,16 +99,17 @@ void blk_recount_segments(struct request_queue *q, struct bio *bio) { bool no_sg_merge = !!test_bit(QUEUE_FLAG_NO_SG_MERGE, &q->queue_flags); + bool merge_not_need = bio->bi_vcnt < queue_max_segments(q); if (no_sg_merge && !bio_flagged(bio, BIO_CLONED) && - bio->bi_vcnt < queue_max_segments(q)) + merge_not_need) bio->bi_phys_segments = bio->bi_vcnt; else { struct bio *nxt = bio->bi_next; bio->bi_next = NULL; bio->bi_phys_segments = __blk_recalc_rq_segments(q, bio, - no_sg_merge); + no_sg_merge && merge_not_need); bio->bi_next = nxt; } -- cgit v1.1 From 31f9690e6eaf549f3e643f6a8f7dab84fd31997a Mon Sep 17 00:00:00 2001 From: Jan Kara Date: Wed, 22 Oct 2014 15:34:21 +0200 Subject: null_blk: Cleanup error recovery in null_add_dev() When creation of queues fails in init_driver_queues(), we free the queues. But null_add_dev() doesn't test for this failure and continues with the setup leading to strange consequences, likely oops. Fix the problem by testing whether init_driver_queues() failed and do proper error cleanup. Coverity-id: 1148005 Signed-off-by: Jan Kara Signed-off-by: Jens Axboe --- drivers/block/null_blk.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/block/null_blk.c b/drivers/block/null_blk.c index 2671a3f..8001e812 100644 --- a/drivers/block/null_blk.c +++ b/drivers/block/null_blk.c @@ -450,14 +450,10 @@ static int init_driver_queues(struct nullb *nullb) ret = setup_commands(nq); if (ret) - goto err_queue; + return ret; nullb->nr_queues++; } - return 0; -err_queue: - cleanup_queues(nullb); - return ret; } static int null_add_dev(void) @@ -507,7 +503,9 @@ static int null_add_dev(void) goto out_cleanup_queues; } blk_queue_make_request(nullb->q, null_queue_bio); - init_driver_queues(nullb); + rv = init_driver_queues(nullb); + if (rv) + goto out_cleanup_blk_queue; } else { nullb->q = blk_init_queue_node(null_request_fn, &nullb->lock, home_node); if (!nullb->q) { @@ -516,7 +514,9 @@ static int null_add_dev(void) } blk_queue_prep_rq(nullb->q, null_rq_prep_fn); blk_queue_softirq_done(nullb->q, null_softirq_done_fn); - init_driver_queues(nullb); + rv = init_driver_queues(nullb); + if (rv) + goto out_cleanup_blk_queue; } nullb->q->queuedata = nullb; -- cgit v1.1 From 84ce0f0e94ac97217398b3b69c21c7a62ebeed05 Mon Sep 17 00:00:00 2001 From: Jan Kara Date: Wed, 22 Oct 2014 20:13:39 -0600 Subject: scsi: Fix error handling in SCSI_IOCTL_SEND_COMMAND When sg_scsi_ioctl() fails to prepare request to submit in blk_rq_map_kern() we jump to a label where we just end up copying (luckily zeroed-out) kernel buffer to userspace instead of reporting error. Fix the problem by jumping to the right label. CC: Jens Axboe CC: linux-scsi@vger.kernel.org CC: stable@vger.kernel.org Coverity-id: 1226871 Signed-off-by: Jan Kara Fixed up the, now unused, out label. Signed-off-by: Jens Axboe --- block/scsi_ioctl.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/block/scsi_ioctl.c b/block/scsi_ioctl.c index abb2e65..1e053d9 100644 --- a/block/scsi_ioctl.c +++ b/block/scsi_ioctl.c @@ -508,7 +508,7 @@ int sg_scsi_ioctl(struct request_queue *q, struct gendisk *disk, fmode_t mode, if (bytes && blk_rq_map_kern(q, rq, buffer, bytes, __GFP_WAIT)) { err = DRIVER_ERROR << 24; - goto out; + goto error; } memset(sense, 0, sizeof(sense)); @@ -517,7 +517,6 @@ int sg_scsi_ioctl(struct request_queue *q, struct gendisk *disk, fmode_t mode, blk_execute_rq(q, disk, rq, 0); -out: err = rq->errors & 0xff; /* only 8 bit SCSI status */ if (err) { if (rq->sense_len && rq->sense) { -- cgit v1.1 From d32f6b57523b6e9b2e39e990e056f9882a6f099a Mon Sep 17 00:00:00 2001 From: Sudip Mukherjee Date: Thu, 23 Oct 2014 22:16:48 +0530 Subject: block: fix wrong error return in elevator_init() while compiling integer err was showing as a set but unused variable. elevator_init_fn can be either cfq_init_queue or deadline_init_queue or noop_init_queue. all three of these functions are returning -ENOMEM if they fail to allocate the queue. so we should actually be returning the error code rather than returning 0 always. Signed-off-by: Sudip Mukherjee Signed-off-by: Jens Axboe --- block/elevator.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/block/elevator.c b/block/elevator.c index 24c28b6..afa3b03 100644 --- a/block/elevator.c +++ b/block/elevator.c @@ -229,7 +229,9 @@ int elevator_init(struct request_queue *q, char *name) } err = e->ops.elevator_init_fn(q, e); - return 0; + if (err) + elevator_put(e); + return err; } EXPORT_SYMBOL(elevator_init); -- cgit v1.1 From c21e59d8dc04b2107bdb4ff0f412a9b7ae3349f3 Mon Sep 17 00:00:00 2001 From: Tony Battersby Date: Thu, 23 Oct 2014 15:10:21 -0400 Subject: lib/scatterlist: fix memory leak with scsi-mq Fix a memory leak with scsi-mq triggered by commands with large data transfer length. Fixes: c53c6d6a68b1 ("scatterlist: allow chaining to preallocated chunks") Cc: # 3.17.x Signed-off-by: Tony Battersby Reviewed-by: Martin K. Petersen Signed-off-by: Jens Axboe --- lib/scatterlist.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/scatterlist.c b/lib/scatterlist.c index 9cdf62f..c9f2e8c 100644 --- a/lib/scatterlist.c +++ b/lib/scatterlist.c @@ -203,10 +203,10 @@ void __sg_free_table(struct sg_table *table, unsigned int max_ents, } table->orig_nents -= sg_size; - if (!skip_first_chunk) { - free_fn(sgl, alloc_size); + if (skip_first_chunk) skip_first_chunk = false; - } + else + free_fn(sgl, alloc_size); sgl = next; } -- cgit v1.1 From cb1a5ab6ece7a37da4ac98ee26b0475b7c3ea79e Mon Sep 17 00:00:00 2001 From: "Martin K. Petersen" Date: Tue, 28 Oct 2014 20:27:43 -0600 Subject: block: Fix merge logic when CONFIG_BLK_DEV_INTEGRITY is not defined Commit 4eaf99beadce switched to returning bool and as a result reversed the logic of the integrity merge checks. However, the empty stubs used when the block integrity code is compiled out were still returning 0. Make these stubs return "true". Signed-off-by: Martin K. Petersen Reported-by: Michael L. Semon Tested-by: Michael L. Semon Signed-off-by: Jens Axboe --- include/linux/blkdev.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index 0207a78..6cbee83 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -1583,13 +1583,13 @@ static inline bool blk_integrity_merge_rq(struct request_queue *rq, struct request *r1, struct request *r2) { - return 0; + return true; } static inline bool blk_integrity_merge_bio(struct request_queue *rq, struct request *r, struct bio *b) { - return 0; + return true; } static inline bool blk_integrity_is_initialized(struct gendisk *g) { -- cgit v1.1