Tauist. Bit twiddler. Advocate for continued logarithms.
- User Since
- Jun 30 2017, 3:18 PM (103 w, 4 d)
It broke boot, so I've reverted it. Now, while I look for the problem, I'll accept your shaming.
Fine-tune a comment that still includes "now will do cow".
Discard the parts of a comment that may mislead.
Apply reviewer suggestions.
Be more articulate.
Back-up one version.
Accept reviewer suggestion.
Accept reviewer advice.
Accept Kostik's suggestion.
Resolve a conflict.
Take Alan's suggestion.
Mon, Jun 24
Remove redundant test.
Retreat. I know I can improve something, and I know that I'm misunderstanding something else. I'll try to take the small win for now.
Move the simplification of entries to a separate, third loop, not because I want to but because I want to see if simplification is what's making my accounting assertion fail, or if the cause is some other thing I don't understand.
Add an assertion that the amount of swap space to reserved has been calculated correctly.
Sun, Jun 23
Remove a redundant condition, one which is tested for at the top of the loop.
If vm_map_entry_back is going to be invoked by clipping, then use the size of the untrimmed entry, which is what vm_map_entry_back will set as the size of the object that it creates. Otherwise, if there's no object, there won't be an object made and use the clipped map entry difference. Otherwise, use the size from the pre-existing object, the size of which won't be modified by vm_map_entry_charge_object, or by clip_start or clip_end.
Don't assume I can ignore ptoa(obj->size). Account for clipping when there's no object.
Address easy suggestions from reviewer.
Don't account for clipping in computing the amount of swap space to reserve. Fix some style glitches.
Sat, Jun 22
I understand that I violated a style(9) header rule too. Fix that.
Reorder events in the final loop now that the significance of 'cred' has been partially revealed to me.
Don't allocate swap space until the in-transition test is passed.
Relative to the previous version, combine the last two loops of vm_map_protect into one.
Reduce the swap_reserve calls to one, and not clip or modify map entry fields until it succeeds. This change makes an assumption about the need to check ptoa(obj->size) - that it isn't necessary when you the start and end of the map entry - that I'm believing because alc told me so.
Fri, Jun 21
Apply most reviewer suggestions. Explain the one I didn't apply.
By renaming, re-indenting, and updating stale comments, seek to answer questions and address reviewer concerns.
Add a header. Drop an extra argument to warn().
Thu, Jun 20
Just to be clear, I understand that this latest patch produces no printfs on amd64, but only on i386, and the previous version produces printfs on both architectures. Is that understanding correct?
I've identified a case where vm_map_protect leaves an entry un-simplified, and attempted to address it. I hope to discover whether the number of simplify_entry calls logged as necessary in clip_start is reduced by this change.
Wed, Jun 19
Addressed reviewer comments about these changes.
Tue, Jun 18
... but keep the boolean_t to bool return value transition for vm_map_entry_lookup.
Too many places depend on the vm_map_entry_lookup name, so leave it alone and rename something else.
Most of these changes are distractions, unrelated to your real purpose. I suggest you get rid of them. What's left is you removing a bug fix from 2003 without any explanation about why that fix is no longer necessary, and you fixing a bug that hasn't been filed with no real explanation of how it is a fix. I don't think anyone has enough information to approve this change.
Mon, Jun 17
Endeavor to apply reviewer suggestions.
Sun, Jun 16
Sat, Jun 15
Just to be clear, I understand you to be saying that the map11 test produces the four lines of output above and no other tests, including buildworld/installworld, produce any such output. Is that correct?
Fri, Jun 14
Make it compile.
Apply reviewer suggestion.
Write information about the cases in which clip_start requires entry simplification, with the hope that the origins of those cases can be identified and that simplification happen elsewhere.
Leave out unrelated patches.
Separate the comments and code blocks for 'create backing object' from 'clone entry'.
Can you update this patch, please, at least so that it can be applied? Part of it was put into a separate patch and has already been put in head.
Update comments in accordance with suggestions from alc.
Thu, Jun 13
Address reviewer comments.
Change bool literals to boolean_t literals in some places. Change u_ints to u_longs, and add a check for overflow. Truncate in computing hi.
Wed, Jun 12
Accept reviewer suggestions for changes to u_int for hi, lo and npages.
Restrict the scope of this patch just to reducing code duplication. On this amd64, it reduces the size of vm_map.o from 456608 to 455816, with no obvious impact on performance.
Ignore this patch while I study an unexpected negative performance impact.
Restore a patch that got accidentally copied over last week.