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. - Ted -- 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
