From owner-freebsd-hackers@freebsd.org Sat Apr 13 15:15:10 2019 Return-Path: Delivered-To: freebsd-hackers@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id B3BC6157B7D4 for ; Sat, 13 Apr 2019 15:15:10 +0000 (UTC) (envelope-from mckusick@mckusick.com) Received: from chez.mckusick.com (chez.mckusick.com [70.36.157.235]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 260E287711; Sat, 13 Apr 2019 15:15:09 +0000 (UTC) (envelope-from mckusick@mckusick.com) Received: from chez.mckusick.com (localhost [IPv6:::1]) by chez.mckusick.com (8.15.2/8.15.2) with ESMTP id x3DFNsu4091963; Sat, 13 Apr 2019 08:23:54 -0700 (PDT) (envelope-from mckusick@mckusick.com) Message-Id: <201904131523.x3DFNsu4091963@chez.mckusick.com> From: Kirk McKusick To: Konstantin Belousov Subject: Re: When can a struct buf's b_lblkno field by < 0 ? cc: Alan Somers , FreeBSD Hackers X-URL: http://WWW.McKusick.COM/ Reply-To: Kirk McKusick In-reply-to: <20190412154117.GD1923@kib.kiev.ua> Comments: In-reply-to Konstantin Belousov message dated "Fri, 12 Apr 2019 18:41:17 +0300." MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-ID: <91961.1555169034.1@chez.mckusick.com> Content-Transfer-Encoding: quoted-printable Date: Sat, 13 Apr 2019 08:23:54 -0700 X-Spam-Status: No, score=-1.4 required=5.0 tests=BAYES_00,MISSING_MID, UNPARSEABLE_RELAY autolearn=no autolearn_force=no version=3.4.1 X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on chez.mckusick.com X-Rspamd-Queue-Id: 260E287711 X-Spamd-Bar: ------ Authentication-Results: mx1.freebsd.org X-Spamd-Result: default: False [-6.96 / 15.00]; NEURAL_HAM_MEDIUM(-1.00)[-0.999,0]; NEURAL_HAM_SHORT(-0.96)[-0.960,0]; REPLY(-4.00)[]; NEURAL_HAM_LONG(-1.00)[-1.000,0] X-Mailman-Approved-At: Sat, 13 Apr 2019 17:12:19 +0000 X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 13 Apr 2019 15:15:11 -0000 > Date: Fri, 12 Apr 2019 18:41:17 +0300 > From: Konstantin Belousov > To: Alan Somers > Cc: FreeBSD Hackers , > Kirk McKusick > Subject: Re: When can a struct buf's b_lblkno field by < 0 ? > = > On Fri, Apr 12, 2019 at 09:10:58AM -0600, Alan Somers wrote: >> In struct buf, b_lblkno is documented as "Logical block number". I >> would expect that to always be nonnegative. However, vtruncbuf loops >> through a list of buffers, skipping those where "bp->b_lblkno > 0". >> Maybe that's just an awkward way of writing "do something for the >> buffer where b_lblkno =3D=3D 0", but SVN archaeology suggests otherwise= . >> Before r112182, the code looked like this, implying that the b_lblkno >> could actually be negative: >> = >> if ((bp->b_flags & B_DELWRI) && (bp->b_lblkno < 0)) { >> = >> Does anybody know under what circumstances that field might be >> negative? > = > Yes, somebody know. Better list for such discussions is fs@. > = > It is up to the filesystem to use any values of b_lblkno as it finds > suitable. For instance, for UFS lblknos -1 and -2 are used for the > blocks carrying the extended attributes of inode. Lower values > are used to cache indirect block' pages in the normal vnode page queue, > so that the pages effectively appear as having very large page indexes. > = >> Also, was r112182 a correct change? It appears to have >> negated "<" and gotten ">", neglecting the "=3D=3D" case. >> https://svnweb.freebsd.org/base/head/sys/kern/vfs_subr.c?r1=3D112182&r2= =3D112181&pathrev=3D112182 > = > This is innocent, in worst case it results in unneeded syncing. But I > wonder why do we still have this loop for 'if (length > 0)' case. I do > not quite follow the purpose of syncing all indirect and extended blocks > on truncation. It might somewhat help SU to ensure that dependencies > carried by indirect blocks are flushed timely after truncation, but this > is too vague. > = > I added Kirk who may know more history there. I don't have much to add to kib's commentary. The change in -r112182 was not meant to have functional change, just reduce indentation and get rid of unnecessary code. The change should have been to = (bp->b_lblkno >=3D 0), but the effect is that if logical data block 0 is in the cache and dirty it will be unnecessarily written. At that time all partial truncations were done synchronously, hence the flushing of all the meta-data. With the additional of journaled soft updates, it became possible to do partial truncations asynchronously. Kirk McKusick