On Mon, Mar 03, 2014 at 01:56:31PM -0800, Linus Torvalds wrote: > On Mon, Mar 3, 2014 at 1:45 PM, Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote: > > > > Um... That's odd - we *could* get there with f.need_put and > > file_count(file) equal to 1, but why would we want to take > > f_pos_lock in that case? > > Because that means that the file table is shared among threads. So > another thread can access the struct file pointer and do a concurrent > read() or write() on it, and so we need to lock f_pos. Yes, but in that case fdget() has grabbed a reference to that sucker, so the only way to end with refcount 1 is to have the damn thing gone from descriptor table in between. And AFAICS in that case we are just fine without f_pos_lock. > - concurrent access due to duplicated "struct file_table" pointers. > This is the "need_put" test, since __fget_light() will have tested the > proper files->count already. Shared descriptor table means that we'd better have a reference grabbed already... I agree that file_count(file) > 1 for any reason requires locking it; it's just that need_put means exactly that we have grabbed a reference ourselves. So having need_put && file_count <= 1 means that we have grabbed it and then somebody dropped all other references. Including the ones in descriptor table(s), shared or not, etc. In that case they'd better not touch that struct file anymore - after all, once we are done with whatever we are doing, we'll do fput() and it will be ripped from under them with no warning. -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
