* Re: split scsi passthrough fields out of struct request V2
From: Bart Van Assche @ 2017-02-03 0:20 UTC (permalink / raw)
To: snitzer@redhat.com; +Cc: hch@lst.de, linux-block@vger.kernel.org, axboe@fb.com
In-Reply-To: <20170202211014.GA27604@redhat.com>
On Thu, 2017-02-02 at 16:10 -0500, Mike Snitzer wrote:
> Care to try moving the dm_get(md) at the end of dm_start_request() to
> the beginning of dm_start_request() and report back on whether it helps
> at all?
Hello Mike,
Sorry but I don't see how that could make a difference. While we are at it:
since dm_start_request() calls dm_get() and since rq_completed() calls
dm_put(), calls to these two functions should always be paired. There is
only one dm_start_request() call in the dm-mq code, namely the one in
dm_mq_queue_rq(). However, if map_request() returns DM_MAPIO_REQUEUE then
rq_completed() is called twice: a first time by
dm_requeue_original_request() and a second time by the dm_mq_queue_rq().
Do you agree with this?
Thanks,
Bart.=
^ permalink raw reply
* Re: split scsi passthrough fields out of struct request V2
From: Mike Snitzer @ 2017-02-03 0:42 UTC (permalink / raw)
To: Bart Van Assche; +Cc: hch@lst.de, linux-block@vger.kernel.org, axboe@fb.com
In-Reply-To: <1486081190.2816.26.camel@sandisk.com>
On Thu, Feb 02 2017 at 7:20pm -0500,
Bart Van Assche <Bart.VanAssche@sandisk.com> wrote:
> On Thu, 2017-02-02 at 16:10 -0500, Mike Snitzer wrote:
> > Care to try moving the dm_get(md) at the end of dm_start_request() to
> > the beginning of dm_start_request() and report back on whether it helps
> > at all?
>
> Hello Mike,
>
> Sorry but I don't see how that could make a difference.
Yeah, I thought about it further after I suggested it and agree that it
shouldn't make a difference (request isn't actually issued during
dm_start_request).
> While we are at it:
> since dm_start_request() calls dm_get() and since rq_completed() calls
> dm_put(), calls to these two functions should always be paired. There is
> only one dm_start_request() call in the dm-mq code, namely the one in
> dm_mq_queue_rq(). However, if map_request() returns DM_MAPIO_REQUEUE then
> rq_completed() is called twice: a first time by
> dm_requeue_original_request() and a second time by the dm_mq_queue_rq().
> Do you agree with this?
Not seeing it.
DM_MAPIO_DELAY_REQUEUE will call dm_requeue_original_request(), but
dm_mq_queue_rq() won't call rq_completed() in that case.
And for DM_MAPIO_REQUEUE dm_mq_queue_rq() will call rq_completed().
^ permalink raw reply
* Re: [PATCH 1/9] virtio_pci: remove struct virtio_pci_vq_info
From: Jason Wang @ 2017-02-03 7:54 UTC (permalink / raw)
To: Christoph Hellwig, mst
Cc: axboe, pbonzini, virtualization, linux-block, linux-kernel
In-Reply-To: <1485504997-17584-2-git-send-email-hch@lst.de>
On 2017年01月27日 16:16, Christoph Hellwig wrote:
> We don't really need struct virtio_pci_vq_info, as most field in there
> are redundant:
>
> - the vq backpointer is not strictly neede to start with
> - the entry in the vqs list is not needed - the generic virtqueue already
> has list, we only need to check if it has a callback to get the same
> semantics
> - we can use a simple array to look up the MSI-X vec if needed.
> - That simple array now also duoble serves to replace the per_vq_vectors
> flag
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> drivers/virtio/virtio_pci_common.c | 117 +++++++++++--------------------------
> drivers/virtio/virtio_pci_common.h | 25 +-------
> drivers/virtio/virtio_pci_legacy.c | 6 +-
> drivers/virtio/virtio_pci_modern.c | 6 +-
> 4 files changed, 39 insertions(+), 115 deletions(-)
>
> diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
> index 186cbab..a3376731 100644
> --- a/drivers/virtio/virtio_pci_common.c
> +++ b/drivers/virtio/virtio_pci_common.c
> @@ -62,16 +62,13 @@ static irqreturn_t vp_config_changed(int irq, void *opaque)
> static irqreturn_t vp_vring_interrupt(int irq, void *opaque)
> {
> struct virtio_pci_device *vp_dev = opaque;
> - struct virtio_pci_vq_info *info;
> irqreturn_t ret = IRQ_NONE;
> - unsigned long flags;
> + struct virtqueue *vq;
>
> - spin_lock_irqsave(&vp_dev->lock, flags);
> - list_for_each_entry(info, &vp_dev->virtqueues, node) {
> - if (vring_interrupt(irq, info->vq) == IRQ_HANDLED)
> + list_for_each_entry(vq, &vp_dev->vdev.vqs, list) {
> + if (vq->callback && vring_interrupt(irq, vq) == IRQ_HANDLED)
The check of vq->callback seems redundant, we will check it soon in
vring_interrupt().
Thanks
^ permalink raw reply
* Re: [PATCH 2/9] virtio_pci: use shared interrupts for virtqueues
From: Jason Wang @ 2017-02-03 7:54 UTC (permalink / raw)
To: Christoph Hellwig, mst
Cc: axboe, pbonzini, virtualization, linux-block, linux-kernel
In-Reply-To: <1485504997-17584-3-git-send-email-hch@lst.de>
On 2017年01月27日 16:16, Christoph Hellwig wrote:
> + snprintf(vp_dev->msix_names[i + 1],
> + sizeof(*vp_dev->msix_names), "%s-%s",
> dev_name(&vp_dev->vdev.dev), names[i]);
> err = request_irq(pci_irq_vector(vp_dev->pci_dev, msix_vec),
> - vring_interrupt, 0,
> - vp_dev->msix_names[msix_vec],
> - vqs[i]);
> + vring_interrupt, IRQF_SHARED,
> + vp_dev->msix_names[i + 1], vqs[i]);
Do we need to check per_vq_vectors before dereferencing msix_names[i + 1] ?
Thanks
^ permalink raw reply
* Re: [PATCH 3/9] virtio_pci: don't duplicate the msix_enable flag in struct pci_dev
From: Jason Wang @ 2017-02-03 7:56 UTC (permalink / raw)
To: Christoph Hellwig, mst
Cc: axboe, pbonzini, virtualization, linux-block, linux-kernel
In-Reply-To: <1485504997-17584-4-git-send-email-hch@lst.de>
On 2017年01月27日 16:16, Christoph Hellwig wrote:
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
Reviewed-by: Jason Wang <jasowang@redhat.com>
> drivers/virtio/virtio_pci_common.c | 5 ++---
> drivers/virtio/virtio_pci_common.h | 2 --
> drivers/virtio/virtio_pci_legacy.c | 2 +-
> drivers/virtio/virtio_pci_modern.c | 2 +-
> include/uapi/linux/virtio_pci.h | 2 +-
> 5 files changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
> index 5880e86..9c4ad7d3f 100644
> --- a/drivers/virtio/virtio_pci_common.c
> +++ b/drivers/virtio/virtio_pci_common.c
> @@ -125,7 +125,7 @@ void vp_del_vqs(struct virtio_device *vdev)
>
> vp_remove_vqs(vdev);
>
> - if (vp_dev->msix_enabled) {
> + if (vp_dev->pci_dev->msix_enabled) {
> for (i = 0; i < vp_dev->msix_vectors; i++)
> free_cpumask_var(vp_dev->msix_affinity_masks[i]);
>
> @@ -245,7 +245,6 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
> allocated_vectors++;
> }
>
> - vp_dev->msix_enabled = 1;
> return 0;
>
> out_remove_vqs:
> @@ -341,7 +340,7 @@ int vp_set_vq_affinity(struct virtqueue *vq, int cpu)
> if (!vq->callback)
> return -EINVAL;
>
> - if (vp_dev->msix_enabled) {
> + if (vp_dev->pci_dev->msix_enabled) {
> int vec = vp_dev->msix_vector_map[vq->index];
> struct cpumask *mask = vp_dev->msix_affinity_masks[vec];
> unsigned int irq = pci_irq_vector(vp_dev->pci_dev, vec);
> diff --git a/drivers/virtio/virtio_pci_common.h b/drivers/virtio/virtio_pci_common.h
> index 8559386..217ca87 100644
> --- a/drivers/virtio/virtio_pci_common.h
> +++ b/drivers/virtio/virtio_pci_common.h
> @@ -64,8 +64,6 @@ struct virtio_pci_device {
> /* the IO mapping for the PCI config space */
> void __iomem *ioaddr;
>
> - /* MSI-X support */
> - int msix_enabled;
> cpumask_var_t *msix_affinity_masks;
> /* Name strings for interrupts. This size should be enough,
> * and I'm too lazy to allocate each name separately. */
> diff --git a/drivers/virtio/virtio_pci_legacy.c b/drivers/virtio/virtio_pci_legacy.c
> index 47292da..2ab6aee 100644
> --- a/drivers/virtio/virtio_pci_legacy.c
> +++ b/drivers/virtio/virtio_pci_legacy.c
> @@ -165,7 +165,7 @@ static void del_vq(struct virtqueue *vq)
>
> iowrite16(vq->index, vp_dev->ioaddr + VIRTIO_PCI_QUEUE_SEL);
>
> - if (vp_dev->msix_enabled) {
> + if (vp_dev->pci_dev->msix_enabled) {
> iowrite16(VIRTIO_MSI_NO_VECTOR,
> vp_dev->ioaddr + VIRTIO_MSI_QUEUE_VECTOR);
> /* Flush the write out to device */
> diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
> index 00e6fc1..e5ce310 100644
> --- a/drivers/virtio/virtio_pci_modern.c
> +++ b/drivers/virtio/virtio_pci_modern.c
> @@ -412,7 +412,7 @@ static void del_vq(struct virtqueue *vq)
>
> vp_iowrite16(vq->index, &vp_dev->common->queue_select);
>
> - if (vp_dev->msix_enabled) {
> + if (vp_dev->pci_dev->msix_enabled) {
> vp_iowrite16(VIRTIO_MSI_NO_VECTOR,
> &vp_dev->common->queue_msix_vector);
> /* Flush the write out to device */
> diff --git a/include/uapi/linux/virtio_pci.h b/include/uapi/linux/virtio_pci.h
> index 90007a1..15b4385 100644
> --- a/include/uapi/linux/virtio_pci.h
> +++ b/include/uapi/linux/virtio_pci.h
> @@ -79,7 +79,7 @@
> * configuration space */
> #define VIRTIO_PCI_CONFIG_OFF(msix_enabled) ((msix_enabled) ? 24 : 20)
> /* Deprecated: please use VIRTIO_PCI_CONFIG_OFF instead */
> -#define VIRTIO_PCI_CONFIG(dev) VIRTIO_PCI_CONFIG_OFF((dev)->msix_enabled)
> +#define VIRTIO_PCI_CONFIG(dev) VIRTIO_PCI_CONFIG_OFF((dev)->pci_dev->msix_enabled)
>
> /* Virtio ABI version, this must match exactly */
> #define VIRTIO_PCI_ABI_VERSION 0
^ permalink raw reply
* [REGRESSION v4.10-rc1] blkdev_issue_zeroout() returns -EREMOTEIO on the first call for SCSI device that doesn't support WRITE SAME
From: Junichi Nomura @ 2017-02-03 7:55 UTC (permalink / raw)
To: linux-block@vger.kernel.org, linux-scsi
I found following ext4 error occurs on a certain storage since v4.10-rc1:
EXT4-fs (sdc1): Delayed block allocation failed for inode 12 at logical offset 100 with max blocks 2 with error 121
EXT4-fs (sdc1): This should not happen!! Data will be lost
Error 121 (EREMOTEIO) was returned from blkdev_issue_zeroout().
That came from sd driver because WRITE SAME was sent to the device
which didn't support it.
The problem was introduced by commit e73c23ff736e ("block: add async
variant of blkdev_issue_zeroout"). Before the commit, blkdev_issue_zeroout
fell back to normal zero writing when WRITE SAME failed and it seems
sd driver's heuristics depends on that behaviour.
Below is a band-aid fix to restore the fallback behaviour for sd. Although
there should be better fix as retrying blindly is not a good idea...
v4.10-rc6:
# cat /sys/block/sdc/queue/write_same_max_bytes
33553920
# fallocate -v -z -l 512 /dev/sdc1
fallocate: fallocate failed: Remote I/O error
# cat /sys/block/sdc/queue/write_same_max_bytes
0
# fallocate -v -z -l 512 /dev/sdc1
# echo $?
0
v4.9 or v4.10-rc6 + this patch:
# grep . /sys/block/sdc/queue/write_same_max_bytes
33553920
# fallocate -v -z -l 512 /dev/sdc1
# echo $?
0
# grep . /sys/block/sdc/queue/write_same_max_bytes
0
diff --git a/block/blk-lib.c b/block/blk-lib.c
index f8c82a9..8e53474 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -360,6 +360,7 @@ int blkdev_issue_zeroout(struct block_device *bdev, sector_t sector,
sector_t nr_sects, gfp_t gfp_mask, bool discard)
{
int ret;
+ int pass = 0;
struct bio *bio = NULL;
struct blk_plug plug;
@@ -369,6 +370,7 @@ int blkdev_issue_zeroout(struct block_device *bdev, sector_t sector,
return 0;
}
+ retry_other_method:
blk_start_plug(&plug);
ret = __blkdev_issue_zeroout(bdev, sector, nr_sects, gfp_mask,
&bio, discard);
@@ -378,6 +380,11 @@ int blkdev_issue_zeroout(struct block_device *bdev, sector_t sector,
}
blk_finish_plug(&plug);
+ if (ret && pass++ == 0) {
+ bio = NULL;
+ goto retry_other_method;
+ }
+
return ret;
}
EXPORT_SYMBOL(blkdev_issue_zeroout);
--
Jun'ichi Nomura, NEC Corporation / NEC Solution Innovators, Ltd.
^ permalink raw reply related
* Re: [PATCH 4/9] virtio_pci: simplify MSI-X setup
From: Jason Wang @ 2017-02-03 7:57 UTC (permalink / raw)
To: Christoph Hellwig, mst
Cc: axboe, pbonzini, virtualization, linux-block, linux-kernel
In-Reply-To: <1485504997-17584-5-git-send-email-hch@lst.de>
On 2017年01月27日 16:16, Christoph Hellwig wrote:
> Try to grab the MSI-X vectors early and fall back to the shared one
> before doing lots of allocations.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
Reviewed-by: Jason Wang <jasowang@redhat.com>
> drivers/virtio/virtio_pci_common.c | 58 +++++++++++++++++++-------------------
> 1 file changed, 29 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
> index 9c4ad7d3f..74ff74c0 100644
> --- a/drivers/virtio/virtio_pci_common.c
> +++ b/drivers/virtio/virtio_pci_common.c
> @@ -142,32 +142,39 @@ void vp_del_vqs(struct virtio_device *vdev)
> }
>
> static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
> - struct virtqueue *vqs[],
> - vq_callback_t *callbacks[],
> - const char * const names[],
> - bool per_vq_vectors)
> + struct virtqueue *vqs[], vq_callback_t *callbacks[],
> + const char * const names[])
> {
> struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> const char *name = dev_name(&vp_dev->vdev.dev);
> int i, err = -ENOMEM, allocated_vectors, nvectors;
> + bool shared = false;
> u16 msix_vec;
>
> - if (per_vq_vectors) {
> - /* Best option: one for change interrupt, one per vq. */
> - nvectors = 1;
> - for (i = 0; i < nvqs; ++i)
> - if (callbacks[i])
> - ++nvectors;
> - } else {
> - /* Second best: one for change, shared for all vqs. */
> + nvectors = 1;
> + for (i = 0; i < nvqs; i++) {
> + if (callbacks[i])
> + nvectors++;
> + }
> +
> + /* Try one vector per queue first. */
> + err = pci_alloc_irq_vectors(vp_dev->pci_dev, nvectors, nvectors,
> + PCI_IRQ_MSIX);
> + if (err < 0) {
> + /* Fallback to one vector for config, one shared for queues. */
> + shared = true;
> nvectors = 2;
> + err = pci_alloc_irq_vectors(vp_dev->pci_dev, nvectors, nvectors,
> + PCI_IRQ_MSIX);
> + if (err < 0)
> + return err;
> }
>
> vp_dev->msix_vectors = nvectors;
> vp_dev->msix_names = kmalloc_array(nvectors,
> sizeof(*vp_dev->msix_names), GFP_KERNEL);
> if (!vp_dev->msix_names)
> - return err;
> + goto out_free_irq_vectors;
>
> vp_dev->msix_affinity_masks = kcalloc(nvectors,
> sizeof(*vp_dev->msix_affinity_masks), GFP_KERNEL);
> @@ -180,18 +187,13 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
> goto out_free_msix_affinity_masks;
> }
>
> - err = pci_alloc_irq_vectors(vp_dev->pci_dev, nvectors, nvectors,
> - PCI_IRQ_MSIX);
> - if (err < 0)
> - goto out_free_msix_affinity_masks;
> -
> /* Set the vector used for configuration */
> snprintf(vp_dev->msix_names[0], sizeof(*vp_dev->msix_names),
> "%s-config", name);
> err = request_irq(pci_irq_vector(vp_dev->pci_dev, 0), vp_config_changed,
> 0, vp_dev->msix_names[0], vp_dev);
> if (err)
> - goto out_free_irq_vectors;
> + goto out_free_msix_affinity_masks;
>
> /* Verify we had enough resources to assign the vector */
> if (vp_dev->config_vector(vp_dev, 0) == VIRTIO_MSI_NO_VECTOR) {
> @@ -241,7 +243,11 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
> }
> vp_dev->msix_vector_map[i] = msix_vec;
>
> - if (per_vq_vectors)
> + /*
> + * Use a different vector for each queue if they are available,
> + * else share the same vector for all VQs.
> + */
> + if (!shared)
> allocated_vectors++;
> }
>
> @@ -254,8 +260,6 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
> vp_dev->config_vector(vp_dev, VIRTIO_MSI_NO_VECTOR);
> out_free_config_irq:
> free_irq(pci_irq_vector(vp_dev->pci_dev, 0), vp_dev);
> -out_free_irq_vectors:
> - pci_free_irq_vectors(vp_dev->pci_dev);
> out_free_msix_affinity_masks:
> for (i = 0; i < nvectors; i++) {
> if (vp_dev->msix_affinity_masks[i])
> @@ -264,6 +268,8 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
> kfree(vp_dev->msix_affinity_masks);
> out_free_msix_names:
> kfree(vp_dev->msix_names);
> +out_free_irq_vectors:
> + pci_free_irq_vectors(vp_dev->pci_dev);
> return err;
> }
>
> @@ -308,15 +314,9 @@ int vp_find_vqs(struct virtio_device *vdev, unsigned nvqs,
> {
> int err;
>
> - /* Try MSI-X with one vector per queue. */
> - err = vp_find_vqs_msix(vdev, nvqs, vqs, callbacks, names, true);
> - if (!err)
> - return 0;
> - /* Fallback: MSI-X with one vector for config, one shared for queues. */
> - err = vp_find_vqs_msix(vdev, nvqs, vqs, callbacks, names, false);
> + err = vp_find_vqs_msix(vdev, nvqs, vqs, callbacks, names);
> if (!err)
> return 0;
> - /* Finally fall back to regular interrupts. */
> return vp_find_vqs_intx(vdev, nvqs, vqs, callbacks, names);
> }
>
^ permalink raw reply
* Re: [PATCH 5/9] virtio: allow drivers to request IRQ affinity when creating VQs
From: Jason Wang @ 2017-02-03 8:01 UTC (permalink / raw)
To: Christoph Hellwig, mst
Cc: axboe, pbonzini, virtualization, linux-block, linux-kernel
In-Reply-To: <1485504997-17584-6-git-send-email-hch@lst.de>
On 2017年01月27日 16:16, Christoph Hellwig wrote:
> Add a struct irq_affinity pointer to the find_vqs methods, which if set
> is used to tell the PCI layer to create the MSI-X vectors for our I/O
> virtqueues with the proper affinity from the start. Compared to after
> the fact affinity hints this gives us an instantly working setup and
> allows to allocate the irq descritors node-local and avoid interconnect
> traffic. Last but not least this will allow blk-mq queues are created
> based on the interrupt affinity for storage drivers.
>
> Signed-off-by: Christoph Hellwig<hch@lst.de>
> ---
Reviewed-by: Jason Wang <jasowang@redhat.com>
^ permalink raw reply
* Re: [PATCH 6/9] virtio: provide a method to get the IRQ affinity mask for a virtqueue
From: Jason Wang @ 2017-02-03 8:02 UTC (permalink / raw)
To: Christoph Hellwig, mst
Cc: axboe, pbonzini, virtualization, linux-block, linux-kernel
In-Reply-To: <1485504997-17584-7-git-send-email-hch@lst.de>
On 2017年01月27日 16:16, Christoph Hellwig wrote:
> This basically passed up the pci_irq_get_affinity information through
> virtio through an optional get_vq_affinity method. It is only implemented
> by the PCI backend for now, and only when we use per-virtqueue IRQs.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
Reviewed-by: Jason Wang <jasowang@redhat.com>
> drivers/virtio/virtio_pci_common.c | 11 +++++++++++
> drivers/virtio/virtio_pci_common.h | 2 ++
> drivers/virtio/virtio_pci_legacy.c | 1 +
> drivers/virtio/virtio_pci_modern.c | 2 ++
> include/linux/virtio_config.h | 3 +++
> 5 files changed, 19 insertions(+)
>
> diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
> index c244212..31ba259 100644
> --- a/drivers/virtio/virtio_pci_common.c
> +++ b/drivers/virtio/virtio_pci_common.c
> @@ -361,6 +361,17 @@ int vp_set_vq_affinity(struct virtqueue *vq, int cpu)
> return 0;
> }
>
> +const struct cpumask *vp_get_vq_affinity(struct virtio_device *vdev, int index)
> +{
> + struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> + unsigned int *map = vp_dev->msix_vector_map;
> +
> + if (!map || map[index] == VIRTIO_MSI_NO_VECTOR)
> + return NULL;
> +
> + return pci_irq_get_affinity(vp_dev->pci_dev, map[index]);
> +}
> +
> #ifdef CONFIG_PM_SLEEP
> static int virtio_pci_freeze(struct device *dev)
> {
> diff --git a/drivers/virtio/virtio_pci_common.h b/drivers/virtio/virtio_pci_common.h
> index a6ad9ec..ac8c9d7 100644
> --- a/drivers/virtio/virtio_pci_common.h
> +++ b/drivers/virtio/virtio_pci_common.h
> @@ -108,6 +108,8 @@ const char *vp_bus_name(struct virtio_device *vdev);
> */
> int vp_set_vq_affinity(struct virtqueue *vq, int cpu);
>
> +const struct cpumask *vp_get_vq_affinity(struct virtio_device *vdev, int index);
> +
> #if IS_ENABLED(CONFIG_VIRTIO_PCI_LEGACY)
> int virtio_pci_legacy_probe(struct virtio_pci_device *);
> void virtio_pci_legacy_remove(struct virtio_pci_device *);
> diff --git a/drivers/virtio/virtio_pci_legacy.c b/drivers/virtio/virtio_pci_legacy.c
> index 2ab6aee..f7362c5 100644
> --- a/drivers/virtio/virtio_pci_legacy.c
> +++ b/drivers/virtio/virtio_pci_legacy.c
> @@ -190,6 +190,7 @@ static const struct virtio_config_ops virtio_pci_config_ops = {
> .finalize_features = vp_finalize_features,
> .bus_name = vp_bus_name,
> .set_vq_affinity = vp_set_vq_affinity,
> + .get_vq_affinity = vp_get_vq_affinity,
> };
>
> /* the PCI probing function */
> diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
> index a7a0981..7bc3004 100644
> --- a/drivers/virtio/virtio_pci_modern.c
> +++ b/drivers/virtio/virtio_pci_modern.c
> @@ -437,6 +437,7 @@ static const struct virtio_config_ops virtio_pci_config_nodev_ops = {
> .finalize_features = vp_finalize_features,
> .bus_name = vp_bus_name,
> .set_vq_affinity = vp_set_vq_affinity,
> + .get_vq_affinity = vp_get_vq_affinity,
> };
>
> static const struct virtio_config_ops virtio_pci_config_ops = {
> @@ -452,6 +453,7 @@ static const struct virtio_config_ops virtio_pci_config_ops = {
> .finalize_features = vp_finalize_features,
> .bus_name = vp_bus_name,
> .set_vq_affinity = vp_set_vq_affinity,
> + .get_vq_affinity = vp_get_vq_affinity,
> };
>
> /**
> diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
> index 2ebe506..8355bab 100644
> --- a/include/linux/virtio_config.h
> +++ b/include/linux/virtio_config.h
> @@ -58,6 +58,7 @@ struct irq_affinity;
> * This returns a pointer to the bus name a la pci_name from which
> * the caller can then copy.
> * @set_vq_affinity: set the affinity for a virtqueue.
> + * @get_vq_affinity: get the affinity for a virtqueue (optional).
> */
> typedef void vq_callback_t(struct virtqueue *);
> struct virtio_config_ops {
> @@ -77,6 +78,8 @@ struct virtio_config_ops {
> int (*finalize_features)(struct virtio_device *vdev);
> const char *(*bus_name)(struct virtio_device *vdev);
> int (*set_vq_affinity)(struct virtqueue *vq, int cpu);
> + const struct cpumask *(*get_vq_affinity)(struct virtio_device *vdev,
> + int index);
> };
>
> /* If driver didn't advertise the feature, it will never appear. */
^ permalink raw reply
* Re: [PATCH 1/9] virtio_pci: remove struct virtio_pci_vq_info
From: Christoph Hellwig @ 2017-02-03 8:22 UTC (permalink / raw)
To: Jason Wang
Cc: Christoph Hellwig, mst, axboe, pbonzini, virtualization,
linux-block, linux-kernel
In-Reply-To: <a5568074-d92e-8322-6955-19c640006fda@redhat.com>
On Fri, Feb 03, 2017 at 03:54:36PM +0800, Jason Wang wrote:
>> + list_for_each_entry(vq, &vp_dev->vdev.vqs, list) {
>> + if (vq->callback && vring_interrupt(irq, vq) == IRQ_HANDLED)
>
> The check of vq->callback seems redundant, we will check it soon in
> vring_interrupt().
Good point - I wanted to keep things exactly as-is and dіdn't notice
we were already protected.
^ permalink raw reply
* Re: [PATCH 2/9] virtio_pci: use shared interrupts for virtqueues
From: Christoph Hellwig @ 2017-02-03 8:26 UTC (permalink / raw)
To: Jason Wang
Cc: Christoph Hellwig, mst, axboe, pbonzini, virtualization,
linux-block, linux-kernel
In-Reply-To: <da34883b-1069-2cfc-aaa3-6aaf3ee0ebf2@redhat.com>
On Fri, Feb 03, 2017 at 03:54:54PM +0800, Jason Wang wrote:
> On 2017年01月27日 16:16, Christoph Hellwig wrote:
>> + snprintf(vp_dev->msix_names[i + 1],
>> + sizeof(*vp_dev->msix_names), "%s-%s",
>> dev_name(&vp_dev->vdev.dev), names[i]);
>> err = request_irq(pci_irq_vector(vp_dev->pci_dev, msix_vec),
>> - vring_interrupt, 0,
>> - vp_dev->msix_names[msix_vec],
>> - vqs[i]);
>> + vring_interrupt, IRQF_SHARED,
>> + vp_dev->msix_names[i + 1], vqs[i]);
>
> Do we need to check per_vq_vectors before dereferencing msix_names[i + 1] ?
No, we need to allocate the array larger in that case as want proper
names for the interrupts.
^ permalink raw reply
* [PATCH RESEND] blkcg: fix double free of new_blkg in blkcg_init_queue
From: Hou Tao @ 2017-02-03 9:19 UTC (permalink / raw)
To: linux-block; +Cc: axboe, tj
If blkg_create fails, new_blkg passed as an argument will
be freed by blkg_create, so there is no need to free it again.
Signed-off-by: Hou Tao <houtao1@huawei.com>
---
block/blk-cgroup.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index 8ba0af7..f570f38 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -1079,10 +1079,8 @@ int blkcg_init_queue(struct request_queue *q)
if (preloaded)
radix_tree_preload_end();
- if (IS_ERR(blkg)) {
- blkg_free(new_blkg);
+ if (IS_ERR(blkg))
return PTR_ERR(blkg);
- }
q->root_blkg = blkg;
q->root_rl.blkg = blkg;
--
2.5.0
^ permalink raw reply related
* Re: [PATCH 2/9] virtio_pci: use shared interrupts for virtqueues
From: Jason Wang @ 2017-02-03 9:47 UTC (permalink / raw)
To: Christoph Hellwig
Cc: mst, axboe, pbonzini, virtualization, linux-block, linux-kernel
In-Reply-To: <20170203082606.GB26800@lst.de>
On 2017年02月03日 16:26, Christoph Hellwig wrote:
> On Fri, Feb 03, 2017 at 03:54:54PM +0800, Jason Wang wrote:
>> On 2017年01月27日 16:16, Christoph Hellwig wrote:
>>> + snprintf(vp_dev->msix_names[i + 1],
>>> + sizeof(*vp_dev->msix_names), "%s-%s",
>>> dev_name(&vp_dev->vdev.dev), names[i]);
>>> err = request_irq(pci_irq_vector(vp_dev->pci_dev, msix_vec),
>>> - vring_interrupt, 0,
>>> - vp_dev->msix_names[msix_vec],
>>> - vqs[i]);
>>> + vring_interrupt, IRQF_SHARED,
>>> + vp_dev->msix_names[i + 1], vqs[i]);
>> Do we need to check per_vq_vectors before dereferencing msix_names[i + 1] ?
> No, we need to allocate the array larger in that case as want proper
> names for the interrupts.
Consider the case of !per_vq_vectors, the size of msix_names is 2, but
snprintf can do out of bound accessing here. (We name the msix shared by
virtqueues with something like "%s-virtqueues" before the patch).
Thanks
^ permalink raw reply
* Re: [PATCH 2/9] virtio_pci: use shared interrupts for virtqueues
From: Christoph Hellwig @ 2017-02-03 9:52 UTC (permalink / raw)
To: Jason Wang
Cc: Christoph Hellwig, mst, axboe, pbonzini, virtualization,
linux-block, linux-kernel
In-Reply-To: <12332f9d-2cf8-d761-032e-a5a89863fd09@redhat.com>
On Fri, Feb 03, 2017 at 05:47:41PM +0800, Jason Wang wrote:
>> No, we need to allocate the array larger in that case as want proper
>> names for the interrupts.
>
> Consider the case of !per_vq_vectors, the size of msix_names is 2, but
> snprintf can do out of bound accessing here. (We name the msix shared by
> virtqueues with something like "%s-virtqueues" before the patch).
Yes, that's what I meant above - we need to allocate a large array
starting with this patch. I'll fix it up for the next version.
^ permalink raw reply
* Re: [PATCH 2/9] virtio_pci: use shared interrupts for virtqueues
From: Jason Wang @ 2017-02-03 9:56 UTC (permalink / raw)
To: Christoph Hellwig
Cc: mst, axboe, pbonzini, virtualization, linux-block, linux-kernel
In-Reply-To: <20170203095219.GA28483@lst.de>
On 2017年02月03日 17:52, Christoph Hellwig wrote:
> On Fri, Feb 03, 2017 at 05:47:41PM +0800, Jason Wang wrote:
>>> No, we need to allocate the array larger in that case as want proper
>>> names for the interrupts.
>> Consider the case of !per_vq_vectors, the size of msix_names is 2, but
>> snprintf can do out of bound accessing here. (We name the msix shared by
>> virtqueues with something like "%s-virtqueues" before the patch).
> Yes, that's what I meant above - we need to allocate a large array
> starting with this patch. I'll fix it up for the next version.
I see.
Thanks
^ permalink raw reply
* [PATCH] dm: don't allow ioctls to targets that don't map to whole devices
From: Christoph Hellwig @ 2017-02-03 10:06 UTC (permalink / raw)
To: axboe, snitzer; +Cc: agk, pbonzini, dm-devel, linux-block
.. at least for unprivilegued users. Before we called into the SCSI
ioctl code to allow excemptions for a few SCSI passthrough ioctls,
but this is pretty unsafe and except for this call dm knows nothing
about SCSI ioctls. As SCSI the SCSI ioctl code is made optionally
now we really don't want to drag it in for DM, and the exception is
not very useful anyway.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Note: this should go into the block tree, as that's where
scsi_verify_blk_ioctl becomes optional.
---
drivers/md/dm.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 9e958bc94fed..adc9dcfd5e9c 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -465,13 +465,16 @@ static int dm_blk_ioctl(struct block_device *bdev, fmode_t mode,
if (r > 0) {
/*
- * Target determined this ioctl is being issued against
- * a logical partition of the parent bdev; so extra
- * validation is needed.
+ * Target determined this ioctl is being issued against a
+ * subset of the parent bdev; require extra privilegues.
*/
- r = scsi_verify_blk_ioctl(NULL, cmd);
- if (r)
+ if (!capable(CAP_SYS_RAWIO)) {
+ printk_ratelimited(KERN_WARNING
+ "%s: sending ioctl %x to DM device!\n",
+ current->comm, cmd);
+ r = -ENOIOCTLCMD;
goto out;
+ }
}
r = __blkdev_driver_ioctl(bdev, mode, cmd, arg);
--
2.11.0
^ permalink raw reply related
* Re: [PATCH] dm: don't allow ioctls to targets that don't map to whole devices
From: Johannes Thumshirn @ 2017-02-03 10:10 UTC (permalink / raw)
To: Christoph Hellwig, axboe, snitzer; +Cc: agk, pbonzini, dm-devel, linux-block
In-Reply-To: <20170203100613.9023-1-hch@lst.de>
On 02/03/2017 11:06 AM, Christoph Hellwig wrote:
> .. at least for unprivilegued users. Before we called into the SCSI
> ioctl code to allow excemptions for a few SCSI passthrough ioctls,
> but this is pretty unsafe and except for this call dm knows nothing
> about SCSI ioctls. As SCSI the SCSI ioctl code is made optionally
^~ duped SCSI or should it
read "as in SCSI the SCSI ioctl code [...]"
> now we really don't want to drag it in for DM, and the exception is
> not very useful anyway.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
>
> Note: this should go into the block tree, as that's where
> scsi_verify_blk_ioctl becomes optional.
>
> ---
Rest looks OK to me,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
--
Johannes Thumshirn Storage
jthumshirn@suse.de +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N�rnberg
GF: Felix Imend�rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N�rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
^ permalink raw reply
* Re: [PATCH] dm: don't allow ioctls to targets that don't map to whole devices
From: Christoph Hellwig @ 2017-02-03 10:34 UTC (permalink / raw)
To: Johannes Thumshirn
Cc: Christoph Hellwig, axboe, snitzer, agk, pbonzini, dm-devel,
linux-block
In-Reply-To: <60c7f620-403c-5ec2-ae9f-e99676410b38@suse.de>
On Fri, Feb 03, 2017 at 11:10:10AM +0100, Johannes Thumshirn wrote:
> On 02/03/2017 11:06 AM, Christoph Hellwig wrote:
>> .. at least for unprivilegued users. Before we called into the SCSI
>> ioctl code to allow excemptions for a few SCSI passthrough ioctls,
>> but this is pretty unsafe and except for this call dm knows nothing
>> about SCSI ioctls. As SCSI the SCSI ioctl code is made optionally
> ^~ duped SCSI or should it
> read "as in SCSI the SCSI ioctl code [...]"
Yeah, thanks.
^ permalink raw reply
* RE: [PATCH] genhd: Do not hold event lock when scheduling workqueue elements
From: Dexuan Cui @ 2017-02-03 12:22 UTC (permalink / raw)
To: Hannes Reinecke, Bart Van Assche, hare@suse.de, axboe@kernel.dk
Cc: hch@lst.de, linux-kernel@vger.kernel.org,
linux-block@vger.kernel.org, jth@kernel.org
In-Reply-To: <532c55c4-15da-d2f9-401c-36bc4343756b@suse.com>
> From: linux-kernel-owner@vger.kernel.org [mailto:linux-kernel-
> owner@vger.kernel.org] On Behalf Of Hannes Reinecke
> Sent: Wednesday, February 1, 2017 00:15
> To: Bart Van Assche <Bart.VanAssche@sandisk.com>; hare@suse.de;
> axboe@kernel.dk
> Cc: hch@lst.de; linux-kernel@vger.kernel.org; linux-block@vger.kernel.org=
;
> jth@kernel.org
> Subject: Re: [PATCH] genhd: Do not hold event lock when scheduling workqu=
eue
> elements
>=20
> On 01/31/2017 01:31 AM, Bart Van Assche wrote:
> > On Wed, 2017-01-18 at 10:48 +0100, Hannes Reinecke wrote:
> >> @@ -1488,26 +1487,13 @@ static unsigned long
> disk_events_poll_jiffies(struct gendisk *disk)
> >> void disk_block_events(struct gendisk *disk)
> >> {
> >> struct disk_events *ev =3D disk->ev;
> >> - unsigned long flags;
> >> - bool cancel;
> >>
> >> if (!ev)
> >> return;
> >>
> >> - /*
> >> - * Outer mutex ensures that the first blocker completes cancel=
ing
> >> - * the event work before further blockers are allowed to finis=
h.
> >> - */
> >> - mutex_lock(&ev->block_mutex);
> >> -
> >> - spin_lock_irqsave(&ev->lock, flags);
> >> - cancel =3D !ev->block++;
> >> - spin_unlock_irqrestore(&ev->lock, flags);
> >> -
> >> - if (cancel)
> >> + if (atomic_inc_return(&ev->block) =3D=3D 1)
> >> cancel_delayed_work_sync(&disk->ev->dwork);
> >>
> >> - mutex_unlock(&ev->block_mutex);
> >> }
> >
> > Hello Hannes,
> >
> > I have already encountered a few times a deadlock that was caused by th=
e
> > event checking code so I agree with you that it would be a big step for=
ward
> > if such deadlocks wouldn't occur anymore. However, this patch realizes =
a
> > change that has not been described in the patch description, namely tha=
t
> > disk_block_events() calls are no longer serialized. Are you sure it is =
safe
> > to drop the serialization of disk_block_events() calls?
> >
> Well, this whole synchronization stuff it a bit weird; I so totally fail
> to see the rationale for it.
> But anyway, once we've converted ev->block to atomics I _think_ the
> mutex_lock can remain; will be checking.
>=20
> Cheers,
>=20
> Hannes
> --
Hi, I think I got the same calltrace with today's linux-next (next-20170203=
).
The issue happened every time when my Linux virtual machine booted and
Hannes's patch could NOT help.
The calltrace is pasted below.
Thanks,
-- Dexuan
[ 9.718802] scsi host2: storvsc_host_t
[ 9.723854] scsi 2:0:0:0: Direct-Access Msft Virtual Disk 1.=
0 PQ: 0 ANSI: 5
[ 9.753161] sd 2:0:0:0: Attached scsi generic sg1 type 0
[ 9.766383] scsi host3: storvsc_host_t
[ 9.771759] scsi 3:0:0:0: Direct-Access Msft Virtual Disk 1.=
0 PQ: 0 ANSI: 5
[ 9.781836] hv_utils: VSS IC version 5.0
[ 9.822511] sd 3:0:0:0: Attached scsi generic sg2 type 0
[ 9.829039] sd 3:0:0:0: [sdb] 266338304 512-byte logical blocks: (136 GB=
/127 GiB)
[ 9.838525] sd 3:0:0:0: [sdb] 4096-byte physical blocks
[ 9.845350] sd 3:0:0:0: [sdb] Write Protect is off
[ 9.851077] sd 3:0:0:0: [sdb] Mode Sense: 0f 00 00 00
[ 9.859765] sd 3:0:0:0: [sdb] Write cache: enabled, read cache: enabled,=
doesn't support DPO or FUA
[ 9.872728] sdb: sdb1
[ 9.877279] sd 3:0:0:0: [sdb] Attached SCSI disk
[ 9.964093] psmouse serio1: trackpoint: failed to get extended button da=
ta
[ 14.864110] psmouse serio1: trackpoint: IBM TrackPoint firmware: 0x01, b=
uttons: 0/0
[ 14.876423] input: TPPS/2 IBM TrackPoint as /devices/platform/i8042/seri=
o1/input/input3
[ 14.887216] input: AT Translated Set 2 keyboard as /devices/LNXSYSTM:00/=
LNXSYBUS:00/PNP0A03:00/device:07/VMBUS:01/d34b2567-b9b6-42b9-8778-0a4ec0b95=
5bf/serio2/input/input5
[ 44.644061] random: crng init done
[ 66.524169] hv_utils: KVP IC version 4.0
Begin: Loading essential drivers ... done.
Begin: Running /scripts/init-premount ... done.
Begin: Mounting root file system ... Begin: Running /scripts/local-top ... =
done.
Begin: Running /scripts/local-premount ... done.
Begin: Waiting for root file system ... Begin: Running /scripts/local-block=
... done.
Begin: Running /scripts/local-block ... done.
Begin: Running /scripts/local-block ... done.
Begin: Running /scripts/local-block ... done.
[ 242.652127] INFO: task systemd-udevd:183 blocked for more than 120 secon=
ds.
[ 242.661008] Not tainted 4.10.0-rc6-next-20170203+ #2
[ 242.697270] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables =
this message.
[ 242.707872] systemd-udevd D 0 183 170 0x00000004
[ 242.714654] Call Trace:
[ 242.717904] __schedule+0x27d/0x8d0
[ 242.724618] schedule+0x36/0x80
[ 242.729042] schedule_timeout+0x235/0x3f0
[ 242.736667] ? sched_clock+0x9/0x10
[ 242.741144] ? try_to_wake_up+0x4a/0x460
[ 242.745580] wait_for_completion+0xa5/0x120
[ 242.751417] ? wake_up_q+0x70/0x70
[ 242.759394] flush_work+0x11a/0x1c0
[ 242.764846] ? worker_detach_from_pool+0xb0/0xb0
[ 242.770943] __cancel_work_timer+0xf3/0x1b0
[ 242.776073] ? disk_map_sector_rcu+0x70/0x70
[ 242.783857] cancel_delayed_work_sync+0x13/0x20
[ 242.789438] disk_block_events+0x34/0x40
[ 242.794641] __blkdev_get+0x10c/0x470
[ 242.799227] blkdev_get+0x11a/0x320
[ 242.803161] ? unlock_new_inode+0x49/0x80
[ 242.807451] ? bdget+0x110/0x130
[ 242.811415] blkdev_open+0x5b/0x70
[ 242.820215] do_dentry_open+0x208/0x310
[ 242.825336] ? blkdev_get_by_dev+0x50/0x50
[ 242.836362] vfs_open+0x4c/0x70
[ 242.839839] ? may_open+0x9b/0x100
[ 242.843803] path_openat+0x297/0x13e0
[ 242.848410] ? _copy_to_user+0x2e/0x40
[ 242.852966] ? move_addr_to_user+0xa3/0xc0
[ 242.857961] do_filp_open+0x7e/0xe0
[ 242.862154] ? _cond_resched+0x1a/0x50
[ 242.867045] ? kmem_cache_alloc+0x156/0x1b0
[ 242.872004] ? getname_flags+0x56/0x1f0
[ 242.881609] ? __alloc_fd+0x46/0x170
[ 242.891812] do_sys_open+0x11b/0x1f0
[ 242.896506] SyS_open+0x1e/0x20
[ 242.900365] entry_SYSCALL_64_fastpath+0x1e/0xad
[ 242.905607] RIP: 0033:0x7efd3827aca0
[ 242.909773] RSP: 002b:00007ffd7ceaeb38 EFLAGS: 00000246 ORIG_RAX: 000000=
0000000002
[ 242.918778] RAX: ffffffffffffffda RBX: 0000555fdf1a2d40 RCX: 00007efd382=
7aca0
[ 242.925725] RDX: 0000555fde106058 RSI: 00000000000a0800 RDI: 0000555fdf1=
a2ff0
[ 242.929715] RBP: 0000000000000000 R08: 0000000000000073 R09: 00000000000=
00003
[ 242.933921] R10: 00007efd37fe6520 R11: 0000000000000246 R12: 00000000000=
00000
[ 242.942260] R13: 0000555fdf1a2d40 R14: 00007ffd7ceaea00 R15: 00000000000=
00000
[ 242.952780] INFO: task systemd-udevd:191 blocked for more than 120 secon=
ds.
[ 242.960994] Not tainted 4.10.0-rc6-next-20170203+ #2
[ 242.966936] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables =
this message.
[ 242.975395] systemd-udevd D 0 191 170 0x00000004
[ 242.983460] Call Trace:
[ 242.986694] __schedule+0x27d/0x8d0
[ 242.992603] ? mutex_lock+0x12/0x40
[ 242.996467] ? kprobes_module_callback+0x15b/0x1d0
[ 243.001934] schedule+0x36/0x80
[ 243.005908] async_synchronize_cookie_domain+0x91/0x130
[ 243.012331] ? wake_atomic_t_function+0x60/0x60
[ 243.017690] async_synchronize_full+0x17/0x20
[ 243.022662] do_init_module+0xc1/0x1ff
[ 243.026965] load_module+0x2313/0x29a0
[ 243.031232] ? __symbol_put+0x40/0x40
[ 243.035347] ? security_kernel_post_read_file+0x6b/0x80
[ 243.042541] SYSC_finit_module+0xbc/0xf0
[ 243.045130] SyS_finit_module+0xe/0x10
[ 243.049116] entry_SYSCALL_64_fastpath+0x1e/0xad
[ 243.054517] RIP: 0033:0x7efd37fa1c19
[ 243.058744] RSP: 002b:00007ffd7ceae188 EFLAGS: 00000246 ORIG_RAX: 000000=
0000000139
[ 243.067483] RAX: ffffffffffffffda RBX: 0000000000000005 RCX: 00007efd37f=
a1c19
[ 243.075801] RDX: 0000000000000000 RSI: 00007efd38497e2a RDI: 00000000000=
0000c
[ 243.084162] RBP: 00007ffd7cead190 R08: 0000000000000000 R09: 00000000000=
00000
[ 243.093738] R10: 000000000000000c R11: 0000000000000246 R12: 0000555fdf1=
a3300
[ 243.103158] R13: 00007ffd7cead170 R14: 0000000000000005 R15: 000000000ab=
a9500
[ 363.484110] INFO: task systemd-udevd:183 blocked for more than 120 secon=
ds.
[ 363.493200] Not tainted 4.10.0-rc6-next-20170203+ #2
[ 363.500109] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables =
this message.
[ 363.509635] systemd-udevd D 0 183 170 0x00000004
[ 363.516428] Call Trace:
[ 363.519492] __schedule+0x27d/0x8d0
[ 363.523850] schedule+0x36/0x80
[ 363.529927] schedule_timeout+0x235/0x3f0
[ 363.534884] ? sched_clock+0x9/0x10
[ 363.539006] ? try_to_wake_up+0x4a/0x460
[ 363.543686] wait_for_completion+0xa5/0x120
[ 363.548831] ? wake_up_q+0x70/0x70
[ 363.552993] flush_work+0x11a/0x1c0
[ 363.557236] ? worker_detach_from_pool+0xb0/0xb0
[ 363.562851] __cancel_work_timer+0xf3/0x1b0
[ 363.567942] ? disk_map_sector_rcu+0x70/0x70
[ 363.572973] cancel_delayed_work_sync+0x13/0x20
[ 363.580271] disk_block_events+0x34/0x40
[ 363.585284] __blkdev_get+0x10c/0x470
[ 363.589774] blkdev_get+0x11a/0x320
[ 363.593851] ? unlock_new_inode+0x49/0x80
[ 363.598509] ? bdget+0x110/0x130
[ 363.602272] blkdev_open+0x5b/0x70
[ 363.606254] do_dentry_open+0x208/0x310
[ 363.610722] ? blkdev_get_by_dev+0x50/0x50
[ 363.615493] vfs_open+0x4c/0x70
[ 363.619287] ? may_open+0x9b/0x100
[ 363.623386] path_openat+0x297/0x13e0
[ 363.627752] ? _copy_to_user+0x2e/0x40
[ 363.633978] ? move_addr_to_user+0xa3/0xc0
[ 363.638833] do_filp_open+0x7e/0xe0
[ 363.642809] ? _cond_resched+0x1a/0x50
[ 363.647070] ? kmem_cache_alloc+0x156/0x1b0
[ 363.651785] ? getname_flags+0x56/0x1f0
[ 363.656285] ? __alloc_fd+0x46/0x170
[ 363.660490] do_sys_open+0x11b/0x1f0
[ 363.664655] SyS_open+0x1e/0x20
[ 363.668405] entry_SYSCALL_64_fastpath+0x1e/0xad
[ 363.674075] RIP: 0033:0x7efd3827aca0
[ 363.678582] RSP: 002b:00007ffd7ceaeb38 EFLAGS: 00000246 ORIG_RAX: 000000=
0000000002
[ 363.689873] RAX: ffffffffffffffda RBX: 0000555fdf1a2d40 RCX: 00007efd382=
7aca0
[ 363.698127] RDX: 0000555fde106058 RSI: 00000000000a0800 RDI: 0000555fdf1=
a2ff0
[ 363.706494] RBP: 0000000000000000 R08: 0000000000000073 R09: 00000000000=
00003
[ 363.714696] R10: 00007efd37fe6520 R11: 0000000000000246 R12: 00000000000=
00000
[ 363.722922] R13: 0000555fdf1a2d40 R14: 00007ffd7ceaea00 R15: 00000000000=
00000
[ 363.731123] INFO: task systemd-udevd:191 blocked for more than 120 secon=
ds.
[ 363.739454] Not tainted 4.10.0-rc6-next-20170203+ #2
[ 363.745246] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables =
this message.
[ 363.753890] systemd-udevd D 0 191 170 0x00000004
[ 363.760085] Call Trace:
[ 363.763006] __schedule+0x27d/0x8d0
[ 363.767016] ? mutex_lock+0x12/0x40
[ 363.770943] ? kprobes_module_callback+0x15b/0x1d0
[ 363.780593] schedule+0x36/0x80
[ 363.785119] async_synchronize_cookie_domain+0x91/0x130
[ 363.793021] ? wake_atomic_t_function+0x60/0x60
[ 363.798351] async_synchronize_full+0x17/0x20
[ 363.803405] do_init_module+0xc1/0x1ff
[ 363.807564] load_module+0x2313/0x29a0
[ 363.811786] ? __symbol_put+0x40/0x40
[ 363.815895] ? security_kernel_post_read_file+0x6b/0x80
[ 363.821852] SYSC_finit_module+0xbc/0xf0
[ 363.826433] SyS_finit_module+0xe/0x10
[ 363.830732] entry_SYSCALL_64_fastpath+0x1e/0xad
[ 363.836311] RIP: 0033:0x7efd37fa1c19
[ 363.842740] RSP: 002b:00007ffd7ceae188 EFLAGS: 00000246 ORIG_RAX: 000000=
0000000139
[ 363.852003] RAX: ffffffffffffffda RBX: 0000000000000005 RCX: 00007efd37f=
a1c19
[ 363.860774] RDX: 0000000000000000 RSI: 00007efd38497e2a RDI: 00000000000=
0000c
[ 363.869669] RBP: 00007ffd7cead190 R08: 0000000000000000 R09: 00000000000=
00000
[ 363.878494] R10: 000000000000000c R11: 0000000000000246 R12: 0000555fdf1=
a3300
[ 363.887931] R13: 00007ffd7cead170 R14: 0000000000000005 R15: 000000000ab=
a9500
[ 484.316121] INFO: task systemd-udevd:183 blocked for more than 120 secon=
ds.
[ 484.327141] Not tainted 4.10.0-rc6-next-20170203+ #2
[ 484.333747] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables =
this message.
[ 484.342486] systemd-udevd D 0 183 170 0x00000004
[ 484.348790] Call Trace:
[ 484.351712] __schedule+0x27d/0x8d0
[ 484.355904] schedule+0x36/0x80
[ 484.359718] schedule_timeout+0x235/0x3f0
[ 484.364932] ? sched_clock+0x9/0x10
[ 484.368892] ? try_to_wake_up+0x4a/0x460
[ 484.376582] wait_for_completion+0xa5/0x120
[ 484.383779] ? wake_up_q+0x70/0x70
[ 484.389885] flush_work+0x11a/0x1c0
[ 484.394048] ? worker_detach_from_pool+0xb0/0xb0
[ 484.399464] __cancel_work_timer+0xf3/0x1b0
[ 484.404326] ? disk_map_sector_rcu+0x70/0x70
[ 484.409379] cancel_delayed_work_sync+0x13/0x20
[ 484.414449] disk_block_events+0x34/0x40
[ 484.418615] __blkdev_get+0x10c/0x470
[ 484.423391] blkdev_get+0x11a/0x320
[ 484.429312] ? unlock_new_inode+0x49/0x80
[ 484.433618] ? bdget+0x110/0x130
[ 484.437300] blkdev_open+0x5b/0x70
[ 484.441376] do_dentry_open+0x208/0x310
[ 484.446108] ? blkdev_get_by_dev+0x50/0x50
[ 484.450946] vfs_open+0x4c/0x70
[ 484.454693] ? may_open+0x9b/0x100
[ 484.458682] path_openat+0x297/0x13e0
[ 484.462656] ? _copy_to_user+0x2e/0x40
[ 484.467125] ? move_addr_to_user+0xa3/0xc0
[ 484.472370] do_filp_open+0x7e/0xe0
[ 484.479195] ? _cond_resched+0x1a/0x50
[ 484.484027] ? kmem_cache_alloc+0x156/0x1b0
[ 484.488851] ? getname_flags+0x56/0x1f0
[ 484.493049] ? __alloc_fd+0x46/0x170
[ 484.496899] do_sys_open+0x11b/0x1f0
[ 484.500784] SyS_open+0x1e/0x20
[ 484.504284] entry_SYSCALL_64_fastpath+0x1e/0xad
[ 484.510767] RIP: 0033:0x7efd3827aca0
[ 484.515460] RSP: 002b:00007ffd7ceaeb38 EFLAGS: 00000246 ORIG_RAX: 000000=
0000000002
[ 484.527246] RAX: ffffffffffffffda RBX: 0000555fdf1a2d40 RCX: 00007efd382=
7aca0
[ 484.537307] RDX: 0000555fde106058 RSI: 00000000000a0800 RDI: 0000555fdf1=
a2ff0
[ 484.545620] RBP: 0000000000000000 R08: 0000000000000073 R09: 00000000000=
00003
[ 484.554372] R10: 00007efd37fe6520 R11: 0000000000000246 R12: 00000000000=
00000
[ 484.563267] R13: 0000555fdf1a2d40 R14: 00007ffd7ceaea00 R15: 00000000000=
00000
[ 484.571966] INFO: task systemd-udevd:191 blocked for more than 120 secon=
ds.
[ 484.580711] Not tainted 4.10.0-rc6-next-20170203+ #2
[ 484.589555] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables =
this message.
[ 484.598799] systemd-udevd D 0 191 170 0x00000004
[ 484.605231] Call Trace:
[ 484.608308] __schedule+0x27d/0x8d0
[ 484.612702] ? mutex_lock+0x12/0x40
[ 484.617071] ? kprobes_module_callback+0x15b/0x1d0
[ 484.621625] schedule+0x36/0x80
[ 484.623511] async_synchronize_cookie_domain+0x91/0x130
[ 484.626443] ? wake_atomic_t_function+0x60/0x60
[ 484.629059] async_synchronize_full+0x17/0x20
[ 484.635723] do_init_module+0xc1/0x1ff
[ 484.640226] load_module+0x2313/0x29a0
[ 484.645030] ? __symbol_put+0x40/0x40
[ 484.649870] ? security_kernel_post_read_file+0x6b/0x80
[ 484.658016] SYSC_finit_module+0xbc/0xf0
[ 484.662676] SyS_finit_module+0xe/0x10
[ 484.667374] entry_SYSCALL_64_fastpath+0x1e/0xad
[ 484.673822] RIP: 0033:0x7efd37fa1c19
[ 484.677771] RSP: 002b:00007ffd7ceae188 EFLAGS: 00000246 ORIG_RAX: 000000=
0000000139
[ 484.688668] RAX: ffffffffffffffda RBX: 0000000000000005 RCX: 00007efd37f=
a1c19
[ 484.697959] RDX: 0000000000000000 RSI: 00007efd38497e2a RDI: 00000000000=
0000c
[ 484.707793] RBP: 00007ffd7cead190 R08: 0000000000000000 R09: 00000000000=
00000
[ 484.716695] R10: 000000000000000c R11: 0000000000000246 R12: 0000555fdf1=
a3300
[ 484.724415] R13: 00007ffd7cead170 R14: 0000000000000005 R15: 000000000ab=
a9500
^ permalink raw reply
* Re: [PATCH 22/24] ubifs: Convert to separately allocated bdi
From: Jan Kara @ 2017-02-03 13:45 UTC (permalink / raw)
To: Richard Weinberger
Cc: Jan Kara, linux-fsdevel, Christoph Hellwig, linux-block,
Artem Bityutskiy, Adrian Hunter, linux-mtd
In-Reply-To: <26c70615-f98c-23a6-abb7-de675fb658b6@nod.at>
On Thu 02-02-17 21:34:32, Richard Weinberger wrote:
> Jan,
>
> Am 02.02.2017 um 18:34 schrieb Jan Kara:
> > Allocate struct backing_dev_info separately instead of embedding it
> > inside the superblock. This unifies handling of bdi among users.
> >
> > CC: Richard Weinberger <richard@nod.at>
> > CC: Artem Bityutskiy <dedekind1@gmail.com>
> > CC: Adrian Hunter <adrian.hunter@intel.com>
> > CC: linux-mtd@lists.infradead.org
> > Signed-off-by: Jan Kara <jack@suse.cz>
>
> Is this series available at some git tree, please?
I've pushed it out to:
git://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs.git bdi
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH 04/24] fs: Provide infrastructure for dynamic BDIs in filesystems
From: Jan Kara @ 2017-02-03 13:50 UTC (permalink / raw)
To: Liu Bo
Cc: Jan Kara, linux-fsdevel, Christoph Hellwig, linux-block,
linux-mtd, linux-nfs, Petr Vandrovec, linux-nilfs, cluster-devel,
osd-dev, codalist, linux-afs, ecryptfs, linux-cifs, ceph-devel,
linux-btrfs, v9fs-developer, lustre-devel
In-Reply-To: <20170202192827.GF6550@localhost.localdomain>
On Thu 02-02-17 11:28:27, Liu Bo wrote:
> Hi,
>
> On Thu, Feb 02, 2017 at 06:34:02PM +0100, Jan Kara wrote:
> > Provide helper functions for setting up dynamically allocated
> > backing_dev_info structures for filesystems and cleaning them up on
> > superblock destruction.
>
> Just one concern, will this cause problems for multiple superblock cases
> like nfs with nosharecache?
Can you ellaborate a bit? I've looked for a while what nfs with
nosharecache does but I didn't see how it would influence anything with
bdis...
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: dm: don't allow ioctls to targets that don't map to whole devices
From: Mike Snitzer @ 2017-02-03 14:35 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: axboe, agk, pbonzini, dm-devel, linux-block
In-Reply-To: <20170203100613.9023-1-hch@lst.de>
On Fri, Feb 03 2017 at 5:06am -0500,
Christoph Hellwig <hch@lst.de> wrote:
> .. at least for unprivilegued users. Before we called into the SCSI
> ioctl code to allow excemptions for a few SCSI passthrough ioctls,
> but this is pretty unsafe and except for this call dm knows nothing
> about SCSI ioctls. As SCSI the SCSI ioctl code is made optionally
> now we really don't want to drag it in for DM, and the exception is
> not very useful anyway.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
>
> Note: this should go into the block tree, as that's where
> scsi_verify_blk_ioctl becomes optional.
>
> ---
> drivers/md/dm.c | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/md/dm.c b/drivers/md/dm.c
> index 9e958bc94fed..adc9dcfd5e9c 100644
> --- a/drivers/md/dm.c
> +++ b/drivers/md/dm.c
> @@ -465,13 +465,16 @@ static int dm_blk_ioctl(struct block_device *bdev, fmode_t mode,
>
> if (r > 0) {
> /*
> - * Target determined this ioctl is being issued against
> - * a logical partition of the parent bdev; so extra
> - * validation is needed.
> + * Target determined this ioctl is being issued against a
> + * subset of the parent bdev; require extra privilegues.
> */
> - r = scsi_verify_blk_ioctl(NULL, cmd);
> - if (r)
> + if (!capable(CAP_SYS_RAWIO)) {
> + printk_ratelimited(KERN_WARNING
> + "%s: sending ioctl %x to DM device!\n",
> + current->comm, cmd);
> + r = -ENOIOCTLCMD;
> goto out;
> + }
> }
>
> r = __blkdev_driver_ioctl(bdev, mode, cmd, arg);
> --
> 2.11.0
>
Would prefer to see the use of DMERR_LIMIT() or DMWARN_LIMIT() as those
wrappers provide error message consistency across DM core and DM
targets. Also, would make sense to say: "sending ioctl %x to DM device
without required privilege (CAP_SYS_RAWIO)."
(you have a couple s/privilegue/privilege typos)
And this patch will need Paolo's ack before being staged.
Otherwise, look good:
Acked-by: Mike Snitzer <snitzer@redhat.com>
^ permalink raw reply
* spread MSI(-X) vectors to all possible CPUs
From: Christoph Hellwig @ 2017-02-03 14:35 UTC (permalink / raw)
To: Thomas Gleixner, Jens Axboe
Cc: Keith Busch, linux-nvme, linux-block, linux-kernel
Hi all,
this series changes our automatic MSI-X vector assignment so that it
takes all present CPUs into account instead of all online ones. This
allows to better deal with cpu hotplug events, which could happen
frequently due to power management for example.
^ permalink raw reply
* [PATCH 1/6] genirq: allow assigning affinity to present but not online CPUs
From: Christoph Hellwig @ 2017-02-03 14:35 UTC (permalink / raw)
To: Thomas Gleixner, Jens Axboe
Cc: Keith Busch, linux-nvme, linux-block, linux-kernel
In-Reply-To: <20170203143600.32307-1-hch@lst.de>
This will allow us to spread MSI/MSI-X affinity over all present CPUs and
thus better deal with systems where cpus are take on and offline all the
time.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
kernel/irq/manage.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index 6b669593e7eb..7a1424330f9c 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -354,22 +354,22 @@ static int setup_affinity(struct irq_desc *desc, struct cpumask *mask)
/*
* Preserve the managed affinity setting and an userspace affinity
- * setup, but make sure that one of the targets is online.
+ * setup, but make sure that one of the targets is present.
*/
if (irqd_affinity_is_managed(&desc->irq_data) ||
irqd_has_set(&desc->irq_data, IRQD_AFFINITY_SET)) {
if (cpumask_intersects(desc->irq_common_data.affinity,
- cpu_online_mask))
+ cpu_present_mask))
set = desc->irq_common_data.affinity;
else
irqd_clear(&desc->irq_data, IRQD_AFFINITY_SET);
}
- cpumask_and(mask, cpu_online_mask, set);
+ cpumask_and(mask, cpu_present_mask, set);
if (node != NUMA_NO_NODE) {
const struct cpumask *nodemask = cpumask_of_node(node);
- /* make sure at least one of the cpus in nodemask is online */
+ /* make sure at least one of the cpus in nodemask is present */
if (cpumask_intersects(mask, nodemask))
cpumask_and(mask, mask, nodemask);
}
--
2.11.0
^ permalink raw reply related
* [PATCH 2/6] genirq/affinity: assign vectors to all present CPUs
From: Christoph Hellwig @ 2017-02-03 14:35 UTC (permalink / raw)
To: Thomas Gleixner, Jens Axboe
Cc: Keith Busch, linux-nvme, linux-block, linux-kernel
In-Reply-To: <20170203143600.32307-1-hch@lst.de>
Currently we only assign spread vectors to online CPUs, which ties the
IRQ mapping to the currently online devices and doesn't deal nicely with
the fact that CPUs could come and go rapidly due to e.g. power management.
Instead assign vectors to all present CPUs to avoid this churn.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
kernel/irq/affinity.c | 43 ++++++++++++++++++++++++++++---------------
1 file changed, 28 insertions(+), 15 deletions(-)
diff --git a/kernel/irq/affinity.c b/kernel/irq/affinity.c
index 4544b115f5eb..6cd20a569359 100644
--- a/kernel/irq/affinity.c
+++ b/kernel/irq/affinity.c
@@ -4,6 +4,8 @@
#include <linux/slab.h>
#include <linux/cpu.h>
+static cpumask_var_t node_to_present_cpumask[MAX_NUMNODES] __read_mostly;
+
static void irq_spread_init_one(struct cpumask *irqmsk, struct cpumask *nmsk,
int cpus_per_vec)
{
@@ -40,8 +42,8 @@ static int get_nodes_in_cpumask(const struct cpumask *mask, nodemask_t *nodemsk)
int n, nodes = 0;
/* Calculate the number of nodes in the supplied affinity mask */
- for_each_online_node(n) {
- if (cpumask_intersects(mask, cpumask_of_node(n))) {
+ for_each_node(n) {
+ if (cpumask_intersects(mask, node_to_present_cpumask[n])) {
node_set(n, *nodemsk);
nodes++;
}
@@ -77,9 +79,7 @@ irq_create_affinity_masks(int nvecs, const struct irq_affinity *affd)
for (curvec = 0; curvec < affd->pre_vectors; curvec++)
cpumask_copy(masks + curvec, irq_default_affinity);
- /* Stabilize the cpumasks */
- get_online_cpus();
- nodes = get_nodes_in_cpumask(cpu_online_mask, &nodemsk);
+ nodes = get_nodes_in_cpumask(cpu_present_mask, &nodemsk);
/*
* If the number of nodes in the mask is greater than or equal the
@@ -87,7 +87,8 @@ irq_create_affinity_masks(int nvecs, const struct irq_affinity *affd)
*/
if (affv <= nodes) {
for_each_node_mask(n, nodemsk) {
- cpumask_copy(masks + curvec, cpumask_of_node(n));
+ cpumask_copy(masks + curvec,
+ node_to_present_cpumask[n]);
if (++curvec == last_affv)
break;
}
@@ -103,7 +104,7 @@ irq_create_affinity_masks(int nvecs, const struct irq_affinity *affd)
int ncpus, v, vecs_to_assign = vecs_per_node;
/* Get the cpus on this node which are in the mask */
- cpumask_and(nmsk, cpu_online_mask, cpumask_of_node(n));
+ cpumask_and(nmsk, cpu_present_mask, node_to_present_cpumask[n]);
/* Calculate the number of cpus per vector */
ncpus = cpumask_weight(nmsk);
@@ -126,8 +127,6 @@ irq_create_affinity_masks(int nvecs, const struct irq_affinity *affd)
}
done:
- put_online_cpus();
-
/* Fill out vectors at the end that don't need affinity */
for (; curvec < nvecs; curvec++)
cpumask_copy(masks + curvec, irq_default_affinity);
@@ -145,12 +144,26 @@ int irq_calc_affinity_vectors(int maxvec, const struct irq_affinity *affd)
{
int resv = affd->pre_vectors + affd->post_vectors;
int vecs = maxvec - resv;
- int cpus;
- /* Stabilize the cpumasks */
- get_online_cpus();
- cpus = cpumask_weight(cpu_online_mask);
- put_online_cpus();
+ return min_t(int, cpumask_weight(cpu_present_mask), vecs) + resv;
+}
+
+static int __init irq_build_cpumap(void)
+{
+ int node, cpu;
+
+ for (node = 0; node < nr_node_ids; node++) {
+ if (!zalloc_cpumask_var(&node_to_present_cpumask[node],
+ GFP_KERNEL))
+ panic("can't allocate early memory\n");
+ }
- return min(cpus, vecs) + resv;
+ for_each_present_cpu(cpu) {
+ node = cpu_to_node(cpu);
+ cpumask_set_cpu(cpu, node_to_present_cpumask[node]);
+ }
+
+ return 0;
}
+
+subsys_initcall(irq_build_cpumap);
--
2.11.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox