Date: Thu, 30 Jul 2020 15:13:19 -0700 From: Conrad Meyer <cem@freebsd.org> To: Konstantin Belousov <kostikbel@gmail.com> Cc: src-committers <src-committers@freebsd.org>, svn-src-all <svn-src-all@freebsd.org>, svn-src-head <svn-src-head@freebsd.org> Subject: Re: svn commit: r363482 - in head/sys: kern sys Message-ID: <CAG6CVpXOKfiCNhF-S3zRVwDrTBBrGFdbz6brF0oBBeUMqVQ8xg@mail.gmail.com> In-Reply-To: <20200728184152.GH2551@kib.kiev.ua> References: <202007241734.06OHY53J080448@repo.freebsd.org> <20200728184152.GH2551@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
Hi Konstantin, On Tue, Jul 28, 2020 at 11:42 AM Konstantin Belousov <kostikbel@gmail.com> wrote: > > On Fri, Jul 24, 2020 at 05:34:05PM +0000, Conrad Meyer wrote: > > ... > > --- head/sys/kern/vfs_bio.c Fri Jul 24 17:32:10 2020 (r363481) > > +++ head/sys/kern/vfs_bio.c Fri Jul 24 17:34:04 2020 (r363482) > > @@ -3849,7 +3849,7 @@ getblkx(struct vnode *vp, daddr_t blkno, daddr_t dblkn > > ... > > + /* Attempt lockless lookup first. */ > > + bp = gbincore_unlocked(bo, blkno); > > + if (bp == NULL) > > + goto newbuf_unlocked; > > + > > + lockflags = LK_EXCLUSIVE | LK_SLEEPFAIL | > > + ((flags & GB_LOCK_NOWAIT) ? LK_NOWAIT : 0); > > + > > + error = BUF_TIMELOCK(bp, lockflags, NULL, "getblku", slpflag, > > + slptimeo); > I realized that this is not safe. There is an ordering between buffer > types that defines which order buffer locks should obey. For instance, > on UFS the critical order is inode buffer -> snaplk -> cg buffer, or > data block -> indirect data block. Since buffer identity can change under > us, we might end up waiting for a lock of type that is incompatible with > the currently owned lock. > > I think the easiest fix is to use LK_NOWAIT always, after all it is lockless > path. ERESTART/EINTR checks below than can be removed. Thanks, that makes sense to me. Please see https://reviews.freebsd.org/D25898 . (For the UFS scenario, I think this requires an on-disk sector changing identity from one kind to another? I believe lblknos are mostly statically typed in UFS, but it could happen with data blocks and indirect blocks? Of course, UFS is not the only filesystem.) Best regards, Conrad
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAG6CVpXOKfiCNhF-S3zRVwDrTBBrGFdbz6brF0oBBeUMqVQ8xg>