* Re: [PATCH] drm/i915: remove duplicated memsets in skl_allocate_pipe_ddb()
From: Matt Roper @ 2016-11-09 1:13 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: Jani Nikula, intel-gfx
In-Reply-To: <1478625682-8955-1-git-send-email-paulo.r.zanoni@intel.com>
On Tue, Nov 08, 2016 at 03:21:22PM -0200, Paulo Zanoni wrote:
> One of the memsets was added by 5a920b85f2c6 ("drm/i915/gen9: fix DDB
> partitioning for multi-screen cases"), and the other was added by
> 01c72d6c17 ("drm/i915/gen9: fix DDB partitioning for multi-screen
> cases"). I'm confused and I'll let the maintainers find out what went
> wrong here.
>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
I think this is just an artifact of having the patch in both -next and
-fixes. The context probably changed later in -next causing a conflict
when the two were merged, and then the merge resolution accidentally
duplicated this hunk. The merge commit ac4139ed7 is where we wound up
with the two copies:
diff --cc drivers/gpu/drm/i915/intel_pm.c
index db24f89,cc9e0c0..88e28c9
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@@ -3361,11 -3404,13 +3404,17 @@@ skl_allocate_pipe_ddb(struct intel_crtc
unsigned int total_data_rate;
int num_active;
int id, i;
+ unsigned plane_data_rate[I915_MAX_PLANES] = {};
+ unsigned plane_y_data_rate[I915_MAX_PLANES] = {};
+
+ /* Clear the partitioning for disabled planes. */
+ memset(ddb->plane[pipe], 0, sizeof(ddb->plane[pipe]));
+ memset(ddb->y_plane[pipe], 0, sizeof(ddb->y_plane[pipe]));
+ /* Clear the partitioning for disabled planes. */
+ memset(ddb->plane[pipe], 0, sizeof(ddb->plane[pipe]));
+ memset(ddb->y_plane[pipe], 0, sizeof(ddb->y_plane[pipe]));
+
if (WARN_ON(!state))
return 0;
Dropping one looks good to me.
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
> ---
> drivers/gpu/drm/i915/intel_pm.c | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 88e28c9..cc9e0c0 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3411,10 +3411,6 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
> memset(ddb->plane[pipe], 0, sizeof(ddb->plane[pipe]));
> memset(ddb->y_plane[pipe], 0, sizeof(ddb->y_plane[pipe]));
>
> - /* Clear the partitioning for disabled planes. */
> - memset(ddb->plane[pipe], 0, sizeof(ddb->plane[pipe]));
> - memset(ddb->y_plane[pipe], 0, sizeof(ddb->y_plane[pipe]));
> -
> if (WARN_ON(!state))
> return 0;
>
> --
> 2.7.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply
* Re: [PATCH v3] drm: move allocation out of drm_get_format_name()
From: Daniel Vetter @ 2016-11-09 1:13 UTC (permalink / raw)
To: Eric Engestrom
Cc: David Airlie, dri-devel, Wei Yongjun, Daniel Vetter, Flora Cui,
Gustavo Padovan, Tom St Denis, Chunming Zhou, Thomas Hellstrom,
Laurent Pinchart, Sinclair Yeh, Xinliang Liu, Xinwei Kong,
VMware Graphics, Vitaly Prosyak, Eric Engestrom, Alexandre Demers,
Jani Nikula, intel-gfx, Emily Deng, Junwei Zhang, Michel
In-Reply-To: <20161109010916.rpsqcswnlyxggovd@phenom.ffwll.local>
On Wed, Nov 09, 2016 at 02:09:16AM +0100, Daniel Vetter wrote:
> On Wed, Nov 09, 2016 at 12:17:52AM +0000, Eric Engestrom wrote:
> > The function's behaviour was changed in 90844f00049e, without changing
> > its signature, causing people to keep using it the old way without
> > realising they were now leaking memory.
> > Rob Clark also noticed it was also allocating GFP_KERNEL memory in
> > atomic contexts, breaking them.
> >
> > Instead of having to allocate GFP_ATOMIC memory and fixing the callers
> > to make them cleanup the memory afterwards, let's change the function's
> > signature by having the caller take care of the memory and passing it to
> > the function.
> > The new parameter is a single-field struct in order to enforce the size
> > of its buffer and help callers to correctly manage their memory.
> >
> > Fixes: 90844f00049e ("drm: make drm_get_format_name thread-safe")
> > Cc: Rob Clark <robdclark@gmail.com>
> > Cc: Christian König <christian.koenig@amd.com>
> > Acked-by: Christian König <christian.koenig@amd.com>
> > Acked-by: Rob Clark <robdclark@gmail.com>
> > Acked-by: Sinclair Yeh <syeh@vmware.com> (vmwgfx)
> > Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> > Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Eric Engestrom <eric@engestrom.ch>
> > ---
> > v3 - fix "Fixes" tag, replace it with an actual commit message
> > - collect ack & r-b
> >
> > v2 - use single-field struct instead of typedef to let the compiler
> > enforce the type (Christian König)
>
> Applied to drm-misc, thanks.
Well, had to drop it again since it didn't compile:
CC [M] drivers/gpu/drm/drm_blend.o
drivers/gpu/drm/drm_atomic.c: In function ‘drm_atomic_plane_print_state’:
drivers/gpu/drm/drm_atomic.c:920:5: error: too few arguments to function ‘drm_get_format_name’
drm_get_format_name(fb->pixel_format));
^~~~~~~~~~~~~~~~~~~
In file included from ./include/drm/drmP.h:71:0,
from drivers/gpu/drm/drm_atomic.c:29:
./include/drm/drm_fourcc.h:65:7: note: declared here
char *drm_get_format_name(uint32_t format, struct drm_format_name_buf *buf);
^~~~~~~~~~~~~~~~~~~
Can you pls rebase onto drm-misc or linux-next or something?
Thanks, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply
* Re: [Intel-gfx] [PATCH v3] drm: move allocation out of drm_get_format_name()
From: Daniel Vetter @ 2016-11-09 1:13 UTC (permalink / raw)
To: Eric Engestrom
Cc: linux-kernel, David Airlie, dri-devel, Wei Yongjun, Daniel Vetter,
Flora Cui, Gustavo Padovan, Tom St Denis, Chunming Zhou,
Thomas Hellstrom, Laurent Pinchart, Sinclair Yeh, Xinliang Liu,
Xinwei Kong, VMware Graphics, Vitaly Prosyak, Eric Engestrom,
Alexandre Demers, Jani Nikula, intel-gfx, Emily Deng,
Colin Ian King, Junwei Zhang, Michel Dänzer, Alex Deucher,
Christian König
In-Reply-To: <20161109010916.rpsqcswnlyxggovd@phenom.ffwll.local>
On Wed, Nov 09, 2016 at 02:09:16AM +0100, Daniel Vetter wrote:
> On Wed, Nov 09, 2016 at 12:17:52AM +0000, Eric Engestrom wrote:
> > The function's behaviour was changed in 90844f00049e, without changing
> > its signature, causing people to keep using it the old way without
> > realising they were now leaking memory.
> > Rob Clark also noticed it was also allocating GFP_KERNEL memory in
> > atomic contexts, breaking them.
> >
> > Instead of having to allocate GFP_ATOMIC memory and fixing the callers
> > to make them cleanup the memory afterwards, let's change the function's
> > signature by having the caller take care of the memory and passing it to
> > the function.
> > The new parameter is a single-field struct in order to enforce the size
> > of its buffer and help callers to correctly manage their memory.
> >
> > Fixes: 90844f00049e ("drm: make drm_get_format_name thread-safe")
> > Cc: Rob Clark <robdclark@gmail.com>
> > Cc: Christian König <christian.koenig@amd.com>
> > Acked-by: Christian König <christian.koenig@amd.com>
> > Acked-by: Rob Clark <robdclark@gmail.com>
> > Acked-by: Sinclair Yeh <syeh@vmware.com> (vmwgfx)
> > Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> > Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Eric Engestrom <eric@engestrom.ch>
> > ---
> > v3 - fix "Fixes" tag, replace it with an actual commit message
> > - collect ack & r-b
> >
> > v2 - use single-field struct instead of typedef to let the compiler
> > enforce the type (Christian König)
>
> Applied to drm-misc, thanks.
Well, had to drop it again since it didn't compile:
CC [M] drivers/gpu/drm/drm_blend.o
drivers/gpu/drm/drm_atomic.c: In function ‘drm_atomic_plane_print_state’:
drivers/gpu/drm/drm_atomic.c:920:5: error: too few arguments to function ‘drm_get_format_name’
drm_get_format_name(fb->pixel_format));
^~~~~~~~~~~~~~~~~~~
In file included from ./include/drm/drmP.h:71:0,
from drivers/gpu/drm/drm_atomic.c:29:
./include/drm/drm_fourcc.h:65:7: note: declared here
char *drm_get_format_name(uint32_t format, struct drm_format_name_buf *buf);
^~~~~~~~~~~~~~~~~~~
Can you pls rebase onto drm-misc or linux-next or something?
Thanks, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply
* Re: [RFC PATCH 18/24] ARM: vITS: handle MOVI command
From: Stefano Stabellini @ 2016-11-09 1:13 UTC (permalink / raw)
To: Andre Przywara; +Cc: xen-devel, Julien Grall, Stefano Stabellini
In-Reply-To: <20160928182457.12433-19-andre.przywara@arm.com>
On Wed, 28 Sep 2016, Andre Przywara wrote:
> The MOVI command moves the interrupt affinity from one redistributor
> (read: VCPU) to another.
> For now migration of "live" LPIs is not yet implemented, but we store
> the changed affinity in the host LPI structure and in our virtual ITTE.
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
> xen/arch/arm/gic-its.c | 16 +++++++++++++++
> xen/arch/arm/vgic-its.c | 46 +++++++++++++++++++++++++++++++++++++++++++
> xen/include/asm-arm/gic-its.h | 1 +
> 3 files changed, 63 insertions(+)
>
> diff --git a/xen/arch/arm/gic-its.c b/xen/arch/arm/gic-its.c
> index 6bac422..d1b1cbb 100644
> --- a/xen/arch/arm/gic-its.c
> +++ b/xen/arch/arm/gic-its.c
> @@ -618,6 +618,22 @@ int gicv3_lpi_drop_host_lpi(struct host_its *its,
> return 0;
> }
>
> +/* Changes the target VCPU for a given host LPI assigned to a domain. */
> +int gicv3_lpi_change_vcpu(struct domain *d, uint32_t host_lpi, int new_vcpu_id)
> +{
> + union host_lpi *hlpip, hlpi;
> +
> + hlpip = gic_find_host_lpi(host_lpi, d);
> + if ( !hlpip )
> + return -1;
> +
> + hlpi.data = hlpip->data;
> + hlpi.vcpu_id = new_vcpu_id;
> + hlpip->data = hlpi.data;
Most surely we need to call vgic_migrate_irq
> + return 0;
> +}
> +
> void gicv3_its_dt_init(const struct dt_device_node *node)
> {
> const struct dt_device_node *its = NULL;
> diff --git a/xen/arch/arm/vgic-its.c b/xen/arch/arm/vgic-its.c
> index 70897dd..c0a60ad 100644
> --- a/xen/arch/arm/vgic-its.c
> +++ b/xen/arch/arm/vgic-its.c
> @@ -327,6 +327,46 @@ out_unlock:
> return ret;
> }
>
> +static int its_handle_movi(struct virt_its *its, uint64_t *cmdptr)
> +{
> + uint32_t devid = its_cmd_get_deviceid(cmdptr);
> + uint32_t eventid = its_cmd_get_id(cmdptr);
> + int collid = its_cmd_get_collection(cmdptr);
> + struct vits_itte *itte;
> + struct vcpu *vcpu;
> + uint32_t host_lpi = 0;
> +
> + if ( collid >= its->max_collections )
> + return -1;
> +
> + spin_lock(&its->its_lock);
> +
> + vcpu = get_vcpu_from_collection(its, collid);
> + if ( !vcpu )
> + goto out_unlock;
> +
> + itte = get_devid_evid(its, devid, eventid);
> + if ( !itte )
> + goto out_unlock;
> +
> + itte->collection = collid;
> + host_lpi = itte->hlpi;
> +
> + /* TODO: lookup currently-in-guest virtual IRQs and migrate them */
> +
> + put_devid_evid(its, itte);
> +
> +out_unlock:
> + spin_unlock(&its->its_lock);
> +
> + if ( !host_lpi )
> + return -1;
> +
> + gicv3_lpi_change_vcpu(its->d, host_lpi, vcpu->vcpu_id);
> +
> + return 0;
> +}
> +
> #define ITS_CMD_BUFFER_SIZE(baser) ((((baser) & 0xff) + 1) << 12)
>
> static int vgic_its_handle_cmds(struct domain *d, struct virt_its *its,
> @@ -363,6 +403,12 @@ static int vgic_its_handle_cmds(struct domain *d, struct virt_its *its,
> case GITS_CMD_MAPTI:
> its_handle_mapti(its, cmdptr);
> break;
> + case GITS_CMD_MOVALL:
> + gdprintk(XENLOG_G_INFO, "ITS: ignoring MOVALL command\n");
> + break;
> + case GITS_CMD_MOVI:
> + its_handle_movi(its, cmdptr);
> + break;
> case GITS_CMD_SYNC:
> /* We handle ITS commands synchronously, so we ignore SYNC. */
> break;
> diff --git a/xen/include/asm-arm/gic-its.h b/xen/include/asm-arm/gic-its.h
> index 3b2e5c0..7e1142f 100644
> --- a/xen/include/asm-arm/gic-its.h
> +++ b/xen/include/asm-arm/gic-its.h
> @@ -133,6 +133,7 @@ int gicv3_its_map_device(struct host_its *hw_its, struct domain *d,
> int gicv3_lpi_allocate_host_lpi(struct host_its *its,
> uint32_t devid, uint32_t eventid,
> struct vcpu *v, int virt_lpi);
> +int gicv3_lpi_change_vcpu(struct domain *d, uint32_t host_lpi, int new_vcpu_id);
> int gicv3_lpi_drop_host_lpi(struct host_its *its,
> uint32_t devid, uint32_t eventid,
> uint32_t host_lpi);
> --
> 2.9.0
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply
* support for partial irq affinity assignment V3
From: Christoph Hellwig @ 2016-11-09 1:15 UTC (permalink / raw)
To: tglx; +Cc: axboe, linux-block, linux-pci, linux-kernel
This series adds support for automatic interrupt assignment to devices
that have a few vectors that are set aside for admin or config purposes
and thus should not fall into the general per-cpu assginment pool.
The first patch adds that support to the core IRQ and PCI/msi code,
and the second is a small tweak to a block layer helper to make use
of it. I'd love to have both go into the same tree so that consumers
of this (e.g. the virtio, scsi and rdma trees) only need to pull in
one of these trees as dependency.
Changes since V3:
- lots of typo fixes
- fixed an argument name
Changes since V1:
- split up into more patches (thanks to Thomas for the initial work)
- simplify internal parameter passing a bit (also thanks to Thomas)
^ permalink raw reply
* [PATCH 1/7] genirq/affinity: Introduce struct irq_affinity
From: Christoph Hellwig @ 2016-11-09 1:15 UTC (permalink / raw)
To: tglx; +Cc: axboe, linux-block, linux-pci, linux-kernel
In-Reply-To: <1478654107-7384-1-git-send-email-hch@lst.de>
Some drivers (various network and RDMA adapter for example) have a MSI-X
vector layout where most of the vectors are used for I/O queues and should
have CPU affinity assigned to them, but some (usually 1 but sometimes more)
at the beginning or end are used for low-performance admin or configuration
work and should not have any explicit affinity assigned to them.
This adds a new irq_affinity structure, which will be passed through a
variant of pci_irq_alloc_vectors that allows to specify these
requirements (and is extensible to any future quirks in that area) so that
the core IRQ affinity algorithm can take this quirks into account.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
Reviewed-by: Hannes Reinecke <hare@suse.com>
---
include/linux/interrupt.h | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index 72f0721..6b52686 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -232,6 +232,18 @@ struct irq_affinity_notify {
void (*release)(struct kref *ref);
};
+/**
+ * struct irq_affinity - Description for automatic irq affinity assignements
+ * @pre_vectors: Don't apply affinity to @pre_vectors at beginning of
+ * the MSI(-X) vector space
+ * @post_vectors: Don't apply affinity to @post_vectors at end of
+ * the MSI(-X) vector space
+ */
+struct irq_affinity {
+ int pre_vectors;
+ int post_vectors;
+};
+
#if defined(CONFIG_SMP)
extern cpumask_var_t irq_default_affinity;
--
2.1.4
^ permalink raw reply related
* [PATCH 2/7] genirq/affinity: Handle pre/post vectors in irq_calc_affinity_vectors()
From: Christoph Hellwig @ 2016-11-09 1:15 UTC (permalink / raw)
To: tglx; +Cc: axboe, linux-block, linux-pci, linux-kernel
In-Reply-To: <1478654107-7384-1-git-send-email-hch@lst.de>
Only calculate the affinity for the main I/O vectors, and skip the
pre or post vectors specified by struct irq_affinity.
Also remove the irq_affinity cpumask argument that has never been used.
If we ever need it in the future we can pass it through struct
irq_affinity.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/pci/msi.c | 8 ++++----
include/linux/interrupt.h | 4 ++--
kernel/irq/affinity.c | 24 ++++++++++--------------
3 files changed, 16 insertions(+), 20 deletions(-)
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index ad70507..dad2da7 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -1061,6 +1061,7 @@ EXPORT_SYMBOL(pci_msi_enabled);
static int __pci_enable_msi_range(struct pci_dev *dev, int minvec, int maxvec,
unsigned int flags)
{
+ static const struct irq_affinity default_affd;
bool affinity = flags & PCI_IRQ_AFFINITY;
int nvec;
int rc;
@@ -1091,8 +1092,7 @@ static int __pci_enable_msi_range(struct pci_dev *dev, int minvec, int maxvec,
for (;;) {
if (affinity) {
- nvec = irq_calc_affinity_vectors(dev->irq_affinity,
- nvec);
+ nvec = irq_calc_affinity_vectors(nvec, &default_affd);
if (nvec < minvec)
return -ENOSPC;
}
@@ -1132,6 +1132,7 @@ static int __pci_enable_msix_range(struct pci_dev *dev,
struct msix_entry *entries, int minvec, int maxvec,
unsigned int flags)
{
+ static const struct irq_affinity default_affd;
bool affinity = flags & PCI_IRQ_AFFINITY;
int rc, nvec = maxvec;
@@ -1140,8 +1141,7 @@ static int __pci_enable_msix_range(struct pci_dev *dev,
for (;;) {
if (affinity) {
- nvec = irq_calc_affinity_vectors(dev->irq_affinity,
- nvec);
+ nvec = irq_calc_affinity_vectors(nvec, &default_affd);
if (nvec < minvec)
return -ENOSPC;
}
diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index 6b52686..9081f23b 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -291,7 +291,7 @@ extern int
irq_set_affinity_notifier(unsigned int irq, struct irq_affinity_notify *notify);
struct cpumask *irq_create_affinity_masks(const struct cpumask *affinity, int nvec);
-int irq_calc_affinity_vectors(const struct cpumask *affinity, int maxvec);
+int irq_calc_affinity_vectors(int maxvec, const struct irq_affinity *affd);
#else /* CONFIG_SMP */
@@ -331,7 +331,7 @@ irq_create_affinity_masks(const struct cpumask *affinity, int nvec)
}
static inline int
-irq_calc_affinity_vectors(const struct cpumask *affinity, int maxvec)
+irq_calc_affinity_vectors(int maxvec, const struct irq_affinity *affd)
{
return maxvec;
}
diff --git a/kernel/irq/affinity.c b/kernel/irq/affinity.c
index 17f51d63..8d92597 100644
--- a/kernel/irq/affinity.c
+++ b/kernel/irq/affinity.c
@@ -131,24 +131,20 @@ struct cpumask *irq_create_affinity_masks(const struct cpumask *affinity,
}
/**
- * irq_calc_affinity_vectors - Calculate to optimal number of vectors for a given affinity mask
- * @affinity: The affinity mask to spread. If NULL cpu_online_mask
- * is used
- * @maxvec: The maximum number of vectors available
+ * irq_calc_affinity_vectors - Calculate the optimal number of vectors
+ * @maxvec: The maximum number of vectors available
+ * @affd: Description of the affinity requirements
*/
-int irq_calc_affinity_vectors(const struct cpumask *affinity, int maxvec)
+int irq_calc_affinity_vectors(int maxvec, const struct irq_affinity *affd)
{
- int cpus, ret;
+ int resv = affd->pre_vectors + affd->post_vectors;
+ int vecs = maxvec - resv;
+ int cpus;
/* Stabilize the cpumasks */
get_online_cpus();
- /* If the supplied affinity mask is NULL, use cpu online mask */
- if (!affinity)
- affinity = cpu_online_mask;
-
- cpus = cpumask_weight(affinity);
- ret = (cpus < maxvec) ? cpus : maxvec;
-
+ cpus = cpumask_weight(cpu_online_mask);
put_online_cpus();
- return ret;
+
+ return min(cpus, vecs) + resv;
}
--
2.1.4
^ permalink raw reply related
* [PATCH 3/7] genirq/affinity: Handle pre/post vectors in irq_create_affinity_masks()
From: Christoph Hellwig @ 2016-11-09 1:15 UTC (permalink / raw)
To: tglx; +Cc: axboe, linux-block, linux-pci, linux-kernel
In-Reply-To: <1478654107-7384-1-git-send-email-hch@lst.de>
Only calculate the affinity for the main I/O vectors, and skip the
pre or post vectors specified by struct irq_affinity.
Also remove the irq_affinity cpumask argument that has never been used.
If we ever need it in the future we can pass it through struct
irq_affinity.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
---
drivers/pci/msi.c | 6 ++++--
include/linux/interrupt.h | 4 ++--
kernel/irq/affinity.c | 46 +++++++++++++++++++++++++---------------------
3 files changed, 31 insertions(+), 25 deletions(-)
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index dad2da7..f4a108b 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -553,12 +553,13 @@ static int populate_msi_sysfs(struct pci_dev *pdev)
static struct msi_desc *
msi_setup_entry(struct pci_dev *dev, int nvec, bool affinity)
{
+ static const struct irq_affinity default_affd;
struct cpumask *masks = NULL;
struct msi_desc *entry;
u16 control;
if (affinity) {
- masks = irq_create_affinity_masks(dev->irq_affinity, nvec);
+ masks = irq_create_affinity_masks(nvec, &default_affd);
if (!masks)
pr_err("Unable to allocate affinity masks, ignoring\n");
}
@@ -692,12 +693,13 @@ static int msix_setup_entries(struct pci_dev *dev, void __iomem *base,
struct msix_entry *entries, int nvec,
bool affinity)
{
+ static const struct irq_affinity default_affd;
struct cpumask *curmsk, *masks = NULL;
struct msi_desc *entry;
int ret, i;
if (affinity) {
- masks = irq_create_affinity_masks(dev->irq_affinity, nvec);
+ masks = irq_create_affinity_masks(nvec, &default_affd);
if (!masks)
pr_err("Unable to allocate affinity masks, ignoring\n");
}
diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index 9081f23b..53144e7 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -290,7 +290,7 @@ extern int irq_set_affinity_hint(unsigned int irq, const struct cpumask *m);
extern int
irq_set_affinity_notifier(unsigned int irq, struct irq_affinity_notify *notify);
-struct cpumask *irq_create_affinity_masks(const struct cpumask *affinity, int nvec);
+struct cpumask *irq_create_affinity_masks(int nvec, const struct irq_affinity *affd);
int irq_calc_affinity_vectors(int maxvec, const struct irq_affinity *affd);
#else /* CONFIG_SMP */
@@ -325,7 +325,7 @@ irq_set_affinity_notifier(unsigned int irq, struct irq_affinity_notify *notify)
}
static inline struct cpumask *
-irq_create_affinity_masks(const struct cpumask *affinity, int nvec)
+irq_create_affinity_masks(int nvec, const struct irq_affinity *affd)
{
return NULL;
}
diff --git a/kernel/irq/affinity.c b/kernel/irq/affinity.c
index 8d92597..17360bd 100644
--- a/kernel/irq/affinity.c
+++ b/kernel/irq/affinity.c
@@ -51,16 +51,16 @@ static int get_nodes_in_cpumask(const struct cpumask *mask, nodemask_t *nodemsk)
/**
* irq_create_affinity_masks - Create affinity masks for multiqueue spreading
- * @affinity: The affinity mask to spread. If NULL cpu_online_mask
- * is used
- * @nvecs: The number of vectors
+ * @nvecs: The total number of vectors
+ * @affd: Description of the affinity requirements
*
* Returns the masks pointer or NULL if allocation failed.
*/
-struct cpumask *irq_create_affinity_masks(const struct cpumask *affinity,
- int nvec)
+struct cpumask *
+irq_create_affinity_masks(int nvecs, const struct irq_affinity *affd)
{
- int n, nodes, vecs_per_node, cpus_per_vec, extra_vecs, curvec = 0;
+ int n, nodes, vecs_per_node, cpus_per_vec, extra_vecs, curvec;
+ int affv = nvecs - affd->pre_vectors - affd->post_vectors;
nodemask_t nodemsk = NODE_MASK_NONE;
struct cpumask *masks;
cpumask_var_t nmsk;
@@ -68,46 +68,46 @@ struct cpumask *irq_create_affinity_masks(const struct cpumask *affinity,
if (!zalloc_cpumask_var(&nmsk, GFP_KERNEL))
return NULL;
- masks = kzalloc(nvec * sizeof(*masks), GFP_KERNEL);
+ masks = kcalloc(nvecs, sizeof(*masks), GFP_KERNEL);
if (!masks)
goto out;
+ /* Fill out vectors at the beginning that don't need affinity */
+ for (curvec = 0; curvec < affd->pre_vectors; curvec++)
+ cpumask_copy(masks + curvec, cpu_possible_mask);
+
/* Stabilize the cpumasks */
get_online_cpus();
- /* If the supplied affinity mask is NULL, use cpu online mask */
- if (!affinity)
- affinity = cpu_online_mask;
-
- nodes = get_nodes_in_cpumask(affinity, &nodemsk);
+ nodes = get_nodes_in_cpumask(cpu_online_mask, &nodemsk);
/*
* If the number of nodes in the mask is less than or equal the
* number of vectors we just spread the vectors across the nodes.
*/
- if (nvec <= nodes) {
+ if (affv <= nodes) {
for_each_node_mask(n, nodemsk) {
cpumask_copy(masks + curvec, cpumask_of_node(n));
- if (++curvec == nvec)
+ if (++curvec == affv)
break;
}
- goto outonl;
+ goto done;
}
/* Spread the vectors per node */
- vecs_per_node = nvec / nodes;
+ vecs_per_node = affv / nodes;
/* Account for rounding errors */
- extra_vecs = nvec - (nodes * vecs_per_node);
+ extra_vecs = affv - (nodes * vecs_per_node);
for_each_node_mask(n, nodemsk) {
int ncpus, v, vecs_to_assign = vecs_per_node;
/* Get the cpus on this node which are in the mask */
- cpumask_and(nmsk, affinity, cpumask_of_node(n));
+ cpumask_and(nmsk, cpu_online_mask, cpumask_of_node(n));
/* Calculate the number of cpus per vector */
ncpus = cpumask_weight(nmsk);
- for (v = 0; curvec < nvec && v < vecs_to_assign; curvec++, v++) {
+ for (v = 0; curvec < affv && v < vecs_to_assign; curvec++, v++) {
cpus_per_vec = ncpus / vecs_to_assign;
/* Account for extra vectors to compensate rounding errors */
@@ -119,12 +119,16 @@ struct cpumask *irq_create_affinity_masks(const struct cpumask *affinity,
irq_spread_init_one(masks + curvec, nmsk, cpus_per_vec);
}
- if (curvec >= nvec)
+ if (curvec >= affv)
break;
}
-outonl:
+done:
put_online_cpus();
+
+ /* Fill out vectors at the end that don't need affinity */
+ for (; curvec < nvecs; curvec++)
+ cpumask_copy(masks + curvec, cpu_possible_mask);
out:
free_cpumask_var(nmsk);
return masks;
--
2.1.4
^ permalink raw reply related
* [PATCH 4/7] PCI/MSI: Propagate IRQ affinity description through the MSI code
From: Christoph Hellwig @ 2016-11-09 1:15 UTC (permalink / raw)
To: tglx; +Cc: axboe, linux-block, linux-pci, linux-kernel
In-Reply-To: <1478654107-7384-1-git-send-email-hch@lst.de>
No API change yet, just pass it down all the way from
pci_alloc_irq_vectors() to the core MSI code.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
---
drivers/pci/msi.c | 66 +++++++++++++++++++++++++++----------------------------
1 file changed, 33 insertions(+), 33 deletions(-)
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index f4a108b..512f388 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -551,15 +551,14 @@ static int populate_msi_sysfs(struct pci_dev *pdev)
}
static struct msi_desc *
-msi_setup_entry(struct pci_dev *dev, int nvec, bool affinity)
+msi_setup_entry(struct pci_dev *dev, int nvec, const struct irq_affinity *affd)
{
- static const struct irq_affinity default_affd;
struct cpumask *masks = NULL;
struct msi_desc *entry;
u16 control;
- if (affinity) {
- masks = irq_create_affinity_masks(nvec, &default_affd);
+ if (affd) {
+ masks = irq_create_affinity_masks(nvec, affd);
if (!masks)
pr_err("Unable to allocate affinity masks, ignoring\n");
}
@@ -619,7 +618,8 @@ static int msi_verify_entries(struct pci_dev *dev)
* an error, and a positive return value indicates the number of interrupts
* which could have been allocated.
*/
-static int msi_capability_init(struct pci_dev *dev, int nvec, bool affinity)
+static int msi_capability_init(struct pci_dev *dev, int nvec,
+ const struct irq_affinity *affd)
{
struct msi_desc *entry;
int ret;
@@ -627,7 +627,7 @@ static int msi_capability_init(struct pci_dev *dev, int nvec, bool affinity)
pci_msi_set_enable(dev, 0); /* Disable MSI during set up */
- entry = msi_setup_entry(dev, nvec, affinity);
+ entry = msi_setup_entry(dev, nvec, affd);
if (!entry)
return -ENOMEM;
@@ -691,15 +691,14 @@ static void __iomem *msix_map_region(struct pci_dev *dev, unsigned nr_entries)
static int msix_setup_entries(struct pci_dev *dev, void __iomem *base,
struct msix_entry *entries, int nvec,
- bool affinity)
+ const struct irq_affinity *affd)
{
- static const struct irq_affinity default_affd;
struct cpumask *curmsk, *masks = NULL;
struct msi_desc *entry;
int ret, i;
- if (affinity) {
- masks = irq_create_affinity_masks(nvec, &default_affd);
+ if (affd) {
+ masks = irq_create_affinity_masks(nvec, affd);
if (!masks)
pr_err("Unable to allocate affinity masks, ignoring\n");
}
@@ -755,14 +754,14 @@ static void msix_program_entries(struct pci_dev *dev,
* @dev: pointer to the pci_dev data structure of MSI-X device function
* @entries: pointer to an array of struct msix_entry entries
* @nvec: number of @entries
- * @affinity: flag to indicate cpu irq affinity mask should be set
+ * @affd: Optional pointer to enable automatic affinity assignement
*
* Setup the MSI-X capability structure of device function with a
* single MSI-X irq. A return of zero indicates the successful setup of
* requested MSI-X entries with allocated irqs or non-zero for otherwise.
**/
static int msix_capability_init(struct pci_dev *dev, struct msix_entry *entries,
- int nvec, bool affinity)
+ int nvec, const struct irq_affinity *affd)
{
int ret;
u16 control;
@@ -777,7 +776,7 @@ static int msix_capability_init(struct pci_dev *dev, struct msix_entry *entries,
if (!base)
return -ENOMEM;
- ret = msix_setup_entries(dev, base, entries, nvec, affinity);
+ ret = msix_setup_entries(dev, base, entries, nvec, affd);
if (ret)
return ret;
@@ -958,7 +957,7 @@ int pci_msix_vec_count(struct pci_dev *dev)
EXPORT_SYMBOL(pci_msix_vec_count);
static int __pci_enable_msix(struct pci_dev *dev, struct msix_entry *entries,
- int nvec, bool affinity)
+ int nvec, const struct irq_affinity *affd)
{
int nr_entries;
int i, j;
@@ -990,7 +989,7 @@ static int __pci_enable_msix(struct pci_dev *dev, struct msix_entry *entries,
dev_info(&dev->dev, "can't enable MSI-X (MSI IRQ already assigned)\n");
return -EINVAL;
}
- return msix_capability_init(dev, entries, nvec, affinity);
+ return msix_capability_init(dev, entries, nvec, affd);
}
/**
@@ -1010,7 +1009,7 @@ static int __pci_enable_msix(struct pci_dev *dev, struct msix_entry *entries,
**/
int pci_enable_msix(struct pci_dev *dev, struct msix_entry *entries, int nvec)
{
- return __pci_enable_msix(dev, entries, nvec, false);
+ return __pci_enable_msix(dev, entries, nvec, NULL);
}
EXPORT_SYMBOL(pci_enable_msix);
@@ -1061,10 +1060,8 @@ int pci_msi_enabled(void)
EXPORT_SYMBOL(pci_msi_enabled);
static int __pci_enable_msi_range(struct pci_dev *dev, int minvec, int maxvec,
- unsigned int flags)
+ const struct irq_affinity *affd)
{
- static const struct irq_affinity default_affd;
- bool affinity = flags & PCI_IRQ_AFFINITY;
int nvec;
int rc;
@@ -1093,13 +1090,13 @@ static int __pci_enable_msi_range(struct pci_dev *dev, int minvec, int maxvec,
nvec = maxvec;
for (;;) {
- if (affinity) {
- nvec = irq_calc_affinity_vectors(nvec, &default_affd);
+ if (affd) {
+ nvec = irq_calc_affinity_vectors(nvec, affd);
if (nvec < minvec)
return -ENOSPC;
}
- rc = msi_capability_init(dev, nvec, affinity);
+ rc = msi_capability_init(dev, nvec, affd);
if (rc == 0)
return nvec;
@@ -1126,29 +1123,27 @@ static int __pci_enable_msi_range(struct pci_dev *dev, int minvec, int maxvec,
**/
int pci_enable_msi_range(struct pci_dev *dev, int minvec, int maxvec)
{
- return __pci_enable_msi_range(dev, minvec, maxvec, 0);
+ return __pci_enable_msi_range(dev, minvec, maxvec, NULL);
}
EXPORT_SYMBOL(pci_enable_msi_range);
static int __pci_enable_msix_range(struct pci_dev *dev,
- struct msix_entry *entries, int minvec, int maxvec,
- unsigned int flags)
+ struct msix_entry *entries, int minvec,
+ int maxvec, const struct irq_affinity *affd)
{
- static const struct irq_affinity default_affd;
- bool affinity = flags & PCI_IRQ_AFFINITY;
int rc, nvec = maxvec;
if (maxvec < minvec)
return -ERANGE;
for (;;) {
- if (affinity) {
- nvec = irq_calc_affinity_vectors(nvec, &default_affd);
+ if (affd) {
+ nvec = irq_calc_affinity_vectors(nvec, affd);
if (nvec < minvec)
return -ENOSPC;
}
- rc = __pci_enable_msix(dev, entries, nvec, affinity);
+ rc = __pci_enable_msix(dev, entries, nvec, affd);
if (rc == 0)
return nvec;
@@ -1179,7 +1174,7 @@ static int __pci_enable_msix_range(struct pci_dev *dev,
int pci_enable_msix_range(struct pci_dev *dev, struct msix_entry *entries,
int minvec, int maxvec)
{
- return __pci_enable_msix_range(dev, entries, minvec, maxvec, 0);
+ return __pci_enable_msix_range(dev, entries, minvec, maxvec, NULL);
}
EXPORT_SYMBOL(pci_enable_msix_range);
@@ -1203,17 +1198,22 @@ EXPORT_SYMBOL(pci_enable_msix_range);
int pci_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs,
unsigned int max_vecs, unsigned int flags)
{
+ static const struct irq_affinity msi_default_affd;
+ const struct irq_affinity *affd = NULL;
int vecs = -ENOSPC;
+ if (flags & PCI_IRQ_AFFINITY)
+ affd = &msi_default_affd;
+
if (flags & PCI_IRQ_MSIX) {
vecs = __pci_enable_msix_range(dev, NULL, min_vecs, max_vecs,
- flags);
+ affd);
if (vecs > 0)
return vecs;
}
if (flags & PCI_IRQ_MSI) {
- vecs = __pci_enable_msi_range(dev, min_vecs, max_vecs, flags);
+ vecs = __pci_enable_msi_range(dev, min_vecs, max_vecs, affd);
if (vecs > 0)
return vecs;
}
--
2.1.4
^ permalink raw reply related
* [PATCH 5/7] pci/msi: Provide pci_alloc_irq_vectors_affinity()
From: Christoph Hellwig @ 2016-11-09 1:15 UTC (permalink / raw)
To: tglx; +Cc: axboe, linux-block, linux-pci, linux-kernel
In-Reply-To: <1478654107-7384-1-git-send-email-hch@lst.de>
This is a variant of pci_alloc_irq_vectors() that allows passing a
struct irq_affinity to provide fine-grained IRQ affinity control.
For now this means being able to exclude vectors at the beginning or
end of the MSI vector space, but it could also be used for any other
quirks needed in the future (e.g. more vectors than CPUs, or excluding
CPUs from the spreading).
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
---
drivers/pci/msi.c | 20 +++++++++++++-------
include/linux/pci.h | 24 +++++++++++++++++++-----
2 files changed, 32 insertions(+), 12 deletions(-)
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 512f388..dd27f73 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -1179,11 +1179,12 @@ int pci_enable_msix_range(struct pci_dev *dev, struct msix_entry *entries,
EXPORT_SYMBOL(pci_enable_msix_range);
/**
- * pci_alloc_irq_vectors - allocate multiple IRQs for a device
+ * pci_alloc_irq_vectors_affinity - allocate multiple IRQs for a device
* @dev: PCI device to operate on
* @min_vecs: minimum number of vectors required (must be >= 1)
* @max_vecs: maximum (desired) number of vectors
* @flags: flags or quirks for the allocation
+ * @affd: optional description of the affinity requirements
*
* Allocate up to @max_vecs interrupt vectors for @dev, using MSI-X or MSI
* vectors if available, and fall back to a single legacy vector
@@ -1195,15 +1196,20 @@ EXPORT_SYMBOL(pci_enable_msix_range);
* To get the Linux IRQ number used for a vector that can be passed to
* request_irq() use the pci_irq_vector() helper.
*/
-int pci_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs,
- unsigned int max_vecs, unsigned int flags)
+int pci_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
+ unsigned int max_vecs, unsigned int flags,
+ const struct irq_affinity *affd)
{
static const struct irq_affinity msi_default_affd;
- const struct irq_affinity *affd = NULL;
int vecs = -ENOSPC;
- if (flags & PCI_IRQ_AFFINITY)
- affd = &msi_default_affd;
+ if (flags & PCI_IRQ_AFFINITY) {
+ if (!affd)
+ affd = &msi_default_affd;
+ } else {
+ if (WARN_ON(affd))
+ affd = NULL;
+ }
if (flags & PCI_IRQ_MSIX) {
vecs = __pci_enable_msix_range(dev, NULL, min_vecs, max_vecs,
@@ -1226,7 +1232,7 @@ int pci_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs,
return vecs;
}
-EXPORT_SYMBOL(pci_alloc_irq_vectors);
+EXPORT_SYMBOL(pci_alloc_irq_vectors_affinity);
/**
* pci_free_irq_vectors - free previously allocated IRQs for a device
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 0e49f70..7090f5f 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -244,6 +244,7 @@ struct pci_cap_saved_state {
struct pci_cap_saved_data cap;
};
+struct irq_affinity;
struct pcie_link_state;
struct pci_vpd;
struct pci_sriov;
@@ -1310,8 +1311,10 @@ static inline int pci_enable_msix_exact(struct pci_dev *dev,
return rc;
return 0;
}
-int pci_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs,
- unsigned int max_vecs, unsigned int flags);
+int pci_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
+ unsigned int max_vecs, unsigned int flags,
+ const struct irq_affinity *affd);
+
void pci_free_irq_vectors(struct pci_dev *dev);
int pci_irq_vector(struct pci_dev *dev, unsigned int nr);
const struct cpumask *pci_irq_get_affinity(struct pci_dev *pdev, int vec);
@@ -1339,14 +1342,17 @@ static inline int pci_enable_msix_range(struct pci_dev *dev,
static inline int pci_enable_msix_exact(struct pci_dev *dev,
struct msix_entry *entries, int nvec)
{ return -ENOSYS; }
-static inline int pci_alloc_irq_vectors(struct pci_dev *dev,
- unsigned int min_vecs, unsigned int max_vecs,
- unsigned int flags)
+
+static inline int
+pci_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
+ unsigned int max_vecs, unsigned int flags,
+ const struct irq_affinity *aff_desc)
{
if (min_vecs > 1)
return -EINVAL;
return 1;
}
+
static inline void pci_free_irq_vectors(struct pci_dev *dev)
{
}
@@ -1364,6 +1370,14 @@ static inline const struct cpumask *pci_irq_get_affinity(struct pci_dev *pdev,
}
#endif
+static inline int
+pci_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs,
+ unsigned int max_vecs, unsigned int flags)
+{
+ return pci_alloc_irq_vectors_affinity(dev, min_vecs, max_vecs, flags,
+ NULL);
+}
+
#ifdef CONFIG_PCIEPORTBUS
extern bool pcie_ports_disabled;
extern bool pcie_ports_auto;
--
2.1.4
^ permalink raw reply related
* [PATCH 6/7] PCI: Remove the irq_affinity mask from struct pci_dev
From: Christoph Hellwig @ 2016-11-09 1:15 UTC (permalink / raw)
To: tglx; +Cc: axboe, linux-block, linux-pci, linux-kernel
In-Reply-To: <1478654107-7384-1-git-send-email-hch@lst.de>
This has never been used, and now is totally unreferenced. Nuke it.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
---
include/linux/pci.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 7090f5f..f2ba6ac 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -333,7 +333,6 @@ struct pci_dev {
* directly, use the values stored here. They might be different!
*/
unsigned int irq;
- struct cpumask *irq_affinity;
struct resource resource[DEVICE_COUNT_RESOURCE]; /* I/O and memory regions + expansion ROMs */
bool match_driver; /* Skip attaching driver */
--
2.1.4
^ permalink raw reply related
* [PATCH 7/7] blk-mq: add a first_vec argument to blk_mq_pci_map_queues
From: Christoph Hellwig @ 2016-11-09 1:15 UTC (permalink / raw)
To: tglx; +Cc: axboe, linux-block, linux-pci, linux-kernel
In-Reply-To: <1478654107-7384-1-git-send-email-hch@lst.de>
This allows skipping the first N IRQ vectors in case they are used for
control or admin interrupts.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
---
block/blk-mq-pci.c | 6 ++++--
drivers/nvme/host/pci.c | 2 +-
include/linux/blk-mq-pci.h | 3 ++-
3 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/block/blk-mq-pci.c b/block/blk-mq-pci.c
index 966c216..03ff7c4 100644
--- a/block/blk-mq-pci.c
+++ b/block/blk-mq-pci.c
@@ -21,6 +21,7 @@
* blk_mq_pci_map_queues - provide a default queue mapping for PCI device
* @set: tagset to provide the mapping for
* @pdev: PCI device associated with @set.
+ * @first_vec: first interrupt vectors to use for queues (usually 0)
*
* This function assumes the PCI device @pdev has at least as many available
* interrupt vetors as @set has queues. It will then queuery the vector
@@ -28,12 +29,13 @@
* that maps a queue to the CPUs that have irq affinity for the corresponding
* vector.
*/
-int blk_mq_pci_map_queues(struct blk_mq_tag_set *set, struct pci_dev *pdev)
+int blk_mq_pci_map_queues(struct blk_mq_tag_set *set, struct pci_dev *pdev,
+ int first_vec)
{
const struct cpumask *mask;
unsigned int queue, cpu;
- for (queue = 0; queue < set->nr_hw_queues; queue++) {
+ for (queue = first_vec; queue < set->nr_hw_queues; queue++) {
mask = pci_irq_get_affinity(pdev, queue);
if (!mask)
return -EINVAL;
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 0248d0e..6e6b917 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -273,7 +273,7 @@ static int nvme_pci_map_queues(struct blk_mq_tag_set *set)
{
struct nvme_dev *dev = set->driver_data;
- return blk_mq_pci_map_queues(set, to_pci_dev(dev->dev));
+ return blk_mq_pci_map_queues(set, to_pci_dev(dev->dev), 0);
}
/**
diff --git a/include/linux/blk-mq-pci.h b/include/linux/blk-mq-pci.h
index 6ab5952..fde26d2 100644
--- a/include/linux/blk-mq-pci.h
+++ b/include/linux/blk-mq-pci.h
@@ -4,6 +4,7 @@
struct blk_mq_tag_set;
struct pci_dev;
-int blk_mq_pci_map_queues(struct blk_mq_tag_set *set, struct pci_dev *pdev);
+int blk_mq_pci_map_queues(struct blk_mq_tag_set *set, struct pci_dev *pdev,
+ int first_vec);
#endif /* _LINUX_BLK_MQ_PCI_H */
--
2.1.4
^ permalink raw reply related
* Re: [1/2] rsi: New firware loading method for RSI 91X devices
From: Kalle Valo @ 2016-11-09 1:15 UTC (permalink / raw)
To: Prameela Rani Garnepudi
Cc: linux-wireless, johannes.berg, hofrat, xypron.glpk,
prameela.garnepudi, Prameela Rani Garnepudi
In-Reply-To: <1477044595-10348-1-git-send-email-prameela.j04cs@gmail.com>
Prameela Rani Garnepudi <prameela.j04cs@gmail.com> wrote:
> RSI deprecated the old firmware loading method and introduced
> new method using soft boot loader for 9113 chipsets.
> Current driver only supports 9113 device model hence firmware
> loading method has been changed.
>
> In the new method, complete RAM image and flash image are present
> in the flash. Two firmwares present in the device, Boot loader firmware
> and functional firmware. Boot loader firmware is fixed but functional
> firmware can be changed. Before loading the functional firmware, host
> issues commands to check whether existing firmware in the chip and the
> firmware file content to load are same or not. If not, host issues
> commands to load the RAM image and then boot loaded switches to the
> functioanl firmware.
>
> Signed-off-by: Prameela Rani Garnepudi <prameela.j04cs@gmail.com>
These two patches are quite big, difficult to review. Smaller changes
would help with that. Will review later.
2 patches set to Deferred.
9388629 [1/2] rsi: New firware loading method for RSI 91X devices
9388627 [2/2] rsi: Device initialization sequence is changed
--
https://patchwork.kernel.org/patch/9388629/
Documentation about submitting wireless patches and checking status
from patchwork:
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* [PATCH RESEND] crypto: gf128mul - remove dead gf128mul_64k_lle code
From: Alex Cope @ 2016-11-09 1:16 UTC (permalink / raw)
To: linux-crypto; +Cc: Alex Cope
This code is unlikely to be useful in the future because transforms
don't know how often keys will be changed, new algorithms are unlikely
to use lle representation, and tables should be replaced with
carryless multiplication instructions when available.
Signed-off-by: Alex Cope <alexcope@google.com>
---
crypto/gf128mul.c | 55 -----------------------------------------------
include/crypto/gf128mul.h | 13 ++++++-----
2 files changed, 6 insertions(+), 62 deletions(-)
diff --git a/crypto/gf128mul.c b/crypto/gf128mul.c
index 5276607..57c85dd 100644
--- a/crypto/gf128mul.c
+++ b/crypto/gf128mul.c
@@ -263,48 +263,6 @@ EXPORT_SYMBOL(gf128mul_bbe);
* t[1][BYTE] contains g*x^8*BYTE
* ..
* t[15][BYTE] contains g*x^120*BYTE */
-struct gf128mul_64k *gf128mul_init_64k_lle(const be128 *g)
-{
- struct gf128mul_64k *t;
- int i, j, k;
-
- t = kzalloc(sizeof(*t), GFP_KERNEL);
- if (!t)
- goto out;
-
- for (i = 0; i < 16; i++) {
- t->t[i] = kzalloc(sizeof(*t->t[i]), GFP_KERNEL);
- if (!t->t[i]) {
- gf128mul_free_64k(t);
- t = NULL;
- goto out;
- }
- }
-
- t->t[0]->t[128] = *g;
- for (j = 64; j > 0; j >>= 1)
- gf128mul_x_lle(&t->t[0]->t[j], &t->t[0]->t[j + j]);
-
- for (i = 0;;) {
- for (j = 2; j < 256; j += j)
- for (k = 1; k < j; ++k)
- be128_xor(&t->t[i]->t[j + k],
- &t->t[i]->t[j], &t->t[i]->t[k]);
-
- if (++i >= 16)
- break;
-
- for (j = 128; j > 0; j >>= 1) {
- t->t[i]->t[j] = t->t[i - 1]->t[j];
- gf128mul_x8_lle(&t->t[i]->t[j]);
- }
- }
-
-out:
- return t;
-}
-EXPORT_SYMBOL(gf128mul_init_64k_lle);
-
struct gf128mul_64k *gf128mul_init_64k_bbe(const be128 *g)
{
struct gf128mul_64k *t;
@@ -357,19 +315,6 @@ void gf128mul_free_64k(struct gf128mul_64k *t)
}
EXPORT_SYMBOL(gf128mul_free_64k);
-void gf128mul_64k_lle(be128 *a, struct gf128mul_64k *t)
-{
- u8 *ap = (u8 *)a;
- be128 r[1];
- int i;
-
- *r = t->t[0]->t[ap[0]];
- for (i = 1; i < 16; ++i)
- be128_xor(r, r, &t->t[i]->t[ap[i]]);
- *a = *r;
-}
-EXPORT_SYMBOL(gf128mul_64k_lle);
-
void gf128mul_64k_bbe(be128 *a, struct gf128mul_64k *t)
{
u8 *ap = (u8 *)a;
diff --git a/include/crypto/gf128mul.h b/include/crypto/gf128mul.h
index da2530e..b611aa9 100644
--- a/include/crypto/gf128mul.h
+++ b/include/crypto/gf128mul.h
@@ -181,20 +181,19 @@ static inline void gf128mul_free_4k(struct gf128mul_4k *t)
}
-/* 64k table optimization, implemented for lle and bbe */
+/* 64k table optimization, implemented for bbe */
struct gf128mul_64k {
struct gf128mul_4k *t[16];
};
-/* first initialize with the constant factor with which you
- * want to multiply and then call gf128_64k_lle with the other
- * factor in the first argument, the table in the second and a
- * scratch register in the third. Afterwards *a = *r. */
-struct gf128mul_64k *gf128mul_init_64k_lle(const be128 *g);
+/* First initialize with the constant factor with which you
+ * want to multiply and then call gf128mul_64k_bbe with the other
+ * factor in the first argument, and the table in the second.
+ * Afterwards, the result is stored in *a.
+ */
struct gf128mul_64k *gf128mul_init_64k_bbe(const be128 *g);
void gf128mul_free_64k(struct gf128mul_64k *t);
-void gf128mul_64k_lle(be128 *a, struct gf128mul_64k *t);
void gf128mul_64k_bbe(be128 *a, struct gf128mul_64k *t);
#endif /* _CRYPTO_GF128MUL_H */
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* Re: [PATCH rdma-core 0/8] libpvrdma: userspace library for PVRDMA
From: Christoph Hellwig @ 2016-11-09 1:17 UTC (permalink / raw)
To: Adit Ranadive
Cc: Doug Ledford, Christoph Hellwig, Jason Gunthorpe,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, pv-drivers
In-Reply-To: <f67c81ff-d358-7242-dc52-e32dff252904-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
FYI, the convention used in scsi is vmw_pvscsi.c, so naming the
RDMA equivalent vmw_pvrdma would make a lot of sense and still
be reasonably short.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: ACPI module-level code (MLC) not working?
From: Peter Wu @ 2016-11-09 1:17 UTC (permalink / raw)
To: Zheng, Lv
Cc: Rick Kerkhof, Bartosz Skrzypczak, Moore, Robert,
linux-acpi@vger.kernel.org
In-Reply-To: <1AE640813FDE7649BE1B193DEA596E886A270285@SHSMSX101.ccr.corp.intel.com>
On Wed, Nov 09, 2016 at 12:43:37AM +0000, Zheng, Lv wrote:
> Hi,
>
> > -----Original Message-----
> > From: Peter Wu [mailto:peter@lekensteyn.nl]
> > Sent: Tuesday, November 8, 2016 4:33 PM
> > To: Zheng, Lv <lv.zheng@intel.com>
> > Cc: Rick Kerkhof <rick.2889@gmail.com>; Bartosz Skrzypczak <barteks2x@gmail.com>; Moore, Robert
> > <robert.moore@intel.com>; linux-acpi@vger.kernel.org
> > Subject: Re: ACPI module-level code (MLC) not working?
> >
> > On Wed, Nov 09, 2016 at 12:07:47AM +0000, Zheng, Lv wrote:
> > > Hi,
> > >
> > > > From: Peter Wu [mailto:peter@lekensteyn.nl]
> > > > Subject: Re: ACPI module-level code (MLC) not working?
> > > >
> > > > On Tue, Nov 08, 2016 at 05:35:07PM +0000, Zheng, Lv wrote:
> > > > > Hi,
> > > > >
> > > > > > From: Peter Wu [mailto:peter@lekensteyn.nl]
> > > > > > Subject: ACPI module-level code (MLC) not working?
> > > > > >
> > > > > > Hi Lv,
> > > > > >
> > > > > > According to some tests, setting acpi_gbl_parse_table_as_term_list to
> > > > > > TRUE does is not effective. The code within the If-block is still not
> > > > > > executed early enough or something else is wrong.
> > > > > >
> > > > > > Previously Rick had an issue with an Acer Aspire V7-582PG where the dGPU
> > > > > > could not be powered off and I demonstrated an isolated test case in
> > > > > > http://www.spinics.net/lists/linux-acpi/msg70069.html
> > > > > >
> > > > > > In Bartosz's case, the dGPU cannot be powered on (also using nouveau),
> > > > > > preventing suspend from working. Situation is as follows (tested with
> > > > > > Linux 3.16, 4.8.4, 4.9-rc2, 4.9-rc4):
> > > > > >
> > > > > > His Lenovo IdeaPad Z510 laptop (BIOS date 2014) enables power resources
> > > > > > and related _PR3 objects under the conditional If(_OSI("Windows 2013")).
> > > > > > Both with and without acpi_gbl_parse_table_as_term_list set to TRUE, the
> > > > > > module-level code is not loaded properly. Via a SSDT override, it was
> > > > > > confirmed that removing the If conditional results in the expected
> > > > > > behavior.
> > > > > >
> > > > > > Various details are given in https://github.com/Bumblebee-Project/bbswitch/issues/142
> > > > > > including lots of dmesg logs (see posts at the bottom).
> > > > > > With above MLC flag set (v4.9-rc4): https://pastebin.com/raw/vCEPGezX
> > > > > > With SSDT override (v4.9-rc2): https://pastebin.com/raw/3Fsf2VPU
> > > > >
> > > > > I checked the post.
> > > > > It seems the current working way is: disabling _OSI(Windows 2013) which disables power resources.
> > > >
> > > > That is how Lenovo probably intended to use it (only add Power Resources
> > > > when Windows 8 is detected).
> > > >
> > > > > I have several questions related to this issue:
> > > > > 1. The following messages are prompt up when "acpi_gbl_parse_table_as_term_list = TRUE":
> > > > > [ 2.519113] ACPI Error: [\_SB_.PCI0.GFX0.DD02._BCL] Namespace lookup failure, AE_NOT_FOUND
> > > > (20160831/psargs-359)
> > > > > [ 2.519121] ACPI Error: Method parse/execution failed [\_SB.PCI0.PEG1.PEGP.DD02._BCL] (Node
> > > > ffff8802568d3cf8), AE_NOT_FOUND (20160831/psparse-543)
> > > > > How was this triggered?
> > > >
> > > > This comes from acpi_video.c, ssdt4.dsl contains a \_SB.PCI0.GFX0.DD02
> > > > device with an _ADR method, but no _BCL one. It is not a regression
> > > > though, let's ignore this for now.
> > > >
> > > > > 2. I noticed the following statement:
> > > > > So, for some reason Lenovo has decided to give all tables the same name.
> > > > > The ACPI table override functionality matches possible candidates by signature, OEM ID and
> > OEM
> > > > Revision ID which are all the same.
> > > > > As a result, the wrong SSDT is overridden.
> > > > > Could you provide the detail of the tables from the platform?
> > > > > What is the reason of doing such kind of craps in BIOS?
> > > >
> > > > I have no idea why Lenovo would do such a silly thing, but that is why
> > > > we had to patch tables.c with:
> > > >
> > > > if (existing_table->checksum != 0xAF) {
> > > > acpi_os_unmap_memory(table, ACPI_HEADER_SIZE);
> > > > pr_info("Skipping next table\n");
> > > > goto next_table;
> > > > }
> > > >
> > > > This is of course an unacceptable hard-coded value, but it was needed in
> > > > as a quick hack. For a longterm solution, maybe we can name the table
> > > > files specially such that additional match conditions can be given. This
> > > > is a different issue though.
> > > >
> > > > What would you like to know about the platform? The acpidump is
> > > > available at
> > > > https://bugs.launchpad.net/lpbugreporter/+bug/752542/+attachment/4773050/+files/LENOVO-
> > 20287.tar.gz
> > > >
> > > > ssdt5.dsl is the file of interest, grep for "Windows 2013".
> > >
> > > The table headers are:
> > >
> > > DefinitionBlock ("dsdt.aml", "DSDT", 1, "LENOVO", "CB-01 ", 0x00000001)
> > > DefinitionBlock ("ssdt1.aml", "SSDT", 1, "LENOVO", "CB-01 ", 0x00000001)
> > > DefinitionBlock ("ssdt2.aml", "SSDT", 1, "LENOVO", "CB-01 ", 0x00000001)
> > > DefinitionBlock ("ssdt3.aml", "SSDT", 1, "LENOVO", "CB-01 ", 0x00000001)
> > > DefinitionBlock ("ssdt4.aml", "SSDT", 1, "LENOVO", "CB-01 ", 0x00000001)
> > > DefinitionBlock ("ssdt5.aml", "SSDT", 1, "LENOVO", "CB-01 ", 0x00000001)
> > >
> > > They will be deemed as different versions of the same table from linux ACPI's point of view.
> > > Whether override will be done or not depends on Linux version.
> >
> > When I mentioned override, I referred to the method described in
> > Documentation/acpi/method-customizing.txt where any ACPI table can be
> > overridden via the initrd. Indeed, that method is unable to pick
> > arbitrary tables when they all have the same ID.
> >
> > > In recent Linux, ssdt1.aml will be used (no override), while in old Linux, ssdt5.aml will be used
> > (override).
> >
> > Does this refer to the same override method as described above? The
> > tables are loaded normally even if they have the same name.
> >
> > > Let me ask further.
> > > 1. Should OSPM load only ssdt5.aml or load only ssdt1.aml, or load
> > ssdt1.aml,ssdt2.aml,ssdt3.aml,ssdt4.aml,ssdt5.aml?
> > > 2. Can you confirm Windows behavior here via amli?
> > >
> > > Anyway, this is a special case that violates spec and seems to be aiming to trigger regressions
> > against changed logic brought by me.
> > > No matter whether the change is reasonable.
> >
> > OSPM should load all tables, they are all different. Perhaps you are
> > confusing this override method with how Linux normally loads tables?
> > (Aren't the identifiers just used for visual purposes and irrelevant for
> > normal operation?)
>
> No, I'm not confusing it.
> I want to know the exact behavior here.
I am a bit confused about what you would like to know and will try my
best to answer it. In the dmesg without patching the kernel, the first
SSDT is overwritten, all others are left intact. This can be seen in the
dmesg from https://pastebin.com/raw/LuTLQnU1:
ACPI: MCFG 0x000000009CFF4000 00003C (v01 LENOVO CB-01 00000001 ACPI 00040000)
ACPI: Table Upgrade: override [SSDT-LENOVO-CB-01 ]
ACPI: SSDT 0x000000009CFE6000 Physical table override, new table: 0x000000009C2AB000
ACPI: SSDT 0x000000009C2AB000 003E32 (v01 LENOVO CB-01 00000002 INTL 20160729)
ACPI: BOOT 0x000000009CFE4000 000028 (v01 LENOVO CB-01 00000001 ACPI 00040000)
ACPI: LPIT 0x000000009CFE3000 00005C (v01 LENOVO CB-01 00000001 ACPI 00040000)
ACPI: ASPT 0x000000009CFE1000 000034 (v07 LENOVO CB-01 00000001 ACPI 00040000)
ACPI: DBGP 0x000000009CFE0000 000034 (v01 LENOVO CB-01 00000001 ACPI 00040000)
ACPI: SSDT 0x000000009CFD8000 000539 (v01 LENOVO CB-01 00000001 ACPI 00040000)
ACPI: SSDT 0x000000009CFD7000 000AD8 (v01 LENOVO CB-01 00000001 ACPI 00040000)
ACPI: SSDT 0x000000009CFD3000 0034C6 (v01 LENOVO CB-01 00000001 ACPI 00040000)
ACPI: SSDT 0x000000009CFCE000 00399A (v01 LENOVO CB-01 00000001 ACPI 00040000)
> Because I'm actually about to change acpi_tb_compare_tables().
> This allows and acpi_install_table() invocations to override matched tables.
> So I'll remove Linux table override callback.
No idea why you have to change acpi_tb_compare_tables, this is only used
for "reloading" functionality.
There are two calls from ACPICA to Linux in acpi_tb_override_table,
namely acpi_os_table_override (for DSDT override) and
acpi_os_physical_table_override (for others, like SSDT). Did you meant
to refer to these as "callback"?
If you want to adjust the matching functionality, I guess that
acpi_table_initrd_override needs to be modified. The ACPI header cannot
be modified, so you would then have to read the checksum, size or index
based on the filename.
Kind regards,
Peter
> Please see my recent comment in the following link:
> https://github.com/Bumblebee-Project/bbswitch/issues/142
>
> Thanks
> Lv
>
> >
> > I have not checked Windows, but I would guess that it loads normally.
> > Otherwise Lenovo QA has really been sleeping.
> >
> > Kind regards,
> > Peter
> >
> > > Linux only need a quirk for this platform.
> > >
> > > Thanks
> > > Lv
> > >
> > > >
> > > > Kind regards,
> > > > Peter
> > > >
> > > > > Thanks and best regards
> > > > > Lv
> > > > >
> > > > > >
> > > > > > If you would like a new bugzilla entry or have some patches to test, you
> > > > > > know where to find us :)
> > > > > > --
> > > > > > Kind regards,
> > > > > > Peter Wu
> > > > > > https://lekensteyn.nl
^ permalink raw reply
* Re: [REGRESSION] 4.9-rc4+ doesn't boot on my test box
From: Jens Axboe @ 2016-11-09 1:19 UTC (permalink / raw)
To: Kashyap Desai, Martin K. Petersen; +Cc: linux-scsi
In-Reply-To: <48f84d6c60f7ac70928c2cb34cd2ed1f@mail.gmail.com>
On 11/08/2016 05:20 PM, Kashyap Desai wrote:
>> -----Original Message-----
>> From: Martin K. Petersen [mailto:martin.petersen@oracle.com]
>> Sent: Wednesday, November 09, 2016 4:45 AM
>> To: Jens Axboe
>> Cc: linux-scsi; Kashyap Desai; Martin K. Petersen
>> Subject: Re: [REGRESSION] 4.9-rc4+ doesn't boot on my test box
>>
>>>>>>> "Jens" == Jens Axboe <axboe@kernel.dk> writes:
>>
>> Jens> I wasted half a day on this, thinking it was something in my
>> Jens> 4.10 branches. But it turns out it is not, the regression is in
>> Jens> mainline.
>
> Jens -
>
> Sorry for trouble. I did not validated this single patch. I validated
> complete patch set.
> Issue is improper MACRO usage MEGASAS_IS_LOGICAL, which gives incorrect
> check condition in qcmd Path.
>
> Below is proposed fix.
>
>
> diff --git a/drivers/scsi/megaraid/megaraid_sas.h
> b/drivers/scsi/megaraid/megaraid_sas.h
> index 74c7b44..0d2625b 100644
> --- a/drivers/scsi/megaraid/megaraid_sas.h
> +++ b/drivers/scsi/megaraid/megaraid_sas.h
> @@ -2236,7 +2236,7 @@ struct megasas_instance_template {
> };
>
> #define MEGASAS_IS_LOGICAL(scp)
> \
> - (scp->device->channel < MEGASAS_MAX_PD_CHANNELS) ? 0 : 1
Ugh... So we're completing everything immediately.
> Martin - I validated whole series. Apologies for this.
>
> Please help me to know how to fix this ? Do I need to send only fix on top
> of latest commit (as posted above - MACRO definition) for this issue ?
Send a fix on top of current -git asap. The current tree is completely
broken for any megaraid user. -rc4 is no time to send in untested
patches, especially not something that claims to fix a 9 year old bug
and is marked for stable as well.
--
Jens Axboe
^ permalink raw reply
* RE: [PATCH v2 1/2] PM / devfreq: rk3399_dmc: Use the resource-managed function to add devfreq dev
From: MyungJoo Ham @ 2016-11-09 1:20 UTC (permalink / raw)
To: Chanwoo Choi, Kyungmin Park
Cc: rjw@rjwysocki.net, subhashj@codeaurora.org,
linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <CGME20161108091331epcas1p252b29eaa3bdd280c9b44c3d1da4d12ca@epcas1p2.samsung.com>
[-- Attachment #1: Type: text/plain, Size: 1093 bytes --]
> This patch uses the resource-managed to add the devfreq device.
> This function will make it easy to handle the devfreq device.
>
> - struct devfreq *devm_devfreq_add_device(struct device *dev,
> struct devfreq_dev_profile *profile,
> const char *governor_name,
> void *data);
>
> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
Acked-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> ---
> drivers/devfreq/rk3399_dmc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c
> index e24b73d66659..3f8f4381cd4a 100644
> --- a/drivers/devfreq/rk3399_dmc.c
> +++ b/drivers/devfreq/rk3399_dmc.c
> @@ -436,7 +436,7 @@ static int rk3399_dmcfreq_probe(struct platform_device *pdev)
>
> rk3399_devfreq_dmc_profile.initial_freq = data->rate;
>
> - data->devfreq = devfreq_add_device(dev,
> + data->devfreq = devm_devfreq_add_device(dev,
> &rk3399_devfreq_dmc_profile,
> "simple_ondemand",
> &data->ondemand_data);
> --
> 1.9.1
>
^ permalink raw reply
* Re: [PATCH] wireless: fix bogus maybe-uninitialized warning
From: Kalle Valo @ 2016-11-09 1:21 UTC (permalink / raw)
To: Arnd Bergmann, Johannes Berg
Cc: Stanislav Yakovlev, Jouni Malinen, David S. Miller,
linux-wireless, netdev, linux-kernel
In-Reply-To: <20161024153918.2810634-2-arnd@arndb.de>
Arnd Bergmann <arnd@arndb.de> writes:
> The hostap_80211_rx() function is supposed to set up the mac addresses
> for four possible cases, based on two bits of input data. For
> some reason, gcc decides that it's possible that none of the these
> four cases apply and the addresses remain uninitialized:
>
> drivers/net/wireless/intersil/hostap/hostap_80211_rx.c: In function =E2=
=80=98hostap_80211_rx=E2=80=99:
> arch/x86/include/asm/string_32.h:77:14: warning: =E2=80=98src=E2=80=99 ma=
y be used uninitialized in this function [-Wmaybe-uninitialized]
> drivers/net/wireless/intel/ipw2x00/libipw_rx.c: In function =E2=80=98libi=
pw_rx=E2=80=99:
> arch/x86/include/asm/string_32.h:77:14: error: =E2=80=98dst=E2=80=99 may =
be used uninitialized in this function [-Werror=3Dmaybe-uninitialized]
> arch/x86/include/asm/string_32.h:78:22: error: =E2=80=98*((void *)&dst+4)=
=E2=80=99 may be used uninitialized in this function [-Werror=3Dmaybe-unini=
tialized]
>
> This warning is clearly nonsense, but changing the last case into
> 'default' makes it obvious to the compiler too, which avoids the
> warning and probably leads to better object code too.
>
> The same code is duplicated several times in the kernel, so this
> patch uses the same workaround for all copies. The exact configuration
> was hit only very rarely in randconfig builds and I only saw it
> in three drivers, but I assume that all of them are potentially
> affected, and it's better to keep the code consistent.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> drivers/net/wireless/ath/ath6kl/wmi.c | 8 ++++----
> drivers/net/wireless/intel/ipw2x00/libipw_rx.c | 2 +-
> drivers/net/wireless/intersil/hostap/hostap_80211_rx.c | 2 +-
> net/wireless/lib80211_crypt_tkip.c | 2 +-
> 4 files changed, 7 insertions(+), 7 deletions(-)
[...]
> --- a/net/wireless/lib80211_crypt_tkip.c
> +++ b/net/wireless/lib80211_crypt_tkip.c
> @@ -556,7 +556,7 @@ static void michael_mic_hdr(struct sk_buff *skb, u8 *=
hdr)
> memcpy(hdr, hdr11->addr3, ETH_ALEN); /* DA */
> memcpy(hdr + ETH_ALEN, hdr11->addr4, ETH_ALEN); /* SA */
> break;
> - case 0:
> + default:
> memcpy(hdr, hdr11->addr1, ETH_ALEN); /* DA */
> memcpy(hdr + ETH_ALEN, hdr11->addr2, ETH_ALEN); /* SA */
> break;
Ideally we prefer that drivers/net/wireless and net/wireless changes are
split into different patches as they get applied to different trees.
Johannes, is it ok if I take this change through my tree this time?
--=20
Kalle Valo
^ permalink raw reply
* Re: [PATCH] wireless: fix bogus maybe-uninitialized warning
From: Kalle Valo @ 2016-11-09 1:21 UTC (permalink / raw)
To: Arnd Bergmann, Johannes Berg
Cc: Stanislav Yakovlev, Jouni Malinen, David S. Miller,
linux-wireless, netdev, linux-kernel
In-Reply-To: <20161024153918.2810634-2-arnd@arndb.de>
Arnd Bergmann <arnd@arndb.de> writes:
> The hostap_80211_rx() function is supposed to set up the mac addresses
> for four possible cases, based on two bits of input data. For
> some reason, gcc decides that it's possible that none of the these
> four cases apply and the addresses remain uninitialized:
>
> drivers/net/wireless/intersil/hostap/hostap_80211_rx.c: In function ‘hostap_80211_rx’:
> arch/x86/include/asm/string_32.h:77:14: warning: ‘src’ may be used uninitialized in this function [-Wmaybe-uninitialized]
> drivers/net/wireless/intel/ipw2x00/libipw_rx.c: In function ‘libipw_rx’:
> arch/x86/include/asm/string_32.h:77:14: error: ‘dst’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> arch/x86/include/asm/string_32.h:78:22: error: ‘*((void *)&dst+4)’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>
> This warning is clearly nonsense, but changing the last case into
> 'default' makes it obvious to the compiler too, which avoids the
> warning and probably leads to better object code too.
>
> The same code is duplicated several times in the kernel, so this
> patch uses the same workaround for all copies. The exact configuration
> was hit only very rarely in randconfig builds and I only saw it
> in three drivers, but I assume that all of them are potentially
> affected, and it's better to keep the code consistent.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> drivers/net/wireless/ath/ath6kl/wmi.c | 8 ++++----
> drivers/net/wireless/intel/ipw2x00/libipw_rx.c | 2 +-
> drivers/net/wireless/intersil/hostap/hostap_80211_rx.c | 2 +-
> net/wireless/lib80211_crypt_tkip.c | 2 +-
> 4 files changed, 7 insertions(+), 7 deletions(-)
[...]
> --- a/net/wireless/lib80211_crypt_tkip.c
> +++ b/net/wireless/lib80211_crypt_tkip.c
> @@ -556,7 +556,7 @@ static void michael_mic_hdr(struct sk_buff *skb, u8 * hdr)
> memcpy(hdr, hdr11->addr3, ETH_ALEN); /* DA */
> memcpy(hdr + ETH_ALEN, hdr11->addr4, ETH_ALEN); /* SA */
> break;
> - case 0:
> + default:
> memcpy(hdr, hdr11->addr1, ETH_ALEN); /* DA */
> memcpy(hdr + ETH_ALEN, hdr11->addr2, ETH_ALEN); /* SA */
> break;
Ideally we prefer that drivers/net/wireless and net/wireless changes are
split into different patches as they get applied to different trees.
Johannes, is it ok if I take this change through my tree this time?
--
Kalle Valo
^ permalink raw reply
* Re: Bug: git config does not respect read-only .gitconfig file
From: Junio C Hamano @ 2016-11-09 1:22 UTC (permalink / raw)
To: Jeff King; +Cc: Jonathan Word, Markus Hitter, git, jword
In-Reply-To: <20161108200110.zvqdm2nlu5zxfyv5@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Probably converting "rename(from, to)" to first check "access(to,
> W_OK)". That's racy, but it's the best we could do.
Hmph, if these (possibly problematic) callers are all following the
usual "lock, write to temp, rename" pattern, perhaps the lock_file()
function can have access(path, W_OK) check before it returns a
tempfile that has been successfully opened?
Having said that, I share your assessment that this is not a code or
design problem. It is unreasonable to drop the write-enable bit of
a file in a writable directory and expect it to stay unmodified. The
W-bit on the file is not usable as a security measure, and we do not
use it as such.
I do not offhand know how much a new feature "this repository can be
modified by pushing into and fetching from, but its configuration
cannot be modified" is a sensible thing to have. But it is quite
clear that even if we were to implement such feature, we wouldn't be
using W-bit on .git/config to signal that.
^ permalink raw reply
* Re: [PATCH rdma-core 0/8] libpvrdma: userspace library for PVRDMA
From: Christoph Hellwig @ 2016-11-09 1:23 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Adit Ranadive, dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, pv-drivers
In-Reply-To: <20161105150136.GA23803-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
On Sat, Nov 05, 2016 at 09:01:36AM -0600, Jason Gunthorpe wrote:
> > Not entirely. You want me to keep the ABI file from the kernel in the
> > fix up folder and also keep a file with the modified structs in
> > providers/pvrdma?
>
> Yes, and the file with the modified structs should include the kernel
> header and duplicate it minimally. This will make it simpler for us to
> eventually get rid of it.
Can we just automate generating the user header, e.g. have a sed script
that recognized a magic comments ala
/* LIBIBVERBS PREAMBLE (DO NOT REMOVE) */
and just insers the needed fields?
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] Avoid that SCSI device removal through sysfs triggers a deadlock
From: Eric W. Biederman @ 2016-11-09 1:22 UTC (permalink / raw)
To: Bart Van Assche
Cc: James Bottomley, Martin K. Petersen, Greg Kroah-Hartman,
Hannes Reinecke, Johannes Thumshirn, Sagi Grimberg,
linux-scsi@vger.kernel.org
In-Reply-To: <0129858f-382b-0903-7157-40e5ec8a9e6a@sandisk.com>
Bart Van Assche <bart.vanassche@sandisk.com> writes:
> On 11/08/2016 11:15 AM, Eric W. Biederman wrote:
>> James Bottomley <jejb@linux.vnet.ibm.com> writes:
>>
>>> On Tue, 2016-11-08 at 08:52 -0800, Bart Van Assche wrote:
>>>> On 11/08/2016 07:28 AM, James Bottomley wrote:
>>>>> On Mon, 2016-11-07 at 16:32 -0800, Bart Van Assche wrote:
>>>>>> diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
>>>>>> index cf4c636..44ec536 100644
>>>>>> --- a/fs/kernfs/dir.c
>>>>>> +++ b/fs/kernfs/dir.c
>>>>>> @@ -1410,7 +1410,7 @@ int kernfs_remove_by_name_ns(struct
>>>>>> kernfs_node
>>>>>> *parent, const char *name,
>>>>>> mutex_lock(&kernfs_mutex);
>>>>>>
>>>>>> kn = kernfs_find_ns(parent, name, ns);
>>>>>> - if (kn)
>>>>>> + if (kn && !(kn->flags & KERNFS_SUICIDED))
>>>>>
>>>>> Actually, wrong flag, you need KERNFS_SUICIDAL. The reason is that
>>>>> kernfs_mutex is actually dropped half way through __kernfs_remove,
>>>>> so KERNFS_SUICIDED is not set atomically with this mutex.
>>>>
>>>> Hello James,
>>>>
>>>> Sorry but what you wrote is not correct.
>>>
>>> I think you agree it is dropped. I don't need to add the bit about the
>>> reacquisition because the race is mediated by the first acquisition not
>>> the second one, if you mediate on KERNFS_SUICIDAL, you only need to
>>> worry about this because the mediation is in the first acquisition. If
>>> you mediate on KERNFS_SUICIDED, you need to explain that the final
>>> thing that means the race can't happen is the unbreak in the sysfs
>>> delete path re-acquiring s_active ... the explanation of what's going
>>> on and why gets about 2x more complex.
>>
>> Is it really the dropping of the lock that is causing this?
>> I don't see that when I read those traces.
>>
>> I am going to put my vote in for doing all of the self removal
>> sem-asynchronously by using task_work_add, and killing
>> device_remove_self, sysfs_remove_self, kernfs_remove_self. Using
>> task_work_add remains synchronous with userspace so userspace should not
>> care, and we get the benefit of not having two different variants of the
>> same code racing with each other.
>>
>> It might take a little more work but will leave code that is much more
>> maintainable in the long run.
>
> Hello Eric,
>
> I'm completely in favor of keeping code maintainable. But what's not clear to me
> is whether asynchronous I/O can be submitted to a sysfs attribute? If so, on
> what context will task work queued through task_work_add() be executed if an aio
> write is used to write into a sysfs attribute?
I don't believe so. I believe the filesystem has to opt into aio. In
either case I don't believe we have to worry about aio because either it
won't be supported or it will be a synchronous write to sysfs.
> Additionally, can a process enter the exiting state after writing into the sysfs
> delete attribute started and before task_work_add() has been called? I'm asking
> this because in that case task_work_add() will fail.
Not before task_work_add has been called because task_work_add is what
will be called synchronously from the write function of the delete
attribute.
Furthermore task_work_run is guaranteed to be called before you return
to userspace. It is remotely possible that someone sends the process a
SIGKILL while the delete attribute is being written. In that case
task_work_run should run before the task gets too deep into the exiting
state.
But the SIGKILL will not be processed until the write syscall finishes
and the kernel returns to the edge of userspace.
So I can't imagine anything that would make task_work_add be a poor
choice in these circumstances.
And basically that fix for just the scsi bits I think is as simple as:
drivers/scsi/scsi_sysfs.c | 20 +++++++++++++++++---
include/scsi/scsi_device.h | 1 +
2 files changed, 18 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 07349270535d..fee88cde7c12 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -12,6 +12,7 @@
#include <linux/blkdev.h>
#include <linux/device.h>
#include <linux/pm_runtime.h>
+#include <linux/task_work.h>
#include <scsi/scsi.h>
#include <scsi/scsi_device.h>
@@ -705,13 +706,26 @@ store_rescan_field (struct device *dev, struct device_attribute *attr,
}
static DEVICE_ATTR(rescan, S_IWUSR, NULL, store_rescan_field);
+static void sdev_remove_self(struct callback_head *cb)
+{
+ struct scsi_device *sdev =
+ container_of(cb, struct scsi_device, delete_work);
+
+ scsi_remove_device(sdev);
+}
+
static ssize_t
sdev_store_delete(struct device *dev, struct device_attribute *attr,
const char *buf, size_t count)
{
- if (device_remove_file_self(dev, attr))
- scsi_remove_device(to_scsi_device(dev));
- return count;
+ struct scsi_device *sdev = to_scsi_device(dev);
+ ssize_t err;
+
+ init_task_work(&sdev->delete_work, sdev_remove_self);
+ err = task_work_add(current, &sdev->delete_work, false);
+ if (!err)
+ err = count;
+ return err;
};
static DEVICE_ATTR(delete, S_IWUSR, NULL, sdev_store_delete);
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 8a9563144890..e82a151c8543 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -206,6 +206,7 @@ struct scsi_device {
struct scsi_device_handler *handler;
void *handler_data;
+ struct callback_head delete_work;
unsigned char access_state;
enum scsi_device_state sdev_state;
unsigned long sdev_data[0];
Eric
^ permalink raw reply related
* [PATCH] drm/amdgpu: Attach exclusive fence to prime exported bo's. (v5)
From: Mario Kleiner @ 2016-11-09 1:25 UTC (permalink / raw)
To: dri-devel; +Cc: Michel Dänzer, Christian König, amd-gfx
External clients which import our bo's wait only
for exclusive dmabuf-fences, not on shared ones,
ditto for bo's which we import from external
providers and write to.
Therefore attach exclusive fences on prime shared buffers
if our exported buffer gets imported by an external
client, or if we import a buffer from an external
exporter.
See discussion in thread:
https://lists.freedesktop.org/archives/dri-devel/2016-October/122370.html
Prime export tested on Intel iGPU + AMD Tonga dGPU as
DRI3/Present Prime render offload, and with the Tonga
standalone as primary gpu.
v2: Add a wait for all shared fences before prime export,
as suggested by Christian Koenig.
v3: - Mark buffer prime_exported in amdgpu_gem_prime_pin,
so we only use the exclusive fence when exporting a
bo to external clients like a separate iGPU, but not
when exporting/importing from/to ourselves as part of
regular DRI3 fd passing.
- Propagate failure of reservation_object_wait_rcu back
to caller.
v4: - Switch to a prime_shared_count counter instead of a
flag, which gets in/decremented on prime_pin/unpin, so
we can switch back to shared fences if all clients
detach from our exported bo.
- Also switch to exclusive fence for prime imported bo's.
v5: - Drop lret, instead use int ret -> long ret, as proposed
by Christian.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95472
Tested-by: Mike Lothian <mike@fireburn.co.uk> (v1)
Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
Reviewed-by: Christian König <christian.koenig@amd.com>.
Cc: Christian König <christian.koenig@amd.com>
Cc: Michel Dänzer <michel.daenzer@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c | 20 +++++++++++++++++++-
3 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 039b57e..496f72b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -459,6 +459,7 @@ struct amdgpu_bo {
u64 metadata_flags;
void *metadata;
u32 metadata_size;
+ unsigned prime_shared_count;
/* list of all virtual address to which this bo
* is associated to
*/
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
index 651115d..c02db01f6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
@@ -132,7 +132,7 @@ static int amdgpu_bo_list_set(struct amdgpu_device *adev,
entry->priority = min(info[i].bo_priority,
AMDGPU_BO_LIST_MAX_PRIORITY);
entry->tv.bo = &entry->robj->tbo;
- entry->tv.shared = true;
+ entry->tv.shared = !entry->robj->prime_shared_count;
if (entry->robj->prefered_domains == AMDGPU_GEM_DOMAIN_GDS)
gds_obj = entry->robj;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
index 7700dc2..3826d5a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
@@ -74,20 +74,36 @@ amdgpu_gem_prime_import_sg_table(struct drm_device *dev,
if (ret)
return ERR_PTR(ret);
+ bo->prime_shared_count = 1;
return &bo->gem_base;
}
int amdgpu_gem_prime_pin(struct drm_gem_object *obj)
{
struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj);
- int ret = 0;
+ long ret = 0;
ret = amdgpu_bo_reserve(bo, false);
if (unlikely(ret != 0))
return ret;
+ /*
+ * Wait for all shared fences to complete before we switch to future
+ * use of exclusive fence on this prime shared bo.
+ */
+ ret = reservation_object_wait_timeout_rcu(bo->tbo.resv, true, false,
+ MAX_SCHEDULE_TIMEOUT);
+ if (unlikely(ret < 0)) {
+ DRM_DEBUG_PRIME("Fence wait failed: %li\n", ret);
+ amdgpu_bo_unreserve(bo);
+ return ret;
+ }
+
/* pin buffer into GTT */
ret = amdgpu_bo_pin(bo, AMDGPU_GEM_DOMAIN_GTT, NULL);
+ if (likely(ret == 0))
+ bo->prime_shared_count++;
+
amdgpu_bo_unreserve(bo);
return ret;
}
@@ -102,6 +118,8 @@ void amdgpu_gem_prime_unpin(struct drm_gem_object *obj)
return;
amdgpu_bo_unpin(bo);
+ if (bo->prime_shared_count)
+ bo->prime_shared_count--;
amdgpu_bo_unreserve(bo);
}
--
2.7.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related
* Re: [PATCH 2/4] usb: musb: Fix sleeping function called from invalid context for hdrc glue
From: Tony Lindgren @ 2016-11-09 1:26 UTC (permalink / raw)
To: Johan Hovold
Cc: Bin Liu, Boris Brezillon, Greg Kroah-Hartman, Andreas Kemnade,
Felipe Balbi, George Cherian, Kishon Vijay Abraham I,
Ivaylo Dimitrov, Ladislav Michl, Laurent Pinchart,
Sergei Shtylyov, linux-usb-u79uwXL29TY76Z2rM5mHXA,
linux-omap-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161108190331.GA14744@localhost>
* Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> [161108 12:03]:
> On Tue, Nov 08, 2016 at 10:34:13AM -0700, Tony Lindgren wrote:
> > * Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> [161108 10:09]:
> > > On Mon, Nov 07, 2016 at 02:50:18PM -0700, Tony Lindgren wrote:
> > > > @@ -2604,6 +2669,9 @@ static int musb_resume(struct device *dev)
> > > > mask = MUSB_DEVCTL_BDEVICE | MUSB_DEVCTL_FSDEV | MUSB_DEVCTL_LSDEV;
> > > > if ((devctl & mask) != (musb->context.devctl & mask))
> > > > musb->port1_status = 0;
> > > > +
> > > > + schedule_delayed_work(&musb->pending_resume_work, 0);
> > > > +
> > >
> > > The interactions with system suspend looks a bit suspicious. It seems
> > > you need to drain any pending resume work on suspend for example.
> > >
> > > And then the above should not be needed, right?
> >
> > Hmm that's an interesting idea. I think that would allow us to get
> > rid of the delayed work if we check the list both on resume and
> > suspend :)
>
> Note that I was referring to draining the runtime-resume work on system
> suspend above. I think we still want the delayed work considering that
> we could stay active for long periods of time.
It seems it's the delayed work causing these race issues, so I'd like
to avoid it. I think we can do it all from PM runtime resume and
checking at the end of musb_queue_resume_work(). We need to introduce
is_runtime_suspended though. This is along the lines of what we have
in Documentation/power/runtime_pm.txt if you search for completion
there.
Below is a version doing things without delayed work, care to check
again if you still see races there?
> > > In fact, the dsps timer must also be cancelled on suspend, or you could
> > > end up calling dsps_check_status() while suspended (it is currently not
> > > cancelled until the parent device is suspended, which could be too
> > > late).
> >
> > And then this should no longer be an issue either.
>
> It would still be an issue as a system-suspending device could already
> have been runtime-resumed so that dsps_check_status() would be called
> directly from the timer function.
The glue layers should do pm_runtime_get_sync(musb->controller) which
dsps glue already does. So that's the musb_core.c device instance. And
looks like we have dsps_suspend() call del_timer_sync(&glue->timer)
already. I think we're safe here.
Regards,
Tony
8< ----------------------
>From tony Mon Sep 17 00:00:00 2001
From: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
Date: Wed, 2 Nov 2016 19:59:05 -0700
Subject: [PATCH] usb: musb: Fix sleeping function called from invalid
context for hdrc glue
Commit 65b3f50ed6fa ("usb: musb: Add PM runtime support for MUSB DSPS
glue layer") wrongly added a call for pm_runtime_get_sync to otg_timer
that runs in softirq context. That causes a "BUG: sleeping function called
from invalid context" every time when polling the cable status:
[<c015ebb4>] (__might_sleep) from [<c0413d60>] (__pm_runtime_resume+0x9c/0xa0)
[<c0413d60>] (__pm_runtime_resume) from [<c04d0bc4>] (otg_timer+0x3c/0x254)
[<c04d0bc4>] (otg_timer) from [<c0191180>] (call_timer_fn+0xfc/0x41c)
[<c0191180>] (call_timer_fn) from [<c01915c0>] (expire_timers+0x120/0x210)
[<c01915c0>] (expire_timers) from [<c0191acc>] (run_timer_softirq+0xa4/0xdc)
[<c0191acc>] (run_timer_softirq) from [<c010168c>] (__do_softirq+0x12c/0x594)
I did not notice that as I did not have CONFIG_DEBUG_ATOMIC_SLEEP enabled.
And looks like also musb_gadget_queue() suffers from the same problem.
Let's fix the issue by using a list of delayed work then call it on
resume. Note that we want to do this only when musb core and it's
parent devices are awake as noted by Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>.
Later on we may be able to remove other delayed work in the musb driver
and just do it from pending_resume_work. But this should be done only
for delayed work that does not have other timing requirements beyond
just being run on resume.
Fixes: 65b3f50ed6fa ("usb: musb: Add PM runtime support for MUSB DSPS
glue layer")
Reported-by: Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
---
drivers/usb/musb/musb_core.c | 114 +++++++++++++++++++++++++++++++++++++++--
drivers/usb/musb/musb_core.h | 7 +++
drivers/usb/musb/musb_dsps.c | 31 +++++++----
drivers/usb/musb/musb_gadget.c | 21 ++++++--
4 files changed, 156 insertions(+), 17 deletions(-)
diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -1969,6 +1969,7 @@ static struct musb *allocate_instance(struct device *dev,
INIT_LIST_HEAD(&musb->control);
INIT_LIST_HEAD(&musb->in_bulk);
INIT_LIST_HEAD(&musb->out_bulk);
+ INIT_LIST_HEAD(&musb->pending_list);
musb->vbuserr_retry = VBUSERR_RETRY_COUNT;
musb->a_wait_bcon = OTG_TIME_A_WAIT_BCON;
@@ -2018,6 +2019,87 @@ static void musb_free(struct musb *musb)
musb_host_free(musb);
}
+struct musb_pending_work {
+ int (*callback)(struct musb *musb, void *data);
+ void *data;
+ struct list_head node;
+};
+
+/*
+ * Called from musb_runtime_resume(), musb_resume(), and
+ * musb_queue_resume_work(). Callers must take musb->lock.
+ */
+static int musb_run_resume_work(struct musb *musb)
+{
+ struct musb_pending_work *w, *_w;
+ unsigned long flags;
+ int error = 0;
+
+ spin_lock_irqsave(&musb->list_lock, flags);
+ list_for_each_entry_safe(w, _w, &musb->pending_list, node) {
+ if (w->callback) {
+ error = w->callback(musb, w->data);
+ if (error < 0) {
+ dev_err(musb->controller,
+ "resume callback %p failed: %i\n",
+ w->callback, error);
+ }
+ }
+ list_del(&w->node);
+ devm_kfree(musb->controller, w);
+ }
+ spin_unlock_irqrestore(&musb->list_lock, flags);
+
+ return error;
+}
+
+/*
+ * Called to run work if device is active or else queue the work to happen
+ * on resume. Caller must take musb->lock.
+ *
+ * Note that we cowardly refuse queuing work after musb PM runtime
+ * resume is done calling musb_run_resume_work() and return -EINPROGRESS
+ * instead.
+ */
+int musb_queue_resume_work(struct musb *musb,
+ int (*callback)(struct musb *musb, void *data),
+ void *data)
+{
+ struct musb_pending_work *w;
+ unsigned long flags;
+ int error;
+
+ if (WARN_ON(!callback))
+ return -EINVAL;
+
+ if (pm_runtime_active(musb->controller))
+ return callback(musb, data);
+
+ w = devm_kzalloc(musb->controller, sizeof(*w), GFP_ATOMIC);
+ if (!w)
+ return -ENOMEM;
+
+ w->callback = callback;
+ w->data = data;
+ spin_lock_irqsave(&musb->list_lock, flags);
+ if (musb->is_runtime_suspended) {
+ list_add_tail(&w->node, &musb->pending_list);
+ error = 0;
+ } else {
+ dev_err(musb->controller, "could not add resume work %p\n",
+ callback);
+ devm_kfree(musb->controller, w);
+ error = -EINPROGRESS;
+ }
+ spin_unlock_irqrestore(&musb->list_lock, flags);
+
+ if (pm_runtime_active(musb->controller))
+ return musb_run_resume_work(musb);
+
+ return error;
+}
+EXPORT_SYMBOL_GPL(musb_queue_resume_work);
+
static void musb_deassert_reset(struct work_struct *work)
{
struct musb *musb;
@@ -2065,6 +2147,7 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl)
}
spin_lock_init(&musb->lock);
+ spin_lock_init(&musb->list_lock);
musb->board_set_power = plat->set_power;
musb->min_power = plat->min_power;
musb->ops = plat->platform_ops;
@@ -2556,6 +2639,7 @@ static int musb_suspend(struct device *dev)
struct musb *musb = dev_to_musb(dev);
unsigned long flags;
+ WARN_ON(!list_empty(&musb->pending_list));
musb_platform_disable(musb);
musb_generic_disable(musb);
@@ -2579,9 +2663,11 @@ static int musb_suspend(struct device *dev)
static int musb_resume(struct device *dev)
{
- struct musb *musb = dev_to_musb(dev);
- u8 devctl;
- u8 mask;
+ struct musb *musb = dev_to_musb(dev);
+ unsigned long flags;
+ int error;
+ u8 devctl;
+ u8 mask;
/*
* For static cmos like DaVinci, register values were preserved
@@ -2599,6 +2685,7 @@ static int musb_resume(struct device *dev)
mask = MUSB_DEVCTL_BDEVICE | MUSB_DEVCTL_FSDEV | MUSB_DEVCTL_LSDEV;
if ((devctl & mask) != (musb->context.devctl & mask))
musb->port1_status = 0;
+
if (musb->need_finish_resume) {
musb->need_finish_resume = 0;
schedule_delayed_work(&musb->finish_resume_work,
@@ -2615,6 +2702,13 @@ static int musb_resume(struct device *dev)
musb_start(musb);
+ spin_lock_irqsave(&musb->lock, flags);
+ error = musb_run_resume_work(musb);
+ if (error)
+ dev_err(musb->controller, "resume work failed with %i\n",
+ error);
+ spin_unlock_irqrestore(&musb->lock, flags);
+
return 0;
}
@@ -2622,14 +2716,18 @@ static int musb_runtime_suspend(struct device *dev)
{
struct musb *musb = dev_to_musb(dev);
+ WARN_ON(!list_empty(&musb->pending_list));
musb_save_context(musb);
+ musb->is_runtime_suspended = 1;
return 0;
}
static int musb_runtime_resume(struct device *dev)
{
- struct musb *musb = dev_to_musb(dev);
+ struct musb *musb = dev_to_musb(dev);
+ unsigned long flags;
+ int error;
/*
* When pm_runtime_get_sync called for the first time in driver
@@ -2651,6 +2749,14 @@ static int musb_runtime_resume(struct device *dev)
msecs_to_jiffies(USB_RESUME_TIMEOUT));
}
+ spin_lock_irqsave(&musb->lock, flags);
+ error = musb_run_resume_work(musb);
+ if (error)
+ dev_err(musb->controller, "resume work failed with %i\n",
+ error);
+ musb->is_runtime_suspended = 0;
+ spin_unlock_irqrestore(&musb->lock, flags);
+
return 0;
}
diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
--- a/drivers/usb/musb/musb_core.h
+++ b/drivers/usb/musb/musb_core.h
@@ -303,6 +303,7 @@ struct musb_context_registers {
struct musb {
/* device lock */
spinlock_t lock;
+ spinlock_t list_lock; /* resume work list lock */
struct musb_io io;
const struct musb_platform_ops *ops;
@@ -337,6 +338,7 @@ struct musb {
struct list_head control; /* of musb_qh */
struct list_head in_bulk; /* of musb_qh */
struct list_head out_bulk; /* of musb_qh */
+ struct list_head pending_list; /* pending work list */
struct timer_list otg_timer;
struct notifier_block nb;
@@ -386,6 +388,7 @@ struct musb {
unsigned long idle_timeout; /* Next timeout in jiffies */
unsigned is_initialized:1;
+ unsigned is_runtime_suspended:1;
/* active means connected and not suspended */
unsigned is_active:1;
@@ -542,6 +545,10 @@ extern irqreturn_t musb_interrupt(struct musb *);
extern void musb_hnp_stop(struct musb *musb);
+int musb_queue_resume_work(struct musb *musb,
+ int (*callback)(struct musb *musb, void *data),
+ void *data);
+
static inline void musb_platform_set_vbus(struct musb *musb, int is_on)
{
if (musb->ops->set_vbus)
diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -188,21 +188,15 @@ static void dsps_musb_disable(struct musb *musb)
musb_writeb(musb->mregs, MUSB_DEVCTL, 0);
}
-static void otg_timer(unsigned long _musb)
+/* Caller must take musb->lock */
+static int dsps_check_status(struct musb *musb, void *unused)
{
- struct musb *musb = (void *)_musb;
void __iomem *mregs = musb->mregs;
struct device *dev = musb->controller;
struct dsps_glue *glue = dev_get_drvdata(dev->parent);
const struct dsps_musb_wrapper *wrp = glue->wrp;
u8 devctl;
- unsigned long flags;
int skip_session = 0;
- int err;
-
- err = pm_runtime_get_sync(dev);
- if (err < 0)
- dev_err(dev, "Poll could not pm_runtime_get: %i\n", err);
/*
* We poll because DSPS IP's won't expose several OTG-critical
@@ -212,7 +206,6 @@ static void otg_timer(unsigned long _musb)
dev_dbg(musb->controller, "Poll devctl %02x (%s)\n", devctl,
usb_otg_state_string(musb->xceiv->otg->state));
- spin_lock_irqsave(&musb->lock, flags);
switch (musb->xceiv->otg->state) {
case OTG_STATE_A_WAIT_VRISE:
mod_timer(&glue->timer, jiffies +
@@ -245,8 +238,26 @@ static void otg_timer(unsigned long _musb)
default:
break;
}
- spin_unlock_irqrestore(&musb->lock, flags);
+ return 0;
+}
+
+static void otg_timer(unsigned long _musb)
+{
+ struct musb *musb = (void *)_musb;
+ struct device *dev = musb->controller;
+ unsigned long flags;
+ int err;
+
+ err = pm_runtime_get(dev);
+ if (err < 0)
+ dev_err(dev, "Poll could not pm_runtime_get: %i\n", err);
+
+ spin_lock_irqsave(&musb->lock, flags);
+ err = musb_queue_resume_work(musb, dsps_check_status, NULL);
+ if (err < 0)
+ dev_err(dev, "%s resume work: %i\n", __func__, err);
+ spin_unlock_irqrestore(&musb->lock, flags);
pm_runtime_mark_last_busy(dev);
pm_runtime_put_autosuspend(dev);
}
diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -1222,6 +1222,15 @@ void musb_ep_restart(struct musb *musb, struct musb_request *req)
rxstate(musb, req);
}
+static int musb_ep_restart_resume_work(struct musb *musb, void *data)
+{
+ struct musb_request *req = data;
+
+ musb_ep_restart(musb, req);
+
+ return 0;
+}
+
static int musb_gadget_queue(struct usb_ep *ep, struct usb_request *req,
gfp_t gfp_flags)
{
@@ -1255,7 +1264,7 @@ static int musb_gadget_queue(struct usb_ep *ep, struct usb_request *req,
map_dma_buffer(request, musb, musb_ep);
- pm_runtime_get_sync(musb->controller);
+ pm_runtime_get(musb->controller);
spin_lock_irqsave(&musb->lock, lockflags);
/* don't queue if the ep is down */
@@ -1271,8 +1280,14 @@ static int musb_gadget_queue(struct usb_ep *ep, struct usb_request *req,
list_add_tail(&request->list, &musb_ep->req_list);
/* it this is the head of the queue, start i/o ... */
- if (!musb_ep->busy && &request->list == musb_ep->req_list.next)
- musb_ep_restart(musb, request);
+ if (!musb_ep->busy && &request->list == musb_ep->req_list.next) {
+ status = musb_queue_resume_work(musb,
+ musb_ep_restart_resume_work,
+ request);
+ if (status < 0)
+ dev_err(musb->controller, "%s resume work: %i\n",
+ __func__, status);
+ }
unlock:
spin_unlock_irqrestore(&musb->lock, lockflags);
--
2.10.2
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.