At boot time, OpenRC checks whether it needs to mount /proc by doing this (in sh/init.sh.Linux.in): mountproc=true if [ -e /proc/uptime ]; then up="$(cat /proc/uptime)" sleep 1 if [ "$up" = "$(cat /proc/uptime)" ]; then eerror "You have cruft in /proc that should be deleted" else einfo "/proc is already mounted, skipping" mountproc=false fi fi It reads /proc/uptime twice to make sure that /proc contains a real proc filesystem and not just some old files. That's a good way of doing it, but it adds a delay of 1 second to every boot if /proc is already mounted. I thought of another way that uses /proc/self/environ instead of /proc/uptime: mountproc=true if [ -e /proc/self/environ ]; then if VAR=1 grep -qz "^VAR=1\$" /proc/self/environ && \ VAR=2 grep -qz "^VAR=2\$" /proc/self/environ; then einfo "/proc is already mounted, skipping" mountproc=false else eerror "You have cruft in /proc that should be deleted" fi fi
Chris: - Care to look at some other parts of openrc for more excess sleep? - What is there is a cruft file at that location that happens to have both of those settings? Use $RANDOM maybe instead? WilliamH: This concept looks good to me. Opinions?
(In reply to comment #1) > Care to look at some other parts of openrc for more excess sleep? I might take a look at the other sleeps at some point. > What if there is a cruft file at that location that happens to have both of > those settings? Use $RANDOM maybe instead? It seemed unlikely to me that there would be two variables in /proc/self/environ with the same name, but it can happen if `environ` is modified directly. Or if the file is corrupt. I did think of using $RANDOM. The probability of the string "VAR=$RANDOM" being in a stale /proc/self/environ seems low, but it could happen. How about launching a command with nothing in the environment except for the magic variable. Then check that the entire contents of /proc/self/environ is as expected. Or just the first few bytes would do: mountproc=true if [ -e /proc/self/environ ]; then if env - VAR=1 head -c 5 /proc/self/environ | grep -q VAR=1 && \ env - VAR=2 head -c 5 /proc/self/environ | grep -q VAR=2; then einfo "/proc is already mounted, skipping" mountproc=false else eerror "You have cruft in /proc that should be deleted" fi fi
the dual grep doesnt gain much i dont think as it isnt like you unset one var and make sure it isnt then in the environ. this test is supposed to be a sanity check rather a sure fire fix, so a single grep should be sufficient. _OPENRC_ENV_="a magic string" grep -q '_OPENRC_ENV_=a magic string' /proc/self/environ
Possibly the dual grep approach is overkill. But, like the /proc/uptime hack, it does verify that /proc is definitely alive. Checking a single magic value would only verify that /proc is probably alive. Then again, someone would have to be amazingly good at buggering up their system to get the magic string, whatever it may be, into their duff /proc.
the code is pretty much always going to be a trade off. a single grep is sufficient imo to cover all likely cases. if you've got a crazy setup which this violates, then fix your setup.
chris: my concern is somebody that does rsync -avP /proc /fakeproc and then has /fakeproc mounted at /proc. Here's a variable I came up with: text='VAR=1' ; [ "$(env - $text cat /proc/self/environ)" == "$text" ] ; rc1=$? text='VAR=2' ; [ "$(env - $text cat /proc/self/environ)" == "$text" ] ; rc2=$? if [ $rc1 -eq 0 -a $rc2 -eq 0 ]; then # /proc is alive fi unset text rc1 rc2
(In reply to comment #6) > chris: my concern is somebody that does rsync -avP /proc /fakeproc and then has > /fakeproc mounted at /proc. I do understand that. Although I was imagining someone tarring up their entire system as a backup, including /proc, and later restoring it, including /proc. Mike's idea of a magic string would work. With the rsync command you mentioned, rsync's environment would end up in /proc/self/environ. The odds are slim that it would contain "_OPENRC_ENV_=FJv2jFiLyddmPDJhDuP6waxy". But, personally, I do prefer the two step approach. It's the only way to be absolutely sure that the contents of /proc are not static. > > Here's a variable I came up with: > text='VAR=1' ; [ "$(env - $text cat /proc/self/environ)" == "$text" ] ; rc1=$? > text='VAR=2' ; [ "$(env - $text cat /proc/self/environ)" == "$text" ] ; rc2=$? > if [ $rc1 -eq 0 -a $rc2 -eq 0 ]; then > # /proc is alive > fi > unset text rc1 rc2 > Yes, that'll do it. And no need to invoke grep. Good call.
`test` uses =, not ==. do it in a single if statement to avoid the variables.
(In reply to comment #8) > `test` uses =, not ==. do it in a single if statement to avoid the variables. SpanK'd!
Incidentally, is the use of [[ acceptable in openrc?
(In reply to comment #10) > Incidentally, is the use of [[ acceptable in openrc? As far as I know, it isn't, because that breaks openrc if the shell is not bash.
(In reply to comment #11) > (In reply to comment #10) > > Incidentally, is the use of [[ acceptable in openrc? > > As far as I know, it isn't, because that breaks openrc if the shell is not > bash. > Which other shells should be supported?
(In reply to comment #12) > (In reply to comment #11) > > (In reply to comment #10) > > > Incidentally, is the use of [[ acceptable in openrc? > > > > As far as I know, it isn't, because that breaks openrc if the shell is not > > bash. > > > > Which other shells should be supported? > any posix compliant shell will be supported. There will be no exceptions.
(In reply to comment #13) > any posix compliant shell will be supported. There will be no exceptions. > That's cool. Are you the lead developer?
(In reply to comment #14) > (In reply to comment #13) > > any posix compliant shell will be supported. There will be no exceptions. > > > That's cool. Are you the lead developer? I'm not Jory, but I think I can answer this. Jory, please correct me if I am wrong. We are a subproject of the base system project, and the lead of base system is Mike. As far as I know, openrc doesn't have a separate lead.
(In reply to comment #15) > (In reply to comment #14) > > (In reply to comment #13) > > > any posix compliant shell will be supported. There will be no exceptions. > > > > > That's cool. Are you the lead developer? > > I'm not Jory, but I think I can answer this. Jory, please correct me if I am > wrong. > > We are a subproject of the base system project, and the lead of base system is > Mike. As far as I know, openrc doesn't have a separate lead. > William this is correct, I let Mike run the lead as we did move openrc under baselayout, I just picked up the code and have had very little time to work on it at moment due to schooling. This will change in a few weeks and will be moving openrc in a much cleaner forward march.
we aim for POSIX shell-ish. we require "local" which isnt in POSIX.
I'm wondering if we can remove all forks entirely and still keep the same result. If we can rely on the minor fault counter always changing between entries to kernelspace and userspace, then I propose this check: ==== f=/proc/self/stat v=a ; exec 9<$f ; read $v <&9 ; exec 9>&- v=b ; exec 9<$f ; read $v <&9 ; exec 9>&- [ "$a" = "$b" ] ; echo $? ==== The minor fault counter increases by at least 2 between the runs. It's pure POSIX sh, has no execs, and is faster than the grep call.
you dont need the "v" variable there, and the stat file provides times with different units. so fault counts shouldnt be a blocking issue.
vapier: Err, I don't care about the actual counts. Just that the file changes. All this bug needs is ANY file in /proc that: - changes between sequential reads - is world-readable (not hardened kernel blocked) - ideally is on a single line (means the entire check is doable without any exec/fork). The v variable was just for ease of showing the code. If the minor faults number in /proc/$PID/stat is always going to change, then it fills the above requirements for detecting a live procfs. /proc/uptime doesn't work with the sleep statement dropped because it only changes every 10ms approximately (the resolution of times in it). Local timing: The original sleep code takes ~1010ms. The grep of environ takes 2.2-2.5ms (mostly fork overhead) My check of /proc/self/stat takes 1.1-1.4ms.
you missed the point of my comment. let's skip the crap and jump to the end: your proposal looks fine. feel free to convert to that.
Fix committed.
Thanks, Robin. Thanks, Mike. You improved my already good idea.
*** Bug 360017 has been marked as a duplicate of this bug. ***
(In reply to comment #18) > If we can rely on the minor fault counter always changing between entries to > kernelspace and userspace, then I propose this check: > ==== > f=/proc/self/stat > v=a ; exec 9<$f ; read $v <&9 ; exec 9>&- > v=b ; exec 9<$f ; read $v <&9 ; exec 9>&- > [ "$a" = "$b" ] ; echo $? > ==== > The minor fault counter increases by at least 2 between the runs. > It's pure POSIX sh, has no execs, and is faster than the grep call. I hate to poke this bug again, but the above method isn't working on my system. I was using my patch until openrc 0.8.0. Now I'm getting "You have cruft...". My processor is an Intel Core i7 920. It has 4 cores and hyperthreading. $ f=/proc/self/stat $ v=a ; exec 9<$f ; read $v <&9 ; exec 9>&-; v=b ; exec 9<$f ; read $v <&9 ; exec 9>&-; [ "$a" = "$b" ] && echo 'a=b' || echo 'a!=b' a=b sleep first: $ sleep 1; v=a ; exec 9<$f ; read $v <&9 ; exec 9>&-; v=b ; exec 9<$f ; read $v <&9 ; exec 9>&-; [ "$a" = "$b" ] && echo 'a=b' || echo 'a!=b' a!=b sleep between: $ v=a ; exec 9<$f ; read $v <&9 ; exec 9>&-; sleep 1; v=b ; exec 9<$f ; read $v <&9 ; exec 9>&-; [ "$a" = "$b" ] && echo 'a=b' || echo 'a!=b' a!=b What is the minor fault counter?
there is already an open bug on the topic
Re-opening per robbat2, We need to find a better solution for this since it caused bug #360027.
*** Bug 360027 has been marked as a duplicate of this bug. ***
(In reply to comment #25) > My processor is an Intel Core i7 920. It has 4 cores and hyperthreading. (snip testcase) Your testcase gives me different results: no sleep a!=b sleep first: a=b sleep between: a!=b > What is the minor fault counter? The fault counter basically counts when things get moved to and from memory. So we need to go back to my original requirements in comment 20. I would like this to be done WITHOUT any call to sleep.
Ok, I think the best we can do is go back to /proc/self/environ. ==== text="VAR=1" [ "$(env - $text cat /proc/self/environ)" == "$text" ] ; rc1=$? text="VAR=2" [ "$(env - $text cat /proc/self/environ)" == "$text" ] ; rc2=$? if [ $rc1 -eq 0 -a $rc2 -eq 0 ]; then echo '# /proc is alive' else echo '# /proc is dead' fi ==== Any final complaints about this? Yes, it takes 2-14ms, and not the 1ms of the stat variant, but that's still better than the 1 second we had before.
compare the two results to each other and you wont need the env step. [ "$(VAR=a cat /proc/self/environ)" = "$(VAR=b cat /proc/self/environ)" ] make sure you use = and not ==
(In reply to comment #31) > compare the two results to each other and you wont need the env step. > [ "$(VAR=a cat /proc/self/environ)" = "$(VAR=b cat /proc/self/environ)" ] > > make sure you use = and not == ====== [ "$(VAR=a cat /proc/self/environ)" != "$(VAR=b cat /proc/self/environ)" ] if [ $? -eq 0 ]; then echo '# /proc is alive' else echo '# /proc is dead' fi ====== 8ms, and I think that should work, other people want to test it please? We should also test for existence of /proc/self/environ before we read it.
(In reply to comment #32) > (In reply to comment #31) > > compare the two results to each other and you wont need the env step. > > [ "$(VAR=a cat /proc/self/environ)" = "$(VAR=b cat /proc/self/environ)" ] > > > > make sure you use = and not == > ====== > [ "$(VAR=a cat /proc/self/environ)" != "$(VAR=b cat /proc/self/environ)" ] > if [ $? -eq 0 ]; then > echo '# /proc is alive' > else > echo '# /proc is dead' > fi > ====== > > 8ms, and I think that should work, other people want to test it please? We > should also test for existence of /proc/self/environ before we read it.
Created attachment 267045 [details] checkproc.patch This patch is based on comment #31. What do the rest of you think?
(In reply to comment #34) > Created attachment 267045 [details] > checkproc.patch > > This patch is based on comment #31. > > What do the rest of you think? Thanks, William. I'll test it tomorrow. I'm too tired now. Sorry about my previous post. I clicked accidentally.
Comment on attachment 267045 [details] checkproc.patch >+ if [ "$(VAR=a cat $f)" = "$(VAR=b cat $f)" ]; then Why not just: if [ "$(VAR=a cat $f)" = "$(cat $f)" ]; then It may not have a big impact, but lets not do unnecessary memory operations, shall we?;)
Created attachment 267081 [details, diff] checkproc.patch This patch is modified according to the suggestion in comment #36. It appears to work for me; what do the rest of you think?
Comment on attachment 267081 [details, diff] checkproc.patch on the off chance that the file in question does have VAR=a, that version fails. this is the same reason people didnt want to use my magic string idea. just go with the previous patch you posted.
(In reply to comment #34) > Created attachment 267045 [details] > checkproc.patch > > This patch is based on comment #31. > > What do the rest of you think? This patch works for me.
This is now in git, commit 1d63e85. Thanks for the report.
*** Bug 360919 has been marked as a duplicate of this bug. ***
*** Bug 362635 has been marked as a duplicate of this bug. ***
