Source-Changes-D archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
Re: CVS commit: src/sys/compat/linux/common
On 2023-08-25 13:30, Taylor R Campbell wrote:
> Since VOP_READDIR requires vp to be locked, I can infer that the
> handle_write caller must already hold vp locked. But that means that
> we have ifd_lock -> vnode lock in one path, and vnode lock -> ifd_lock
> in another path, which is forbidden (unless there's some reason we can
> prove these paths never happen concurrently).
Hmm... I did not realize this, so I don't think NOTE_WRITE
disambiguation is (or has ever been) safe.
The real pain here is that we inherit a held vp_interlock and vnode lock
in the needs_lock=false case from the kevent callback. So this may
require a pretty substantial locking revision(?)
Anyways, I'll take a closer look later this weekend.
> I'm also suspicious of logic like this:
>
> mutex_enter(&fp->f_lock);
> uio.uio_offset = fp->f_offset;
> mutex_exit(&fp->f_lock);
> ...
> mutex_enter(&fp->f_lock);
> fp->f_offset = uio.uio_offset;
> mutex_exit(&fp->f_lock);
>
> If fp->f_offset can change concurrently while f_lock is released, this
> looks like a problem. But if it can't, why do we need the lock at
> all? I suspect this really does need a lock over the whole
> transaction (maybe fp->f_lock, maybe something else), which is not
> released while I/O is happening -- possibly not with mutex(9) but
> something interruptible instead.
I think you're right about needing a lock for the whole transaction.
Since this is called from get_inotify_dir_entries(), perhaps it would be
better for get_inotify_dir_entries() to keep track of the offset and
pass it in instead?
Theo(dore)
Home |
Main Index |
Thread Index |
Old Index