[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20090115133530.GA30522@mit.edu>
Date: Thu, 15 Jan 2009 08:35:30 -0500
From: Theodore Tso <tytso@....edu>
To: Jamie Lokier <jamie@...reable.org>
Cc: Dave Kleikamp <shaggy@...ux.vnet.ibm.com>,
Hisashi Hifumi <hifumi.hisashi@....ntt.co.jp>,
akpm@...ux-foundation.org, linux-ext4@...r.kernel.org,
linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH] ext2/3/4: change i_mutex usage on lseek
On Thu, Jan 15, 2009 at 01:01:54PM +0000, Jamie Lokier wrote:
> Dave Kleikamp wrote:
> > Is there any reason you couldn't have just changed generic_file_llseek()
> > to do this rather than making identical changes to the individual file
> > systems. I would think this optimization would be safe for any file
> > system.
>
> Is it safe on 32-bit systems where 64-bit updates are not atomic?
Protection of 64-bit updates of i_size where 32-bit systems do not
have atomic 64-bit updates is done via i_size_seqcount, which is done
automatically as long as the code in question uses i_size_write(), an
inline function defined in include/linux/fs.h.
> You may say that doing multiple parallel lseek() calls is undefined
> behaviour, so it's ok to end up with file position that none of the
> individual lseek() calls asked for.
>
> But if it's undefined behaviour, no programs should be doing parallel
> lseek() calls on the same open file, so why optimise it at all?
i_mutex gets used for a lot more than protecting i_size updates for
lseek(), so removing the need for i_mutex could very well be helpful
for non-micro-benchmark tests. As Hirashi-san mentioned in his patch,
this could avoid contention between lseek() and write() (possibly via
a different file descriptor, so it wouldn't be an undefined race
condition) and fsync().
I do agree this is an optimization that should be done in
generic_file_llseek(), though.
Regards,
- Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists