Use next_end

In D54229#1240206, @des wrote:

Can I please get a decision on this?

This looks fine to me, but I prefer to have more eyes on it before is is engraved into the libc.

In D54258#1239897, @markj wrote:

In D54258#1239826, @kib wrote:

This is only relevant for 32bit arches, am I right? (I do not claim that it is not a bug to fix).

Hmm, now that I look at it again, I think this patch is just bogus, and there is no problem. If size < 0 and size is promoted to an unsigned type, addition with tm_ea_memory_inuse will cause wraparound and the result will be correct. It does not make much sense to perform the comparison at all if size < 0, but I think there is no problem, even on 32-bit arches. I did not test it at all yet.

Correct the article in comment.

This is only relevant for 32bit arches, am I right? (I do not claim that it is not a bug to fix).

Fix pend (and eliminate).

Ok, if somebody from the powerpc crowd confirm that the isync is redundant there, I am fine with that.
If not, I put a sketch for my proposal at D54246.

Not even compiled.

Remove pstart.
Remove clipping from swap_pager_seek_data(), so that PG_ZERO check can see all pages in the object.

swap_pager_seek_data(): check for the swap object, this is what the function assumes

In D54221#1238704, @jrtc27 wrote:

IMO the commits that added the icache sync were done due to some actual problems seen.

Probably due to a broken pmap (at least at the time) then.

in principle, non-plt relocations can modify any kind of segments. There are DT_TEXTREL binries, as well as nothing in ELF spec prevents existence of PF_W|PF_X segment with relocs.
IMO the commits that added the icache sync were done due to some actual problems seen.

In D54214#1238647, @des wrote:

@kib can you also please take a look at the PR? the originator claims this used to work as recently as 14.2 but I see no evidence that it has ever worked, the code to zero out the fsid has been there since 1997.

Indeed, p_klist initialization should not be moved into constructor, since it is cleared in exit1() (and should not be done in destructor, I believe). Then it would leak in each case where exit1() is not called, i.e. fork failures.
For mac, I have no idea what modules expect, so IMO it is better to keep them as is.

So lets split this review, hopefully to revive it.

In D54215#1238587, @kib wrote:

IMO it would be more proper to move the initialization into proc_init().

IMO it would be more proper to move the initialization into proc_init().

I think users must have ability to get back to the previous behavior. Might be add a sysctl to tweak it.

It is really eventfd data, so might be slightly better to call it eventfd_data. I do not object to proposal to rename the FreeBSD own structure,

Switch to macros.

This cannot work, alloca result is only valid for the duration of the current frame.
The functions can be moved to string.h as __always_inline, but I suspect that this would cause compat issues with weird compilers.

strndupa is from glibc as well

Thanks.

In D54033#1234654, @markj wrote:

In D54033#1234624, @kib wrote:

Indeed, the explicitly stated motivation, at least in the commit message, would be quite useful.

BTW, should LD_BIND_NOW be marked as unsecure, it potentially changes the bindings of the image?

I'm not sure, what happens if we perform lazy binding and the application dlopens a DSO which provides a duplicate symbol? How else can LD_BIND_NOW change the bindings of the image?

Indeed, the explicitly stated motivation, at least in the commit message, would be quite useful.