Date: Fri, 21 Jul 2023 10:37:12 +0200 From: Peter Eriksson <pen@lysator.liu.se> To: FreeBSD FS <freebsd-fs@freebsd.org> Cc: kevans@freebsd.org, Rick Macklem <rick.macklem@gmail.com> Subject: Re: RFC: Patch for mountd to handle a database for exports Message-ID: <7AC56987-BAB0-4291-8717-2D9EFB2487DB@lysator.liu.se> In-Reply-To: <2D8B626E-82BB-410C-B7C7-35B4D3834A44@lysator.liu.se> References: <CAM5tNy7GkMLgy-wjonVu%2BgOMnexoA2W8vSZvMo4NDikrg3-A1A@mail.gmail.com> <2D8B626E-82BB-410C-B7C7-35B4D3834A44@lysator.liu.se>
next in thread | previous in thread | raw e-mail | index | archive | help
Regarding the potential for speed improvement here is a quick little = benchmark=E2=80=A6 > # wc -l /etc/zfs/exports > 13116 /etc/zfs/exports With the original FreeBSD 13.2 code: > # /usr/bin/time zfs share -a > 40.08 real 34.00 user 6.04 sys > # /usr/bin/time zfs share -a > 40.15 real 34.06 user 6.04 sys With the DB code in use: > # service mountd stop ; mv /usr/sbin/mountd.DB /usr/sbin/mountd ; mv = /lib/libzfs.so.4.DB /lib/libzfs.so.4 > # rm -f /etc/zfs/exports.db > # /usr/bin/time zfs share -a > 2.75 real 0.88 user 1.87 sys > # /usr/bin/time zfs share -a > 2.77 real 1.01 user 1.75 sys This translates into a shorter boot time if nothing else... - Peter > On 21 Jul 2023, at 10:22, Peter Eriksson <pen@lysator.liu.se> wrote: >=20 > Hi! >=20 > Here=E2=80=99s a couple of updated patches with some bug fixes. I=E2=80=99= ve also moved the database location for ZFS to /etc/zfs/exports.db to = mirror the current location. >=20 > I also changed the patch for mountd so that for each =E2=80=9Cexports=E2= =80=9D source specified it first tries to open <path>.db which I think = makes it easier to use (no need for the =E2=80=9C-D=E2=80=9D option and = it supports multiple sources). Cleaner I think. >=20 >=20 > Btw, you can create the database manually from /etc/zfs/exports using = =E2=80=9Cmakemap=E2=80=9D like this: >=20 > makemap -f btree /etc/zfs/exports.db </etc/zfs/exports >=20 >=20 > Known =E2=80=9Cbugs=E2=80=9D: > =E2=80=9CV4:=E2=80=9D=20 > Even though you could create an /etc/exports.db with the contents of = /etc/exports it will fail with the =E2=80=9CV4:=E2=80=9D lines since the = BTree database will sort the exported entries so that the V4: key will = appear last and then mountd will complain when scanning the DB.=20 >=20 > Also when using the file /etc/exports you can either write = =E2=80=9CV4:/export -sec=3Dsys=E2=80=9D or =E2=80=9CV4: /export = -sec=3Dsys=E2=80=9D (with a space between V4: and the path) so this = probably needs some special handling (can=E2=80=99t simply use = =E2=80=9Cmakemap -f btree /etc/exports.db </etc/exports=E2=80=9D to = create it.=20 >=20 > I did try to fix that but the code quickly got hairy so I didn=E2=80=99t= like it. If we really want/need that I=E2=80=99m thinking of creating a = special case for the V4: handling, sort of like prefixing the database = key with a NUL byte or something (so that it will be sorted first).=20 >=20 >=20 > Multiline options - well, the current ZFS code doesn=E2=80=99t support = it either so no change from the current setup but it would be nice to = have. > Ie, support for things like: > /export -sec=3Dkrb5 > /export -sec=3Dsys ro >=20 >=20 >=20 > <mountd-sharedb.patch><libshare-nfs-sharedb.patch><share.c> >=20 >=20 > (I also agree that the USE_SHAREDB probably should be removed, I just = have that here for now so that I can quickly disable the code) >=20 > Regarding the patch to the zfs part - I=E2=80=99m not sure which way = to go there - the current patch switches to always use the DB. But one = could argue that the code could check for an existing = /etc/zfs/exports.db and only use the DB-writing code if that already = exists. That way it will support both the old way and the new way, but = it requires an empty /etc/zfs/exports.db to be pre-created at initial = boot time for it to start using it.=20 >=20 > - Peter >=20 >=20 >> On 21 Jul 2023, at 00:50, Rick Macklem <rick.macklem@gmail.com> = wrote: >>=20 >> Hi, >>=20 >> Peter Eriksson has submitted an interesting patch that adds support >> for a database file for exports. My understanding is that this = improves >> performance when used with the ZFS share property for exporting a >> large number of file systems. >>=20 >> There are a couple of user visible issues that I'd like feedback from >> others. (Hopefully Peter can correct me if I get any of these wrong.) >>=20 >> - The patch uses a single database file and a new "-D" option to >> specify it on the command line. >> --> I think it might be less confusing to just put the database = file(s) >> in the exports list and identify them via a ".db" filename = suffix. >> What do others think? >>=20 >> The changes are #ifdef'd on USE_SHAREDB. I'm thinking that this >> change will be always built, so maybe USE_SHAREDB is not needed? >> (Obviously mountd(8)'s semantics will only changed if/when database >> file(s) are provided.) >>=20 >> Once I have feedback on the above, I will put a patch up on >> phabricator. >>=20 >> rick >> ps: I believe kevans@ has volunteered to shepperd the ZFS share >> property changes. >>=20 >=20
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?7AC56987-BAB0-4291-8717-2D9EFB2487DB>