On Thu, Aug 03, 2023 at 11:35:53AM -0700, Linus Torvalds wrote: > On Thu, 3 Aug 2023 at 11:03, Christian Brauner <brauner@xxxxxxxxxx> wrote: > > > > Only thing that's missing is exclusion with seek on directories > > as that's the heinous part. > > Bah. I forgot about lseek entirely, because for some completely stupid > reason I just thought "Oh, that will always get the lock". > > So I guess we'd just have to do that "unconditional fdget_dir()" thing > in the header file after all, and make llseek() and ksys_lseek() use > it. > > Bah. And then we'd still have to worry about any filesystem that > allows 'read()' and 'write()' on the directory - which can also update > f_pos. > > And yes, those exist. See at least 'adfs_dir_ops', and > 'ceph_dir_fops'. They may be broken, but people actually did do things > like that historically, maybe there's a reason adfs and ceph allow it. > > End result: we can forget about fdget_dir(). We'd need to split > FMODE_ATOMIC_POS into two instead. > > I don't think we have any free flags, but I didn't check. The ugly We do. Christoph freed up 3 last cycle. I think I mentioned it in one my prs. (And btw, we should probably try to get rid of a few more.) > thing to do is to just special-case S_ISDIR. Not lovely, but whatever. > > So something like this instead? It's a smaller diff anyway, and it > gets the crazy afds/ceph cases right too. If you really care about this we can do it. But if we can live with just unconditionally locking then I think we're better off. As I said, I've explicitly had requested the lkp performance ci that (I think Intel?) runs for us to be run on this with a focus on single threaded performance and so far there was nothing that wasn't noise.
