linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Nick Piggin <nickpiggin@yahoo.com.au>
To: Hugh Dickins <hugh@veritas.com>
Cc: Linus Torvalds <torvalds@osdl.org>, Andrew Morton <akpm@osdl.org>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org, mason@suse.com,
	andrea@suse.de, axboe@suse.de
Subject: Re: [rfc][patch] remove racy sync_page?
Date: Wed, 31 May 2006 10:21:55 +1000	[thread overview]
Message-ID: <447CE1A3.60507@yahoo.com.au> (raw)
In-Reply-To: <Pine.LNX.4.64.0605301911480.10355@blonde.wat.veritas.com>

Hugh Dickins wrote:
> On Tue, 30 May 2006, Nick Piggin wrote:
> 
>>But for 2.6.17, how's this?
> 
> 
> It was a great emperor's-clothes-like discovery.  But we've survived
> for so many years without noticing, does it have to be fixed right
> now for 2.6.17?  (I bet I'd be insisting yes if I'd found it.)

It's up to Linus and Andrew I guess. I don't see why not, but I
don't much care one way or the other. But thanks having a quick
look at it, we may want it for the Suse kernel.

> 
> The thing I don't like about your lock_page_nosync (reasonable as
> it is) is that the one case you're using it, set_page_dirty_nolock,
> would be so much happier not to have to lock the page in the first
> place - it's only doing _that_ to stabilize page->mapping, and the
> lock_page forbids it from being called from anywhere that can't
> sleep, which is often just where we want to call it from.  Neil's
> suggestion, using a spin_lock against the mapping changing, would
> help there; but seems like more work than I'd want to get into.

But making PG_lock a spinning lock is completely unrelated to the
bug at hand.

> 
> So, although I think lock_page_nosync fixes the bug (at least in
> that one place we've identified there's likely to be such a bug),
> it seems to be aiming at the wrong target.  I'm pacing and thinking,
> doubt I'll come up with anything better, please don't hold breath.

It is the correct target. I know all about your set_page_dirty_lock
problems, but they aren't what I'm trying to fix.

AFAIKS, you could also make set_page_dirty_lock non sleeping quite
easily by making inode slabs RCU freed.

What places want to use set_page_dirty_lock without sleeping?
The only place in drivers/ apart from sg/st that SetPageDirty are
rd.c and via_dmablit.c, both of which look OK, if a bit crufty.

-- 
SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2006-05-31  0:21 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-05-29  9:34 Nick Piggin
2006-05-29 19:15 ` Andrew Morton
2006-05-30  0:08   ` Nick Piggin
2006-05-30  1:32     ` Andrew Morton
2006-05-30  2:54       ` Nick Piggin
2006-05-30  3:14         ` Andrew Morton
2006-05-30  4:13           ` Nick Piggin
2006-05-30  9:05           ` Jens Axboe
2006-05-31 13:43             ` Nick Piggin
2006-05-31 15:09               ` Hugh Dickins
2006-05-31 15:22                 ` Nick Piggin
2006-05-31 17:51                   ` Jens Axboe
2006-05-31 17:50               ` Jens Axboe
2006-05-30  4:20         ` Linus Torvalds
2006-05-30  5:07           ` Nick Piggin
2006-05-30  5:21             ` Nick Piggin
2006-05-30  6:12               ` Neil Brown
2006-05-30  7:10                 ` Nick Piggin
2006-05-31  4:34                   ` Neil Brown
2006-05-30  8:24               ` Nikita Danilov
2006-05-30 17:55               ` Linus Torvalds
2006-05-31  0:32                 ` Nick Piggin
2006-05-31  0:56                   ` Linus Torvalds
2006-05-31  1:33                     ` Mark Lord
2006-05-31  6:11                       ` Jens Axboe
2006-05-31 12:55                         ` Mark Lord
2006-05-31 13:02                           ` Jens Axboe
2006-06-01 13:19                           ` NCQ performance (was Re: [rfc][patch] remove racy sync_page?) Jens Axboe
2006-06-01 14:56                             ` Avi Kivity
2006-06-01 15:03                               ` Jens Axboe
2006-06-01 18:04                                 ` Jens Axboe
2006-06-05  5:30                                   ` Avi Kivity
2006-06-05  7:59                                     ` Jens Axboe
2006-05-31 12:31                     ` [rfc][patch] remove racy sync_page? Helge Hafting
2006-05-31 12:36                       ` Arjan van de Ven
2006-05-31 13:29                     ` Nick Piggin
2006-05-31 13:41                       ` Jens Axboe
2006-05-31 13:54                         ` Nick Piggin
2006-05-31 14:43                       ` Linus Torvalds
2006-05-31 14:57                         ` Nick Piggin
2006-05-31 15:13                           ` Linus Torvalds
2006-05-31 15:09                         ` Linus Torvalds
2006-05-31 18:13                           ` Jens Axboe
2006-05-31 18:26                             ` Linus Torvalds
2006-05-30  5:36             ` Nick Piggin
2006-05-30 18:31               ` Hugh Dickins
2006-05-31  0:21                 ` Nick Piggin [this message]
2006-05-31  3:06                   ` Hugh Dickins
2006-05-31 14:30                     ` Hugh Dickins
2006-05-31 17:56                     ` Jens Axboe
2006-05-30  5:51 ` Josef Sipek
2006-05-30  6:44   ` Nick Piggin
2006-05-30  6:50     ` Nick Piggin
2006-05-30 13:12     ` Josef Sipek

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=447CE1A3.60507@yahoo.com.au \
    --to=nickpiggin@yahoo.com.au \
    --cc=akpm@osdl.org \
    --cc=andrea@suse.de \
    --cc=axboe@suse.de \
    --cc=hugh@veritas.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mason@suse.com \
    --cc=torvalds@osdl.org \
    /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