From: David Sterba <dsterba@suse.cz>
To: Qu Wenruo <wqu@suse.com>
Cc: linux-btrfs@vger.kernel.org
Subject: Re: [PATCH v2 07/17] btrfs: make rbio_add_io_page() subpage compatible
Date: Wed, 13 Apr 2022 21:14:56 +0200 [thread overview]
Message-ID: <20220413191456.GN15609@twin.jikos.cz> (raw)
In-Reply-To: <d2873b5f3a00e9bb966150b4dd0253f4db107c12.1649753690.git.wqu@suse.com>
There's an assertion failure reported by btrfs/023
On Tue, Apr 12, 2022 at 05:32:57PM +0800, Qu Wenruo wrote:
> +static int rbio_add_io_sector(struct btrfs_raid_bio *rbio,
> + struct bio_list *bio_list,
> + struct sector_ptr *sector,
> + unsigned int stripe_nr,
> + unsigned int sector_nr,
> + unsigned long bio_max_len, unsigned int opf)
> {
> + const u32 sectorsize = rbio->bioc->fs_info->sectorsize;
> struct bio *last = bio_list->tail;
> int ret;
> struct bio *bio;
> struct btrfs_io_stripe *stripe;
> u64 disk_start;
>
> + /*
> + * NOTE: here stripe_nr has taken device replace into consideration,
> + * thus it can be larger than rbio->real_stripe.
> + * So here we check against bioc->num_stripes, not rbio->real_stripes.
> + */
> + ASSERT(stripe_nr >= 0 && stripe_nr < rbio->bioc->num_stripes);
> + ASSERT(sector_nr >= 0 && sector_nr < rbio->stripe_nsectors);
> + ASSERT(sector->page);
This one ^^^
[ 2280.705765] assertion failed: sector->page, in fs/btrfs/raid56.c:1145
[ 2280.707844] ------------[ cut here ]------------
[ 2280.709401] kernel BUG at fs/btrfs/ctree.h:3614!
[ 2280.711084] invalid opcode: 0000 [#1] PREEMPT SMP
[ 2280.712822] CPU: 3 PID: 4084 Comm: kworker/u8:2 Not tainted 5.18.0-rc2-default+ #1697
[ 2280.715648] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.14.0-0-g155821a-rebuilt.opensuse.org 04/01/2014
[ 2280.719656] Workqueue: btrfs-rmw btrfs_work_helper [btrfs]
[ 2280.721775] RIP: 0010:assertfail.constprop.0+0x18/0x1a [btrfs]
[ 2280.729575] RSP: 0018:ffffad6d071afda0 EFLAGS: 00010246
[ 2280.730844] RAX: 0000000000000039 RBX: 0000000000000000 RCX: 0000000000000000
[ 2280.732449] RDX: 0000000000000000 RSI: 0000000000000003 RDI: 00000000ffffffff
[ 2280.733992] RBP: ffff8e51d5465000 R08: 0000000000000003 R09: 0000000000000002
[ 2280.735535] R10: 0000000000000000 R11: 0000000000000001 R12: 0000000000000003
[ 2280.737093] R13: ffff8e51d5465000 R14: ffff8e51d5465d78 R15: 0000000000001000
[ 2280.738613] FS: 0000000000000000(0000) GS:ffff8e523dc00000(0000) knlGS:0000000000000000
[ 2280.740392] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 2280.741794] CR2: 000055f48fe51ab0 CR3: 000000001f805001 CR4: 0000000000170ea0
[ 2280.743532] Call Trace:
[ 2280.744136] <TASK>
[ 2280.744701] rbio_add_io_sector.cold+0x11/0x33 [btrfs]
[ 2280.745846] ? _raw_spin_unlock_irq+0x2f/0x50
[ 2280.746782] raid56_rmw_stripe.isra.0+0x153/0x320 [btrfs]
[ 2280.747965] btrfs_work_helper+0xd6/0x1d0 [btrfs]
[ 2280.749018] process_one_work+0x264/0x5f0
[ 2280.749806] worker_thread+0x52/0x3b0
[ 2280.750523] ? process_one_work+0x5f0/0x5f0
[ 2280.751395] kthread+0xea/0x110
[ 2280.752097] ? kthread_complete_and_exit+0x20/0x20
[ 2280.753112] ret_from_fork+0x1f/0x30
> +
> + /* We don't yet support subpage, thus pgoff should always be 0 */
> + ASSERT(sector->pgoff == 0);
> +
> stripe = &rbio->bioc->stripes[stripe_nr];
> - disk_start = stripe->physical + (page_index << PAGE_SHIFT);
> + disk_start = stripe->physical + sector_nr * sectorsize;
>
> /* if the device is missing, just fail this stripe */
> if (!stripe->dev->bdev)
> @@ -1156,8 +1226,9 @@ static int rbio_add_io_page(struct btrfs_raid_bio *rbio,
> */
> if (last_end == disk_start && !last->bi_status &&
> last->bi_bdev == stripe->dev->bdev) {
> - ret = bio_add_page(last, page, PAGE_SIZE, 0);
> - if (ret == PAGE_SIZE)
> + ret = bio_add_page(last, sector->page, sectorsize,
> + sector->pgoff);
> + if (ret == sectorsize)
> return 0;
> }
> }
> @@ -1168,7 +1239,7 @@ static int rbio_add_io_page(struct btrfs_raid_bio *rbio,
> bio->bi_iter.bi_sector = disk_start >> 9;
> bio->bi_private = rbio;
>
> - bio_add_page(bio, page, PAGE_SIZE, 0);
> + bio_add_page(bio, sector->page, sectorsize, sector->pgoff);
> bio_list_add(bio_list, bio);
> return 0;
> }
> @@ -1265,7 +1336,7 @@ static noinline void finish_rmw(struct btrfs_raid_bio *rbio)
> void **pointers = rbio->finish_pointers;
> int nr_data = rbio->nr_data;
> int stripe;
> - int pagenr;
> + int sectornr;
> bool has_qstripe;
> struct bio_list bio_list;
> struct bio *bio;
> @@ -1309,16 +1380,16 @@ static noinline void finish_rmw(struct btrfs_raid_bio *rbio)
> else
> clear_bit(RBIO_CACHE_READY_BIT, &rbio->flags);
>
> - for (pagenr = 0; pagenr < rbio->stripe_npages; pagenr++) {
> + for (sectornr = 0; sectornr < rbio->stripe_nsectors; sectornr++) {
> struct page *p;
> /* first collect one page from each data stripe */
> for (stripe = 0; stripe < nr_data; stripe++) {
> - p = page_in_rbio(rbio, stripe, pagenr, 0);
> + p = page_in_rbio(rbio, stripe, sectornr, 0);
> pointers[stripe] = kmap_local_page(p);
> }
>
> /* then add the parity stripe */
> - p = rbio_pstripe_page(rbio, pagenr);
> + p = rbio_pstripe_page(rbio, sectornr);
> SetPageUptodate(p);
> pointers[stripe++] = kmap_local_page(p);
>
> @@ -1328,7 +1399,7 @@ static noinline void finish_rmw(struct btrfs_raid_bio *rbio)
> * raid6, add the qstripe and call the
> * library function to fill in our p/q
> */
> - p = rbio_qstripe_page(rbio, pagenr);
> + p = rbio_qstripe_page(rbio, sectornr);
> SetPageUptodate(p);
> pointers[stripe++] = kmap_local_page(p);
>
> @@ -1349,19 +1420,19 @@ static noinline void finish_rmw(struct btrfs_raid_bio *rbio)
> * everything else.
> */
> for (stripe = 0; stripe < rbio->real_stripes; stripe++) {
> - for (pagenr = 0; pagenr < rbio->stripe_npages; pagenr++) {
> - struct page *page;
> + for (sectornr = 0; sectornr < rbio->stripe_nsectors; sectornr++) {
> + struct sector_ptr *sector;
> +
> if (stripe < rbio->nr_data) {
> - page = page_in_rbio(rbio, stripe, pagenr, 1);
> - if (!page)
> + sector = sector_in_rbio(rbio, stripe, sectornr, 1);
> + if (!sector)
> continue;
> } else {
> - page = rbio_stripe_page(rbio, stripe, pagenr);
> + sector = rbio_stripe_sector(rbio, stripe, sectornr);
> }
>
> - ret = rbio_add_io_page(rbio, &bio_list,
> - page, stripe, pagenr, rbio->stripe_len,
> - REQ_OP_WRITE);
> + ret = rbio_add_io_sector(rbio, &bio_list, sector, stripe,
> + sectornr, rbio->stripe_len, REQ_OP_WRITE);
> if (ret)
> goto cleanup;
> }
> @@ -1374,20 +1445,21 @@ static noinline void finish_rmw(struct btrfs_raid_bio *rbio)
> if (!bioc->tgtdev_map[stripe])
> continue;
>
> - for (pagenr = 0; pagenr < rbio->stripe_npages; pagenr++) {
> - struct page *page;
> + for (sectornr = 0; sectornr < rbio->stripe_nsectors; sectornr++) {
> + struct sector_ptr *sector;
> +
> if (stripe < rbio->nr_data) {
> - page = page_in_rbio(rbio, stripe, pagenr, 1);
> - if (!page)
> + sector = sector_in_rbio(rbio, stripe, sectornr, 1);
> + if (!sector)
> continue;
> } else {
> - page = rbio_stripe_page(rbio, stripe, pagenr);
> + sector = rbio_stripe_sector(rbio, stripe, sectornr);
> }
>
> - ret = rbio_add_io_page(rbio, &bio_list, page,
> - rbio->bioc->tgtdev_map[stripe],
> - pagenr, rbio->stripe_len,
> - REQ_OP_WRITE);
> + ret = rbio_add_io_sector(rbio, &bio_list, sector,
> + rbio->bioc->tgtdev_map[stripe],
> + sectornr, rbio->stripe_len,
> + REQ_OP_WRITE);
> if (ret)
> goto cleanup;
> }
> @@ -1563,7 +1635,7 @@ static int raid56_rmw_stripe(struct btrfs_raid_bio *rbio)
> int bios_to_read = 0;
> struct bio_list bio_list;
> int ret;
> - int pagenr;
> + int sectornr;
> int stripe;
> struct bio *bio;
>
> @@ -1581,28 +1653,29 @@ static int raid56_rmw_stripe(struct btrfs_raid_bio *rbio)
> * stripe
> */
> for (stripe = 0; stripe < rbio->nr_data; stripe++) {
> - for (pagenr = 0; pagenr < rbio->stripe_npages; pagenr++) {
> - struct page *page;
> + for (sectornr = 0; sectornr < rbio->stripe_nsectors; sectornr++) {
> + struct sector_ptr *sector;
> +
> /*
> - * we want to find all the pages missing from
> + * We want to find all the sectors missing from
> * the rbio and read them from the disk. If
> - * page_in_rbio finds a page in the bio list
> + * sector_in_rbio() finds a page in the bio list
> * we don't need to read it off the stripe.
> */
> - page = page_in_rbio(rbio, stripe, pagenr, 1);
> - if (page)
> + sector = sector_in_rbio(rbio, stripe, sectornr, 1);
> + if (sector)
> continue;
>
> - page = rbio_stripe_page(rbio, stripe, pagenr);
> + sector = rbio_stripe_sector(rbio, stripe, sectornr);
> /*
> - * the bio cache may have handed us an uptodate
> + * The bio cache may have handed us an uptodate
> * page. If so, be happy and use it
> */
> - if (PageUptodate(page))
> + if (sector->uptodate)
> continue;
>
> - ret = rbio_add_io_page(rbio, &bio_list, page,
> - stripe, pagenr, rbio->stripe_len,
> + ret = rbio_add_io_sector(rbio, &bio_list, sector,
> + stripe, sectornr, rbio->stripe_len,
> REQ_OP_READ);
> if (ret)
> goto cleanup;
> @@ -2107,7 +2180,7 @@ static int __raid56_parity_recover(struct btrfs_raid_bio *rbio)
> int bios_to_read = 0;
> struct bio_list bio_list;
> int ret;
> - int pagenr;
> + int sectornr;
> int stripe;
> struct bio *bio;
>
> @@ -2130,21 +2203,20 @@ static int __raid56_parity_recover(struct btrfs_raid_bio *rbio)
> continue;
> }
>
> - for (pagenr = 0; pagenr < rbio->stripe_npages; pagenr++) {
> - struct page *p;
> + for (sectornr = 0; sectornr < rbio->stripe_nsectors; sectornr++) {
> + struct sector_ptr *sector;
>
> /*
> * the rmw code may have already read this
> * page in
> */
> - p = rbio_stripe_page(rbio, stripe, pagenr);
> - if (PageUptodate(p))
> + sector = rbio_stripe_sector(rbio, stripe, sectornr);
> + if (sector->uptodate)
> continue;
>
> - ret = rbio_add_io_page(rbio, &bio_list,
> - rbio_stripe_page(rbio, stripe, pagenr),
> - stripe, pagenr, rbio->stripe_len,
> - REQ_OP_READ);
> + ret = rbio_add_io_sector(rbio, &bio_list, sector,
> + stripe, sectornr,
> + rbio->stripe_len, REQ_OP_READ);
> if (ret < 0)
> goto cleanup;
> }
> @@ -2399,7 +2471,7 @@ static noinline void finish_parity_scrub(struct btrfs_raid_bio *rbio,
> unsigned long *pbitmap = rbio->finish_pbitmap;
> int nr_data = rbio->nr_data;
> int stripe;
> - int pagenr;
> + int sectornr;
> bool has_qstripe;
> struct page *p_page = NULL;
> struct page *q_page = NULL;
> @@ -2419,7 +2491,7 @@ static noinline void finish_parity_scrub(struct btrfs_raid_bio *rbio,
>
> if (bioc->num_tgtdevs && bioc->tgtdev_map[rbio->scrubp]) {
> is_replace = 1;
> - bitmap_copy(pbitmap, rbio->dbitmap, rbio->stripe_npages);
> + bitmap_copy(pbitmap, rbio->dbitmap, rbio->stripe_nsectors);
> }
>
> /*
> @@ -2453,12 +2525,12 @@ static noinline void finish_parity_scrub(struct btrfs_raid_bio *rbio,
> /* Map the parity stripe just once */
> pointers[nr_data] = kmap_local_page(p_page);
>
> - for_each_set_bit(pagenr, rbio->dbitmap, rbio->stripe_npages) {
> + for_each_set_bit(sectornr, rbio->dbitmap, rbio->stripe_nsectors) {
> struct page *p;
> void *parity;
> /* first collect one page from each data stripe */
> for (stripe = 0; stripe < nr_data; stripe++) {
> - p = page_in_rbio(rbio, stripe, pagenr, 0);
> + p = page_in_rbio(rbio, stripe, sectornr, 0);
> pointers[stripe] = kmap_local_page(p);
> }
>
> @@ -2473,13 +2545,13 @@ static noinline void finish_parity_scrub(struct btrfs_raid_bio *rbio,
> }
>
> /* Check scrubbing parity and repair it */
> - p = rbio_stripe_page(rbio, rbio->scrubp, pagenr);
> + p = rbio_stripe_page(rbio, rbio->scrubp, sectornr);
> parity = kmap_local_page(p);
> if (memcmp(parity, pointers[rbio->scrubp], PAGE_SIZE))
> copy_page(parity, pointers[rbio->scrubp]);
> else
> /* Parity is right, needn't writeback */
> - bitmap_clear(rbio->dbitmap, pagenr, 1);
> + bitmap_clear(rbio->dbitmap, sectornr, 1);
> kunmap_local(parity);
>
> for (stripe = nr_data - 1; stripe >= 0; stripe--)
> @@ -2499,12 +2571,13 @@ static noinline void finish_parity_scrub(struct btrfs_raid_bio *rbio,
> * higher layers (the bio_list in our rbio) and our p/q. Ignore
> * everything else.
> */
> - for_each_set_bit(pagenr, rbio->dbitmap, rbio->stripe_npages) {
> - struct page *page;
> + for_each_set_bit(sectornr, rbio->dbitmap, rbio->stripe_nsectors) {
> + struct sector_ptr *sector;
>
> - page = rbio_stripe_page(rbio, rbio->scrubp, pagenr);
> - ret = rbio_add_io_page(rbio, &bio_list, page, rbio->scrubp,
> - pagenr, rbio->stripe_len, REQ_OP_WRITE);
> + sector = rbio_stripe_sector(rbio, rbio->scrubp, sectornr);
> + ret = rbio_add_io_sector(rbio, &bio_list, sector, rbio->scrubp,
> + sectornr, rbio->stripe_len,
> + REQ_OP_WRITE);
> if (ret)
> goto cleanup;
> }
> @@ -2512,13 +2585,13 @@ static noinline void finish_parity_scrub(struct btrfs_raid_bio *rbio,
> if (!is_replace)
> goto submit_write;
>
> - for_each_set_bit(pagenr, pbitmap, rbio->stripe_npages) {
> - struct page *page;
> + for_each_set_bit(sectornr, pbitmap, rbio->stripe_nsectors) {
> + struct sector_ptr *sector;
>
> - page = rbio_stripe_page(rbio, rbio->scrubp, pagenr);
> - ret = rbio_add_io_page(rbio, &bio_list, page,
> + sector = rbio_stripe_sector(rbio, rbio->scrubp, sectornr);
> + ret = rbio_add_io_sector(rbio, &bio_list, sector,
> bioc->tgtdev_map[rbio->scrubp],
> - pagenr, rbio->stripe_len, REQ_OP_WRITE);
> + sectornr, rbio->stripe_len, REQ_OP_WRITE);
> if (ret)
> goto cleanup;
> }
> @@ -2650,7 +2723,7 @@ static void raid56_parity_scrub_stripe(struct btrfs_raid_bio *rbio)
> int bios_to_read = 0;
> struct bio_list bio_list;
> int ret;
> - int pagenr;
> + int sectornr;
> int stripe;
> struct bio *bio;
>
> @@ -2666,28 +2739,30 @@ static void raid56_parity_scrub_stripe(struct btrfs_raid_bio *rbio)
> * stripe
> */
> for (stripe = 0; stripe < rbio->real_stripes; stripe++) {
> - for_each_set_bit(pagenr, rbio->dbitmap, rbio->stripe_npages) {
> - struct page *page;
> + for_each_set_bit(sectornr, rbio->dbitmap,
> + rbio->stripe_nsectors) {
> + struct sector_ptr *sector;
> /*
> - * we want to find all the pages missing from
> + * We want to find all the sectors missing from
> * the rbio and read them from the disk. If
> - * page_in_rbio finds a page in the bio list
> + * sector_in_rbio() finds a sector in the bio list
> * we don't need to read it off the stripe.
> */
> - page = page_in_rbio(rbio, stripe, pagenr, 1);
> - if (page)
> + sector = sector_in_rbio(rbio, stripe, sectornr, 1);
> + if (sector)
> continue;
>
> - page = rbio_stripe_page(rbio, stripe, pagenr);
> + sector = rbio_stripe_sector(rbio, stripe, sectornr);
> /*
> - * the bio cache may have handed us an uptodate
> - * page. If so, be happy and use it
> + * The bio cache may have handed us an uptodate
> + * sector. If so, be happy and use it
> */
> - if (PageUptodate(page))
> + if (sector->uptodate)
> continue;
>
> - ret = rbio_add_io_page(rbio, &bio_list, page, stripe,
> - pagenr, rbio->stripe_len, REQ_OP_READ);
> + ret = rbio_add_io_sector(rbio, &bio_list, sector,
> + stripe, sectornr,
> + rbio->stripe_len, REQ_OP_READ);
> if (ret)
> goto cleanup;
> }
> --
> 2.35.1
next prev parent reply other threads:[~2022-04-13 19:19 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-12 9:32 [PATCH v2 00/17] btrfs: add subpage support for RAID56 Qu Wenruo
2022-04-12 9:32 ` [PATCH v2 01/17] btrfs: reduce width for stripe_len from u64 to u32 Qu Wenruo
2022-04-12 9:32 ` [PATCH v2 02/17] btrfs: open-code rbio_nr_pages() Qu Wenruo
2022-04-12 9:32 ` [PATCH v2 03/17] btrfs: make btrfs_raid_bio more compact Qu Wenruo
2022-04-12 9:32 ` [PATCH v2 04/17] btrfs: introduce new cached members for btrfs_raid_bio Qu Wenruo
2022-04-15 5:31 ` Christoph Hellwig
2022-04-15 5:34 ` Qu Wenruo
2022-04-15 5:45 ` Christoph Hellwig
2022-04-12 9:32 ` [PATCH v2 05/17] btrfs: introduce btrfs_raid_bio::stripe_sectors Qu Wenruo
2022-04-12 9:32 ` [PATCH v2 06/17] btrfs: introduce btrfs_raid_bio::bio_sectors Qu Wenruo
2022-04-12 9:32 ` [PATCH v2 07/17] btrfs: make rbio_add_io_page() subpage compatible Qu Wenruo
2022-04-13 19:14 ` David Sterba [this message]
2022-04-13 23:28 ` Qu Wenruo
2022-04-14 0:43 ` Qu Wenruo
2022-04-14 10:59 ` Qu Wenruo
2022-04-14 15:51 ` David Sterba
2022-04-14 22:48 ` Qu Wenruo
2022-04-21 15:44 ` David Sterba
2022-04-14 15:43 ` David Sterba
2022-04-14 17:51 ` David Sterba
2022-04-14 22:28 ` Qu Wenruo
2022-04-21 16:24 ` David Sterba
2022-04-12 9:32 ` [PATCH v2 08/17] btrfs: make finish_parity_scrub() " Qu Wenruo
2022-04-12 9:32 ` [PATCH v2 09/17] btrfs: make __raid_recover_endio_io() subpage compatibable Qu Wenruo
2022-04-12 9:33 ` [PATCH v2 10/17] btrfs: make finish_rmw() subpage compatible Qu Wenruo
2022-04-12 9:33 ` [PATCH v2 11/17] btrfs: open-code rbio_stripe_page_index() Qu Wenruo
2022-04-12 9:33 ` [PATCH v2 12/17] btrfs: make raid56_add_scrub_pages() subpage compatible Qu Wenruo
2022-04-12 9:33 ` [PATCH v2 13/17] btrfs: remove btrfs_raid_bio::bio_pages array Qu Wenruo
2022-04-12 9:33 ` [PATCH v2 14/17] btrfs: make set_bio_pages_uptodate() subpage compatible Qu Wenruo
2022-04-12 9:33 ` [PATCH v2 15/17] btrfs: make steal_rbio() " Qu Wenruo
2022-04-12 9:33 ` [PATCH v2 16/17] btrfs: make alloc_rbio_essential_pages() " Qu Wenruo
2022-04-12 9:33 ` [PATCH v2 17/17] btrfs: enable subpage support for RAID56 Qu Wenruo
2022-04-12 17:42 ` [PATCH v2 00/17] btrfs: add " David Sterba
2022-04-13 14:46 ` David Sterba
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=20220413191456.GN15609@twin.jikos.cz \
--to=dsterba@suse.cz \
--cc=linux-btrfs@vger.kernel.org \
--cc=wqu@suse.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox