At 10:49 09/01/16, Andrew Morton wrote: >On Fri, 16 Jan 2009 09:53:02 +0900 Hisashi Hifumi ><hifumi.hisashi@xxxxxxxxxxxxx> wrote: > >> >> At 09:40 09/01/16, Andrew Morton wrote: >> >On Thu, 15 Jan 2009 09:21:13 -0500 >> >Theodore Tso <tytso@xxxxxxx> 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. >> > >> >bug ;) >> > >> >> 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. >> > >> >I think our thinking was that if two threads are racily updating f_pos >> >with different values, then it should end up with one of those values. >> > >> >From a quality-of-implementation POV (what _is_ that, anyway) it would >> >be bad if the kernel were to set f_pos to the upper 32 bits of position >> >A and the lower 32 bits of position B. Which could happen if we remove >> >the i_mutex protection on 32-bits. >> > >> >We could perhaps omit some locking if CONFIG_64BIT. There's probably >> >quite a bit of locking which could be omitted in that case. >> >> Updating f_pos value on 32bit is not atomic, so we discussed about this >> but we concluded that it does not matter whether f_pos is atomic or not > >It's unclear what you're saying here. > >I see three issues here: > >a) two racing threads update f_pos. One of them wins, and the > outcome in indeterminate. > >b) two racing threads update f_pos and the end result is that f_pos > contains a value which *neither* thread tried to write. > >c) one thread is writing and the other reading. There is a window > where the reader can see an intermediate value which is a mix of the > old and new values. > >I think we decided that a) is acceptable, b) is not and that c) can only >occur on multiple-of-4G wraparounds and isn't worth bothering about. > >> See, >> Subject:[RESEND] [PATCH] VFS: make file->f_pos access atomic on 32bit >> http://marc.info/?l=linux-fsdevel&m=122335627224515 > >Sorry, I'm disinclined to re-read a long thread, trying to work out >which bit you might be referring to. Following is Linus's post about this issue. http://marc.info/?l=linux-kernel&m=122356445226680&w=2 If we decide that a) is ok and we mind f_pos value being atomic on 32bit arch, we should use seq_counter to f_pos. > >> I think even i_mutex is not needed. When we touch i_size, i_size_read is >enough, >> and we can remove i_mutex at all on lseek. > >Why are we talking about i_size now? > >Confused. When caller of lseek set SEEK_END, i_size is referenced. I mentioned about this. I thought that the reason of i_mutex existence on lseek is only touching i_size. So if i_size_read is used to touch i_size value, i_mutex could be removed. -- 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
