Re: [PATCH] file: always lock position

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux