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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 19 Nov 2018 17:00:22 +0800 From: Ming Lei <ming.lei@...hat.com> To: Christoph Hellwig <hch@....de> Cc: 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 Subject: Re: [PATCH V10 14/19] block: enable multipage bvecs On Fri, Nov 16, 2018 at 02:53:08PM +0100, Christoph Hellwig wrote: > > - > > - if (page == bv->bv_page && off == bv->bv_offset + bv->bv_len) { > > - bv->bv_len += len; > > - bio->bi_iter.bi_size += len; > > - return true; > > - } > > + struct request_queue *q = NULL; > > + > > + if (page == bv->bv_page && off == (bv->bv_offset + bv->bv_len) > > + && (off + len) <= PAGE_SIZE) > > How could the page struct be the same, but the range beyond PAGE_SIZE > (at least with the existing callers)? > > Also no need for the inner btraces, and the && always goes on the > first line. OK. > > > + if (bio->bi_disk) > > + q = bio->bi_disk->queue; > > + > > + /* disable multi-page bvec too if cluster isn't enabled */ > > + if (!q || !blk_queue_cluster(q) || > > + ((page_to_phys(bv->bv_page) + bv->bv_offset + bv->bv_len) != > > + (page_to_phys(page) + off))) > > + return false; > > + merge: > > + bv->bv_len += len; > > + bio->bi_iter.bi_size += len; > > + return true; > > Ok, this is scary, as it will give differen results depending on when > bi_disk is assigned. It is just merge or not, both can be handled well now. > But then again we shouldn't really do the cluster > check here, but rather when splitting the bio for the actual low-level > driver. Yeah, I thought of this way too, but it may cause tons of bio split for no-clustering, and there are quite a few scsi devices which require to disable clustering. [linux]$ git grep -n DISABLE_CLUSTERING ./drivers/scsi/ | wc -l 28 Or we may introduce bio_split_to_single_page_bvec() to allocate & convert to single-page bvec table for non-clustering, will try this approach in next version. > > (and eventually we should kill this clustering setting off in favor > of our normal segment limits). Yeah, it has been in my post-multi-page todo list already, :-) thanks, Ming
Powered by blists - more mailing lists