Re: [PATCH v3 37/49] fs/mpage: convert to bio_for_each_segment_all_sp()

On Thu, Aug 10, 2017 at 05:16:12AM -0700, Christoph Hellwig wrote: > > struct bio_vec *bv; > > + struct bvec_iter_all bia; > > int i; > > > > - bio_for_each_segment_all(bv, bio, i) { > > + bio_for_each_segment_all_sp(bv, bio, i, bia) { > > struct page *page =

Re: [PATCH v3 41/49] xfs: convert to bio_for_each_segment_all_sp()

On Tue, Aug 08, 2017 at 09:32:32AM -0700, Darrick J. Wong wrote: > On Tue, Aug 08, 2017 at 04:45:40PM +0800, Ming Lei wrote: > > Sure would be nice to have a changelog explaining why we're doing this. > > > Cc: "Darrick J. Wong" > > Cc: linux-...@vger.kernel.org > >

Re: [PATCH v3 20/49] block: introduce bio_for_each_segment_mp()

On Thu, Aug 10, 2017 at 05:11:10AM -0700, Christoph Hellwig wrote: > First: as mentioned in the previous patches I really hate the name > scheme with the _sp and _mp postfixes. > > To be clear and understandable we should always name the versions > that iterate over segments *segment* and the

Re: [RESEND PATCH 1/3] completion: Add support for initializing completion with lockdep_map

On Wed, 2017-10-18 at 18:38 +0900, Byungchul Park wrote: > Sometimes, we want to initialize completions with sparate lockdep maps > to assign lock classes under control. For example, the workqueue code > manages lockdep maps, as it can classify lockdep maps properly. > Provided a function for that

Re: [PATCH v3 11/49] btrfs: avoid access to .bi_vcnt directly

On Thu, Aug 10, 2017 at 04:29:59AM -0700, Christoph Hellwig wrote: > > +static unsigned int get_bio_pages(struct bio *bio) > > +{ > > + unsigned i; > > + struct bio_vec *bv; > > + > > + bio_for_each_segment_all(bv, bio, i) > > + ; > > + > > + return i; > > +} > >

Re: [PATCH v3 09/49] block: comment on bio_iov_iter_get_pages()

On Thu, Aug 10, 2017 at 04:28:05AM -0700, Christoph Hellwig wrote: > > + * The hacking way of using bvec table as page pointer array is safe > > + * even after multipage bvec is introduced because that space can be > > + * thought as unused by bio_add_page(). > > I'm not sure what value this

Re: [PATCH v3 07/49] bcache: comment on direct access to bvec table

On Thu, Aug 10, 2017 at 04:26:03AM -0700, Christoph Hellwig wrote: > I think all this bcache code needs bigger attention. For one > bio_alloc_pages is only used in bcache, so we should move it in there. Looks a good idea. > > Second the way bio_alloc_pages is currently written looks

[PATCH 2/2] nbd: don't start req until after the dead connection logic

From: Josef Bacik We can end up sleeping for a while waiting for the dead timeout, which means we could get the per request timer to fire. We did handle this case, but if the dead timeout happened right after we submitted we'd either tear down the connection or possibly requeue

[PATCH 1/2] nbd: wait uninterruptible for the dead timeout

From: Josef Bacik If we have a pending signal or the user kills their application then it'll bring down the whole device, which is less than awesome. Instead wait uninterruptible for the dead timeout so we're sure we gave it our best shot. Fixes: 560bc4b39952 ("nbd: handle dead

Re: [PATCH 1/1] [RFC] blk-mq: fix queue stalling on shared hctx restart

On Wed, 2017-10-18 at 12:22 +0200, Roman Pen wrote: > the patch below fixes queue stalling when shared hctx marked for restart > (BLK_MQ_S_SCHED_RESTART bit) but q->shared_hctx_restart stays zero. The > root cause is that hctxs are shared between queues, but 'shared_hctx_restart' > belongs to the

[PATCH] block: Fix a race between blk_cleanup_queue() and timeout handling

Make sure that if the timeout timer fires after a queue has been marked "dying" that the affected requests are finished. Reported-by: chenxiang (M) Fixes: commit 287922eb0b18 ("block: defer timeouts to a workqueue") Signed-off-by: Bart Van Assche

Re: [PATCH 9/9] bsg: split handling of SCSI CDBs vs transport requeues

Hey Christoph, better late than never I guess. On Tue, Oct 03, 2017 at 12:48:45PM +0200, Christoph Hellwig wrote: > The current BSG design tries to shoe-horn the transport-specific passthrough > commands into the overall framework for SCSI passthrough requests. This > has a couple problems: > >

Re: [PATCH 17/17] nvme: also expose the namespace identification sysfs files for mpath nodes

I like the fact that we're expose only the device nodes. Reviewed-by: Sagi Grimberg

Re: [PATCH 07/17] nvme: use ida_simple_{get,remove} for the controller instance

On Thu, Oct 19, 2017 at 09:14:27AM -0600, Keith Busch wrote: > This is a good cleanup, and I'd support this patch going in ahead of > this series on its own if you want to apply to 4.15 immediately. Ok, will do.

Re: [PATCH 07/17] nvme: use ida_simple_{get,remove} for the controller instance

This is a good cleanup, and I'd support this patch going in ahead of this series on its own if you want to apply to 4.15 immediately. Reviewed-by: Keith Busch

Re: Fix false positive by LOCKDEP_CROSSRELEASE

On Thu, 2017-10-19 at 10:57 +0900, Byungchul Park wrote: > On Wed, Oct 18, 2017 at 02:29:56PM +, Bart Van Assche wrote: > > On Wed, 2017-10-18 at 18:38 +0900, Byungchul Park wrote: > > > Several false positives were reported, so I tried to fix them. > > > > > > It would be appreciated if you

Re: [PATCH 03/17] block: provide a direct_make_request helper

+/** + * direct_make_request - hand a buffer directly to its device driver for I/O + * @bio: The bio describing the location in memory and on the device. + * + * This function behaves like generic_make_request(), but does not protect + * against recursion. Must only be used if the called

Re: [PATCH 15/17] nvme: track shared namespaces

On Thu, Oct 19, 2017 at 02:06:07PM +0300, Sagi Grimberg wrote: > Christoph, > >> static void nvme_free_ns(struct kref *kref) >> { >> struct nvme_ns *ns = container_of(kref, struct nvme_ns, kref); >> + if (ns->head) >> +nvme_put_ns_head(ns->head); >> + > > When can we not

Re: [PATCH 10/17] nvme: switch controller refcounting to use struct device

On Thu, Oct 19, 2017 at 01:33:55PM +0300, Sagi Grimberg wrote: >> Well, the ctrl device integration is what enables us to do this. >> Before that the ctrl refcount could have reached null by the time >> we call the delete_ctrl method. > > OK, maybe a change log mention then? I thought I did that,

Re: [PATCH 03/17] block: provide a direct_make_request helper

On Thu, Oct 19, 2017 at 01:35:58PM +0300, Sagi Grimberg wrote: > >> +/** >> + * direct_make_request - hand a buffer directly to its device driver for I/O >> + * @bio: The bio describing the location in memory and on the device. >> + * >> + * This function behaves like generic_make_request(), but

Re: [PATCH 06/17] block: introduce GENHD_FL_HIDDEN

On 10/18/2017 06:52 PM, Christoph Hellwig wrote: > With this flag a driver can create a gendisk that can be used for I/O > submission inside the kernel, but which is not registered as user > facing block device. This will be useful for the NVMe multipath > implementation. > > Signed-off-by:

Re: [PATCH V8 00/14] mmc: Add Command Queue support

On 18/10/17 09:16, Adrian Hunter wrote: > On 11/10/17 16:58, Ulf Hansson wrote: >> On 11 October 2017 at 14:58, Adrian Hunter wrote: >>> On 11/10/17 15:13, Ulf Hansson wrote: On 10 October 2017 at 15:31, Adrian Hunter wrote: > On

Re: [PATCH 15/17] nvme: track shared namespaces

Christoph, static void nvme_free_ns(struct kref *kref) { struct nvme_ns *ns = container_of(kref, struct nvme_ns, kref); + if (ns->head) + nvme_put_ns_head(ns->head); + When can we not have a ns-head set? AFAICT, if nvme_alloc_ns succeeded, we have it set don't

Re: [PATCH 03/17] block: provide a direct_make_request helper

+/** + * direct_make_request - hand a buffer directly to its device driver for I/O + * @bio:  The bio describing the location in memory and on the device. + * + * This function behaves like generic_make_request(), but does not protect + * against recursion.  Must only be used if the called

Re: [PATCH 03/17] block: provide a direct_make_request helper

+/** + * direct_make_request - hand a buffer directly to its device driver for I/O + * @bio: The bio describing the location in memory and on the device. + * + * This function behaves like generic_make_request(), but does not protect + * against recursion. Must only be used if the called

Re: [PATCH 10/17] nvme: switch controller refcounting to use struct device

By the time we call ->delete_ctrl in nvmf_create_ctrl we must still have that reference because we are still in a ->write call and thus ->release can't have been called yet. Thanks for the clarification. Is it worth a preparatory patch to change that before the ctrl device integration for

Re: [PATCH 10/17] nvme: switch controller refcounting to use struct device

On Thu, Oct 19, 2017 at 01:02:59PM +0300, Sagi Grimberg wrote: >> By the time we call ->delete_ctrl in nvmf_create_ctrl we must still >> have that reference because we are still in a ->write call and thus >> ->release can't have been called yet. > > Thanks for the clarification. Is it worth a

Re: [PATCH 10/17] nvme: switch controller refcounting to use struct device

I don't think the fabrics device does not help us to keep the ctrl allocated until we finish removal. All fabrics drivers grab an extra reference during ->create_ctrl, which we will drop when releasing the file descriptor that we used to create the device. By the time we call ->delete_ctrl

Re: [PATCH 05/17] block: don't look at the struct device dev_t in disk_devt

Looks good, Reviewed-by: Johannes Thumshirn -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham

Re: [PATCH 06/17] block: introduce GENHD_FL_HIDDEN

Looks good, Reviewed-by: Johannes Thumshirn -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham

Re: [PATCH 17/17] nvme: also expose the namespace identification sysfs files for mpath nodes

Looks good, Reviewed-by: Johannes Thumshirn -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham

Re: [PATCH 10/17] nvme: switch controller refcounting to use struct device

On Thu, Oct 19, 2017 at 10:31:20AM +0300, Sagi Grimberg wrote: > I don't think the fabrics device does not help us to keep the ctrl > allocated until we finish removal. All fabrics drivers grab an extra reference during ->create_ctrl, which we will drop when releasing the file descriptor that we

Re: [PATCH 15/17] nvme: track shared namespaces

Looks good, Reviewed-by: Johannes Thumshirn -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham

Re: [PATCH 10/17] nvme: switch controller refcounting to use struct device

- if (!kref_get_unless_zero(>ctrl.kref)) - return -EBUSY; + nvme_get_ctrl(>ctrl); Given that we take this reference before we are protected with the state change I think this should still be get_unless_zero. Because we now refcount the device we must have a

Re: [PATCH 11/17] nvme: get rid of nvme_ctrl_list

On Thu, Oct 19, 2017 at 09:22:09AM +0200, Johannes Thumshirn wrote: > I'm not sure if the removel of the major number module_param is good > from an ABI POV. This could lead to some unexpected results for those > poor souls that used it. It's a completely broken interface, so I have no pity for

Re: [PATCH 12/17] nvme: check for a live controller in nvme_dev_open

Looks good, Reviewed-by: Johannes Thumshirn -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham

Re: [PATCH 11/17] nvme: get rid of nvme_ctrl_list

I'm not sure if the removel of the major number module_param is good from an ABI POV. This could lead to some unexpected results for those poor souls that used it. Apart from that, Reviewed-by: Johannes Thumshirn -- Johannes Thumshirn

Re: [PATCH 10/17] nvme: switch controller refcounting to use struct device

On Thu, Oct 19, 2017 at 10:17:01AM +0300, Sagi Grimberg wrote: > > >> -if (!kref_get_unless_zero(>ctrl.kref)) >> -return -EBUSY; >> +nvme_get_ctrl(>ctrl); > > Given that we take this reference before we are protected with > the state change I think this should still be

Re: [PATCH 12/17] nvme: check for a live controller in nvme_dev_open

Looks good, Reviewed-by: Sagi Grimberg

Re: [PATCH 11/17] nvme: get rid of nvme_ctrl_list

Looks good! Reviewed-by: Sagi Grimberg

Re: [PATCH 10/17] nvme: switch controller refcounting to use struct device

- if (!kref_get_unless_zero(>ctrl.kref)) - return -EBUSY; + nvme_get_ctrl(>ctrl); Given that we take this reference before we are protected with the state change I think this should still be get_unless_zero. Maybe we can introduce get_device_unless_zero() for this?

Re: [PATCH 13/17] nvme: track subsystems

> I'm having some trouble with this one. The device_initialize() initializes > the kobj's reference counter, and then the device_add() takes another > reference on it. > > The teardown, though, only calls put_device(). Where's the call to > device_del() supposed to go that ultimately drops the

[PATCH v2 4/4] lockdep: Assign a lock_class per gendisk used for wait_for_completion()

Darrick and Dave Chinner posted the following warning: > == > WARNING: possible circular locking dependency detected > 4.14.0-rc1-fixes #1 Tainted: GW > -- > loop0/31693 is trying to

[PATCH v2 3/4] genhd.h: Remove trailing white space

Trailing white space is not accepted in kernel coding style. Remove them. Signed-off-by: Byungchul Park --- include/linux/genhd.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/linux/genhd.h b/include/linux/genhd.h index ea652bf..6d85a75

[PATCH v2 2/4] lockdep: Remove unnecessary acquisitions wrt workqueue flush

The workqueue added manual acquisitions to catch deadlock cases. Now crossrelease was introduced, some of those are redundant, since wait_for_completion() already includes the acquisition for itself. Removed it. Signed-off-by: Byungchul Park --- include/linux/workqueue.h

[PATCH v2 1/4] completion: Add support for initializing completion with lockdep_map

Sometimes, we want to initialize completions with sparate lockdep maps to assign lock classes under control. For example, the workqueue code manages lockdep maps, as it can classify lockdep maps properly. Provided a function for that purpose. Signed-off-by: Byungchul Park

Re: [PATCH 09/17] nvme: simplify nvme_open

Reviewed-by: Sagi Grimberg

Re: [PATCH 08/17] nvme: use kref_get_unless_zero in nvme_find_get_ns

Reviewed-by: Sagi Grimberg

Re: [PATCH 09/17] nvme: simplify nvme_open

Looks good, Reviewed-by: Johannes Thumshirn -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham

Re: [PATCH 08/17] nvme: use kref_get_unless_zero in nvme_find_get_ns

Looks good, Reviewed-by: Johannes Thumshirn -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham

Re: [PATCH 07/17] nvme: use ida_simple_{get,remove} for the controller instance

Looks good, Reviewed-by: Johannes Thumshirn -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham

Re: high overhead of functions blkg_*stats_* in bfq

> Il giorno 19 ott 2017, alle ore 08:46, Paolo Valente > ha scritto: > ... >>> >>> blkgs are, but the blkg_stat objects passed to the blkg_*stat_* >>> functions by bfq are not. In particular, these objects are contained >>> in bfq_group objects. I talked partial

Re: [PATCH 02/17] block: add REQ_DRV bit

Looks good, Reviewed-by: Johannes Thumshirn -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham

Re: [PATCH 01/17] block: move REQ_NOWAIT

Looks good, Reviewed-by: Johannes Thumshirn -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham