Date: Mon, 7 Jun 2004 02:31:10 +0200 From: "Cyrille Lefevre" <cyrille.lefevre@laposte.net> To: "Garance A Drosihn" <drosih@rpi.edu>, <arch@freebsd.org> Cc: "gnats-submit @FreeBSD.org" <FreeBSD-gnats-submit@freebsd.org> Subject: Re: Re: ps enhencements (posix syntax, and more) Message-ID: <05a201c44c82$d94fc680$7890a8c0@dyndns.org> References: <p0602042abce687fa7dd9@[128.113.24.47]>
next in thread | previous in thread | raw e-mail | index | archive | help
Garance A Drosihn <drosih@rpi.edu> wrote: >At 12:03 PM +0200 4/27/04, Cyrille Lefevre wrote: >>here is a description of the last PR#64803 updates : > >The latest info I see in PR #65803 does not match some >things that you describe in the rest of this message. >The following comments are based on what I have looked >at in the updates in the PR. I have not had much sleep, >so this message may be a little confusing in parts. > >>*** the kernel part has been reworked and validated in >> the last patch set. ...OpenBSD -k ... > >I haven't looked at what you have for -k, but I did try >what you had for KERN_PROC_SESSION and it didn't seem to >work. using the all in one kernel patch I've just submited, all would work as expected. it seems I forgot some important part to what I've submitted, sorry. >> implement KERN_PROC_SESSION in /sys/kern/kern_proc.c >> as well as system processes filtering in KERN_PROC_PROC >> (not sure about that). both features have not been >> tested yet, but will be soon. > >I am skipping the change to KERN_PROC_PROC for now. I >suspect we will want that for at least some situations, >but I haven't decided what situations yet. as I'm writting the email, all the submitted stuffs have been tested and works well. >>bug fix : -L is missing a break. > >It does not need one in the present FreeBSD source, but I >did not check to see if there is some reason it needs one >in your updated source. this was related to my ps patches. I'll submit a new all in one patch related to the the -current ps, so, you could try again the new stuffs w/o conflicts and w/o missing pieces, is any :( >>*** this is the last patch set before dynamic sizing... > >I expect that I am missing this 'the last patch set' >which you refer to here. the emul keyword :) >>kernel level added features : >>KERN_PROC_SESSION, KERN_PROC_GID and KERN_PROC_RGID have >>been implemented at kernel level as well as > >I also wanted KERN_PROC_RGID, so I had added the similar >clause to the case statement. Your KERN_PROC_SESSION and >this KERN_PROC_RGID which I added did not seem to work, so >I looked around a bit and decided I needed to add four >SYSCTL_NODE() entries. With those added, the new features >seem to work fine. see my above comment. >>KERN_PROC_TTY_NODEV and KERN_PROC_TTY_REVOKED (the last >>two are based on what NetBSD does but differently since >>we have to lock p_session). > >I obviously did not add these, or KERN_PROC_GID, since the >update I have does not include them. They sound like >reasonable things to add. there are in the all in one kernel patch. >>KERN_PROC_PROC now handles KERN_PROC_INC_THREADS and, in >>this case, is similar to KERN_PROC_ALL. > >I have been awake many hours due to work, so I may not be >thinking clearly right now. But the above does not sound >right to me. Wasn't the whole point of KERN_PROC_PROC to >*not* include threads? > >Also, there are other commands which call the same routine, >so I don't think we can just blindly rework everything. We >are now working towards a "5.x-stable" release, so I do not >want to make incompatible changes in any API's unless there >is some pretty major benefit. If all we are doing is getting >rid of an unnecessary variable, I do not believe that will be >worth the trouble at this point. top ignore system processes by default except if you uses -S. unfortunatelly, w/ this implementation of KERN_PROC_PROC, -H will be needed to see kernel threads. well, a better way would have to add a new macro like KERN_PROC_INC_KTHREADS to get or not system processes (aka kernel threads) appart than user threads ? >>P_SYSTEM processes aren't selected by default (as OpenBSD). > >I think this is a repeat of what you said earlier in this >message. I will want to think about that. no, this isn't a repeat. this is the way system processes are ignored at user level before to be implemented using KERN_PROC_PROC w/o KERN_PROC_INC_THREADS at kernel level. >>PS : take care about the struct kinfo alignment. as >> currently implemented, ki_spare has been validated >> under i386 and ia64 (and should work under ...) > >I have some changes to improve the checking of struct-kinfo >size & alignment, but they aren't finished. the ones from NetBSD/OpenBSD ? :) >>-G may now uses KERN_PROC_RGID if WITH_RGID is defined. >>-s may now uses KERN_PROC_SESSION if WITH_SESSION is >> defined. > >Note that for the base-system version of `ps', we do not >have to clutter things up with a lot of #if's. Either >the matching system supports a feature, or it doesn't. The I've added these #if's to help you to distinguish changes between them and to have the ability to test the base patch w/ or w/o kernel support :P >few '#if's that I added in my update were just placemarkers, >for something that I intend to pick up on with my later >updates. > >>some int variables have been changed to size_t >> (list->count, prtheader, i, lineno, nkept, nselectors). > >What was the advantage in that? For some of those, it is >pretty much impossible that the value will get over MAX_INT. it's just to match the system calls expected arguments or return values. using the right types avoid many bugs. >'nselectors' is the number of selectors that the user has >typed in as parameters to the `ps' command. If they type >in more than 32767 different selectors, they will run into >problems with command-line length long before they overflow >some of these counters. > >>-j now have sid by default. > >Hmm. Yeah, I *think* there was a PR about that. One of >those format-groups, at least. > >>PS : Garance, if you want a fresh patch set, let me know. > >I probably should, but I fear that before I have the time to >read & understand & test & install those updates, you will >just have rewritten them all over again. This is a bit I didn't understant why you want to reinvent the weel ? >frustrating, as I have much more modest goals for `ps' than yes, I'm frustrated to have worked hours and days w/o knowing if it's not for nothing... >you seem to. I like a lot of what you have done, but I am >not likely to pick it all up, and it is bound to irritate >you if you rewrite something six or seven times and then I >only install pieces of it. And I still have my own updates >which I need to merge in, as well going through all the work >you have been doing. could you submit your pending updates somewhere ? >>the current size of ps is (optimized and stripped and >>w/o extra aliases) : >>-current : 30788 >>this one : 38776 > >I am certainly hoping to keep `ps' smaller than that... When >I started out on my earlier big update, I was able to keep >the size down to 28000 for most the time I worked on it. But >in the last few days I worked on it, the size ballooned up >by about 15% without me noticing it. That still annoys me! I >would actually like to get the size back down, if I could. >I suspect that I will not be able to, but I will not want to >add *another* 24% to the size. whith todays so big disks, I didn't really understand why such limitations ? for instance, IMHO, the problem is more the kernel size which grow by more than 100% between 4.x and 5.x ! CC -arch, -gnats Cyrille Lefevre. -- home: mailto:cyrille.lefevre@laposte.net
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?05a201c44c82$d94fc680$7890a8c0>