Theodore Tso wrote:
On Thu, Jan 15, 2009 at 06:22:52AM -0700, Matthew Wilcox wrote:
Of course if you have multiple threads, they will share a struct file,
and you're updating f_pos and f_version without locking. Maybe that's
OK, but it's soemthing you didn't discuss.
f_pos is updated by sys_write(), and friends without locking, so we're
fine on that front, or at least no worse off. SUSv3 doesn't seem to
say one way or another what should happen if two threads try to
write() to a file at the same time using the same file descriptor in
terms of whether or not f_pos gets updated intelligently. We've opted
for speed over determinism already.
Zero'ing out f_version is fine to do without locking. It's only used
so we know that we need to revalidate in the readdir() case so that we
know it's pointing at a valid directory pointer.
That being said, I do see a race in fs/ext*/dir.c, but i_mutex locking
isn't the problem and it's not going to save us. ext[234]_readdir()
uses f_pos through the routine, even between calls that might block;
so if one thread is randomly calling seekdir() (or lseek() directly)
while another read is calling readdir(), ext[234]_readdir() could get
potentially very confused. If someone wants to take a look at it,
that would be great. Otherwise I'll put it on my low-priority queue
of things to look at.
I think it's the only reason to have the mutex here. Otherwise we could
simply use i_size_read() in generic_file_llseek_unlocked() and there
would be no need for a mutex at all.
That's a good point. Maybe I'm missing something, but I'm not sure we
need the mutex in generic_file_llseek() at all.
I'm not arguing against this. Just pointing out what
I suspect Jamie was thinking about and what a difference
3 months makes to the patch author :)
Subject:[RESEND] [PATCH] VFS: make file->f_pos access atomic on 32bit
http://marc.info/?l=linux-fsdevel&m=122335627224515
...that ended with Linus rightly saying users expecting to
do stuff like that are insane.
*However*, we should remember that any time you remove locks
from a path, you run the risk the lock was providing some
unplanned consistency and removing it will increase the window
for unexpected behavior. And maybe finding more buggy apps
is good or bad only depending on your point of view.
jim
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
