From: Nick Piggin <nickpiggin@yahoo.com.au>
To: Jens Axboe <axboe@suse.de>
Cc: Andrew Morton <akpm@osdl.org>,
linux-kernel@vger.kernel.org, linux-mm@kvack.org, mason@suse.com,
andrea@suse.de, hugh@veritas.com, torvalds@osdl.org
Subject: Re: [rfc][patch] remove racy sync_page?
Date: Wed, 31 May 2006 23:43:56 +1000 [thread overview]
Message-ID: <447D9D9C.1030602@yahoo.com.au> (raw)
In-Reply-To: <20060530090549.GF4199@suse.de>
Hi Jens,
Sorry, I don't think I gave you any reply to this...
Jens Axboe wrote:
> On Mon, May 29 2006, Andrew Morton wrote:
>
>>
>>Mysterious question, that. A few years ago I think Jens tried pulling
>>unplugging out, but some devices still want it (magneto-optical
>>storage iirc). And I think we did try removing it, and it caused
>>hurt.
>
>
> I did, back when we had problems due to the blk_plug_lock being a global
> one. I first wanted to investigate if plugging still made a difference,
> otherwise we could've just ripped it out back than and the problem would
> be solved. But it did get us about a 10% boost on normal SCSI drives
> (don't think I tested MO drives at all), so it was fixed up.
Interesting. I'd like to know where from. I wonder if my idea of a
process context plug/unplug would solve it...
>
>
>>>With splice, the mapping can change, so you can have the wrong
>>>sync_page callback run against the page.
>>
>>Oh.
>
>
> Maybe I'm being dense, but I don't see a problem there. You _should_
> call the new mapping sync page if it has been migrated.
But can some other thread calling lock_page first find the old mapping,
and then run its ->sync_page which finds the new mapping? While it may
not matter for anyone in-tree, it does break the API so it would be
better to either fix it or rip it out than be silently buggy.
>>>
>>>The ->pin() calls in pipe_to_file and pipe_to_sendpage?
>>
>>One for Jens...
>
>
> splice never incs/decs any inode related reference counts, so if it
> needs to then yes it's broken. Any references to kernel code that deals
> with that?
Most code in the VM that has an inode/mapping gets called from the VFS,
which already does its thing somehow (I guess something like the file
pins the dentry which pins the inode). An iget might solve it. Or you
could use the lock_page_nosync() if/when the patch goes in (although I
don't want that to spread too far just yet).
--
SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com
next prev parent reply other threads:[~2006-05-31 13:44 UTC|newest]
Thread overview: 66+ messages / expand[flat|nested] mbox.gz Atom feed top
2006-05-29 9:34 [rfc][patch] remove racy sync_page? 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 [this message]
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:33 ` Nick Piggin
2006-05-31 15:57 ` Linus Torvalds
2006-05-31 16:12 ` Linus Torvalds
2006-05-31 16:26 ` Nick Piggin
2006-05-31 16:19 ` Nick Piggin
2006-05-31 16:22 ` Nick Piggin
2006-05-31 16:41 ` Linus Torvalds
2006-06-02 2:34 ` Nick Piggin
2006-06-02 2:39 ` Nick Piggin
2006-05-31 16:39 ` Linus Torvalds
2006-06-02 2:21 ` Nick Piggin
2006-05-31 23:59 ` Neil Brown
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
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=447D9D9C.1030602@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