Date: Sun, 21 Aug 2022 00:02:48 +0000 From: Rick Macklem <rmacklem@uoguelph.ca> To: Konstantin Belousov <kostikbel@gmail.com> Cc: FreeBSD Filesystems <freebsd-fs@freebsd.org> Subject: Re: SEEK_DATA/SEEK_HOLE with vnode locked Message-ID: <YT4PR01MB9736B24FDE64C945C2C9EC8EDD6F9@YT4PR01MB9736.CANPRD01.PROD.OUTLOOK.COM> In-Reply-To: <YvQ7MYXPl0AugojS@kib.kiev.ua> References: <YQBPR0101MB97420AD41791E544519A0A2DDD659@YQBPR0101MB9742.CANPRD01.PROD.OUTLOOK.COM> <YvQ7MYXPl0AugojS@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
Just to summarize this... I was able to do a VOP_SEEK() which would be called with a LK_SHARED locked vnode and it seemed to work fine. However, ReadPlus (which is like Read, but allows for holes to be represented as <offset, length> in the reply instead of a stream of 0 bytes) seems to be a performance dud. I was surprised how poorly it performed compares to ordinary Read. Typically it would take 60% longer to read a file. I tried sparse and non-sparse files of various sizes and they always took longer. (If I disabled SEEK_DATA/SEEK_HOLE in the server code, so it never actually did holes, it worked comparably to regular Read, so somehow the overhead of doing SEEK_DATA/SEEK_HOLE was a big performance hit. It was using LK_SHARED locks, so it wasn't serializing the reads, but I don't really know why it performed so poorly?) Anyhow, unless the performance issue gets resolved, there is no reason to commit the code to FreeBSD's main. (NFSv4.2 operations, like ReadPlus, are all optional and are not required for an RFC conformant implementation.) rick ________________________________________ From: Konstantin Belousov <kostikbel@gmail.com> Sent: Wednesday, August 10, 2022 7:11 PM To: Rick Macklem Cc: FreeBSD Filesystems Subject: Re: SEEK_DATA/SEEK_HOLE with vnode locked CAUTION: This email originated from outside of the University of Guelph. Do= not click links or open attachments unless you recognize the sender and kn= ow the content is safe. If in doubt, forward suspicious emails to IThelp@uo= guelph.ca On Wed, Aug 10, 2022 at 10:17:36PM +0000, Rick Macklem wrote: > Hi, > > When implementing what is called Read_Plus for the NFSv4.2 > server, I need to do SEEK_DATA/SEEK_HOLE. However, if I > use VOP_IOCTL(), I need to unlock/relock the vnode. > > This can result in problems if another RPC changes the size > of the file or allocates/deallocates data in the file while the > vnode is unlocked. > > >From what I can see, UFS does SEEK_DATA/SEEK_HOLE with > the vnode locked and ZFS doesn't seem to care/notice if it > is locked. > (Actually, ZFS looks like it might be unsafe, since it seems to > assume it can use the unlocked vnode that might be doomed, > but I do not know ZFS.) The statement about UFS needs more precision. UFS VOP_IOCTL() takes unlocked but references vnode, and the first thing vn_bmap_seekhole/data() and ufs_vmap_seekdata() do is vn_lock(vp, LK_SHARED). Since LK_RETRY flag is not specified, doomed vnodes result in ENOENT. I believe this interface was done because I wrote bmap-based seekhold/data while ZFS had its implementation already in place. It avoided taking the vnode lock, so UFS needed to do the same. I am not sure was the ZFS interface decision due to some internal consisten= cy guarantees already present, or because internal locking causing ordering issue with the vnode lock. > > Anyhow, does implementing a new vnode op VOP_SEEK(), which > takes a locked vnode and does SEEK_DATA/SEEK_HOLE sound > reasonable? You need to make some decision about ZFS first, I believe. UFS and generic buffer cache consumers fs are trivial to adopt, of course.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?YT4PR01MB9736B24FDE64C945C2C9EC8EDD6F9>