Date: Thu, 20 Jul 2006 19:07:45 GMT From: John Baldwin <jhb@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 102018 for review Message-ID: <200607201907.k6KJ7jca038509@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=102018 Change 102018 by jhb@jhb_mutex on 2006/07/20 19:07:21 Don't allocate a stream structure for a socket on demand but instead just do it in streamsopen(). This avoids changing f_ops during the life of a struct file which is a Bad Thing(tm). I think it is also unnecessary since all sockets for which stream operations make sense should already be created via streamsopen() anyway. Affected files ... .. //depot/projects/smpng/sys/dev/streams/streams.c#30 edit .. //depot/projects/smpng/sys/notes#86 edit Differences ... ==== //depot/projects/smpng/sys/dev/streams/streams.c#30 (text+ko) ==== @@ -188,6 +188,7 @@ int fd, extraref; struct file *fp; struct socket *so; + struct svr4_strm *st; int error; int family; struct proc *p = td->td_proc; @@ -268,7 +269,19 @@ fp->f_type = DTYPE_SOCKET; FILEDESC_UNLOCK_FAST(p->p_fd); - (void)svr4_stream_get(fp); + /* + * Allocate a stream structure and attach it to this socket. + * We don't bother locking so_emuldata for SVR4 stream sockets as + * its value is constant for the lifetime of the stream once it + * is initialized here. + */ + st = malloc(sizeof(struct svr4_strm), M_TEMP, M_WAITOK); + st->s_family = so->so_proto->pr_domain->dom_family; + st->s_cmd = ~0; + st->s_afd = -1; + st->s_eventmask = 0; + so->so_emuldata = st; + fdrop(fp, td); td->td_dupfd = fd; return ENXIO; @@ -331,41 +344,12 @@ struct file *fp; { struct socket *so; - struct svr4_strm *st; if (fp == NULL || fp->f_type != DTYPE_SOCKET) return NULL; so = fp->f_data; - - /* - * mpfixme: lock socketbuffer here - */ - if (so->so_emuldata) { - return so->so_emuldata; - } - - /* Allocate a new one. */ - st = malloc(sizeof(struct svr4_strm), M_TEMP, M_WAITOK); - st->s_family = so->so_proto->pr_domain->dom_family; - st->s_cmd = ~0; - st->s_afd = -1; - st->s_eventmask = 0; - /* - * avoid a race where we loose due to concurrancy issues - * of two threads trying to allocate the so_emuldata. - */ - if (so->so_emuldata) { - /* lost the race, use the existing emuldata */ - FREE(st, M_TEMP); - st = so->so_emuldata; - } else { - /* we won, or there was no race, use our copy */ - so->so_emuldata = st; - fp->f_ops = &svr4_netops; - } - - return st; + return so->so_emuldata; } static int ==== //depot/projects/smpng/sys/notes#86 (text+ko) ==== @@ -85,7 +85,7 @@ + compat32 - svr4 - svr4_stream_get() and friends - - need to see where this is called and see if all of the files + + need to see where this is called and see if all of the files should already have f_ops set correctly and if we can just allocate so_emuldata directly in streamsopen() + XXX: svr4_add_socket() can add duplicates? it's ok, just
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200607201907.k6KJ7jca038509>