Re: [dm-devel] [PATCH] nvme: allow ANA support to be independent of native multipathing

2018-11-16 Thread Hannes Reinecke
On 11/16/18 10:14 AM, Christoph Hellwig wrote: On Thu, Nov 15, 2018 at 12:46:05PM -0500, Mike Snitzer wrote: Whether or not ANA is present is a choice of the target implementation; the host (and whether it supports multipathing) has _zero_ influence on this. If the target declares a path as

Re: [dm-devel] [PATCH] nvme: allow ANA support to be independent of native multipathing

2018-11-16 Thread Christoph Hellwig
On Thu, Nov 15, 2018 at 12:46:05PM -0500, Mike Snitzer wrote: > Whether or not ANA is present is a choice of the target implementation; > the host (and whether it supports multipathing) has _zero_ influence on > this. If the target declares a path as 'inaccessible' the path _is_ > inaccessible to

Re: [dm-devel] [PATCH V10 03/19] block: use bio_for_each_bvec() to compute multi-page bvec count

2018-11-16 Thread Christoph Hellwig
On Thu, Nov 15, 2018 at 02:18:47PM -0800, Omar Sandoval wrote: > My only reason to prefer unsigned int is consistency. unsigned int is > much more common in the kernel: > > $ ag --cc -s 'unsigned\s+int' | wc -l > 129632 > $ ag --cc -s 'unsigned\s+(?!char|short|int|long)' | wc -l > 22435 > >

Re: [dm-devel] [PATCH 3/3] block: use a driver-specific handler for the "inflight" value

2018-11-16 Thread Christoph Hellwig
On Fri, Nov 16, 2018 at 01:04:19AM +0100, Mikulas Patocka wrote: > Device mapper was converted to percpu inflight counters. In order to > display the correct values in the "inflight" sysfs file and in > /proc/diskstats, we need a custom callback that sums the percpu counters. > > The function

Re: [dm-devel] [PATCH V10 01/19] block: introduce multi-page page bvec helpers

2018-11-16 Thread Omar Sandoval
On Thu, Nov 15, 2018 at 04:52:48PM +0800, Ming Lei wrote: > This patch introduces helpers of 'mp_bvec_iter_*' for multipage > bvec support. > > The introduced helpers treate one bvec as real multi-page segment, > which may include more than one pages. > > The existed helpers of bvec_iter_* are

Re: [dm-devel] [PATCH V10 03/19] block: use bio_for_each_bvec() to compute multi-page bvec count

2018-11-16 Thread Gao Xiang
On 2018/11/16 17:19, Christoph Hellwig wrote: > On Thu, Nov 15, 2018 at 02:18:47PM -0800, Omar Sandoval wrote: >> My only reason to prefer unsigned int is consistency. unsigned int is >> much more common in the kernel: >> >> $ ag --cc -s 'unsigned\s+int' | wc -l >> 129632 >> $ ag --cc -s

Re: [dm-devel] [PATCH V10 03/19] block: use bio_for_each_bvec() to compute multi-page bvec count

2018-11-16 Thread Omar Sandoval
On Thu, Nov 15, 2018 at 04:52:50PM +0800, Ming Lei wrote: > First it is more efficient to use bio_for_each_bvec() in both > blk_bio_segment_split() and __blk_recalc_rq_segments() to compute how > many multi-page bvecs there are in the bio. > > Secondly once bio_for_each_bvec() is used, the bvec

Re: [dm-devel] [PATCH V10 08/19] btrfs: move bio_pages_all() to btrfs

2018-11-16 Thread Omar Sandoval
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. Shouldn't you also get rid

Re: [dm-devel] [PATCH V10 17/19] block: don't use bio->bi_vcnt to figure out segment number

2018-11-16 Thread Omar Sandoval
On Thu, Nov 15, 2018 at 04:53:04PM +0800, Ming Lei wrote: > It is wrong to use bio->bi_vcnt to figure out how many segments > there are in the bio even though CLONED flag isn't set on this bio, > because this bio may be splitted or advanced. > > So always use bio_segments() in

Re: [dm-devel] [PATCH V10 14/19] block: enable multipage bvecs

2018-11-16 Thread Omar Sandoval
On Thu, Nov 15, 2018 at 04:53:01PM +0800, Ming Lei wrote: > This patch pulls the trigger for multi-page bvecs. > > Now any request queue which supports queue cluster will see multi-page > bvecs. > > Cc: Dave Chinner > Cc: Kent Overstreet > Cc: Mike Snitzer > Cc: dm-devel@redhat.com > Cc:

Re: [dm-devel] [PATCH V10 18/19] block: kill QUEUE_FLAG_NO_SG_MERGE

2018-11-16 Thread Omar Sandoval
On Thu, Nov 15, 2018 at 04:53:05PM +0800, Ming Lei wrote: > Since bdced438acd83ad83a6c ("block: setup bi_phys_segments after splitting"), > physical segment number is mainly figured out in blk_queue_split() for > fast path, and the flag of BIO_SEG_VALID is set there too. > > Now only

Re: [dm-devel] [PATCH V10 07/19] btrfs: use bvec_last_segment to get bio's last page

2018-11-16 Thread Omar Sandoval
On Thu, Nov 15, 2018 at 04:52:54PM +0800, Ming Lei wrote: > Preparing for supporting multi-page bvec. > > Cc: Dave Chinner > Cc: Kent Overstreet > Cc: Mike Snitzer > Cc: dm-devel@redhat.com > Cc: Alexander Viro > Cc: linux-fsde...@vger.kernel.org > Cc: Shaohua Li > Cc:

Re: [dm-devel] [PATCH V10 16/19] block: document usage of bio iterator helpers

2018-11-16 Thread Omar Sandoval
On Thu, Nov 15, 2018 at 04:53:03PM +0800, Ming Lei wrote: > Now multi-page bvec is supported, some helpers may return page by > page, meantime some may return segment by segment, this patch > documents the usage. > > Cc: Dave Chinner > Cc: Kent Overstreet > Cc: Mike Snitzer > Cc:

Re: [dm-devel] [PATCH V10 09/19] block: introduce bio_bvecs()

2018-11-16 Thread Omar Sandoval
On Thu, Nov 15, 2018 at 04:52:56PM +0800, Ming Lei wrote: > There are still cases in which we need to use bio_bvecs() for get the > number of multi-page segment, so introduce it. > > Cc: Dave Chinner > Cc: Kent Overstreet > Cc: Mike Snitzer > Cc: dm-devel@redhat.com > Cc: Alexander Viro > Cc:

Re: [dm-devel] [PATCH] nvme: allow ANA support to be independent of native multipathing

2018-11-16 Thread Christoph Hellwig
On Fri, Nov 16, 2018 at 11:06:32AM +0100, Hannes Reinecke wrote: > Ok, so would you be happy with making ANA support configurable? I've looked a bit over the whole situation, and what I think we need to do is: a) warn if we see a ANA capable device without multipath support so people know

Re: [dm-devel] [PATCH V10 05/19] block: introduce bvec_last_segment()

2018-11-16 Thread Omar Sandoval
On Thu, Nov 15, 2018 at 04:52:52PM +0800, Ming Lei wrote: > BTRFS and guard_bio_eod() need to get the last singlepage segment > from one multipage bvec, so introduce this helper to make them happy. > > Cc: Dave Chinner > Cc: Kent Overstreet > Cc: Mike Snitzer > Cc: dm-devel@redhat.com > Cc:

Re: [dm-devel] [PATCH V10 03/19] block: use bio_for_each_bvec() to compute multi-page bvec count

2018-11-16 Thread Omar Sandoval
On Thu, Nov 15, 2018 at 04:05:10PM -0500, Mike Snitzer wrote: > On Thu, Nov 15 2018 at 3:20pm -0500, > Omar Sandoval wrote: > > > On Thu, Nov 15, 2018 at 04:52:50PM +0800, Ming Lei wrote: > > > First it is more efficient to use bio_for_each_bvec() in both > > > blk_bio_segment_split() and

Re: [dm-devel] [PATCH V10 02/19] block: introduce bio_for_each_bvec()

2018-11-16 Thread Omar Sandoval
On Thu, Nov 15, 2018 at 04:52:49PM +0800, Ming Lei wrote: > This helper is used for iterating over multi-page bvec for bio > split & merge code. > > Cc: Dave Chinner > Cc: Kent Overstreet > Cc: Mike Snitzer > Cc: dm-devel@redhat.com > Cc: Alexander Viro > Cc: linux-fsde...@vger.kernel.org >

Re: [dm-devel] [PATCH V10 11/19] bcache: avoid to use bio_for_each_segment_all() in bch_bio_alloc_pages()

2018-11-16 Thread Omar Sandoval
On Thu, Nov 15, 2018 at 04:52:58PM +0800, Ming Lei wrote: > bch_bio_alloc_pages() is always called on one new bio, so it is safe > to access the bvec table directly. Given it is the only kind of this > case, open code the bvec table access since bio_for_each_segment_all() > will be changed to

Re: [dm-devel] [PATCH V10 10/19] block: loop: pass multi-page bvec to iov_iter

2018-11-16 Thread Omar Sandoval
On Thu, Nov 15, 2018 at 04:52:57PM +0800, Ming Lei wrote: > iov_iter is implemented with bvec itererator, so it is safe to pass > multipage bvec to it, and this way is much more efficient than > passing one page in each bvec. > > Cc: Dave Chinner > Cc: Kent Overstreet > Cc: Mike Snitzer > Cc:

Re: [dm-devel] [PATCH V10 13/19] iomap & xfs: only account for new added page

2018-11-16 Thread Omar Sandoval
On Thu, Nov 15, 2018 at 04:53:00PM +0800, Ming Lei wrote: > After multi-page is enabled, one new page may be merged to a segment > even though it is a new added page. > > This patch deals with this issue by post-check in case of merge, and > only a freshly new added page need to be dealt with for

Re: [dm-devel] [PATCH V10 04/19] block: use bio_for_each_bvec() to map sg

2018-11-16 Thread Omar Sandoval
On Thu, Nov 15, 2018 at 04:52:51PM +0800, Ming Lei wrote: > It is more efficient to use bio_for_each_bvec() to map sg, meantime > we have to consider splitting multipage bvec as done in > blk_bio_segment_split(). > > Cc: Dave Chinner > Cc: Kent Overstreet > Cc: Mike Snitzer > Cc:

Re: [dm-devel] [PATCH V10 06/19] fs/buffer.c: use bvec iterator to truncate the bio

2018-11-16 Thread Omar Sandoval
On Thu, Nov 15, 2018 at 04:52:53PM +0800, Ming Lei wrote: > Once multi-page bvec is enabled, the last bvec may include more than one > page, this patch use bvec_last_segment() to truncate the bio. > > Cc: Dave Chinner > Cc: Kent Overstreet > Cc: Mike Snitzer > Cc: dm-devel@redhat.com > Cc:

Re: [dm-devel] [PATCH] nvme: allow ANA support to be independent of native multipathing

2018-11-16 Thread Christoph Hellwig
On Fri, Nov 16, 2018 at 10:40:40AM +0100, Hannes Reinecke wrote: >>> Introduce ability to always re-read ANA log page as required due to ANA >>> error and make current ANA state available via sysfs -- even if native >>> multipathing is disabled on the host (e.g. nvme_core.multipath=N). >> >> The

Re: [dm-devel] [PATCH] nvme: allow ANA support to be independent of native multipathing

2018-11-16 Thread Hannes Reinecke
On 11/16/18 10:49 AM, Christoph Hellwig wrote: On Fri, Nov 16, 2018 at 10:40:40AM +0100, Hannes Reinecke wrote: Introduce ability to always re-read ANA log page as required due to ANA error and make current ANA state available via sysfs -- even if native multipathing is disabled on the host

Re: [dm-devel] [PATCH V10 15/19] block: always define BIO_MAX_PAGES as 256

2018-11-16 Thread Omar Sandoval
On Thu, Nov 15, 2018 at 04:53:02PM +0800, Ming Lei wrote: > Now multi-page bvec can cover CONFIG_THP_SWAP, so we don't need to > increase BIO_MAX_PAGES for it. You mentioned to it in the cover letter, but this needs more explanation in the commit message. Why did CONFIG_THP_SWAP require > 256?

Re: [dm-devel] [PATCH V10 02/19] block: introduce bio_for_each_bvec()

2018-11-16 Thread Christoph Hellwig
> +static inline void __bio_advance_iter(struct bio *bio, struct bvec_iter > *iter, > + unsigned bytes, bool mp) I think these magic 'bool np' arguments and wrappers over wrapper don't help anyone to actually understand the code. I'd vote for removing as many

Re: [dm-devel] [PATCH V10 04/19] block: use bio_for_each_bvec() to map sg

2018-11-16 Thread Christoph Hellwig
> + if (!*sg) > + return sglist; > + else { No need for an else after an early return. -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH V10 06/19] fs/buffer.c: use bvec iterator to truncate the bio

2018-11-16 Thread Christoph Hellwig
Looks fine, Reviewed-by: Christoph Hellwig -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH V10 07/19] btrfs: use bvec_last_segment to get bio's last page

2018-11-16 Thread Christoph Hellwig
On Thu, Nov 15, 2018 at 04:52:54PM +0800, Ming Lei wrote: > index 2955a4ea2fa8..161e14b8b180 100644 > --- a/fs/btrfs/compression.c > +++ b/fs/btrfs/compression.c > @@ -400,8 +400,11 @@ blk_status_t btrfs_submit_compressed_write(struct inode > *inode, u64 start, > static u64 bio_end_offset(struct

Re: [dm-devel] [PATCH V10 08/19] btrfs: move bio_pages_all() to btrfs

2018-11-16 Thread Christoph Hellwig
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

Re: [dm-devel] [PATCH V10 11/19] bcache: avoid to use bio_for_each_segment_all() in bch_bio_alloc_pages()

2018-11-16 Thread Christoph Hellwig
> - bio_for_each_segment_all(bv, bio, i) { > + for (i = 0, bv = bio->bi_io_vec; i < bio->bi_vcnt; bv++) { This really needs a comment. Otherwise it looks fine to me. -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH V10 13/19] iomap & xfs: only account for new added page

2018-11-16 Thread Christoph Hellwig
I'd much rather have __bio_try_merge_page only do merges in the same page, and have a new __bio_try_merge_segment that does multi-page merges. This will keep the accounting a lot simpler. -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH V10 09/19] block: introduce bio_bvecs()

2018-11-16 Thread Christoph Hellwig
On Thu, Nov 15, 2018 at 04:52:56PM +0800, Ming Lei wrote: > There are still cases in which we need to use bio_bvecs() for get the > number of multi-page segment, so introduce it. The only user in your final tree seems to be the loop driver, and even that one only uses the helper for read/write

Re: [dm-devel] [PATCH 3/3] block: use a driver-specific handler for the "inflight" value

2018-11-16 Thread Mike Snitzer
On Fri, Nov 16 2018 at 4:11am -0500, Christoph Hellwig wrote: > On Fri, Nov 16, 2018 at 01:04:19AM +0100, Mikulas Patocka wrote: > > Device mapper was converted to percpu inflight counters. In order to > > display the correct values in the "inflight" sysfs file and in > > /proc/diskstats, we

Re: [dm-devel] [PATCH V10 14/19] block: enable multipage bvecs

2018-11-16 Thread Christoph Hellwig
> - > - 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; > + > +

Re: [dm-devel] [PATCH V10 17/19] block: don't use bio->bi_vcnt to figure out segment number

2018-11-16 Thread Christoph Hellwig
On Thu, Nov 15, 2018 at 04:53:04PM +0800, Ming Lei wrote: > It is wrong to use bio->bi_vcnt to figure out how many segments > there are in the bio even though CLONED flag isn't set on this bio, > because this bio may be splitted or advanced. > > So always use bio_segments() in

Re: [dm-devel] [PATCH V10 01/19] block: introduce multi-page page bvec helpers

2018-11-16 Thread Christoph Hellwig
> -#define bvec_iter_page(bvec, iter) \ > +#define mp_bvec_iter_page(bvec, iter)\ > (__bvec_iter_bvec((bvec), (iter))->bv_page) > > -#define bvec_iter_len(bvec, iter)\ > +#define mp_bvec_iter_len(bvec,

Re: [dm-devel] [PATCH V10 05/19] block: introduce bvec_last_segment()

2018-11-16 Thread Christoph Hellwig
> + unsigned last_page = total / PAGE_SIZE; > + > + if (last_page * PAGE_SIZE == total) Not sure it matters much with modern compilers, but generally we shit by PAGE_SHIFT instead of multiplying with or dividing by PAGE_SIZE. -- dm-devel mailing list dm-devel@redhat.com

Re: [dm-devel] [PATCH V10 19/19] block: kill BLK_MQ_F_SG_MERGE

2018-11-16 Thread Christoph Hellwig
On Thu, Nov 15, 2018 at 04:53:06PM +0800, Ming Lei wrote: > QUEUE_FLAG_NO_SG_MERGE has been killed, so kill BLK_MQ_F_SG_MERGE too. Looks fine, Reviewed-by: Christoph Hellwig -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH V10 00/19] block: support multi-page bvec

2018-11-16 Thread Christoph Hellwig
It seems like bi_phys_segments is still around of this series. Shouldn't it be superflous now? -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH V10 18/19] block: kill QUEUE_FLAG_NO_SG_MERGE

2018-11-16 Thread Christoph Hellwig
On Thu, Nov 15, 2018 at 04:53:05PM +0800, Ming Lei wrote: > Since bdced438acd83ad83a6c ("block: setup bi_phys_segments after splitting"), > physical segment number is mainly figured out in blk_queue_split() for > fast path, and the flag of BIO_SEG_VALID is set there too. > > Now only

Re: [dm-devel] [PATCH V10 18/19] block: kill QUEUE_FLAG_NO_SG_MERGE

2018-11-16 Thread Christoph Hellwig
On Thu, Nov 15, 2018 at 06:18:11PM -0800, Omar Sandoval wrote: > This commit message wasn't very clear. Is it the case that > QUEUE_FLAG_NO_SG_MERGE is no longer set by any drivers? I think he wants to say that not doing S/G merging is rather pointless with the current setup of the I/O path, as

Re: [dm-devel] nvme: allow ANA support to be independent of native multipathing

2018-11-16 Thread Mike Snitzer
On Fri, Nov 16 2018 at 4:14am -0500, Christoph Hellwig wrote: > On Thu, Nov 15, 2018 at 12:46:05PM -0500, Mike Snitzer wrote: > > Whether or not ANA is present is a choice of the target implementation; > > the host (and whether it supports multipathing) has _zero_ influence on > > this. If the

Re: [dm-devel] nvme: allow ANA support to be independent of native multipathing

2018-11-16 Thread Mike Snitzer
On Fri, Nov 16 2018 at 2:25am -0500, Hannes Reinecke wrote: > On 11/15/18 6:46 PM, Mike Snitzer wrote: > >Whether or not ANA is present is a choice of the target implementation; > >the host (and whether it supports multipathing) has _zero_ influence on > >this. If the target declares a path as

Re: [dm-devel] [PATCH 3/3] block: use a driver-specific handler for the "inflight" value

2018-11-16 Thread Jens Axboe
On 11/16/18 6:55 AM, Mike Snitzer wrote: > On Fri, Nov 16 2018 at 4:11am -0500, > Christoph Hellwig wrote: > >> On Fri, Nov 16, 2018 at 01:04:19AM +0100, Mikulas Patocka wrote: >>> Device mapper was converted to percpu inflight counters. In order to >>> display the correct values in the

[dm-devel] [PATCH v2] nvme: allow ANA support to be independent of native multipathing

2018-11-16 Thread Mike Snitzer
Whether or not ANA is present is a choice of the target implementation; the host (and whether it supports multipathing) has _zero_ influence on this. If the target declares a path as 'inaccessible' the path _is_ inaccessible to the host. As such, ANA support should be functional even if native

Re: [dm-devel] nvme: allow ANA support to be independent of native multipathing

2018-11-16 Thread Mike Snitzer
On Fri, Nov 16 2018 at 5:17am -0500, Christoph Hellwig wrote: > On Fri, Nov 16, 2018 at 11:06:32AM +0100, Hannes Reinecke wrote: > > Ok, so would you be happy with making ANA support configurable? > > I've looked a bit over the whole situation, and what I think we need > to do is: > > a)

Re: [dm-devel] nvme: allow ANA support to be independent of native multipathing

2018-11-16 Thread Laurence Oberman
On Fri, 2018-11-16 at 14:28 -0500, Mike Snitzer wrote: > On Fri, Nov 16 2018 at  5:17am -0500, > Christoph Hellwig wrote: > > > On Fri, Nov 16, 2018 at 11:06:32AM +0100, Hannes Reinecke wrote: > > > Ok, so would you be happy with making ANA support configurable? > > > > I've looked a bit over

Re: [dm-devel] [PATCH V10 15/19] block: always define BIO_MAX_PAGES as 256

2018-11-16 Thread Christoph Hellwig
> diff --git a/include/linux/bio.h b/include/linux/bio.h > index 5040e9a2eb09..277921ad42e7 100644 > --- a/include/linux/bio.h > +++ b/include/linux/bio.h > @@ -34,15 +34,7 @@ > #define BIO_BUG_ON > #endif > > -#ifdef CONFIG_THP_SWAP > -#if HPAGE_PMD_NR > 256 > -#define BIO_MAX_PAGES

Re: [dm-devel] [PATCH V10 00/19] block: support multi-page bvec

2018-11-16 Thread Ming Lei
On Fri, Nov 16, 2018 at 03:03:14PM +0100, Christoph Hellwig wrote: > It seems like bi_phys_segments is still around of this series. > Shouldn't it be superflous now? Even though multi-page bvec is supported, the segment number doesn't equal to the actual bvec count yet, for example, one bvec may