On Sat, Nov 01, 2025 at 05:03:44PM +0100, Mark Kettenis wrote:
> > Date: Sat, 1 Nov 2025 13:45:41 +0100
> > From: Martin Pieuchot <mpi@grenadille.net>
> > 
> > On 31/10/25(Fri) 16:08, Vitaliy Makkoveev wrote:
> > > On Mon, Oct 27, 2025 at 10:08:08AM +0100, Martin Pieuchot wrote:
> > > > Diff below uses foffset() in vn_read() and getdents(2) to push the
> > > > KERNEL_LOCK() a bit further.  This is not much in itself but is a
> > > > requirement for upcoming changes.
> > > > 
> > > > By reading the current offset before grabbing the vnode lock we
> > > > accept a race: another thread could change it.
> > > > 
> > > > While here, grab the corresponding mutex before writing `f_offset' in
> > > > dt(4).
> > > > 
> > > > ok?
> > > > 
> > > 
> > > As I understand the code, vn_lock() and VOP_LOCK() paths do not require
> > > kernel lock to be held, so you can leave vn_lock() as is and avoid
> > > race for `f_offset'.
> > 
> > You're right. That said I'm interested in doing exactly the opposite of
> > what you suggest.  That's why I sent this diff in the first place.
> > 
> > The race you mentioned is not a problem for the kernel to solve.  Is
> > that clear or does that need explanations?
> 
> I think an explanation is needed.  POSIX does make certain guarantees
> about atomicity of file operations (See 2.9.7 Thread Interactions with
> File Operations in the System Interfaces part).  And while
> posix_getdents() isn't listed there, read() is.
> 

I like the idea to push kernel lock deeper and keep it around
VOP_READ()/VOP_WRITE(). We don't need to move vn_lock()/VOP_LOCK(), so
the race for `f_offset' will be excluded.

I can make this diff by myself if mpi@ has no interest in this
direction.