All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@lst.de>
To: Qu Wenruo <quwenruo.btrfs@gmx.com>
Cc: Christoph Hellwig <hch@lst.de>, Chris Mason <clm@fb.com>,
	Josef Bacik <josef@toxicpanda.com>,
	David Sterba <dsterba@suse.com>,
	Johannes Thumshirn <jth@kernel.org>,
	linux-btrfs@vger.kernel.org
Subject: Re: [PATCH 03/10] btrfs: offload all write I/O completions to a workqueue
Date: Mon, 20 Mar 2023 13:30:59 +0100	[thread overview]
Message-ID: <20230320123059.GB9008@lst.de> (raw)
In-Reply-To: <2aa047a7-984e-8f6f-163e-8fe6d12a41d8@gmx.com>

On Mon, Mar 20, 2023 at 07:29:38PM +0800, Qu Wenruo wrote:
> Sure, they are called in very strict context, thus we should keep them 
> short.
> But on the other hand, we're already having too many workqueues, and I'm 
> always wondering under what situation they can lead to deadlock.
> (e.g. why we need to queue endios for free space and regular data inodes 
> into different workqueues?)

In general the reason for separate workqueues is if one workqueue depends
on the execution of another.  It seems like this is Josef's area, but
my impression is that finishing and ordered extent can cause writeback
and a possible wait for data in the freespace inode.  Normally such
workqueue splits should have comments in the code to explain them, but
so far I haven't found one.

> My current method is always consider the workqueue has only 1 max_active, 
> but I'm still not sure for such case, what would happen if one work slept?

That's my understanding of the workqueue mechanisms, yes.

> Would the workqueue being able to choose the next work item? Or that 
> workqueue is stalled until the only active got woken?

I think it is stalled.  That's why the workqueue heavily discourages
limiting max_active unless you have a good reason to, and most callers
follow that advise.

> Personally speaking, I'd like to keep the btrfs bio endio function calls in 
> the old soft/hard irq context, and let the higher layer to queue the work.

Can you explain why?

> However we have already loosen the endio context for btrfs bio, from the 
> old soft/hard irq to the current workqueue context already...

read I/O are executed in a workqueue.  For write completions there also
are various offloads, but none that is consistent and dependable so far.

  reply	other threads:[~2023-03-20 12:31 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-14 16:59 defer all write I/O completions to process context Christoph Hellwig
2023-03-14 16:59 ` [PATCH 01/10] btrfs: use a plain workqueue for ordered_extent processing Christoph Hellwig
2023-03-16 17:10   ` Johannes Thumshirn
2023-03-16 17:31   ` David Sterba
2023-03-20  6:12     ` Christoph Hellwig
2023-03-20 11:08   ` Qu Wenruo
2023-03-20 11:35     ` Qu Wenruo
2023-03-20 12:24       ` Christoph Hellwig
2023-03-20 23:19         ` Qu Wenruo
2023-03-21 12:48           ` Christoph Hellwig
2023-03-14 16:59 ` [PATCH 02/10] btrfs: refactor btrfs_end_io_wq Christoph Hellwig
2023-03-16 17:12   ` Johannes Thumshirn
2023-03-20 11:09   ` Qu Wenruo
2023-03-14 16:59 ` [PATCH 03/10] btrfs: offload all write I/O completions to a workqueue Christoph Hellwig
2023-03-16 17:14   ` Johannes Thumshirn
2023-03-20 11:29   ` Qu Wenruo
2023-03-20 12:30     ` Christoph Hellwig [this message]
2023-03-20 23:37       ` Qu Wenruo
2023-03-21 12:55         ` Christoph Hellwig
2023-03-21 23:37           ` Qu Wenruo
2023-03-22  8:32             ` Christoph Hellwig
2023-03-23  8:07               ` Qu Wenruo
2023-03-23  8:12                 ` Christoph Hellwig
2023-03-23  8:20                   ` Qu Wenruo
2023-03-24  1:11                     ` Christoph Hellwig
2023-03-23 14:53               ` Chris Mason
2023-03-24  1:09                 ` Christoph Hellwig
2023-03-24 13:25                   ` Chris Mason
2023-03-24 19:20                     ` Chris Mason
2023-03-25  8:13                       ` Christoph Hellwig
2023-03-25 17:16                         ` Chris Mason
2023-03-25  8:15                     ` Christoph Hellwig
2023-03-25  8:42                       ` Qu Wenruo
2023-03-14 16:59 ` [PATCH 04/10] btrfs: remove the compressed_write_workers workqueue Christoph Hellwig
2023-03-14 16:59 ` [PATCH 05/10] btrfs: remove irq disabling for btrfs_workqueue.list_lock Christoph Hellwig
2023-03-17 10:34   ` Johannes Thumshirn
2023-03-14 16:59 ` [PATCH 06/10] btrfs: remove irq disabling for subpage.list_lock Christoph Hellwig
2023-03-14 16:59 ` [PATCH 07/10] btrfs: remove irq disabling for leak_lock Christoph Hellwig
2023-03-17 10:35   ` Johannes Thumshirn
2023-03-14 16:59 ` [PATCH 08/10] btrfs: remove irq disabling for fs_info.ebleak_lock Christoph Hellwig
2023-03-17 10:35   ` Johannes Thumshirn
2023-03-14 16:59 ` [PATCH 09/10] btrfs: remove irq_disabling for ordered_tree.lock Christoph Hellwig
2023-03-17 10:36   ` Johannes Thumshirn
2023-03-20  6:12     ` Christoph Hellwig
2023-03-14 16:59 ` [PATCH 10/10] btrfs: remove confusing comments Christoph Hellwig
2023-03-17 10:37   ` Johannes Thumshirn
2023-03-17 10:39 ` defer all write I/O completions to process context Johannes Thumshirn
2023-03-20  6:14   ` 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=20230320123059.GB9008@lst.de \
    --to=hch@lst.de \
    --cc=clm@fb.com \
    --cc=dsterba@suse.com \
    --cc=josef@toxicpanda.com \
    --cc=jth@kernel.org \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=quwenruo.btrfs@gmx.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 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.