- User Since
- Oct 29 2015, 5:25 PM (115 w, 6 d)
Wed, Jan 10
I think this should be committed after addressing @wblock's comments.
I think this should be committed basically "as is", unless someone raises serious objections in the next week or so.
After addressing @ian's commands, I think this should be committed "as is", unless someone raises serious objections in the next week or so.
I think this should be committed basically "as is" (perhaps with the few tweaks suggested in the review), unless someone raises serious objections in the next week or so.
Tue, Jan 9
I think one of the few weaknesses I see is the way the hash result is cached.
It seems like this could use a man page to describe the mechanism. There are some subtleties that are not immediately obvious, such as the way that shared libraries are protected. In addition, the O_VERIFY flag should probably be documented in the open() man page with a pointer to the verified exec man page.
Mon, Jan 8
Nov 3 2017
Added a sysctl to control whether KASSERTs are suppressed after a panic.
Nov 2 2017
I ended up posting this for review, since I didn't want to lose the diff. However, while thinking about this more, I realized that there is probably a better way to do this.
Oct 24 2017
Seems like a good idea! See in-line comments.
Oct 19 2017
For what its worth, here is my 2c on this.
Oct 11 2017
I remain concerned about the performance overhead of activating this where not needed. However, I have no performance information to either alleviate or confirm my fears. Has someone done the work to gather performance information to compare a non-VIMAGE kernel to a VIMAGE kernel with a single VNET?
Sep 26 2017
I'm not sure what I think of this change. It seems like an API change again: now EAGAIN always means that pru_send added the bytes instead of freeing them, while other errors are either fatal or mean that the bytes were lost.
Sep 25 2017
I'm not sure I understand the problem this patch is trying to solve. Can you explain the problem in sufficient detail so we can understand how this patch addresses it? Thanks!
Can you explain how these are different? Both functionally set error to so->so_error, and then set so->so_error to 0. If (so) is properly locked, it looks like these should be equivalent. If (so) is not properly locked, then the new formulation is not guaranteed to overcome this deficiency.
Sep 8 2017
Aug 14 2017
Looks good (with one minor nit inline).
Aug 12 2017
Aug 11 2017
At a high level...
Aug 9 2017
Jul 5 2017
Jul 3 2017
Jun 10 2017
Jun 8 2017
Jun 7 2017
May 31 2017
This is so trivial that I plan to commit it within 24 hours unless someone complains.
I had a long discussion with @ed in the bug. You can see that for context. I think the summary is that I still think we should commit this code, even if @ed thinks a more robust fix requires a larger change. I would like to do that, but do also support @ed making a further bug fix in the future.
May 30 2017
May 26 2017
May 25 2017
In general, I think the code would benefit from more comments.
Apr 13 2017
Feb 28 2017
Enhance the comment. Move an atomic operation to non-atomic.
Feb 27 2017
Feb 26 2017
Feb 24 2017
Invalidate TLB when restarting other CPUs.
Feb 23 2017
This has been lingering for a while. Any comments before I commit it?
Feb 22 2017
Feb 8 2017
Jan 26 2017
Nov 17 2016
Thanks for finding this bug and proposing a solution! Your solution looks fine in the sense that it mimics the rest of the behavior of this function when validation fails.
Oct 20 2016
Summary of verbal feedback today:
- If it can be done without major surgery, please consider changing (u_)long to (u)int32_t.
- Because this changes the assumption about who sets snd_cwnd, please verify that all the congestion control modules follow the new assumption.