All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nikolay Borisov <nborisov@suse.com>
To: Christoph Hellwig <hch@lst.de>, David Sterba <dsterba@suse.com>,
	Josef Bacik <josef@toxicpanda.com>, Qu Wenruo <wqu@suse.com>
Cc: linux-btrfs@vger.kernel.org
Subject: Re: [PATCH 06/10] btrfs: don't use btrfs_bio_wq_end_io for compressed writes
Date: Wed, 11 May 2022 22:28:02 +0300	[thread overview]
Message-ID: <63d980ff-29cf-19e9-e3ea-a5587fabc534@suse.com> (raw)
In-Reply-To: <f8d90519-9911-fde0-9b18-3e4f339590c3@suse.com>



On 11.05.22 г. 22:20 ч., Nikolay Borisov wrote:
> 
> 
> On 4.05.22 г. 15:25 ч., Christoph Hellwig wrote:
>> Compressed write bio completion is the only user of btrfs_bio_wq_end_io
>> for writes, and the use of btrfs_bio_wq_end_io is a little suboptimal
>> here as we only real need user context for the final completion of a
>> compressed_bio structure, and not every single bio completion.
>>
>> Add a work_struct to struct compressed_bio instead and use that to call
>> finish_compressed_bio_write.  This allows to remove all handling of
>> write bios in the btrfs_bio_wq_end_io infrastructure.
>>
>> Signed-off-by: Christoph Hellwig <hch@lst.de>

<snip>


>> @@ -771,6 +762,9 @@ blk_status_t btrfs_bio_wq_end_io(struct 
>> btrfs_fs_info *info, struct bio *bio,
>>   {
>>       struct btrfs_end_io_wq *end_io_wq;
>> +    if (WARN_ON_ONCE(btrfs_op(bio) != BTRFS_MAP_WRITE))
>> +        return BLK_STS_IOERR;
> 
> This is broken w.r.t to btrfs_submit_metadata_bio since in
> that function the code is:
> 
>   if (btrfs_op(bio) != BTRFS_MAP_WRITE) {
>                        /*
>                         * called for a read, do the setup so that 
> checksum validation
>                         * can happen in the async kernel threads
>                         */
>                      ret = btrfs_bio_wq_end_io(fs_info, bio,
>                                                  BTRFS_WQ_ENDIO_METADATA);
>                        if (!ret)
>                                ret = btrfs_map_bio(fs_info, bio, 
> mirror_num);
> 
> 
> 
> So metadata reads  will end up in function which disallows reads..
> This results in failure to mount when reading various metadata.
> 
> I guess the correct fix is to include the following hunk from patch 8 in 
> this series:
> 
> @@ -916,14 +839,7 @@ void btrfs_submit_metadata_bio(struct inode *inode, 
> struct bio *bio, int mirror_
>       bio->bi_opf |= REQ_META;
> 
>       if (btrfs_op(bio) != BTRFS_MAP_WRITE) {
> -        /*
> -         * called for a read, do the setup so that checksum validation
> -         * can happen in the async kernel threads
> -         */
> -        ret = btrfs_bio_wq_end_io(fs_info, bio,
> -                      BTRFS_WQ_ENDIO_METADATA);
> -        if (!ret)
> -            ret = btrfs_map_bio(fs_info, bio, mirror_num);
> +        ret = btrfs_map_bio(fs_info, bio, mirror_num);
> 
> 
> 
> 
> 

Even if I apply this hunk to patch 6 I get more io failures, this time 
from btrfs_submit_data_read_bio, again there is relevant hunk in patch 8:

-	ret = btrfs_bio_wq_end_io(fs_info, bio,
-			btrfs_is_free_space_inode(BTRFS_I(inode)) ?
-			BTRFS_WQ_ENDIO_FREE_SPACE : BTRFS_WQ_ENDIO_DATA);
-	if (ret)
-		goto out;
-

And then there is another due to DIO in btrfs_submit_dio_bio's call of 
btrfs_bio_wq_end_io if we want to read.


Please fix those to ensure the series is actually bisectable.


<Snip>

  reply	other threads:[~2022-05-11 19:28 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-04 12:25 cleanup btrfs bio handling, part 2 v3 Christoph Hellwig
2022-05-04 12:25 ` [PATCH 01/10] btrfs: don't double-defer bio completions for compressed reads Christoph Hellwig
2022-05-04 12:25 ` [PATCH 02/10] btrfs: move more work into btrfs_end_bioc Christoph Hellwig
2022-05-04 12:25 ` [PATCH 03/10] btrfs: cleanup btrfs_submit_dio_bio Christoph Hellwig
2022-05-04 12:25 ` [PATCH 04/10] btrfs: split btrfs_submit_data_bio Christoph Hellwig
2022-05-04 12:38   ` Qu Wenruo
2022-05-04 14:08     ` Christoph Hellwig
2022-05-04 22:41       ` Qu Wenruo
2022-05-04 22:44         ` Qu Wenruo
2022-05-05 15:08         ` Christoph Hellwig
2022-05-04 12:25 ` [PATCH 05/10] btrfs: defer I/O completion based on the btrfs_raid_bio Christoph Hellwig
2022-05-04 12:25 ` [PATCH 06/10] btrfs: don't use btrfs_bio_wq_end_io for compressed writes Christoph Hellwig
2022-05-11 19:20   ` Nikolay Borisov
2022-05-11 19:28     ` Nikolay Borisov [this message]
2022-05-12  5:56       ` Christoph Hellwig
2022-05-04 12:25 ` [PATCH 07/10] btrfs: centralize setting REQ_META Christoph Hellwig
2022-05-04 12:25 ` [PATCH 08/10] btrfs: remove btrfs_end_io_wq Christoph Hellwig
2022-05-05  2:34   ` Qu Wenruo
2022-05-05 15:09     ` Christoph Hellwig
2022-05-04 12:25 ` [PATCH 09/10] btrfs: refactor btrfs_map_bio Christoph Hellwig
2022-05-04 12:46   ` Qu Wenruo
2022-05-04 12:25 ` [PATCH 10/10] btrfs: do not allocate a btrfs_bio for low-level bios Christoph Hellwig
2022-05-05 11:23   ` Qu Wenruo
2022-05-05  6:56 ` cleanup btrfs bio handling, part 2 v3 Qu Wenruo
2022-05-05 15:11   ` Christoph Hellwig
2022-05-12  6:22     ` Anand Jain
2022-05-12  6:30       ` Anand Jain
2022-05-05 15:34   ` David Sterba
  -- strict thread matches above, loose matches on Subject: below --
2022-05-26  7:36 cleanup btrfs bio handling, part 2 v4 Christoph Hellwig
2022-05-26  7:36 ` [PATCH 06/10] btrfs: don't use btrfs_bio_wq_end_io for compressed writes Christoph Hellwig
2022-06-01 19:33   ` David Sterba
2022-04-29 14:30 cleanup btrfs bio handling, part 2 v2 Christoph Hellwig
2022-04-29 14:30 ` [PATCH 06/10] btrfs: don't use btrfs_bio_wq_end_io for compressed writes Christoph Hellwig
2022-04-25  7:54 cleanup btrfs bio handling, part 2 Christoph Hellwig
2022-04-25  7:54 ` [PATCH 06/10] btrfs: don't use btrfs_bio_wq_end_io for compressed writes Christoph Hellwig

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=63d980ff-29cf-19e9-e3ea-a5587fabc534@suse.com \
    --to=nborisov@suse.com \
    --cc=dsterba@suse.com \
    --cc=hch@lst.de \
    --cc=josef@toxicpanda.com \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=wqu@suse.com \
    --subject='Re: [PATCH 06/10] btrfs: don'\''t use btrfs_bio_wq_end_io for compressed writes' \
    /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

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.