From owner-freebsd-hackers@freebsd.org Sat Apr 13 15:43:50 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 F107B157BE3F for ; Sat, 13 Apr 2019 15:43:49 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) (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 C89A8881BC; Sat, 13 Apr 2019 15:43:48 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from tom.home (kib@localhost [127.0.0.1]) by kib.kiev.ua (8.15.2/8.15.2) with ESMTPS id x3DFheJD084610 (version=TLSv1.2 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Sat, 13 Apr 2019 18:43:43 +0300 (EEST) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.10.3 kib.kiev.ua x3DFheJD084610 Received: (from kostik@localhost) by tom.home (8.15.2/8.15.2/Submit) id x3DFheEq084608; Sat, 13 Apr 2019 18:43:40 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Sat, 13 Apr 2019 18:43:40 +0300 From: Konstantin Belousov To: Kirk McKusick Cc: Alan Somers , FreeBSD Hackers Subject: Re: When can a struct buf's b_lblkno field by < 0 ? Message-ID: <20190413154340.GJ1923@kib.kiev.ua> References: <20190412154117.GD1923@kib.kiev.ua> <201904131523.x3DFNsu4091963@chez.mckusick.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <201904131523.x3DFNsu4091963@chez.mckusick.com> User-Agent: Mutt/1.11.4 (2019-03-13) X-Spam-Status: No, score=-1.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FORGED_GMAIL_RCVD,FREEMAIL_FROM, NML_ADSP_CUSTOM_MED autolearn=no autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on tom.home 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:43:50 -0000 On Sat, Apr 13, 2019 at 08:23:54AM -0700, Kirk McKusick wrote: > > 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 == 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 "==" case. > >> https://svnweb.freebsd.org/base/head/sys/kern/vfs_subr.c?r1=112182&r2=112181&pathrev=112182 > > > > 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 >= 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. Do you agree with the statement that the last loop in vtruncbuf() is useless ? Its removal could only make a difference for ffs_truncate(), and there, I do not think that b(a)write() is enough to ensure that the indirect buffers are clean, due to dependencies.