- User Since
- Jan 13 2015, 10:58 PM (166 w, 2 d)
Feb 20 2018
Looks ok to me at a glance. I don't know of any "standard" for what errors should be
returned. (Long ago it would have been "what Solaris does", but that's no longer the
case. If you wanted to, you could check to see if the error returns are "Linux compatible",
which is as close to a "standard" as there is these days.
Feb 9 2018
I'm sure there's a way to add this to the above comment box,
but I don't know how to do it.
- W.r.t. the syscall_helper() maintaining a counter of caller threads...
---> For the NFS/Krpc modules it isn't normally syscall threads.
Usually they are called through the VFS or via a set of function pointers or... - There really isn't any reason to unload any of these as far as I know.
Feb 8 2018
Since MOD_UNLOAD is not allowed and just fails, I don't see a reason
to call kgss_unload() and get rid of the syscalls.
Feb 7 2018
Looks ok to me, but since I know nothing about the interface,
I've taken myself off the review.
Looks ok to me, although, since I know nothing about the interface,
I've taken myself off the review.
Feb 6 2018
The NFS ones look ok to me. To be honest, some of them were inherited from the old NFS
and I'm not even sure what the do (without looking at the code). However, the descriptions
look reasonable to me.
Jan 29 2018
At a glance, it looks ok. However, since I know absolutely nothing about libxo and
have no way of testing it before April, I don't feel qualified to review it.
Since it has already received a positive review, that seems good enough to me.
Jan 25 2018
The replacement of MALLOC/FREE in the NFS code looks ok to me.
As I noted in the inline comment, I used to care about portability to
other BSDen, but I don't any more. (None of them seem interested
in the code..)
Dec 25 2017
The ones for nfs look ok. (I have not reviewed the rest of them.)
I would suggest that they be done as separate commits.
Dec 19 2017
Dec 18 2017
Dec 9 2017
Dec 6 2017
It looks ok to me, but I am not familiar enough with socket handling to
be sure doing this is ok. As such, I suspect it's fine, but won't actually
accept it. (Someone else already has accepted it.)
Dec 4 2017
Adding a KASSERT() to check for correct lock acquisition when incrementing/decrementing
the counter is more awkward than it might seem.
The obvious place for such a check would be nfsrv_freedeleg(), which is where
this counter is decremented. Unfortunately this function is called during module
unload, when there are no nfsd threads and no locks held.
The locations where the counter is incremented are all in rather long chunks of
code that is updating state. Putting a KASSERT() at the beginning of these chunks
could be done, but would be almost silly, since the call would be right after the
code that acquires the locking.
Dec 2 2017
Modify the patch to not use atomic ops, which do not appear to be needed.
Add an inline comment related to nfsd thread locking.
Added inline reply to kib@'s comment.
Dec 1 2017
Nov 11 2017
Nov 9 2017
Nov 5 2017
Nov 4 2017
Nov 2 2017
Oct 25 2017
Oct 16 2017
Oct 15 2017
Oct 11 2017
Oct 10 2017
This variant of the patch sets the default # of threads to 8 * mp_ncpus instead of
a fixed 32. The sysctl allows a sysadmin to override this default.
Suggested by Julian Eischler.
The previous patch had a fundamental flaw, which was the code slept
waiting on completion of each RPC, serializing them.
This version of the patch doesn't sleep waiting for completion until
after all the RPCs have been started.
Oct 9 2017
Oct 8 2017
Oct 5 2017
Oct 4 2017
NFS part looks ok to me. (I might have used UINT32_MAX instead of 0xffffffff for NFS_MAX_LINK, but
that's a very minor nit..)
Oct 3 2017
I've accepted this. As bapt@ noted, it loses the alphabetical order.
I don't know if that matters. I believe it means that the reply to
"showmount -e" will not be alphabetically ordered. Does any
client care? I don't know.
Looks ok to me. I think the only effect is a change in the order of entries in the list
and I don't believe that matters.
Oct 2 2017
Oct 1 2017
Sep 29 2017
Sep 28 2017
Sep 27 2017
Sep 26 2017
Sep 24 2017
Sep 21 2017
Sep 19 2017
Sep 17 2017
Sep 4 2017
Sep 3 2017
Aug 25 2017
Aug 19 2017
This version looks fine to me.
Thanks for doing this, rick.
I've accepted this revision, but I do have one concern.
If a future code change resulted in the vnode lock changing from shared->exclusive
before the ncl_downgrade_vnlock(), it would erroneously do an unlock of this
- If you left ncl_upgrade_vnlock() returning oldlock and passed that into ncl_downgrade_lock() to use to decide whether or not to unlock the new lock, this would be avoided.