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