Date: Fri, 25 Mar 2011 16:56:14 +0200 From: Kostik Belousov <kostikbel@gmail.com> To: John Baldwin <jhb@freebsd.org> Cc: freebsd-fs@freebsd.org Subject: Re: O_CLOEXEC Message-ID: <20110325145614.GN78089@deviant.kiev.zoral.com.ua> In-Reply-To: <201103250936.56512.jhb@freebsd.org> References: <20110325005923.GI78089@deviant.kiev.zoral.com.ua> <201103250814.47903.jhb@freebsd.org> <20110325123422.GK78089@deviant.kiev.zoral.com.ua> <201103250936.56512.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--TBIaq1yoUA4huN1+
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On Fri, Mar 25, 2011 at 09:36:56AM -0400, John Baldwin wrote:
> On Friday, March 25, 2011 8:34:22 am Kostik Belousov wrote:
> > On Fri, Mar 25, 2011 at 08:14:47AM -0400, John Baldwin wrote:
> > > On Thursday, March 24, 2011 8:59:24 pm Kostik Belousov wrote:
> > > > Hi,
> > > > below is the implementation of O_CLOEXEC flag for open(2). I also
> > > > handle the fhopen(2), since the man page states that fhopen(2) takes
> > > > the same flags as open(2), and it is more logical to change code
> > > > then man page.
> > > >=20
> > > > It is somewhat curious that SUSv4 did not specified O_CLOEXEC behav=
iour
> > > > for posix_openpt(). I left it out, but it probably makes sense to
> > > > allow O_CLOEXEC there ?
> > > >=20
> > > > The falloc() KPI is left as is because the function is often used
> > > > in the kernel and probably in the third-party modules. fdallocf()
> > > > takes additional flag argument to set close-on-exec before any other
> > > > thread might see new file descriptor.
> > >=20
> > > Hmm, I don't actually expect falloc() to be used in 3rd party modules=
and=20
> > > would be fine with just adding a new flags parameter to it.
> >=20
> > The calls to falloc() appear in such modules as cryptodev(4).
> > I do not mind changing falloc interface, but I also intend to merge
> > O_CLOEXEC to stable/8. Are you fine with merging your suggestion to
> > stable branch, while falloc() is called from cryptodev, zlib,
> > linux (later is not a big issue if I bump __FreeBSD_version) ?
>=20
> Hmmm, there are a few ways, but perhaps the simplest is to commit the
> current approach (and MFC it), but to do a followup commit to HEAD to
> remove fallocf() and add the flags argument to falloc(). That changes
> the KPI for 9+, but avoids growing the future KPI.
I will commit the change below after r219999 is merged to 8.
diff --git a/sys/dev/streams/streams.c b/sys/dev/streams/streams.c
index 7b99d20..00f65a3 100644
--- a/sys/dev/streams/streams.c
+++ b/sys/dev/streams/streams.c
@@ -241,7 +241,7 @@ streamsopen(struct cdev *dev, int oflags, int devtype, =
struct thread *td)
}
=20
fdp =3D td->td_proc->p_fd;
- if ((error =3D falloc(td, &fp, &fd)) !=3D 0)
+ if ((error =3D falloc(td, &fp, &fd, 0)) !=3D 0)
return error;
/* An extra reference on `fp' has been held for us by falloc(). */
=20
diff --git a/sys/kern/kern_descrip.c b/sys/kern/kern_descrip.c
index 21590d3..b69460a 100644
--- a/sys/kern/kern_descrip.c
+++ b/sys/kern/kern_descrip.c
@@ -1516,7 +1516,7 @@ fdavail(struct thread *td, int n)
* release the FILEDESC lock.
*/
int
-fallocf(struct thread *td, struct file **resultfp, int *resultfd, int flag=
s)
+falloc(struct thread *td, struct file **resultfp, int *resultfd, int flags)
{
struct proc *p =3D td->td_proc;
struct file *fp;
@@ -1569,13 +1569,6 @@ fallocf(struct thread *td, struct file **resultfp, i=
nt *resultfd, int flags)
return (0);
}
=20
-int
-falloc(struct thread *td, struct file **resultfp, int *resultfd)
-{
-
- return (fallocf(td, resultfp, resultfd, 0));
-}
-
/*
* Build a new filedesc structure from another.
* Copy the current, root, and jail root vnode references.
diff --git a/sys/kern/kern_event.c b/sys/kern/kern_event.c
index 5df669d..e14ae02 100644
--- a/sys/kern/kern_event.c
+++ b/sys/kern/kern_event.c
@@ -684,7 +684,7 @@ kqueue(struct thread *td, struct kqueue_args *uap)
int fd, error;
=20
fdp =3D td->td_proc->p_fd;
- error =3D falloc(td, &fp, &fd);
+ error =3D falloc(td, &fp, &fd, 0);
if (error)
goto done2;
=20
diff --git a/sys/kern/sys_pipe.c b/sys/kern/sys_pipe.c
index d3929b4..50b0389 100644
--- a/sys/kern/sys_pipe.c
+++ b/sys/kern/sys_pipe.c
@@ -348,7 +348,7 @@ kern_pipe(struct thread *td, int fildes[2])
rpipe->pipe_state |=3D PIPE_DIRECTOK;
wpipe->pipe_state |=3D PIPE_DIRECTOK;
=20
- error =3D falloc(td, &rf, &fd);
+ error =3D falloc(td, &rf, &fd, 0);
if (error) {
pipeclose(rpipe);
pipeclose(wpipe);
@@ -364,7 +364,7 @@ kern_pipe(struct thread *td, int fildes[2])
* side while we are blocked trying to allocate the write side.
*/
finit(rf, FREAD | FWRITE, DTYPE_PIPE, rpipe, &pipeops);
- error =3D falloc(td, &wf, &fd);
+ error =3D falloc(td, &wf, &fd, 0);
if (error) {
fdclose(fdp, rf, fildes[0], td);
fdrop(rf, td);
diff --git a/sys/kern/tty_pts.c b/sys/kern/tty_pts.c
index afbef1f..b749f3f 100644
--- a/sys/kern/tty_pts.c
+++ b/sys/kern/tty_pts.c
@@ -805,7 +805,7 @@ posix_openpt(struct thread *td, struct posix_openpt_arg=
s *uap)
if (uap->flags & ~(O_RDWR|O_NOCTTY))
return (EINVAL);
=09
- error =3D falloc(td, &fp, &fd);
+ error =3D falloc(td, &fp, &fd, 0);
if (error)
return (error);
=20
diff --git a/sys/kern/uipc_mqueue.c b/sys/kern/uipc_mqueue.c
index f757cc5..9b334ac 100644
--- a/sys/kern/uipc_mqueue.c
+++ b/sys/kern/uipc_mqueue.c
@@ -1974,7 +1974,7 @@ kern_kmq_open(struct thread *td, const char *upath, i=
nt flags, mode_t mode,
if (len < 2 || path[0] !=3D '/' || index(path + 1, '/') !=3D NULL)
return (EINVAL);
=20
- error =3D falloc(td, &fp, &fd);
+ error =3D falloc(td, &fp, &fd, 0);
if (error)
return (error);
=20
diff --git a/sys/kern/uipc_sem.c b/sys/kern/uipc_sem.c
index 6ade212..917c343 100644
--- a/sys/kern/uipc_sem.c
+++ b/sys/kern/uipc_sem.c
@@ -422,7 +422,7 @@ ksem_create(struct thread *td, const char *name, semid_=
t *semidp, mode_t mode,
=20
fdp =3D td->td_proc->p_fd;
mode =3D (mode & ~fdp->fd_cmask) & ACCESSPERMS;
- error =3D falloc(td, &fp, &fd);
+ error =3D falloc(td, &fp, &fd, 0);
if (error) {
if (name =3D=3D NULL)
error =3D ENOSPC;
diff --git a/sys/kern/uipc_shm.c b/sys/kern/uipc_shm.c
index cef8317..00496af 100644
--- a/sys/kern/uipc_shm.c
+++ b/sys/kern/uipc_shm.c
@@ -496,7 +496,7 @@ shm_open(struct thread *td, struct shm_open_args *uap)
fdp =3D td->td_proc->p_fd;
cmode =3D (uap->mode & ~fdp->fd_cmask) & ACCESSPERMS;
=20
- error =3D falloc(td, &fp, &fd);
+ error =3D falloc(td, &fp, &fd, 0);
if (error)
return (error);
=20
diff --git a/sys/kern/uipc_syscalls.c b/sys/kern/uipc_syscalls.c
index d3326d4..a4bbdba 100644
--- a/sys/kern/uipc_syscalls.c
+++ b/sys/kern/uipc_syscalls.c
@@ -176,7 +176,7 @@ socket(td, uap)
return (error);
#endif
fdp =3D td->td_proc->p_fd;
- error =3D falloc(td, &fp, &fd);
+ error =3D falloc(td, &fp, &fd, 0);
if (error)
return (error);
/* An extra reference on `fp' has been held for us by falloc(). */
@@ -358,7 +358,7 @@ kern_accept(struct thread *td, int s, struct sockaddr *=
*name,
if (error !=3D 0)
goto done;
#endif
- error =3D falloc(td, &nfp, &fd);
+ error =3D falloc(td, &nfp, &fd, 0);
if (error)
goto done;
ACCEPT_LOCK();
@@ -606,12 +606,12 @@ kern_socketpair(struct thread *td, int domain, int ty=
pe, int protocol,
if (error)
goto free1;
/* On success extra reference to `fp1' and 'fp2' is set by falloc. */
- error =3D falloc(td, &fp1, &fd);
+ error =3D falloc(td, &fp1, &fd, 0);
if (error)
goto free2;
rsv[0] =3D fd;
fp1->f_data =3D so1; /* so1 already has ref count */
- error =3D falloc(td, &fp2, &fd);
+ error =3D falloc(td, &fp2, &fd, 0);
if (error)
goto free3;
fp2->f_data =3D so2; /* so2 already has ref count */
@@ -2299,7 +2299,7 @@ sctp_peeloff(td, uap)
* but that is ok.
*/
=20
- error =3D falloc(td, &nfp, &fd);
+ error =3D falloc(td, &nfp, &fd, 0);
if (error)
goto done;
td->td_retval[0] =3D fd;
diff --git a/sys/kern/vfs_syscalls.c b/sys/kern/vfs_syscalls.c
index fe66591..dc1e262 100644
--- a/sys/kern/vfs_syscalls.c
+++ b/sys/kern/vfs_syscalls.c
@@ -1069,7 +1069,7 @@ kern_openat(struct thread *td, int fd, char *path, en=
um uio_seg pathseg,
else
flags =3D FFLAGS(flags);
=20
- error =3D fallocf(td, &nfp, &indx, flags);
+ error =3D falloc(td, &nfp, &indx, flags);
if (error)
return (error);
/* An extra reference on `nfp' has been held for us by falloc(). */
@@ -4488,7 +4488,7 @@ fhopen(td, uap)
* end of vn_open code
*/
=20
- if ((error =3D fallocf(td, &nfp, &indx, fmode)) !=3D 0) {
+ if ((error =3D falloc(td, &nfp, &indx, fmode)) !=3D 0) {
if (fmode & FWRITE)
vp->v_writecount--;
goto bad;
diff --git a/sys/opencrypto/cryptodev.c b/sys/opencrypto/cryptodev.c
index 6a10f9a..2c0c503 100644
--- a/sys/opencrypto/cryptodev.c
+++ b/sys/opencrypto/cryptodev.c
@@ -1109,7 +1109,7 @@ cryptoioctl(struct cdev *dev, u_long cmd, caddr_t dat=
a, int flag, struct thread
TAILQ_INIT(&fcr->csessions);
fcr->sesn =3D 0;
=20
- error =3D falloc(td, &f, &fd);
+ error =3D falloc(td, &f, &fd, 0);
=20
if (error) {
free(fcr, M_XDATA);
diff --git a/sys/sys/filedesc.h b/sys/sys/filedesc.h
index c96d6f9..33dddca 100644
--- a/sys/sys/filedesc.h
+++ b/sys/sys/filedesc.h
@@ -111,8 +111,7 @@ struct thread;
int closef(struct file *fp, struct thread *td);
int dupfdopen(struct thread *td, struct filedesc *fdp, int indx, int dfd,
int mode, int error);
-int falloc(struct thread *td, struct file **resultfp, int *resultfd);
-int fallocf(struct thread *td, struct file **resultfp, int *resultfd,
+int falloc(struct thread *td, struct file **resultfp, int *resultfd,
int flags);
int fdalloc(struct thread *td, int minfd, int *result);
int fdavail(struct thread *td, int n);
--TBIaq1yoUA4huN1+
Content-Type: application/pgp-signature
Content-Disposition: inline
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (FreeBSD)
iEYEARECAAYFAk2MrQ4ACgkQC3+MBN1Mb4hF4QCeMaR13gNAiSkbLLu2Kiu17XdN
nR4AoOBpFoLNf5czWKfhLudum9dlmHIh
=4FHm
-----END PGP SIGNATURE-----
--TBIaq1yoUA4huN1+--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20110325145614.GN78089>