Date: Wed, 26 Aug 2020 21:15:20 +0000 From: Rick Macklem <rmacklem@uoguelph.ca> To: J David <j.david.lists@gmail.com>, Brandon Bergren <bdragon@FreeBSD.org> Cc: FreeBSD Hackers <freebsd-hackers@freebsd.org> Subject: Re: pidfile_open() usage in "mount" Message-ID: <QB1PR01MB3364C5F4640D161DEE96E579DD540@QB1PR01MB3364.CANPRD01.PROD.OUTLOOK.COM> In-Reply-To: <CABXB=RQe8Epvr=7oZv9C3D4NXyd9TRvRctxM0yy_c5oB_%2B7P4A@mail.gmail.com> References: <CABXB=RRM7YusQL1gGVGD4s9xi9AB4F5AR5-Jqbp4G1WTZWWA%2BQ@mail.gmail.com> <20200826144013.GV2551@kib.kiev.ua> <dcf65725-af6f-4a89-bcac-b1ca46d49cae@www.fastmail.com>, <CABXB=RQe8Epvr=7oZv9C3D4NXyd9TRvRctxM0yy_c5oB_%2B7P4A@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
Brandon Bergren wrote:
>> The pidfile_* stuff is meant for the daemon side.
>
>As written, agreed, it clearly is. But alas "meant for" and "used
>for" appear to differ. It looks like there's a client-side use in ZFS
>as well (cddl/compat/opensolaris/misc/fsshare.c).
>
>> Why not just regular open(2)?
>
>That's also a solution I'd be fine with submitting. Certainly less
>work. But there are a couple of reasons not to do that, mostly to do
>with code reuse.
>
>First, the pidfile library does some stuff around path handling that
>is useful not to try to reinvent.
>
>Second, while races do exist, use of pidfiles in this way probably
>isn't going away, so having the code that tries to minimize those
>races in one place makes a lot of sense, rather than having
>potentially multiple idiosyncratic approaches/cases on an ad-hoc
>basis. ("Don't let the perfect be the enemy of the good" and all
>that.)
>
>The CDDL thing is actually a good example of this, because it looks
>lifted straight from mount.c, but when mount.c got updated with the
>mountdpid <= 0 check, fsshare.c didn't, and that's almost certainly a
>bug because it could lead to inadvertent kill( -1, SIGHUP ) with
>superuser permissions. Whoops! ZFS just randomly HUP'd every process
>on the system.
>
>Having mount HUP mountd (iff it's running) seems odd to me anyway, but
>today's filesystems are so weird and complex and diverse that I'm sure
>it's essential for somebody.
Yes, it is weird and I don't think I was the guilty party.
The reason is that, if the local file system being mounted happens to be in
/etc/exports, then /etc/exports has to be re-loaded for the update.
Ideally, all local file systems would be mounted and then one HUP would
be sent to mountd, but that isn't the way it has worked for a long time
and changing it would be a POLA violation, imho.
Imagine how many times this happens for a server with 20,000 file
systems that happen to be exported (such a server does exist, apparently).
I ended up coming up with an incremental exports update to improve this
and Peter (the guy that has the 20,000+ file system servers) has a patch that
uses a database instead of the flat exports file. (Hopefully a variant of his
patch can make it into ZFS someday. Now that the big switch of ZFS to ZoL
has happened, maybe someday soon?)
rick
Thanks!
_______________________________________________
freebsd-hackers@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "freebsd-hackers-unsubscribe@freebsd.org"
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?QB1PR01MB3364C5F4640D161DEE96E579DD540>