Changeset View
Standalone View
sys/kern/kern_descrip.c
Show First 20 Lines • Show All 138 Lines • ▼ Show 20 Lines | |||||
* linked list and freed only when the struct filedesc is released. | * linked list and freed only when the struct filedesc is released. | ||||
*/ | */ | ||||
#define NDFILE 20 | #define NDFILE 20 | ||||
#define NDSLOTSIZE sizeof(NDSLOTTYPE) | #define NDSLOTSIZE sizeof(NDSLOTTYPE) | ||||
#define NDENTRIES (NDSLOTSIZE * __CHAR_BIT) | #define NDENTRIES (NDSLOTSIZE * __CHAR_BIT) | ||||
#define NDSLOT(x) ((x) / NDENTRIES) | #define NDSLOT(x) ((x) / NDENTRIES) | ||||
#define NDBIT(x) ((NDSLOTTYPE)1 << ((x) % NDENTRIES)) | #define NDBIT(x) ((NDSLOTTYPE)1 << ((x) % NDENTRIES)) | ||||
#define NDSLOTS(x) (((x) + NDENTRIES - 1) / NDENTRIES) | #define NDSLOTS(x) (((x) + NDENTRIES - 1) / NDENTRIES) | ||||
#define NFD2MAPSIZE(x) (NDSLOTS(x) * NDSLOTSIZE) | |||||
/* | /* | ||||
* SLIST entry used to keep track of ofiles which must be reclaimed when | * SLIST entry used to keep track of ofiles which must be reclaimed when | ||||
* the process exits. | * the process exits. | ||||
*/ | */ | ||||
struct freetable { | struct freetable { | ||||
struct fdescenttbl *ft_table; | struct fdescenttbl *ft_table; | ||||
SLIST_ENTRY(freetable) ft_next; | SLIST_ENTRY(freetable) ft_next; | ||||
▲ Show 20 Lines • Show All 1,520 Lines • ▼ Show 20 Lines | memcpy(ntable->fdt_ofiles, otable->fdt_ofiles, | ||||
onfiles * sizeof(ntable->fdt_ofiles[0])); | onfiles * sizeof(ntable->fdt_ofiles[0])); | ||||
/* | /* | ||||
* Allocate a new map only if the old is not large enough. It will | * Allocate a new map only if the old is not large enough. It will | ||||
* grow at a slower rate than the table as it can map more | * grow at a slower rate than the table as it can map more | ||||
* entries than the table can hold. | * entries than the table can hold. | ||||
*/ | */ | ||||
if (NDSLOTS(nnfiles) > NDSLOTS(onfiles)) { | if (NDSLOTS(nnfiles) > NDSLOTS(onfiles)) { | ||||
nmap = malloc(NDSLOTS(nnfiles) * NDSLOTSIZE, M_FILEDESC, | nmap = malloc(NFD2MAPSIZE(nnfiles), M_FILEDESC, | ||||
M_ZERO | M_WAITOK); | M_ZERO | M_WAITOK); | ||||
/* copy over the old data and update the pointer */ | /* copy over the old data and update the pointer */ | ||||
memcpy(nmap, omap, NDSLOTS(onfiles) * sizeof(*omap)); | memcpy(nmap, omap, NDSLOTS(onfiles) * sizeof(*omap)); | ||||
fdp->fd_map = nmap; | fdp->fd_map = nmap; | ||||
} | } | ||||
/* | /* | ||||
* Make sure that ntable is correctly initialized before we replace | * Make sure that ntable is correctly initialized before we replace | ||||
▲ Show 20 Lines • Show All 1,615 Lines • ▼ Show 20 Lines | sysctl_kern_proc_nfds(SYSCTL_HANDLER_ARGS) | ||||
FILEDESC_SUNLOCK(fdp); | FILEDESC_SUNLOCK(fdp); | ||||
return (SYSCTL_OUT(req, &count, sizeof(count))); | return (SYSCTL_OUT(req, &count, sizeof(count))); | ||||
} | } | ||||
static SYSCTL_NODE(_kern_proc, KERN_PROC_NFDS, nfds, | static SYSCTL_NODE(_kern_proc, KERN_PROC_NFDS, nfds, | ||||
CTLFLAG_RD|CTLFLAG_CAPRD|CTLFLAG_MPSAFE, sysctl_kern_proc_nfds, | CTLFLAG_RD|CTLFLAG_CAPRD|CTLFLAG_MPSAFE, sysctl_kern_proc_nfds, | ||||
"Number of open file descriptors"); | "Number of open file descriptors"); | ||||
static int | |||||
sysctl_kern_proc_fdmap(SYSCTL_HANDLER_ARGS) | |||||
{ | |||||
struct filedesc *fdp; | |||||
NDSLOTTYPE lmap[NDSLOTS(NDFILE) * 16]; | |||||
NDSLOTTYPE *map; | |||||
size_t bsize, csize; | |||||
int error; | |||||
CTASSERT(sizeof(lmap) <= 128); | |||||
if (*(int *)arg1 != 0) | |||||
return (EINVAL); | |||||
fdp = curproc->p_fd; | |||||
if (req->oldptr == NULL) | |||||
return (SYSCTL_OUT(req, NULL, NFD2MAPSIZE(fdp->fd_nfiles))); | |||||
if (curproc->p_numthreads == 1 && fdp->fd_refcnt == 1) | |||||
/* No need to lock anything as only we can modify the map. */ | |||||
return (SYSCTL_OUT(req, fdp->fd_map, NFD2MAPSIZE(fdp->fd_nfiles))); | |||||
/* Potential other threads modifying the map. */ | |||||
map = lmap; | |||||
bsize = sizeof(lmap); | |||||
for (;;) { | |||||
FILEDESC_SLOCK(fdp); | |||||
csize = NFD2MAPSIZE(fdp->fd_nfiles); | |||||
if (bsize >= csize) | |||||
break; | |||||
FILEDESC_SUNLOCK(fdp); | |||||
if (map != lmap) | |||||
free(map, M_TEMP); | |||||
bsize = csize; | |||||
map = malloc(bsize, M_TEMP, M_WAITOK); | |||||
} | |||||
memcpy(map, fdp->fd_map, csize); | |||||
FILEDESC_SUNLOCK(fdp); | |||||
error = SYSCTL_OUT(req, map, csize); | |||||
if (map != lmap) | |||||
free(map, M_TEMP); | |||||
return (error); | |||||
} | |||||
static SYSCTL_NODE(_kern_proc, KERN_PROC_FDMAP, fdmap, | |||||
CTLFLAG_RD|CTLFLAG_CAPRD|CTLFLAG_MPSAFE, sysctl_kern_proc_fdmap, | |||||
"File descriptor map"); | |||||
/* | /* | ||||
* Get file structures globally. | * Get file structures globally. | ||||
*/ | */ | ||||
static int | static int | ||||
sysctl_kern_file(SYSCTL_HANDLER_ARGS) | sysctl_kern_file(SYSCTL_HANDLER_ARGS) | ||||
{ | { | ||||
struct xfile xf; | struct xfile xf; | ||||
▲ Show 20 Lines • Show All 502 Lines • ▼ Show 20 Lines | |||||
* Store a process current working directory information to sbuf. | * Store a process current working directory information to sbuf. | ||||
* | * | ||||
* Takes a locked proc as argument, and returns with the proc unlocked. | * Takes a locked proc as argument, and returns with the proc unlocked. | ||||
*/ | */ | ||||
int | int | ||||
kern_proc_cwd_out(struct proc *p, struct sbuf *sb, ssize_t maxlen) | kern_proc_cwd_out(struct proc *p, struct sbuf *sb, ssize_t maxlen) | ||||
{ | { | ||||
struct filedesc *fdp; | struct filedesc *fdp; | ||||
struct export_fd_buf *efbuf; | struct export_fd_buf *efbuf; | ||||
mjg: I don't think we should bother with arbitrary processes and even if we do, the common case of… | |||||
int error; | int error; | ||||
PROC_LOCK_ASSERT(p, MA_OWNED); | PROC_LOCK_ASSERT(p, MA_OWNED); | ||||
Not Done Inline ActionsThis + fdhold + fddrop business is avoidable for curproc. It should be special cased. mjg: This + fdhold + fddrop business is avoidable for curproc. It should be special cased. | |||||
fdp = fdhold(p); | fdp = fdhold(p); | ||||
PROC_UNLOCK(p); | PROC_UNLOCK(p); | ||||
if (fdp == NULL) | if (fdp == NULL) | ||||
return (EINVAL); | return (EINVAL); | ||||
Done Inline ActionsExtra {}. kib: Extra {}. | |||||
efbuf = malloc(sizeof(*efbuf), M_TEMP, M_WAITOK); | efbuf = malloc(sizeof(*efbuf), M_TEMP, M_WAITOK); | ||||
Not Done Inline ActionsThis is legal but not recommended - should it block due to a page fault we can go off while holding the lock and blocking others for an indeterminate amount of time. Preferably the code would do a copy with disallowing page faults and fallback to something else if necessary. I don't know if there is a handy way to do it with sysctl. It is possible to wire the target buffer but I don't think it's worth it. So I think this would be good enough for now with making a local copy before doing SYSCTL_OUT. You can have a small on stack buffer to cover the common case and malloc in the worst case. mjg: This is legal but not recommended - should it block due to a page fault we can go off while… | |||||
Not Done Inline ActionsAlso note if only the current process is of interest, the above concern goes out of the window. bitmaps are only freed when the entire struct is getting freed. Then you can simply lock the struct, read off the pointer and the size, unlock the struct and SYSCTL_OUT. mjg: Also note if only the current process is of interest, the above concern goes out of the window. | |||||
Not Done Inline Actionsroughly this (untested): static int sysctl_kern_proc_fdmap(SYSCTL_HANDLER_ARGS) { struct filedesc *fdp; NDSLOTTYPE *map; size_t size; if (*(int *)arg1 != 0) return (EINVAL); fdp = curproc->p_fd; FILEDESC_SLOCK(fdp); /* * We can safely access the map after dropping the lock because it can * only get freed when the struct filedesc is getting freed. * See fdgrowtable. */ size = NDSLOTS(fdp->fd_nfiles) * NDSLOTSIZE; map = fdp->fd_map; FILEDESC_SUNLOCK(fdp); return (SYSCTL_OUT(req, map, size)); } static SYSCTL_NODE(_kern_proc, KERN_PROC_FDMAP, fdmap, CTLFLAG_RD|CTLFLAG_CAPRD|CTLFLAG_MPSAFE, sysctl_kern_proc_fdmap, "File descriptor map"); mjg: roughly this (untested):
```
static int
sysctl_kern_proc_fdmap(SYSCTL_HANDLER_ARGS)
{… | |||||
Not Done Inline Actionsapologies for spam, scratch that - only fd tables survive, old maps do get freed so a copying it will be necessary. I stand by not doing copyout with the lock held. mjg: apologies for spam, scratch that - only fd tables survive, old maps do get freed so a copying… | |||||
efbuf->fdp = fdp; | efbuf->fdp = fdp; | ||||
efbuf->sb = sb; | efbuf->sb = sb; | ||||
efbuf->remainder = maxlen; | efbuf->remainder = maxlen; | ||||
FILEDESC_SLOCK(fdp); | FILEDESC_SLOCK(fdp); | ||||
if (fdp->fd_cdir == NULL) | if (fdp->fd_cdir == NULL) | ||||
error = EINVAL; | error = EINVAL; | ||||
else { | else { | ||||
Not Done Inline ActionsI remember mjg already pointed out that malloc under FILEDESC_SLOCK is undesirable. kib: I remember mjg already pointed out that malloc under FILEDESC_SLOCK is undesirable. | |||||
vrefact(fdp->fd_cdir); | vrefact(fdp->fd_cdir); | ||||
error = export_vnode_to_sb(fdp->fd_cdir, KF_FD_TYPE_CWD, | error = export_vnode_to_sb(fdp->fd_cdir, KF_FD_TYPE_CWD, | ||||
FREAD, efbuf); | FREAD, efbuf); | ||||
} | } | ||||
Done Inline ActionsDon't you need to free map ? kib: Don't you need to free map ? | |||||
Done Inline ActionsD'oh, yes. jhibbits: D'oh, yes. | |||||
FILEDESC_SUNLOCK(fdp); | FILEDESC_SUNLOCK(fdp); | ||||
fddrop(fdp); | fddrop(fdp); | ||||
Not Done Inline ActionsYou can read off the size without taking the lock, do malloc and take the lock. Then verify the size fits (it will almost always will), if it does not free and retry. The table will very rarely change the size and it will always grow. I would use a small on stack buffer by default, sized just like the initial map to avoid malloc in the common case. I'm also increasingly leaning towards extending sysctl api to add SYSCTL_OUT_NOFAULT or so to fast path and avoid all this crap in the common case. General support is already there (based on pre-wired mappings), the code just needs cosmetic changes + api extension. It should come in handy in other code. I'll probably put a review later. With this in place you can avoid the double copy. This can be taken care of at a later time if fdwalk is expected to land for 12.1 mjg: You can read off the size without taking the lock, do malloc and take the lock. Then verify the… | |||||
free(efbuf, M_TEMP); | free(efbuf, M_TEMP); | ||||
return (error); | return (error); | ||||
Done Inline Actionsspace around '|' binary op. kib: space around '|' binary op. | |||||
Done Inline ActionsThis follows style of the rest of the file (copy&paste of another node). All other SYSCTL_NODE()s have this style violation. I will, however, fix this one. jhibbits: This follows style of the rest of the file (copy&paste of another node). All other SYSCTL_NODE… | |||||
} | } | ||||
/* | /* | ||||
* Get per-process current working directory. | * Get per-process current working directory. | ||||
*/ | */ | ||||
static int | static int | ||||
sysctl_kern_proc_cwd(SYSCTL_HANDLER_ARGS) | sysctl_kern_proc_cwd(SYSCTL_HANDLER_ARGS) | ||||
{ | { | ||||
Not Done Inline ActionsThis needs CTLFLAG_CAPRD so that it works in capability mode. mjg: This needs CTLFLAG_CAPRD so that it works in capability mode. | |||||
struct sbuf sb; | struct sbuf sb; | ||||
struct proc *p; | struct proc *p; | ||||
ssize_t maxlen; | ssize_t maxlen; | ||||
int error, error2, *name; | int error, error2, *name; | ||||
name = (int *)arg1; | name = (int *)arg1; | ||||
sbuf_new_for_sysctl(&sb, NULL, sizeof(struct kinfo_file), req); | sbuf_new_for_sysctl(&sb, NULL, sizeof(struct kinfo_file), req); | ||||
▲ Show 20 Lines • Show All 385 Lines • Show Last 20 Lines |
I don't think we should bother with arbitrary processes and even if we do, the common case of inspecting the current one can skip some of the work including locking the process and holding the table.