All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ming Lin <mlin@minggr.net>
To: Dongsu Park <dongsu.park@profitbricks.com>
Cc: Kent Overstreet <kmo@daterainc.com>,
	linux-fsdevel@vger.kernel.org,
	lkml <linux-kernel@vger.kernel.org>, Jens Axboe <axboe@kernel.dk>,
	Christoph Hellwig <hch@infradead.org>
Subject: Re: Block layer projects that I haven't had time for
Date: Thu, 11 Dec 2014 22:32:31 -0800	[thread overview]
Message-ID: <548A8BFF.9020801@minggr.net> (raw)
In-Reply-To: <20141211100751.GA2409@gmail.com>

On 12/11/2014 02:07 AM, Dongsu Park wrote:
> Hi Ming & Kent,
> 
> On 10.12.2014 23:11, Ming Lin wrote:
>>> On Wed, Dec 10, 2014 at 02:42:14PM -0800, Ming Lin wrote:
>>> Try this fix:
>> Yes, it fixed ext4 problem.
> 
> @kent: Thank you for the patch. Indeed it fixes the ext4 lockup I've seen.
> I've applied it to my tree, under the branch block-mpage-bvecs-for-next.
> See 0d2e05525a58 ("fs/ext4: fix a lockup when writing blocks into ext4
> rootfs") <https://github.com/dongsupark/linux/commit/0d2e05525a58>.
> 
> After that of course, more bugs start to appear, e.g. crash with virtio-blk,
> like we'd have opened a can of worms. ;-)
> 
>> Just tried to edit a btrfs file.
>>
>> [   45.216351] BTRFS error (device sdb1): partial page write in btrfs with
>> offset 0 and length 8192
>> [   45.217522] BTRFS critical (device sdb1): bad ordered accounting left 0
>> size 4096
> 
> @ming: I guess you managed to see this error as you're testing with a
> SCSI device, not virtio-blk device like me.
> Are you seeing it without any back traces?
> Does the attached patch fix your issue?
> (This is already included in the branch block-mpage-bvecs-for-next.)

How about below fix?
"bvec.bv_len != PAGE_CACHE_SIZE" should be not valid any more, because now bio
handles arbitrary size, right?

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 2ac08e2..482c89c 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -2468,7 +2468,7 @@ static void end_bio_extent_writepage(struct bio *bio, int err)
 		 * advance bv_offset and adjust bv_len to compensate.
 		 * Print a warning for nonzero offsets, and an error
 		 * if they don't add up to a full page.  */
-		if (bvec.bv_offset || bvec.bv_len != PAGE_CACHE_SIZE) {
+		if (bvec.bv_offset) {
 			if (bvec.bv_offset + bvec.bv_len != PAGE_CACHE_SIZE)
 				btrfs_err(BTRFS_I(page->mapping->host)->root->fs_info,
 				   "partial page write in btrfs with offset %u and length %u",
@@ -2481,7 +2481,7 @@ static void end_bio_extent_writepage(struct bio *bio, int err)
 		}
 
 		start = page_offset(page);
-		end = start + bvec.bv_offset + bvec.bv_len - 1;
+		end = start + bvec.bv_offset + min(bvec.bv_len, PAGE_CACHE_SIZE) - 1;
 
 		if (end_extent_writepage(page, err, start, end))
 			continue;

Thanks,
Ming

> 
> Thanks,
> Dongsu
> 
> ====
> 
> From 7cef37e357b4fd636b3d4aa296e8b67ba8db66d1 Mon Sep 17 00:00:00 2001
> From: Dongsu Park <dongsu.park@profitbricks.com>
> Date: Tue, 9 Dec 2014 18:10:10 +0100
> Subject: [PATCH] btrfs: use a correct function for bvec iteration in
>  btrfs_csum_one_bio()
> 
> Commit 94607a8a("block: Convert various code to bio_for_each_page()")
> introduced a critical bug in btrfs_csum_one_bio() using
> bio_for_each_page_all() for iterating through each bvec.
> That should actually call bio_for_each_page() to take the current
> offset into account. Without this fix, xfstests/btrfs/012 would
> end up with lockup with warnings in btrfs_add_ordered_sum(), because
> iter.bi_size becomes < 0.
> 
> Signed-off-by: Dongsu Park <dongsu.park@profitbricks.com>
> ---
>  fs/btrfs/file-item.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
> index 6a81176..c7ae23c 100644
> --- a/fs/btrfs/file-item.c
> +++ b/fs/btrfs/file-item.c
> @@ -447,7 +447,7 @@ int btrfs_csum_one_bio(struct btrfs_root *root, struct inode *inode,
>  	sums->bytenr = (u64)bio->bi_iter.bi_sector << 9;
>  	index = 0;
>  
> -	bio_for_each_page_all(bvec, bio, iter) {
> +	bio_for_each_page(bvec, bio, iter) {
>  		if (!contig)
>  			offset = page_offset(bvec.bv_page) + bvec.bv_offset;
>  
> 

  parent reply	other threads:[~2014-12-12  6:32 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-24  4:16 Block layer projects that I haven't had time for Kent Overstreet
2014-12-04 11:00 ` Dongsu Park
2014-12-06  3:02   ` Kent Overstreet
2014-12-08 11:48     ` Dongsu Park
2014-12-10 22:42       ` Ming Lin
2014-12-10 22:57         ` Kent Overstreet
2014-12-10 23:11           ` Ming Lin
2014-12-11 10:07             ` Dongsu Park
2014-12-11 10:14               ` Kent Overstreet
2014-12-11 19:16               ` Ming Lin
2014-12-12  6:32               ` Ming Lin [this message]
2014-12-12 12:40                 ` Dongsu Park
2014-12-10 22:49       ` Kent Overstreet
2014-12-11 10:21         ` Dongsu Park

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=548A8BFF.9020801@minggr.net \
    --to=mlin@minggr.net \
    --cc=axboe@kernel.dk \
    --cc=dongsu.park@profitbricks.com \
    --cc=hch@infradead.org \
    --cc=kmo@daterainc.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.