All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Sterba <dave@jikos.cz>
To: Miao Xie <miaox@cn.fujitsu.com>
Cc: Linux Kernel <linux-kernel@vger.kernel.org>,
	Linux Btrfs <linux-btrfs@vger.kernel.org>,
	Linux Fsdevel <linux-fsdevel@vger.kernel.org>,
	Linux Ext4 <linux-ext4@vger.kernel.org>,
	viro <viro@zeniv.linux.org.uk>,
	Christoph Hellwig <hch@infradead.org>,
	Kamal Mostafa <kamal@canonical.com>
Subject: Re: [PATCH 1/2 RESEND] vfs: re-implement writeback_inodes_sb(_nr)_if_idle() and rename them
Date: Wed, 5 Sep 2012 17:54:04 +0200	[thread overview]
Message-ID: <20120905155403.GL17430@twin.jikos.cz> (raw)
In-Reply-To: <503DB3E8.1000308@cn.fujitsu.com>

On Wed, Aug 29, 2012 at 02:17:12PM +0800, Miao Xie wrote:
> writeback_inodes_sb(_nr)_if_idle() is re-implemented by replacing down_read()
> with down_read_trylock() because
> - If ->s_umount is write locked, then the sb is not idle. That is
>   writeback_inodes_sb(_nr)_if_idle() needn't wait for the lock.
> - writeback_inodes_sb(_nr)_if_idle() grabs s_umount lock when it want to start
>   writeback, it may bring us deadlock problem when doing umount.
>   (Ex. Btrfs has such a problem, see the following URL
>        http://marc.info/?l=linux-btrfs&m=133540923510561&w=2)
> 
> The name of these two functions is cumbersome, so rename them to
> try_to_writeback_inodes_sb(_nr).
> 
> This idea came from Christoph Hellwig.
> Some code is from the patch of Kamal Mostafa.
> 
> Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
Tested-by: David Sterba <dsterba@suse.cz>

> ---
> Many users who use btrfs met the deadlock problem caused by writeback_inodes_sb(_nr)_if_idle(),
> so I send this patch again and hope it will be received as soon as possible if nobody objects.

Yes please, this makes testing of the 'autodefrag' mount option hard as
the deadlock occurs frequently upon umount (when there's a background
snapshot cleanup in progress).

>From the user's POV, the autodefrag improves performance, it's not yet
on by default until it proves to be stable, the deadlocks were standing
in the way for long.

thanks,
david

  reply	other threads:[~2012-09-05 15:54 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-29  6:17 [PATCH 1/2 RESEND] vfs: re-implement writeback_inodes_sb(_nr)_if_idle() and rename them Miao Xie
2012-09-05 15:54 ` David Sterba [this message]
2012-10-09 10:25   ` 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=20120905155403.GL17430@twin.jikos.cz \
    --to=dave@jikos.cz \
    --cc=hch@infradead.org \
    --cc=kamal@canonical.com \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=miaox@cn.fujitsu.com \
    --cc=viro@zeniv.linux.org.uk \
    /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.