<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Jan 26, 2024 at 4:10 PM enh <<a href="mailto:enh@google.com">enh@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Jan 25, 2024 at 10:00 PM Rob Landley <<a href="mailto:rob@landley.net" target="_blank">rob@landley.net</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 1/25/24 17:46, enh wrote:<br>
> On Wed, Jan 24, 2024 at 11:56 PM Rob Landley wrote:<br>
> Should the mlock() be optional? (Maybe -m? Or for backwards compatibility keep<br>
> it the default and -M to _not_ lock? Dirtying isn't optional because without<br>
> that the malloc just updates a couple pointers and maybe extends a mapping.)<br>
> <br>
> dunno... the specific Android copy i set out to remove doesn't have the mlock(),<br>
> but spent most of its initial checkin comment effectively saying "this tool<br>
> doesn't do quite what you want because it doesn't mlock()", so it does seem like<br>
> it should be there and -- because i'm told this is mainly used by non-engineers<br>
> under instruction -- probably on by default. (that's why the original daemonizes<br>
> itself, which is why i was looking at xvdaemon() yesterday, but it offended my<br>
> sensibilities not to just say `adb shell nohup memeater` until/unless that's<br>
> proven to be too error-prone or whatever.)<br>
<br>
You can & in the shell pretty easily, and then it's accessible to job control.<br></blockquote><div><br></div><div>(that gets more interesting when you're usually on the other end of `adb shell`.)</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> Since this is just dirtying pages do you object to += 4096 in the loop? Or do<br>
> you want the ascending numbers over the memory? Or maybe a -f option for fast to<br>
> do that and once again leave the default?<br>
> <br>
> no, when i asked the heap guys if they had any comments, the only thing they<br>
> said was "why are you wasting time touching more than one byte per page?",<br>
<br>
I had it do register sized writes because _smaller_ than that is sometimes more<br>
expensive. :)<br></blockquote><div><br></div><div>(i remembered the other bikeshed that meant i went with every byte --- "are you going to check the _actual_ page size, or just assume one?".)</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> and<br>
> my only answer was "because the toybox guy will probably prefer the shorter<br>
> code, which is this".<br>
<br>
It's a tradeoff, as most things are. :)<br>
<br>
The kernel's already zeroing all the pages before handing them to userspace for<br>
security reasons, writing over that is just eating cycles for no obvious reason.<br>
<br>
> like i said in the checkin comment, until/unless proven<br>
> insufficient, i don't think there's any reason but cargo cult for the current<br>
> behavior.<br>
<br>
I took out -f and left -M switching off the default mlock(), and promoted it.<br>
Commit ebcd678451fe and lemme know if I broke anything.<br></blockquote><div><br></div><div>i have quite a backlog of broken stuff from this week, so next week seems more likely, but i'll let you know when i've tried this on a device...</div></div></div></blockquote><div><br></div><div>"works for me!"</div><div><br></div><div>(obviously i'm not the actual user, but the mlock() is instantaneous and successful for me, so i'm optimistic. i really need to get a Go device for other reasons anyway, so i'll try it there too when i do.)</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> Sigh, almost all toybox command line numbers understand unit suffixes. I wince<br>
> documenting it here the same as repeatedly explaining "-" in file arguments,<br>
> although you know your audience better than I do...<br>
> <br>
> heh, that was actually my compromise between _my_ preference for nothing (even<br>
> for this tool, i assume they'll be told `adb shell memeater 512m` or whatever,<br>
> not left to their own devices) and the toy i copied from [truncate.c] actually<br>
> listing out a subset of the prefixes and what numbers they correspond to (!).<br>
<br>
Semi-historical: it has prefixes as well as suffixes so documented both because<br>
explaining "weird prefixes but the usual suffixes" was about as long.<br></blockquote><div><br></div><div>yeah, that makes it more defensible.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Also, commit 89701da1f predated commit cefc0a218 by about a year. :)<br>
<br>
> Anyway, quick stab at what cleaning it up might look like attached...<br>
> <br>
> lgtm, though i'd probably say "YAGNI" to the -f flag and just always have the<br>
> fast behavior? gkaiser (already cc:ed) will be along shortly to tell us if that<br>
> doesn't work for him for some reason :-)<br>
<br>
I yanked -f, but can put it back if they object. (You'd pretty much have to<br>
strace it to tell the difference though. The different value at the start of<br>
each page prevents VM page collation from garbage collecting them back together<br>
behind your back with the "balloon" driver or whatever else strange tricks Mel<br>
Gorman and friends come up with since I last paid attention...)<br>
<br>
I mean yeah, if the kernel zeroes it and then we overwrite that while it's still<br>
hot in L1 cache using 64 bit writes (512 writes per 4k page) it's presumably not<br>
THAT expensive, but still. (And more likely it's L2 with the system call<br>
happening on a different stack...)<br>
<br>
> toolbox is probably best left as-is. we moved getprop/setprop back into toolbox<br>
> (so they could reuse some OS libraries that aren't exposed in the NDK), and<br>
> start/stop don't make any sense outside Android. that only really leaves<br>
> getevent which -- as long as we have kernel headers and you don't -- is slightly<br>
> better off in toolbox because we can just auto-generate the list from<br>
> <linux/input.h> based on the current kernel headers, without ever having to<br>
> remember to manually update.<br>
<br>
Toybox doesn't have kernel headers? Or do you mean doesn't have _current_ kernel<br>
headers, but the older ones from the toolchain...<br></blockquote><div><br></div><div>yeah --- toolbox knows it's in AOSP, so it knows there are current kernel headers in external/kernel-headers/ and can run a python script against them to pull out all the input constants. the alternative probably looks like the constant busywork that strace has, updating those xlat files all the time for every kernel release.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> Rob<br>
<br>
Rob<br>
</blockquote></div></div>
</blockquote></div></div>