Date: Sun, 13 Jul 2014 11:56:42 +0300 From: Konstantin Belousov <kostikbel@gmail.com> To: Mateusz Guzik <mjguzik@gmail.com> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Mateusz Guzik <mjg@FreeBSD.org> Subject: Re: svn commit: r268570 - head/sys/kern Message-ID: <20140713085642.GV93733@kib.kiev.ua> In-Reply-To: <20140713080115.GD16884@dft-labs.eu> References: <201407121535.s6CFZ42f063120@svn.freebsd.org> <20140712161801.GS93733@kib.kiev.ua> <20140712165346.GA16884@dft-labs.eu> <20140712171015.GT93733@kib.kiev.ua> <20140713080115.GD16884@dft-labs.eu>
next in thread | previous in thread | raw e-mail | index | archive | help
--/MDAAvgDg55PPszM
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On Sun, Jul 13, 2014 at 10:01:16AM +0200, Mateusz Guzik wrote:
> On Sat, Jul 12, 2014 at 08:10:15PM +0300, Konstantin Belousov wrote:
> > On Sat, Jul 12, 2014 at 06:53:47PM +0200, Mateusz Guzik wrote:
> > > There can be only one 'struct file' for devctl and devclose is only
> > > called when it is about to be destroyed.
> > >=20
> > > fd =3D open("/dev/devctl");
> > > close(dup(fd));
> > >=20
> > > does not result in calling devclose.
> > >=20
> > > If devclose is indeed reachable whlie fds are active this code needs
> > > serious help since devsoftc.inuse is of no use whatsoever.
> > >=20
> > > There is no support for multiple readers in the sense that each event
> > > can be read only once, hence the restriction on open.
> > >=20
> > > On the other hand it is indeed possible to obtain multiple fds for
> > > devctl which is harmless as far as consistency in the kernel goes.
> > > Concurrent reads are serialized with a mutex and closes are invisible=
to
> > > the device, except for the last one which destroys fp.
> > Well, I argue that devsoftc.inuse is broken too. It was introduced in
> > time when the only way of tracking the shared use of cdev was cloning.
> > Note that it does not prevent multiple threads from simultaneously
> > fall into the cdevsw methods; e.g. cv_wait_sig() in devread() drops
> > devsoftc.mtx etc.
> >=20
>=20
> But this is again harmless as far as kernel consistency goes.
>=20
> > IMO the right thing to do is to allow multiple opens and to keep
> > non-blocking attribute and async bindings in the per-file structure.
> > Then your change would be real nop.
> >=20
>=20
> This would be an option, but then what to do with events? Whoever
> happens to read one consumes it? Current behaviour of denying further
> opens seems safer since the process which opened can be sure nobody
> suddenly steals any. If it decides to 'share' device fd, well, it is its
> own problem.
>=20
> Assignments in devclose are not going to be executed as long as
> there is an active fp, thus this is a nop from perspective of devctl
> users.
>=20
> > BTW, another, this time really big, user of the private (yes) cloning
> > implementation is snd(4). The conversion of it to devfs_cdevpriv(9)
> > would be also highly desirable.
>=20
>=20
> This was meant to be a cosmetic change, I'm not interested in working on
> this, sorry. I can revert the change if you want.
I do not see a point in reverting it.
I am more interested in (somebody) converting such places to use cdevpriv.
>=20
> I plan to merge the following to stable/10:
> - r264114 Fix SIGIO delivery. Use fsetown() to handle file descriptor own=
er
> ioctl and use pgsigio() to send SIGIO.
> - r264310 Add kqueue support for devctl.
>=20
> Do you have any objections?
>=20
> If you don't want a revert of this patch, I'll MFC it as well.
I have no objections.
--/MDAAvgDg55PPszM
Content-Type: application/pgp-signature
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
iQIcBAEBAgAGBQJTwknKAAoJEJDCuSvBvK1ByqsP/00RBKrDB3lcodbnXvCh2yf6
smXfVeHx8N6unK+oNuEQPUV6K9Gxav0rPoyKk0+3LLKaQ+U44kMy46m5wUL3Lo4e
9fCtFIU2p4QJBT1FD7bYsJHbDyuhv3WZcRVmGiBj5I2YMu1Gi362ES/SWHrdAqEP
6fVsB3+BtMfMyAFTbftUOqC6SeoJGKTyOVCuXhAQQACKBzjpQQdUwbSKkhvbbRY2
yzsT5V3mAVvKCOt0AyrwxVME4MQUXc6h8QYXzFXau8Ikfmu7npAiGCIZnxwgeKpS
ad7vcRUQZp/GRtJ84YCMJKkI4cV+wUbzCmUsWCEcOVZqQOP9QmBW6d1A82SRyKnR
zbg/7wwHinTjpBH0hf9LDuv5pYCrccnfwW/A9/eM7otmBpP90T76JGO52k5VpysU
wC0MeMuHjJWHQXh1oelyXtRj5eH+BJ5HwydQIOVG11JjFEyRXXGu71u2uX3QdazX
WhYg37WplovZ9ewp4ftsnbT0lbgVELSQvXbJUd6RGFXFsOxDV/ua0mPE4OZlPg1Y
W32kXS1maIeG7W6cqdbsHXZE6/5ZU2psQG3vkoqFAa1adLL2RQ3MKY3N9NzU/yBt
VA7xevmtuFIY5mkgqvjXp7Ex2iD++KjgaRxbKKseTk/14o8EVQQ91zOsbdpMGP1X
aO6lT8Hb6keaCYaGr1+b
=CjSt
-----END PGP SIGNATURE-----
--/MDAAvgDg55PPszM--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140713085642.GV93733>