On Wed, Jul 26, 2023 at 10:31:07AM +0000, David Laight wrote: > From: Christian Brauner > > Sent: 26 July 2023 09:37 > ... > > Yes, and to summarize which I tried in my description for the commit. > > The getdents support patchset would have introduced a bug because the > > patchset copied the fdget_pos() file_count(file) > 1 optimization into > > io_uring. > > > > That works fine as long as the original file descriptor used to register > > the fixed file is kept. The locking will work correctly as > > file_count(file) > 1 and no races are possible neither via getdent calls > > using the original file descriptor nor via io_uring using the fixed file > > or even mixing both. > > > > But as soon as the original file descriptor is closed the f_count for > > the file drops back to 1 but continues to be usable from io_uring via > > the fixed file. Now the optimization that the patchset wanted to copy > > over would cause bugs as multiple racing getdent requests would be > > possible using the fixed file. > > Could the io_uring code grab two references? > That would stop the optimisation without affecting any > normal code paths? io_uring doesn't use fdget variants and can't for it's purposes as fdget is for short term references while io_uring holds on to the file. This whole thing was about the logic that was copied in a patchset not the actual helper itself. I thought that was clear from "copied the [...] optimization into io_uring". It should just not do that.
