linux-kernel - Re: [PATCH V10 08/19] btrfs: move bio_pages

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20181119082414.GA10678@lst.de>
Date:   Mon, 19 Nov 2018 09:24:14 +0100
From:   Christoph Hellwig <hch@....de>
To:     Ming Lei <ming.lei@...hat.com>
Cc:     Christoph Hellwig <hch@....de>, Jens Axboe <axboe@...nel.dk>,
        linux-block@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-mm@...ck.org, Dave Chinner <dchinner@...hat.com>,
        Kent Overstreet <kent.overstreet@...il.com>,
        Mike Snitzer <snitzer@...hat.com>, dm-devel@...hat.com,
        Alexander Viro <viro@...iv.linux.org.uk>,
        linux-fsdevel@...r.kernel.org, Shaohua Li <shli@...nel.org>,
        linux-raid@...r.kernel.org, linux-erofs@...ts.ozlabs.org,
        David Sterba <dsterba@...e.com>, linux-btrfs@...r.kernel.org,
        "Darrick J . Wong" <darrick.wong@...cle.com>,
        linux-xfs@...r.kernel.org, Gao Xiang <gaoxiang25@...wei.com>,
        Theodore Ts'o <tytso@....edu>, linux-ext4@...r.kernel.org,
        Coly Li <colyli@...e.de>, linux-bcache@...r.kernel.org,
        Boaz Harrosh <ooo@...ctrozaur.com>,
        Bob Peterson <rpeterso@...hat.com>, cluster-devel@...hat.com,
        Chandan Rajendra <chandan@...ux.vnet.ibm.com>
Subject: Re: [PATCH V10 08/19] btrfs: move bio_pages_all() to btrfs

On Mon, Nov 19, 2018 at 04:19:24PM +0800, Ming Lei wrote:
> On Fri, Nov 16, 2018 at 02:38:45PM +0100, Christoph Hellwig wrote:
> > On Thu, Nov 15, 2018 at 04:52:55PM +0800, Ming Lei wrote:
> > > BTRFS is the only user of this helper, so move this helper into
> > > BTRFS, and implement it via bio_for_each_segment_all(), since
> > > bio->bi_vcnt may not equal to number of pages after multipage bvec
> > > is enabled.
> > 
> > btrfs only uses the value to check if it is larger than 1.  No amount
> > of multipage bio merging should ever make bi_vcnt go from 0 to 1 or
> > vice versa.
> 
> Could you explain a bit why?
> 
> Suppose 2 physically continuous pages are added to this bio, .bi_vcnt
> can be 1 in case of multi-page bvec, but it is 2 in case of single-page
> bvec.

True, I did think of 0 vs 1.

The magic here in btrfs still doesn't make much sense.  The comments
down in btrfs_check_repairable talk about sectors, so it might be a
leftover from the blocksize == PAGE_SIZE days (assuming those patches
actually got merged).  I'd like to have the btrfs folks chime in,
but in the end we should probably check if the bio was larger than
a single sector here.