From: Andi Kleen <ak@suse.de>
To: Trond Myklebust <Trond.Myklebust@netapp.com>
Cc: Steve French <smfrench@gmail.com>,
swhiteho@redhat.com, sfrench@samba.org, vandrove@vc.cvut.cz,
linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
akpm@osdl.org
Subject: Re: [PATCH] [8/18] BKL-removal: Remove BKL from remote_llseek
Date: Mon, 28 Jan 2008 03:58:13 +0100 [thread overview]
Message-ID: <200801280358.14024.ak@suse.de> (raw)
In-Reply-To: <1201475336.7346.37.camel@heimdal.trondhjem.org>
On Monday 28 January 2008 00:08:56 Trond Myklebust wrote:
>
> On Sun, 2008-01-27 at 16:18 -0600, Steve French wrote:
> > If two seeks overlap, can't you end up with an f_pos value that is
> > different than what either thread seeked to? or if you have a seek and
> > a read overlap can't you end up with the read occurring in the midst
> > of an update of f_pos (which takes more than one instruction on
> > various architectures), e.g. reading an f_pos, which has only the
> > lower half of a 64 bit field updated? I agree that you shouldn't
> > have seeks racing in parallel but I think it is preferable to get
> > either the updated f_pos or the earlier f_pos not something 1/2
> > updated.
>
> Why? The threads are doing something inherently liable to corrupt data
> anyway. If they can race over the seek, why wouldn't they race over the
> read or write too?
> The race in lseek() should probably be the least of your worries in this
> case.
The problem is that it's not a race in who gets to do its thing first, but a
parallel reader can actually see a corrupted value from the two independent
words on 32bit (e.g. during a 4GB). And this could actually completely corrupt
f_pos when it happens with two racing relative seeks or read/write()s
I would consider that a bug.
Fixes would be either to always take a spinlock to update this (nasty on
platforms where spinlocks are expensive like P4) or define some architecture
specific way to read/write 64bit values consistently. In theory also some
lazy locking seqlock like mechanism could be used, but that had the disadvantage
of being theoretically starvable.
-Andi
next prev parent reply other threads:[~2008-01-28 2:58 UTC|newest]
Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-01-27 2:17 [PATCH] [0/18] Implement some low hanging BKL removal fruit in fs/* Andi Kleen
2008-01-27 2:17 ` [PATCH] [1/18] BKL-removal: Convert ext2 over to use unlocked_ioctl Andi Kleen
2008-01-27 2:17 ` [PATCH] [2/18] BKL-removal: Remove incorrect BKL comment in ext2 Andi Kleen
2008-01-27 2:17 ` [PATCH] [3/18] BKL-removal: Convert ext3 to use unlocked_ioctl Andi Kleen
2008-01-28 5:33 ` Andrew Morton
2008-01-28 6:02 ` Andi Kleen
2008-01-27 2:17 ` [PATCH] [4/18] ext3: Remove incorrect BKL comment Andi Kleen
2008-01-27 2:17 ` [PATCH] [5/18] BKL-removal: Remove incorrect comment refering to lock_kernel() from jbd/jbd2 Andi Kleen
2008-01-27 2:17 ` [PATCH] [6/18] BKL-removal: Convert ext4 to use unlocked_ioctl Andi Kleen
2008-01-27 2:17 ` [PATCH] [7/18] BKL-removal: Remove incorrect comments refering to BKL from ext4 Andi Kleen
2008-01-27 2:17 ` [PATCH] [8/18] BKL-removal: Remove BKL from remote_llseek Andi Kleen
2008-01-27 16:57 ` Steve French
2008-01-27 17:56 ` Trond Myklebust
2008-01-27 22:18 ` Steve French
2008-01-27 23:08 ` Trond Myklebust
2008-01-28 2:58 ` Andi Kleen [this message]
2008-01-28 4:13 ` Trond Myklebust
2008-01-28 4:38 ` Andi Kleen
2008-01-28 4:51 ` Trond Myklebust
2008-01-28 5:13 ` Andrew Morton
2008-01-28 8:17 ` Andi Kleen
2008-01-28 18:33 ` Steve French
2008-01-28 19:34 ` Dave Kleikamp
2008-01-28 12:56 ` Alan Cox
2008-01-28 13:27 ` Andi Kleen
2008-01-28 13:38 ` Alan Cox
2008-01-28 14:10 ` Andi Kleen
2008-01-28 14:50 ` Alan Cox
2008-01-28 15:13 ` Diego Calleja
2008-01-28 15:13 ` Diego Calleja
2008-01-28 2:44 ` Andi Kleen
2008-01-28 2:43 ` Andi Kleen
2008-01-27 2:17 ` [PATCH] [9/18] BKL-removal: Use unlocked_ioctl for jfs Andi Kleen
2008-01-27 23:05 ` Dave Kleikamp
2008-01-27 2:17 ` [PATCH] [10/18] BKL-removal: Implement a compat_ioctl handler for JFS Andi Kleen
2008-01-27 23:05 ` Dave Kleikamp
2008-01-27 2:17 ` [PATCH] [11/18] BKL-removal: Convert ocfs2 over to unlocked_ioctl Andi Kleen
2008-01-29 4:13 ` [Ocfs2-devel] [PATCH] [11/18] BKL-removal: Convert ocfs2 over to unlocked_ioctl II Andi Kleen
2008-01-28 20:29 ` [Ocfs2-devel] " Mark Fasheh
2008-01-27 2:17 ` [PATCH] [12/18] BKL-removal: Convert CIFS over to unlocked_ioctl Andi Kleen
2008-01-27 2:17 ` [PATCH] [13/18] BKL-removal: Add compat_ioctl for cifs Andi Kleen
2008-01-27 2:17 ` [PATCH] [14/18] BKL-removal: Add unlocked_fasync Andi Kleen
2008-01-27 7:05 ` KOSAKI Motohiro
2008-01-27 2:17 ` [PATCH] [15/18] BKL-removal: Convert pipe over to unlocked_fasync Andi Kleen
2008-01-27 2:17 ` [PATCH] [16/18] BKL-removal: Convert socket fasync " Andi Kleen
2008-01-27 2:17 ` [PATCH] [17/18] BKL-removal: Convert fuse " Andi Kleen
2008-01-27 2:17 ` [PATCH] [18/18] BKL-removal: Convert bad_inode " Andi Kleen
2008-01-28 1:59 ` [PATCH] [0/18] Implement some low hanging BKL removal fruit in fs/* Nick Piggin
2008-01-28 3:15 ` Andi Kleen
[not found] <9Q5hR-3MI-9@gated-at.bofh.it>
[not found] ` <9Qsob-7is-1@gated-at.bofh.it>
[not found] ` <9QtDz-121-11@gated-at.bofh.it>
[not found] ` <9QtWX-1qg-15@gated-at.bofh.it>
[not found] ` <9Qugj-1PK-1@gated-at.bofh.it>
2008-01-28 9:44 ` [PATCH] [8/18] BKL-removal: Remove BKL from remote_llseek Bodo Eggert
2008-01-28 9:44 ` Bodo Eggert
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=200801280358.14024.ak@suse.de \
--to=ak@suse.de \
--cc=Trond.Myklebust@netapp.com \
--cc=akpm@osdl.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=sfrench@samba.org \
--cc=smfrench@gmail.com \
--cc=swhiteho@redhat.com \
--cc=vandrove@vc.cvut.cz \
/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.