On Tue, Dec 19, 2023 at 07:50:25AM +1030, Brett Lymn wrote:
> On Sat, Sep 02, 2023 at 08:57:03AM +0930, Brett Lymn wrote:
> > On Thu, Aug 31, 2023 at 10:24:07AM +0200, J. Hannken-Illjes wrote:
> > >
> > > I'm short on time, some remarks:
> > >
> >
> > Thanks, I will have a closer look at these issues. This could be where the
> > deadlock I was
> > seeing is coming from.
> >
>
> OK, I have, finally, updated the diff in line with the comments (thanks
> again for the good feedback).
Brett,
first it is difficult to read a diff that also introduces a "notyet" operation
on file pages.
Looks like you replace global and item rw-locks with refcounts/signals.
I tried to understand the rationale behind and failed. Your implementation
seems at least vulnerable to livelocks as the writer side is missing priority.
In general, what problem(s) are you trying to solve here?
What is wrong with a quite simple hashtable to hold fileid<->fingerprint
mapping accessed as:
mutex_enter table lock
lookup fileid -> item
mutex_enter item
mutex_exit table lock
while item state is busy
cv_wait item cv
if item state is unknown
set item state busy
mutex_exit item
compute fingerprint
mutex_enter item
set item state valid/invalid
cv_broadcast item cv
Here the item state is either valid or invalid
mutex_exit item
Some more problems (line numbers with your diff applied):
476 veriexec_fp_calc(struct lwp *l, struct vnode *vp, int lock_state,
477 struct veriexec_file_entry *vfe, u_char *fp)
478 {
524 error = vn_rdwr(UIO_READ, vp, buf, len, offset,
525 UIO_SYSSPACE, lock_state,
lock_state is sometimes VERIEXEC_UNLOCKED, sometimes IO_NODELOCKED.
1629 veriexec_dump(struct lwp *l, prop_array_t rarray)
1630 {
1631 struct mount *mp;
1632 mount_iterator_t *mpiter;
1633
1634 mountlist_iterator_init(&mpiter);
1635
1636 while ((mp = mountlist_iterator_trynext(mpiter)) != NULL) {
1637 /* If it fails, the file-system is [being] unmounted. */
1638 if (vfs_busy(mp) != 0)
1639 continue;
This is completely wrong. Operation mountlist_iterator_trynext() already
returns the mount busy and skips mounts currently unmounting or remounting.
Operation mountlist_iterator_next() was right or does this function get
called from inside unmount?
1653 veriexec_flush(struct lwp *l)
1654 {
1655 struct mount *mp;
1656 mount_iterator_t *mpiter;
1657 int error = 0;
1658
1659 mountlist_iterator_init(&mpiter);
1660
1661 while ((mp = mountlist_iterator_trynext(mpiter)) != NULL) {
1662 int lerror;
1663
1664 /* If it fails, the file-system is [being] unmounted. */
1665 if (vfs_busy(mp) != 0)
1666 continue;
This is completely wrong, see above.
--
J. Hannken-Illjes
