Page MenuHomeFreeBSD

Show shm_open() and shmget() path/key in procstat -v.
Needs ReviewPublic

Authored by tmunro on May 23 2019, 10:23 PM.

Details

Reviewers
kib
markj
jhb
Summary
Add a new callback facility to build the path shown in procstat -v.
Teach SysV shm to show its key, and POSIX shm to show its path.
Test Plan
# sudo posixshmcontrol ls
MODE    	OWNER	GROUP	SIZE	PATH
rw-------	postgres	postgres	6928	/PostgreSQL.314942518
rwx------	tmunro	tmunro	67108904	/tmp/pulse-shm-1517061554
rwx------	tmunro	tmunro	67108904	/tmp/pulse-shm-2640157626
rwx------	gdm	gdm	67108904	/tmp/pulse-shm-4143995000
rwx------	gdm	gdm	67108904	/tmp/pulse-shm-4056659034
rwx------	tmunro	tmunro	67108904	/tmp/pulse-shm-2104754199
# procstat -v 1494 | grep PostgreSQL
 1494        0x844eeb000        0x844eed000 rw-    1    1   7   0 ----- df /PostgreSQL.314942518

Diff Detail

Repository
rS FreeBSD src repository
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

tmunro created this revision.May 23 2019, 10:23 PM

An obvious example of another user would be SysV shm. I suppose it should show the key (in hex format?) as the "path". Then of course you would use jail_adjust = false.

kib added a comment.May 24 2019, 10:14 AM

I dislike the idea of allocating and keeping the string around as long as the object is alive. The string is almost never used, also it is always a duplicate of some other string that kernel already has allocated.

I could suggest you to make some helper function which would centralize the knowledge of possible meaningful paths for given vm object. For instance, I can list the following cases:

  • vnode object, use vn_fullpath() to get the string
  • swap object backing tmpfs node, same vn_fullpath() (this case should be already specially handled somewhere)
  • shmfd, return the name

For shmfd you might need to store the back pointer from object to shmfd, perhaps reuse swp_tmpfs, this field is only used for back-linking tmpfs node. You might also need to change the type of the shmfd backing object to be swap from the beginning.

tmunro edited the summary of this revision. (Show Details)EditedMay 25 2019, 12:20 AM
tmunro updated this revision to Diff 57875.

Got rid of allocation/copying.
Show POSIX segments as "(unlinked)" if their name has gone.
Show SysV segments with their key_t (same way it is displayed in ipcs output).

Implemented using a callback. Vnodes are still a special case though, because they have different locking requirements (don't want to do that stuff while holding the vmobject lock).

Better?

tmunro retitled this revision from Show shm_open() paths in procstat -v. to Show shm_open() and shmget() path/key in procstat -v..May 25 2019, 12:31 AM
tmunro edited the summary of this revision. (Show Details)
kib added a comment.May 25 2019, 11:19 AM

Implemented using a callback. Vnodes are still a special case though, because they have different locking requirements (don't want to do that stuff while holding the vmobject lock).
Better?

In fact, not. Look at the struct vm_object change. You are adding 16 bytes to it. vm_object_t is allocated for each once-opened vnode in the system, and e.g. on my low-end 16G router I have 1M vnodes. So the change, if applied, takes 16M away for almost never used memory.

Please re-read what I suggested in previous reply.

tmunro updated this revision to Diff 58333.EditedJun 6 2019, 10:25 PM

Please re-read what I suggested in previous reply.

Point taken. Here's a WIP patch along those lines.

I didn't want this feature to be restricted to OBJT_SWAP objects though, because I also want it to work when kern.ipc.shm_use_phys=1. So I tried aliasing it with a new union member, which can be used even when there is no pager, for OBJT_DEFAULT and OBJT_PHYS. It is also used for OBJT_SWAP but only if the OBJ_TMPFS flag isn't set.

I still need a way to know if it's a SysV or POSIX segment, so I stole a flag bit that should never be used when OBJ_TEMPFS is set, IIUC. That is not beautiful... I think I need to find a better way to record which type of path_info it is.

kib added inline comments.Jun 7 2019, 10:48 AM
sys/vm/vm_object.h
179

This is very fragile. It depends on the layout details for swap object and breaks if any rearrangement is done there.

If anything, I would expect you to provide explicit members for each type you handle, and perhaps wrapping swp_tmpfs into union with your new pointer.

That said, I am now looking at this from the higher point of view, and wonder do we need to store something in the object at all. How many SysV or POSIX shm objects are alive in the typical system ? What if we calculate the names using brute-force search for the corresponding shm segment ? The only thing you need to store in the object is the SysV/POSIX shm indicator.