The following reply was made to PR kern/175674; it has been noted by GNATS.
From: David Xu <davi...@freebsd.org> To: Giorgos Keramidas <keram...@freebsd.org> Cc: Jukka Ukkonen <j...@iki.fi>, freebsd-gnats-sub...@freebsd.org, jil...@freebsd.org Subject: Re: kern/175674: sem_open() should use O_EXLOCK with open() instead of a separate flock() call Date: Mon, 04 Feb 2013 10:05:31 +0800 On 2013/02/03 13:25, Giorgos Keramidas wrote: > On 2013-01-29 18:03, Jukka Ukkonen <j...@iki.fi> wrote: >>> Number: 175674 >>> Category: kern >>> Synopsis: sem_open() should use O_EXLOCK with open() instead of a >>> separate flock() call > >>> Environment: >> FreeBSD sleipnir 9.1-STABLE FreeBSD 9.1-STABLE #2 r246056M: Tue Jan 29 >> 07:33:01 EET 2013 root@sleipnir:/usr/obj/usr/src/sys/Sleipnir amd64 >>> Description: >> sem_open() is calling flock() to set a lock on a newly created file >> descriptor. >> That is pointless. The open() call a few lines before the flock() could, and >> in my opinion should, be done with the O_EXLOCK flag set. > > It's also a bit safer to obtain the exclusive lock atomically before > open() returns. Waiting for open() to complete and then calling flock() > has a race condition. > > Jilles and David, do you think this patch looks ok for libc? > >> Patch attached with submission follows: >> >> --- lib/libc/gen/sem_new.c.flock 2012-11-09 18:50:05.000000000 +0200 >> +++ lib/libc/gen/sem_new.c 2012-11-09 18:44:59.000000000 +0200 >> @@ -198,11 +198,13 @@ >> goto error; >> } >> >> - fd = _open(path, flags|O_RDWR|O_CLOEXEC, mode); >> + fd = _open(path, flags|O_RDWR|O_CLOEXEC|O_EXLOCK, mode); >> if (fd == -1) >> goto error; >> +#if 0 >> if (flock(fd, LOCK_EX) == -1) >> goto error; >> +#endif >> if (_fstat(fd, &sb)) { >> flock(fd, LOCK_UN); >> goto error; > > I don't think there is a race condition, the flock is used to ensure the semaphore is already initialized, whether you use flock or E_EXLOCK, they are same, because only one thread can lock the file at same time, and in locked state, the code checks and initializes semaphore if it is needed. Regards, David Xu _______________________________________________ freebsd-bugs@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-bugs To unsubscribe, send any mail to "freebsd-bugs-unsubscr...@freebsd.org"