[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20070131003215.GA7163@wotan.suse.de>
Date: Wed, 31 Jan 2007 01:32:16 +0100
From: Nick Piggin <npiggin@...e.de>
To: Andrew Morton <akpm@...l.org>
Cc: Hugh Dickins <hugh@...itas.com>,
Linux Kernel <linux-kernel@...r.kernel.org>,
Linux Filesystems <linux-fsdevel@...r.kernel.org>,
Linux Memory Management <linux-mm@...ck.org>
Subject: Re: [patch 0/9] buffered write deadlock fix
On Tue, Jan 30, 2007 at 12:55:58PM -0800, Andrew Morton wrote:
> On Mon, 29 Jan 2007 11:31:37 +0100 (CET)
> Nick Piggin <npiggin@...e.de> wrote:
>
> > The following set of patches attempt to fix the buffered write
> > locking problems
>
> y'know, four or five years back I fixed this bug by doing
>
> current->locked_page = page;
>
> in the write() code, and then teaching the pagefault code to avoid locking
> the same page. Patch below.
>
> But then evil mean Hugh pointed out that the patch is still vulnerable to
> ab/ba deadlocking so I dropped it.
>
> But Hugh lied! There is no ab/ba deadlock because both a and b need
> i_mutex to get into the write() path.
Not only is there still the abba deadlock on page locks, as you point
out in your next mail, but there is also an abba on page lock vs mmap_sem.
> This approach doesn't fix the writev() performance regresson which
> nobody has measured yet but which the NFS guys reported.
>
> But I think with this fix in place we can go back to a modified version of
> the 2.6.17 filemap.c code and get that performance back, but I haven't
> thought about that.
>
> It's a heck of a lot simpler than your patches though ;)
Ignoring the cleanup patches, the only thing mine really do is to use
get_user_pages in generic_file_buffered_write if the destination page
is not uptodate, and use atomic copies if it is. Conceptually pretty simple.
Mine fixes the writev performance regression iff the destination page
it uptodate. More importantly, they fix 3 deadlocks in the core mm.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists