LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: "Steve French" <smfrench@gmail.com>
To: "Trond Myklebust" <Trond.Myklebust@netapp.com>
Cc: "Andi Kleen" <ak@suse.de>,
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: Sun, 27 Jan 2008 16:18:26 -0600 [thread overview]
Message-ID: <524f69650801271418s16f88928xc58dcbe9f5ede9e4@mail.gmail.com> (raw)
In-Reply-To: <1201456562.7346.13.camel@heimdal.trondhjem.org>
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.
On Jan 27, 2008 11:56 AM, Trond Myklebust <Trond.Myklebust@netapp.com> wrote:
>
> On Sun, 2008-01-27 at 10:57 -0600, Steve French wrote:
> > Don't you need to a spinlock/spinunlock(i_lock) or something similar
> > (there isn't a spinlock in the file struct unfortunately) around the
> > reads and writes from f_pos in fs/read_write.c in remote_llseek with
> > your patch since the reads/writes from that field are not necessarily
> > atomic and threads could be racing in seek on the same file struct?
>
> Where does is state in POSIX or SUS that we need to cater to that kind
> of application?
> In any case, the current behaviour of f_pos if two threads are sharing
> the file struct is undefined no matter whether you spinlock or not,
> since there is no special locking around sys_read() or sys_write().
>
> Trond
>
--
Thanks,
Steve
next prev parent reply other threads:[~2008-01-27 22:18 UTC|newest]
Thread overview: 47+ 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 [this message]
2008-01-27 23:08 ` Trond Myklebust
2008-01-28 2:58 ` Andi Kleen
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 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-27 2:17 ` [PATCH] [12/18] BKL-removal: Convert CIFS " 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
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=524f69650801271418s16f88928xc58dcbe9f5ede9e4@mail.gmail.com \
--to=smfrench@gmail.com \
--cc=Trond.Myklebust@netapp.com \
--cc=ak@suse.de \
--cc=akpm@osdl.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=sfrench@samba.org \
--cc=swhiteho@redhat.com \
--cc=vandrove@vc.cvut.cz \
--subject='Re: [PATCH] [8/18] BKL-removal: Remove BKL from remote_llseek' \
/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
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).