* [PATCH kernel v3 1/2] dma: Allow mixing bypass and mapped DMA operation
From: Alexey Kardashevskiy @ 2020-10-28 7:00 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Alexey Kardashevskiy, iommu, Christoph Hellwig, linux-kernel
In-Reply-To: <20201028070030.60643-1-aik@ozlabs.ru>
At the moment we allow bypassing DMA ops only when we can do this for
the entire RAM. However there are configs with mixed type memory
where we could still allow bypassing IOMMU in most cases;
POWERPC with persistent memory is one example.
This adds an arch hook to determine where bypass can still work and
we invoke direct DMA API. The following patch checks the bus limit
on POWERPC to allow or disallow direct mapping.
This adds a CONFIG_ARCH_HAS_DMA_SET_MASK config option to make arch_xxxx
hooks no-op by default.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
kernel/dma/mapping.c | 24 ++++++++++++++++++++----
kernel/dma/Kconfig | 4 ++++
2 files changed, 24 insertions(+), 4 deletions(-)
diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
index 51bb8fa8eb89..a0bc9eb876ed 100644
--- a/kernel/dma/mapping.c
+++ b/kernel/dma/mapping.c
@@ -137,6 +137,18 @@ static inline bool dma_map_direct(struct device *dev,
return dma_go_direct(dev, *dev->dma_mask, ops);
}
+#ifdef CONFIG_ARCH_HAS_DMA_MAP_DIRECT
+bool arch_dma_map_page_direct(struct device *dev, phys_addr_t addr);
+bool arch_dma_unmap_page_direct(struct device *dev, dma_addr_t dma_handle);
+bool arch_dma_map_sg_direct(struct device *dev, struct scatterlist *sg, int nents);
+bool arch_dma_unmap_sg_direct(struct device *dev, struct scatterlist *sg, int nents);
+#else
+#define arch_dma_map_page_direct(d, a) (0)
+#define arch_dma_unmap_page_direct(d, a) (0)
+#define arch_dma_map_sg_direct(d, s, n) (0)
+#define arch_dma_unmap_sg_direct(d, s, n) (0)
+#endif
+
dma_addr_t dma_map_page_attrs(struct device *dev, struct page *page,
size_t offset, size_t size, enum dma_data_direction dir,
unsigned long attrs)
@@ -149,7 +161,8 @@ dma_addr_t dma_map_page_attrs(struct device *dev, struct page *page,
if (WARN_ON_ONCE(!dev->dma_mask))
return DMA_MAPPING_ERROR;
- if (dma_map_direct(dev, ops))
+ if (dma_map_direct(dev, ops) ||
+ arch_dma_map_page_direct(dev, page_to_phys(page) + offset + size))
addr = dma_direct_map_page(dev, page, offset, size, dir, attrs);
else
addr = ops->map_page(dev, page, offset, size, dir, attrs);
@@ -165,7 +178,8 @@ void dma_unmap_page_attrs(struct device *dev, dma_addr_t addr, size_t size,
const struct dma_map_ops *ops = get_dma_ops(dev);
BUG_ON(!valid_dma_direction(dir));
- if (dma_map_direct(dev, ops))
+ if (dma_map_direct(dev, ops) ||
+ arch_dma_unmap_page_direct(dev, addr + size))
dma_direct_unmap_page(dev, addr, size, dir, attrs);
else if (ops->unmap_page)
ops->unmap_page(dev, addr, size, dir, attrs);
@@ -188,7 +202,8 @@ int dma_map_sg_attrs(struct device *dev, struct scatterlist *sg, int nents,
if (WARN_ON_ONCE(!dev->dma_mask))
return 0;
- if (dma_map_direct(dev, ops))
+ if (dma_map_direct(dev, ops) ||
+ arch_dma_map_sg_direct(dev, sg, nents))
ents = dma_direct_map_sg(dev, sg, nents, dir, attrs);
else
ents = ops->map_sg(dev, sg, nents, dir, attrs);
@@ -207,7 +222,8 @@ void dma_unmap_sg_attrs(struct device *dev, struct scatterlist *sg,
BUG_ON(!valid_dma_direction(dir));
debug_dma_unmap_sg(dev, sg, nents, dir);
- if (dma_map_direct(dev, ops))
+ if (dma_map_direct(dev, ops) ||
+ arch_dma_unmap_sg_direct(dev, sg, nents))
dma_direct_unmap_sg(dev, sg, nents, dir, attrs);
else if (ops->unmap_sg)
ops->unmap_sg(dev, sg, nents, dir, attrs);
diff --git a/kernel/dma/Kconfig b/kernel/dma/Kconfig
index c99de4a21458..43d106598e82 100644
--- a/kernel/dma/Kconfig
+++ b/kernel/dma/Kconfig
@@ -20,6 +20,10 @@ config DMA_OPS
config DMA_OPS_BYPASS
bool
+# Lets platform IOMMU driver choose between bypass and IOMMU
+config ARCH_HAS_DMA_MAP_DIRECT
+ bool
+
config NEED_SG_DMA_LENGTH
bool
--
2.17.1
^ permalink raw reply related
* Re: [PATCH kernel v2 1/2] dma: Allow mixing bypass and normal IOMMU operation
From: Alexey Kardashevskiy @ 2020-10-28 6:55 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: iommu, linuxppc-dev, linux-kernel
In-Reply-To: <20201027164858.GA30651@lst.de>
On 28/10/2020 03:48, Christoph Hellwig wrote:
>> +static inline bool dma_handle_direct(struct device *dev, dma_addr_t dma_handle)
>> +{
>> + return dma_handle >= dev->archdata.dma_offset;
>> +}
>
> This won't compile except for powerpc, and directly accesing arch members
> in common code is a bad idea. Maybe both your helpers need to be
> supplied by arch code to better abstract this out.
rats, overlooked it :( bus_dma_limit is generic but dma_offset is in
archdata :-/
>
>> if (dma_map_direct(dev, ops))
>> addr = dma_direct_map_page(dev, page, offset, size, dir, attrs);
>> +#ifdef CONFIG_DMA_OPS_BYPASS_BUS_LIMIT
>> + else if (dev->bus_dma_limit &&
>> + can_map_direct(dev, (phys_addr_t) page_to_phys(page) + offset + size))
>> + addr = dma_direct_map_page(dev, page, offset, size, dir, attrs);
>> +#endif
>
> I don't think page_to_phys needs a phys_addr_t on the return value.
> I'd also much prefer if we make this a little more beautiful, here
> are a few suggestions:
>
> - hide the bus_dma_limit check inside can_map_direct, and provide a
> stub so that we can avoid the ifdef
> - use a better name for can_map_direct, and maybe also a better calling
> convention by passing the page (the sg code also has the page),
It is passing an address of the end of the mapped area so passing a page
struct means passing page and offset which is an extra parameter and we
do not want to do anything with the page in those hooks anyway so I'd
keep it as is.
> and
> maybe even hide the dma_map_direct inside it.
Call dma_map_direct() from arch_dma_map_page_direct() if
arch_dma_map_page_direct() is defined? Seems suboptimal as it is going
to be bypass=true in most cases and we save one call by avoiding calling
arch_dma_map_page_direct(). Unless I missed something?
>
> if (dma_map_direct(dev, ops) ||
> arch_dma_map_page_direct(dev, page, offset, size))
> addr = dma_direct_map_page(dev, page, offset, size, dir, attrs);
>
>> BUG_ON(!valid_dma_direction(dir));
>> if (dma_map_direct(dev, ops))
>> dma_direct_unmap_page(dev, addr, size, dir, attrs);
>> +#ifdef CONFIG_DMA_OPS_BYPASS_BUS_LIMIT
>> + else if (dev->bus_dma_limit && dma_handle_direct(dev, addr + size))
>> + dma_direct_unmap_page(dev, addr, size, dir, attrs);
>> +#endif
>
> Same here.
>
>> if (dma_map_direct(dev, ops))
>> ents = dma_direct_map_sg(dev, sg, nents, dir, attrs);
>> +#ifdef CONFIG_DMA_OPS_BYPASS_BUS_LIMIT
>> + else if (dev->bus_dma_limit) {
>> + struct scatterlist *s;
>> + bool direct = true;
>> + int i;
>> +
>> + for_each_sg(sg, s, nents, i) {
>> + direct = can_map_direct(dev, sg_phys(s) + s->offset + s->length);
>> + if (!direct)
>> + break;
>> + }
>> + if (direct)
>> + ents = dma_direct_map_sg(dev, sg, nents, dir, attrs);
>> + else
>> + ents = ops->map_sg(dev, sg, nents, dir, attrs);
>> + }
>> +#endif
>
> This needs to go into a helper as well. I think the same style as
> above would work pretty nicely as well:
Yup. I'll repost v3 soon with this change. Thanks for the review.
>
> if (dma_map_direct(dev, ops) ||
> arch_dma_map_sg_direct(dev, sg, nents))
> ents = dma_direct_map_sg(dev, sg, nents, dir, attrs);
> else
> ents = ops->map_sg(dev, sg, nents, dir, attrs);
>
>> +#ifdef CONFIG_DMA_OPS_BYPASS_BUS_LIMIT
>> + if (dev->bus_dma_limit) {
>> + struct scatterlist *s;
>> + bool direct = true;
>> + int i;
>> +
>> + for_each_sg(sg, s, nents, i) {
>> + direct = dma_handle_direct(dev, s->dma_address + s->length);
>> + if (!direct)
>> + break;
>> + }
>> + if (direct) {
>> + dma_direct_unmap_sg(dev, sg, nents, dir, attrs);
>> + return;
>> + }
>> + }
>> +#endif
>
> One more time here..
>
--
Alexey
^ permalink raw reply
* Re: [PATCH] ibmvscsi: fix race potential race after loss of transport
From: Michael Ellerman @ 2020-10-28 5:21 UTC (permalink / raw)
To: Tyrel Datwyler, james.bottomley
Cc: Tyrel Datwyler, martin.petersen, linux-scsi, linux-kernel, brking,
linuxppc-dev
In-Reply-To: <20201025001355.4527-1-tyreld@linux.ibm.com>
Tyrel Datwyler <tyreld@linux.ibm.com> writes:
> After a loss of tranport due to an adatper migration or crash/disconnect from
> the host partner there is a tiny window where we can race adjusting the
> request_limit of the adapter. The request limit is atomically inc/dec to track
> the number of inflight requests against the allowed limit of our VIOS partner.
> After a transport loss we set the request_limit to zero to reflect this state.
> However, there is a window where the adapter may attempt to queue a command
> because the transport loss event hasn't been fully processed yet and
> request_limit is still greater than zero. The hypercall to send the event will
> fail and the error path will increment the request_limit as a result. If the
> adapter processes the transport event prior to this increment the request_limit
> becomes out of sync with the adapter state and can result in scsi commands being
> submitted on the now reset connection prior to an SRP Login resulting in a
> protocol violation.
>
> Fix this race by protecting request_limit with the host lock when changing the
> value via atomic_set() to indicate no transport.
>
> Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
> ---
> drivers/scsi/ibmvscsi/ibmvscsi.c | 36 +++++++++++++++++++++++---------
> 1 file changed, 26 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c
> index b1f3017b6547..188ed75417a5 100644
> --- a/drivers/scsi/ibmvscsi/ibmvscsi.c
> +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
> @@ -806,6 +806,22 @@ static void purge_requests(struct ibmvscsi_host_data *hostdata, int error_code)
> spin_unlock_irqrestore(hostdata->host->host_lock, flags);
> }
>
> +/**
> + * ibmvscsi_set_request_limit - Set the adapter request_limit in response to
> + * an adapter failure, reset, or SRP Login. Done under host lock to prevent
> + * race with scsi command submission.
> + * @hostdata: adapter to adjust
> + * @limit: new request limit
> + */
> +static void ibmvscsi_set_request_limit(struct ibmvscsi_host_data *hostdata, int limit)
> +{
> + unsigned long flags;
> +
> + spin_lock_irqsave(hostdata->host->host_lock, flags);
> + atomic_set(&hostdata->request_limit, limit);
> + spin_unlock_irqrestore(hostdata->host->host_lock, flags);
> +}
> +
> /**
> * ibmvscsi_reset_host - Reset the connection to the server
> * @hostdata: struct ibmvscsi_host_data to reset
...
> @@ -2137,12 +2153,12 @@ static void ibmvscsi_do_work(struct ibmvscsi_host_data *hostdata)
> }
>
> hostdata->action = IBMVSCSI_HOST_ACTION_NONE;
> + spin_unlock_irqrestore(hostdata->host->host_lock, flags);
You drop the lock ...
> if (rc) {
> - atomic_set(&hostdata->request_limit, -1);
> + ibmvscsi_set_request_limit(hostdata, -1);
.. then retake it, then drop it again in ibmvscsi_set_request_limit().
Which introduces the possibility that something else gets the lock
before you can set the limit to -1.
I'm not sure that's a bug, but it's not obviously correct either?
cheers
> dev_err(hostdata->dev, "error after %s\n", action);
> }
> - spin_unlock_irqrestore(hostdata->host->host_lock, flags);
>
> scsi_unblock_requests(hostdata->host);
> }
^ permalink raw reply
* Re: [PATCH 1/2] ASoC: dt-bindings: fsl_aud2htx: Add binding doc for aud2htx module
From: Shengjiu Wang @ 2020-10-28 5:20 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
alsa-devel, Timur Tabi, Xiubo Li, Fabio Estevam, Shengjiu Wang,
Liam Girdwood, Takashi Iwai, Nicolin Chen, Rob Herring,
Mark Brown, linuxppc-dev, linux-kernel
In-Reply-To: <20201027110840.GA23076@kozik-lap>
On Tue, Oct 27, 2020 at 7:09 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On Mon, Oct 26, 2020 at 06:40:54PM +0800, Shengjiu Wang wrote:
> > AUD2HTX (Audio Subsystem TO HDMI TX Subsystem) is a new
> > IP module found on i.MX8MP.
> >
> > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > ---
> > .../bindings/sound/fsl,aud2htx.yaml | 67 +++++++++++++++++++
> > 1 file changed, 67 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/sound/fsl,aud2htx.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/sound/fsl,aud2htx.yaml b/Documentation/devicetree/bindings/sound/fsl,aud2htx.yaml
> > new file mode 100644
> > index 000000000000..18548d0889a8
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/sound/fsl,aud2htx.yaml
> > @@ -0,0 +1,67 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/sound/fsl,aud2htx.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: NXP Audio Subsystem to HDMI RTX Subsystem Controller
> > +
> > +maintainers:
> > + - Shengjiu Wang <shengjiu.wang@nxp.com>
> > +
> > +properties:
> > + $nodename:
> > + pattern: "^aud2htx@.*"
>
> aud2htx is not a generic class of a device so it should not be enforced.
>
> > +
> > + compatible:
> > + const: fsl,imx8mp-aud2htx
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + interrupts:
> > + maxItems: 1
> > +
> > + clocks:
> > + items:
> > + - description: Peripheral clock
> > +
> > + clock-names:
> > + items:
> > + - const: bus
> > +
> > + dmas:
> > + items:
> > + - description: DMA controller phandle and request line for TX
> > +
> > + dma-names:
> > + items:
> > + - const: tx
> > +
> > + power-domains:
> > + maxItems: 1
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - interrupts
> > + - clocks
> > + - clock-names
> > + - dmas
> > + - dma-names
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/interrupt-controller/arm-gic.h>
> > + #include <dt-bindings/clock/imx8mp-clock.h>
> > +
> > + aud2htx: aud2htx@30cb0000 {
> > + compatible = "fsl,imx8mp-aud2htx";
>
> Wrong indentation. Most of examples are indented with 4 spaces.
>
ok, I will update it.
best regards
wang shengjiu
^ permalink raw reply
* Re: [PATCH 1/3] powerpc/uaccess: Switch __put_user_size_allowed() to __put_user_asm_goto()
From: Michael Ellerman @ 2020-10-28 5:19 UTC (permalink / raw)
To: Andreas Schwab, Christophe Leroy
Cc: Paul Mackerras, linuxppc-dev, linux-kernel
In-Reply-To: <87imav9r64.fsf@igel.home>
Andreas Schwab <schwab@linux-m68k.org> writes:
> On Okt 28 2020, Andreas Schwab wrote:
>
>> On Sep 04 2020, Christophe Leroy wrote:
>>
>>> __put_user_asm_goto() provides more flexibility to GCC and avoids using
>>> a local variable to tell if the write succeeded or not.
>>> GCC can then avoid implementing a cmp in the fast path.
>>
>> That breaks CLONE_CHILD_SETTID. I'm getting an assertion failure in
>> __libc_fork (THREAD_GETMEM (self, tid) != ppid).
>
> This is what schedule_tail now looks like. As you can see, put_user has
> become a nop:
>
> 000000000000455c <.schedule_tail>:
> 455c: 7c 08 02 a6 mflr r0
> 4560: f8 01 00 10 std r0,16(r1)
> 4564: f8 21 ff 91 stdu r1,-112(r1)
> 4568: 4b ff cd 4d bl 12b4 <.finish_task_switch>
> 456c: 4b ff c0 99 bl 604 <.balance_callback>
> 4570: e8 6d 01 88 ld r3,392(r13)
> 4574: e9 23 06 b0 ld r9,1712(r3)
> 4578: 2f a9 00 00 cmpdi cr7,r9,0
> 457c: 41 9e 00 14 beq cr7,4590 <.schedule_tail+0x34>
> 4580: 38 80 00 00 li r4,0
> 4584: 38 a0 00 00 li r5,0
> 4588: 48 00 00 01 bl 4588 <.schedule_tail+0x2c>
> 4588: R_PPC64_REL24 .__task_pid_nr_ns
> 458c: 60 00 00 00 nop
> 4590: 48 00 00 01 bl 4590 <.schedule_tail+0x34>
> 4590: R_PPC64_REL24 .calculate_sigpending
> 4594: 60 00 00 00 nop
> 4598: 38 21 00 70 addi r1,r1,112
> 459c: e8 01 00 10 ld r0,16(r1)
> 45a0: 7c 08 03 a6 mtlr r0
> 45a4: 4e 80 00 20 blr
Not for me, see below.
What config and compiler are you using?
cheers
c000000000181aa0 <schedule_tail>:
c000000000181aa0: 82 01 4c 3c addis r2,r12,386
c000000000181aa4: 60 1b 42 38 addi r2,r2,7008
c000000000181aa8: a6 02 08 7c mflr r0
c000000000181aac: cd b5 ee 4b bl c00000000006d078 <_mcount>
c000000000181ab0: a6 02 08 7c mflr r0
c000000000181ab4: f8 ff e1 fb std r31,-8(r1)
c000000000181ab8: 10 00 01 f8 std r0,16(r1)
c000000000181abc: d1 ff 21 f8 stdu r1,-48(r1)
c000000000181ac0: c9 7d ff 4b bl c000000000179888 <finish_task_switch+0x8>
c000000000181ac4: 40 0a 23 e9 ld r9,2624(r3)
c000000000181ac8: 00 00 a9 2f cmpdi cr7,r9,0
c000000000181acc: b4 00 9e 40 bne cr7,c000000000181b80 <schedule_tail+0xe0>
c000000000181ad0: 68 09 6d e8 ld r3,2408(r13)
c000000000181ad4: 48 07 e3 eb ld r31,1864(r3)
c000000000181ad8: 00 00 bf 2f cmpdi cr7,r31,0
c000000000181adc: 88 00 9e 41 beq cr7,c000000000181b64 <schedule_tail+0xc4>
c000000000181ae0: 00 00 a0 38 li r5,0
c000000000181ae4: 00 00 80 38 li r4,0
c000000000181ae8: 21 4b fe 4b bl c000000000166608 <__task_pid_nr_ns+0x8>
c000000000181aec: 00 00 00 60 nop
c000000000181af0: ff ff 20 39 li r9,-1
c000000000181af4: 00 03 29 79 clrldi r9,r9,12
c000000000181af8: 40 48 bf 7f cmpld cr7,r31,r9
c000000000181afc: 68 00 9d 41 bgt cr7,c000000000181b64 <schedule_tail+0xc4>
c000000000181b00: 01 00 29 39 addi r9,r9,1
c000000000181b04: 50 48 3f 7d subf r9,r31,r9
c000000000181b08: 03 00 a9 2b cmpldi cr7,r9,3
c000000000181b0c: 58 00 9d 40 ble cr7,c000000000181b64 <schedule_tail+0xc4>
c000000000181b10: 02 00 42 3d addis r10,r2,2
c000000000181b14: 18 25 4a 39 addi r10,r10,9496
c000000000181b18: 00 00 2a e9 ld r9,0(r10)
c000000000181b1c: 22 00 29 e9 lwa r9,32(r9)
c000000000181b20: 00 00 89 2f cmpwi cr7,r9,0
c000000000181b24: 24 00 9c 40 bge cr7,c000000000181b48 <schedule_tail+0xa8>
c000000000181b28: 2c 01 00 4c isync
c000000000181b2c: 00 40 20 3d lis r9,16384
c000000000181b30: c6 07 29 79 rldicr r9,r9,32,31
c000000000181b34: a6 03 3d 7d mtspr 29,r9 # put_user() begins here
c000000000181b38: 2c 01 00 4c isync
c000000000181b3c: 00 00 2a e9 ld r9,0(r10)
c000000000181b40: 22 00 29 e9 lwa r9,32(r9)
c000000000181b44: 00 00 89 2f cmpwi cr7,r9,0
c000000000181b48: 00 00 7f 90 stw r3,0(r31)
c000000000181b4c: 18 00 9c 40 bge cr7,c000000000181b64 <schedule_tail+0xc4>
c000000000181b50: 2c 01 00 4c isync
c000000000181b54: ff ff 20 39 li r9,-1
c000000000181b58: 44 00 29 79 rldicr r9,r9,0,1
c000000000181b5c: a6 03 3d 7d mtspr 29,r9
c000000000181b60: 2c 01 00 4c isync
c000000000181b64: b5 c9 fc 4b bl c00000000014e518 <calculate_sigpending+0x8>
c000000000181b68: 00 00 00 60 nop
c000000000181b6c: 30 00 21 38 addi r1,r1,48
c000000000181b70: 10 00 01 e8 ld r0,16(r1)
c000000000181b74: f8 ff e1 eb ld r31,-8(r1)
c000000000181b78: a6 03 08 7c mtlr r0
c000000000181b7c: 20 00 80 4e blr
c000000000181b80: 39 40 ff 4b bl c000000000175bb8 <__balance_callback+0x8>
c000000000181b84: 4c ff ff 4b b c000000000181ad0 <schedule_tail+0x30>
^ permalink raw reply
* Re: [PATCH] ibmveth: Fix use of ibmveth in a bridge.
From: Jakub Kicinski @ 2020-10-28 0:54 UTC (permalink / raw)
To: Thomas Bogendoerfer
Cc: Cristobal Forno, netdev, linux-kernel, Paul Mackerras,
Michal Suchanek, linuxppc-dev, David S. Miller, Cris Forno
In-Reply-To: <20201026200407.fcf43678ba4cef7fe0cb7c98@suse.de>
On Mon, 26 Oct 2020 20:04:07 +0100 Thomas Bogendoerfer wrote:
> > On Mon, 26 Oct 2020 11:42:21 +0100 Michal Suchanek wrote:
> > > From: Thomas Bogendoerfer <tbogendoerfer@suse.de>
> > >
> > > The check for src mac address in ibmveth_is_packet_unsupported is wrong.
> > > Commit 6f2275433a2f wanted to shut down messages for loopback packets,
> > > but now suppresses bridged frames, which are accepted by the hypervisor
> > > otherwise bridging won't work at all.
> > >
> > > Fixes: 6f2275433a2f ("ibmveth: Detect unsupported packets before sending to the hypervisor")
> > > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> >
> > Since the From: line says Thomas we need a signoff from him.
>
> you can add
>
> Signed-off-by: tbogendoerfer@suse.de
Applied, thanks.
^ permalink raw reply
* Re: [PATCH 1/3] powerpc/uaccess: Switch __put_user_size_allowed() to __put_user_asm_goto()
From: Andreas Schwab @ 2020-10-27 23:37 UTC (permalink / raw)
To: Christophe Leroy; +Cc: Paul Mackerras, linuxppc-dev, linux-kernel
In-Reply-To: <87mu079ron.fsf@igel.home>
On Okt 28 2020, Andreas Schwab wrote:
> On Sep 04 2020, Christophe Leroy wrote:
>
>> __put_user_asm_goto() provides more flexibility to GCC and avoids using
>> a local variable to tell if the write succeeded or not.
>> GCC can then avoid implementing a cmp in the fast path.
>
> That breaks CLONE_CHILD_SETTID. I'm getting an assertion failure in
> __libc_fork (THREAD_GETMEM (self, tid) != ppid).
This is what schedule_tail now looks like. As you can see, put_user has
become a nop:
000000000000455c <.schedule_tail>:
455c: 7c 08 02 a6 mflr r0
4560: f8 01 00 10 std r0,16(r1)
4564: f8 21 ff 91 stdu r1,-112(r1)
4568: 4b ff cd 4d bl 12b4 <.finish_task_switch>
456c: 4b ff c0 99 bl 604 <.balance_callback>
4570: e8 6d 01 88 ld r3,392(r13)
4574: e9 23 06 b0 ld r9,1712(r3)
4578: 2f a9 00 00 cmpdi cr7,r9,0
457c: 41 9e 00 14 beq cr7,4590 <.schedule_tail+0x34>
4580: 38 80 00 00 li r4,0
4584: 38 a0 00 00 li r5,0
4588: 48 00 00 01 bl 4588 <.schedule_tail+0x2c>
4588: R_PPC64_REL24 .__task_pid_nr_ns
458c: 60 00 00 00 nop
4590: 48 00 00 01 bl 4590 <.schedule_tail+0x34>
4590: R_PPC64_REL24 .calculate_sigpending
4594: 60 00 00 00 nop
4598: 38 21 00 70 addi r1,r1,112
459c: e8 01 00 10 ld r0,16(r1)
45a0: 7c 08 03 a6 mtlr r0
45a4: 4e 80 00 20 blr
This is schedule_tail in 5.9:
000000000000455c <.schedule_tail>:
455c: 7c 08 02 a6 mflr r0
4560: fb c1 ff f0 std r30,-16(r1)
4564: fb e1 ff f8 std r31,-8(r1)
4568: f8 01 00 10 std r0,16(r1)
456c: f8 21 ff 81 stdu r1,-128(r1)
4570: 4b ff cd 45 bl 12b4 <.finish_task_switch>
4574: 4b ff c0 91 bl 604 <.balance_callback>
4578: eb cd 01 88 ld r30,392(r13)
457c: eb fe 06 b0 ld r31,1712(r30)
4580: 2f bf 00 00 cmpdi cr7,r31,0
4584: 41 9e 00 2c beq cr7,45b0 <.schedule_tail+0x54>
4588: 7f c3 f3 78 mr r3,r30
458c: 38 80 00 00 li r4,0
4590: 38 a0 00 00 li r5,0
4594: 48 00 00 01 bl 4594 <.schedule_tail+0x38>
4594: R_PPC64_REL24 .__task_pid_nr_ns
4598: 60 00 00 00 nop
459c: e9 3e 0a b8 ld r9,2744(r30)
45a0: 7f bf 48 40 cmpld cr7,r31,r9
45a4: 41 9d 00 0c bgt cr7,45b0 <.schedule_tail+0x54>
45a8: 2b a9 00 03 cmpldi cr7,r9,3
45ac: 41 9d 00 14 bgt cr7,45c0 <.schedule_tail+0x64>
45b0: 48 00 00 01 bl 45b0 <.schedule_tail+0x54>
45b0: R_PPC64_REL24 .calculate_sigpending
45b4: 60 00 00 00 nop
45b8: 38 21 00 80 addi r1,r1,128
45bc: 48 00 00 00 b 45bc <.schedule_tail+0x60>
45bc: R_PPC64_REL24 _restgpr0_30
45c0: 39 20 00 00 li r9,0
45c4: 90 7f 00 00 stw r3,0(r31)
45c8: 4b ff ff e8 b 45b0 <.schedule_tail+0x54>
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
"And now for something completely different."
^ permalink raw reply
* Re: [PATCH 1/3] powerpc/uaccess: Switch __put_user_size_allowed() to __put_user_asm_goto()
From: Andreas Schwab @ 2020-10-27 23:26 UTC (permalink / raw)
To: Christophe Leroy; +Cc: Paul Mackerras, linuxppc-dev, linux-kernel
In-Reply-To: <94ba5a5138f99522e1562dbcdb38d31aa790dc89.1599216721.git.christophe.leroy__44535.5968013004$1599217383$gmane$org@csgroup.eu>
On Sep 04 2020, Christophe Leroy wrote:
> __put_user_asm_goto() provides more flexibility to GCC and avoids using
> a local variable to tell if the write succeeded or not.
> GCC can then avoid implementing a cmp in the fast path.
That breaks CLONE_CHILD_SETTID. I'm getting an assertion failure in
__libc_fork (THREAD_GETMEM (self, tid) != ppid).
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
"And now for something completely different."
^ permalink raw reply
* [PATCH net-next 13/15] soc/fsl/qbman: Add an argument to signal if NAPI processing is required.
From: Sebastian Andrzej Siewior @ 2020-10-27 22:54 UTC (permalink / raw)
To: netdev
Cc: Aymen Sghaier, Madalin Bucur, Sebastian Andrzej Siewior,
Zhu Yanjun, Samuel Chessman, Ping-Ke Shih, Herbert Xu,
Horia Geantă, linux-rdma, Rain River, Kalle Valo,
Ulrich Kunitz, Jouni Malinen, Daniel Drake, Jakub Kicinski,
Thomas Gleixner, linux-arm-kernel, Leon Romanovsky, linuxppc-dev,
linux-wireless, Li Yang, linux-crypto, Jon Mason, Saeed Mahameed,
David S. Miller
In-Reply-To: <20201027225454.3492351-1-bigeasy@linutronix.de>
dpaa_eth_napi_schedule() and caam_qi_napi_schedule() schedule NAPI if
invoked from:
- Hard interrupt context
- Any context which is not serving soft interrupts
Any context which is not serving soft interrupts includes hard interrupts
so the in_irq() check is redundant. caam_qi_napi_schedule() has a comment
about this:
/*
* In case of threaded ISR, for RT kernels in_irq() does not return
* appropriate value, so use in_serving_softirq to distinguish between
* softirq and irq contexts.
*/
if (in_irq() || !in_serving_softirq())
This has nothing to do with RT. Even on a non RT kernel force threaded
interrupts run obviously in thread context and therefore in_irq() returns
false when invoked from the handler.
The extension of the in_irq() check with !in_serving_softirq() was there
when the drivers were added, but in the out of tree FSL BSP the original
condition was in_irq() which got extended due to failures on RT.
The usage of in_xxx() in drivers is phased out and Linus clearly requested
that code which changes behaviour depending on context should either be
seperated or the context be conveyed in an argument passed by the caller,
which usually knows the context. Right he is, the above construct is
clearly showing why.
The following callchains have been analyzed to end up in
dpaa_eth_napi_schedule():
qman_p_poll_dqrr()
__poll_portal_fast()
fq->cb.dqrr()
dpaa_eth_napi_schedule()
portal_isr()
__poll_portal_fast()
fq->cb.dqrr()
dpaa_eth_napi_schedule()
Both need to schedule NAPI.
The crypto part has another code path leading up to this:
kill_fq()
empty_retired_fq()
qman_p_poll_dqrr()
__poll_portal_fast()
fq->cb.dqrr()
dpaa_eth_napi_schedule()
kill_fq() is called from task context and ends up scheduling NAPI, but
that's pointless and an unintended side effect of the !in_serving_softirq()
check.
The code path:
caam_qi_poll() -> qman_p_poll_dqrr()
is invoked from NAPI and I *assume* from crypto's NAPI device and not
from qbman's NAPI device. I *guess* it is okay to skip scheduling NAPI
(because this is what happens now) but could be changed if it is wrong
due to `budget' handling.
Add an argument to __poll_portal_fast() which is true if NAPI needs to be
scheduled. This requires propagating the value to the caller including
`qman_cb_dqrr' typedef which is used by the dpaa and the crypto driver.
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: "Horia Geantă" <horia.geanta@nxp.com>
Cc: Aymen Sghaier <aymen.sghaier@nxp.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Madalin Bucur <madalin.bucur@nxp.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Li Yang <leoyang.li@nxp.com>
Cc: linux-crypto@vger.kernel.org
Cc: netdev@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-arm-kernel@lists.infradead.org
---
drivers/crypto/caam/qi.c | 3 ++-
drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 12 ++++++++----
drivers/soc/fsl/qbman/qman.c | 12 ++++++------
drivers/soc/fsl/qbman/qman_test_api.c | 6 ++++--
drivers/soc/fsl/qbman/qman_test_stash.c | 6 ++++--
include/soc/fsl/qman.h | 3 ++-
6 files changed, 26 insertions(+), 16 deletions(-)
diff --git a/drivers/crypto/caam/qi.c b/drivers/crypto/caam/qi.c
index ec53528d82058..09ea398304c8b 100644
--- a/drivers/crypto/caam/qi.c
+++ b/drivers/crypto/caam/qi.c
@@ -564,7 +564,8 @@ static int caam_qi_napi_schedule(struct qman_portal *p, struct caam_napi *np)
static enum qman_cb_dqrr_result caam_rsp_fq_dqrr_cb(struct qman_portal *p,
struct qman_fq *rsp_fq,
- const struct qm_dqrr_entry *dqrr)
+ const struct qm_dqrr_entry *dqrr,
+ bool napi)
{
struct caam_napi *caam_napi = raw_cpu_ptr(&pcpu_qipriv.caam_napi);
struct caam_drv_req *drv_req;
diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
index 06cc863f4dd63..27835310b718e 100644
--- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
+++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
@@ -2316,7 +2316,8 @@ static inline int dpaa_eth_napi_schedule(struct dpaa_percpu_priv *percpu_priv,
static enum qman_cb_dqrr_result rx_error_dqrr(struct qman_portal *portal,
struct qman_fq *fq,
- const struct qm_dqrr_entry *dq)
+ const struct qm_dqrr_entry *dq,
+ bool napi)
{
struct dpaa_fq *dpaa_fq = container_of(fq, struct dpaa_fq, fq_base);
struct dpaa_percpu_priv *percpu_priv;
@@ -2343,7 +2344,8 @@ static enum qman_cb_dqrr_result rx_error_dqrr(struct qman_portal *portal,
static enum qman_cb_dqrr_result rx_default_dqrr(struct qman_portal *portal,
struct qman_fq *fq,
- const struct qm_dqrr_entry *dq)
+ const struct qm_dqrr_entry *dq,
+ bool napi)
{
struct skb_shared_hwtstamps *shhwtstamps;
struct rtnl_link_stats64 *percpu_stats;
@@ -2460,7 +2462,8 @@ static enum qman_cb_dqrr_result rx_default_dqrr(struct qman_portal *portal,
static enum qman_cb_dqrr_result conf_error_dqrr(struct qman_portal *portal,
struct qman_fq *fq,
- const struct qm_dqrr_entry *dq)
+ const struct qm_dqrr_entry *dq,
+ bool napi)
{
struct dpaa_percpu_priv *percpu_priv;
struct net_device *net_dev;
@@ -2481,7 +2484,8 @@ static enum qman_cb_dqrr_result conf_error_dqrr(struct qman_portal *portal,
static enum qman_cb_dqrr_result conf_dflt_dqrr(struct qman_portal *portal,
struct qman_fq *fq,
- const struct qm_dqrr_entry *dq)
+ const struct qm_dqrr_entry *dq,
+ bool napi)
{
struct dpaa_percpu_priv *percpu_priv;
struct net_device *net_dev;
diff --git a/drivers/soc/fsl/qbman/qman.c b/drivers/soc/fsl/qbman/qman.c
index 9888a70618730..a92bd032fc36a 100644
--- a/drivers/soc/fsl/qbman/qman.c
+++ b/drivers/soc/fsl/qbman/qman.c
@@ -1159,7 +1159,7 @@ static u32 fq_to_tag(struct qman_fq *fq)
static u32 __poll_portal_slow(struct qman_portal *p, u32 is);
static inline unsigned int __poll_portal_fast(struct qman_portal *p,
- unsigned int poll_limit);
+ unsigned int poll_limit, bool napi);
static void qm_congestion_task(struct work_struct *work);
static void qm_mr_process_task(struct work_struct *work);
@@ -1174,7 +1174,7 @@ static irqreturn_t portal_isr(int irq, void *ptr)
/* DQRR-handling if it's interrupt-driven */
if (is & QM_PIRQ_DQRI) {
- __poll_portal_fast(p, QMAN_POLL_LIMIT);
+ __poll_portal_fast(p, QMAN_POLL_LIMIT, true);
clear = QM_DQAVAIL_MASK | QM_PIRQ_DQRI;
}
/* Handling of anything else that's interrupt-driven */
@@ -1602,7 +1602,7 @@ static noinline void clear_vdqcr(struct qman_portal *p, struct qman_fq *fq)
* user callbacks to call into any QMan API.
*/
static inline unsigned int __poll_portal_fast(struct qman_portal *p,
- unsigned int poll_limit)
+ unsigned int poll_limit, bool napi)
{
const struct qm_dqrr_entry *dq;
struct qman_fq *fq;
@@ -1636,7 +1636,7 @@ static inline unsigned int __poll_portal_fast(struct qman_portal *p,
* and we don't want multiple if()s in the critical
* path (SDQCR).
*/
- res = fq->cb.dqrr(p, fq, dq);
+ res = fq->cb.dqrr(p, fq, dq, napi);
if (res == qman_cb_dqrr_stop)
break;
/* Check for VDQCR completion */
@@ -1646,7 +1646,7 @@ static inline unsigned int __poll_portal_fast(struct qman_portal *p,
/* SDQCR: context_b points to the FQ */
fq = tag_to_fq(be32_to_cpu(dq->context_b));
/* Now let the callback do its stuff */
- res = fq->cb.dqrr(p, fq, dq);
+ res = fq->cb.dqrr(p, fq, dq, napi);
/*
* The callback can request that we exit without
* consuming this entry nor advancing;
@@ -1753,7 +1753,7 @@ EXPORT_SYMBOL(qman_start_using_portal);
int qman_p_poll_dqrr(struct qman_portal *p, unsigned int limit)
{
- return __poll_portal_fast(p, limit);
+ return __poll_portal_fast(p, limit, false);
}
EXPORT_SYMBOL(qman_p_poll_dqrr);
diff --git a/drivers/soc/fsl/qbman/qman_test_api.c b/drivers/soc/fsl/qbman/qman_test_api.c
index 7066b2f1467cc..93eb86923e44f 100644
--- a/drivers/soc/fsl/qbman/qman_test_api.c
+++ b/drivers/soc/fsl/qbman/qman_test_api.c
@@ -45,7 +45,8 @@
static enum qman_cb_dqrr_result cb_dqrr(struct qman_portal *,
struct qman_fq *,
- const struct qm_dqrr_entry *);
+ const struct qm_dqrr_entry *,
+ bool napi);
static void cb_ern(struct qman_portal *, struct qman_fq *,
const union qm_mr_entry *);
static void cb_fqs(struct qman_portal *, struct qman_fq *,
@@ -208,7 +209,8 @@ int qman_test_api(void)
static enum qman_cb_dqrr_result cb_dqrr(struct qman_portal *p,
struct qman_fq *fq,
- const struct qm_dqrr_entry *dq)
+ const struct qm_dqrr_entry *dq,
+ bool napi)
{
if (WARN_ON(fd_neq(&fd_dq, &dq->fd))) {
pr_err("BADNESS: dequeued frame doesn't match;\n");
diff --git a/drivers/soc/fsl/qbman/qman_test_stash.c b/drivers/soc/fsl/qbman/qman_test_stash.c
index e87b65403b672..9b8caceedf7f3 100644
--- a/drivers/soc/fsl/qbman/qman_test_stash.c
+++ b/drivers/soc/fsl/qbman/qman_test_stash.c
@@ -275,7 +275,8 @@ static inline int process_frame_data(struct hp_handler *handler,
static enum qman_cb_dqrr_result normal_dqrr(struct qman_portal *portal,
struct qman_fq *fq,
- const struct qm_dqrr_entry *dqrr)
+ const struct qm_dqrr_entry *dqrr,
+ bool napi)
{
struct hp_handler *handler = (struct hp_handler *)fq;
@@ -293,7 +294,8 @@ static enum qman_cb_dqrr_result normal_dqrr(struct qman_portal *portal,
static enum qman_cb_dqrr_result special_dqrr(struct qman_portal *portal,
struct qman_fq *fq,
- const struct qm_dqrr_entry *dqrr)
+ const struct qm_dqrr_entry *dqrr,
+ bool napi)
{
struct hp_handler *handler = (struct hp_handler *)fq;
diff --git a/include/soc/fsl/qman.h b/include/soc/fsl/qman.h
index 9f484113cfda7..fe3faa2d64f16 100644
--- a/include/soc/fsl/qman.h
+++ b/include/soc/fsl/qman.h
@@ -689,7 +689,8 @@ enum qman_cb_dqrr_result {
};
typedef enum qman_cb_dqrr_result (*qman_cb_dqrr)(struct qman_portal *qm,
struct qman_fq *fq,
- const struct qm_dqrr_entry *dqrr);
+ const struct qm_dqrr_entry *dqrr,
+ bool napi);
/*
* This callback type is used when handling ERNs, FQRNs and FQRLs via MR. They
--
2.28.0
^ permalink raw reply related
* [PATCH net-next 14/15] net: dpaa: Replace in_irq() usage.
From: Sebastian Andrzej Siewior @ 2020-10-27 22:54 UTC (permalink / raw)
To: netdev
Cc: Aymen Sghaier, Madalin Bucur, Sebastian Andrzej Siewior,
Zhu Yanjun, Samuel Chessman, Ping-Ke Shih, Herbert Xu,
Horia Geantă, linux-rdma, Rain River, Kalle Valo,
Ulrich Kunitz, Jouni Malinen, Daniel Drake, Jakub Kicinski,
Thomas Gleixner, linux-arm-kernel, Leon Romanovsky, linuxppc-dev,
linux-wireless, Li Yang, linux-crypto, Jon Mason, Saeed Mahameed,
David S. Miller
In-Reply-To: <20201027225454.3492351-1-bigeasy@linutronix.de>
The driver uses in_irq() + in_serving_softirq() magic to decide if NAPI
scheduling is required or packet processing.
The usage of in_*() in drivers is phased out and Linus clearly requested
that code which changes behaviour depending on context should either be
seperated or the context be conveyed in an argument passed by the caller,
which usually knows the context.
Use the `napi' argument passed by the callback. It is set true if
called from the interrupt handler and NAPI should be scheduled.
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: "Horia Geantă" <horia.geanta@nxp.com>
Cc: Aymen Sghaier <aymen.sghaier@nxp.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Madalin Bucur <madalin.bucur@nxp.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Li Yang <leoyang.li@nxp.com>
Cc: linux-crypto@vger.kernel.org
Cc: netdev@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-arm-kernel@lists.infradead.org
---
drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
index 27835310b718e..2c949acd74c67 100644
--- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
+++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
@@ -2300,9 +2300,9 @@ static void dpaa_tx_conf(struct net_device *net_dev,
}
static inline int dpaa_eth_napi_schedule(struct dpaa_percpu_priv *percpu_priv,
- struct qman_portal *portal)
+ struct qman_portal *portal, bool napi)
{
- if (unlikely(in_irq() || !in_serving_softirq())) {
+ if (napi) {
/* Disable QMan IRQ and invoke NAPI */
qman_p_irqsource_remove(portal, QM_PIRQ_DQRI);
@@ -2333,7 +2333,7 @@ static enum qman_cb_dqrr_result rx_error_dqrr(struct qman_portal *portal,
percpu_priv = this_cpu_ptr(priv->percpu_priv);
- if (dpaa_eth_napi_schedule(percpu_priv, portal))
+ if (dpaa_eth_napi_schedule(percpu_priv, portal, napi))
return qman_cb_dqrr_stop;
dpaa_eth_refill_bpools(priv);
@@ -2377,7 +2377,7 @@ static enum qman_cb_dqrr_result rx_default_dqrr(struct qman_portal *portal,
percpu_priv = this_cpu_ptr(priv->percpu_priv);
percpu_stats = &percpu_priv->stats;
- if (unlikely(dpaa_eth_napi_schedule(percpu_priv, portal)))
+ if (unlikely(dpaa_eth_napi_schedule(percpu_priv, portal, napi)))
return qman_cb_dqrr_stop;
/* Make sure we didn't run out of buffers */
@@ -2474,7 +2474,7 @@ static enum qman_cb_dqrr_result conf_error_dqrr(struct qman_portal *portal,
percpu_priv = this_cpu_ptr(priv->percpu_priv);
- if (dpaa_eth_napi_schedule(percpu_priv, portal))
+ if (dpaa_eth_napi_schedule(percpu_priv, portal, napi))
return qman_cb_dqrr_stop;
dpaa_tx_error(net_dev, priv, percpu_priv, &dq->fd, fq->fqid);
@@ -2499,7 +2499,7 @@ static enum qman_cb_dqrr_result conf_dflt_dqrr(struct qman_portal *portal,
percpu_priv = this_cpu_ptr(priv->percpu_priv);
- if (dpaa_eth_napi_schedule(percpu_priv, portal))
+ if (dpaa_eth_napi_schedule(percpu_priv, portal, napi))
return qman_cb_dqrr_stop;
dpaa_tx_conf(net_dev, priv, percpu_priv, &dq->fd, fq->fqid);
--
2.28.0
^ permalink raw reply related
* [PATCH net-next 12/15] net: rtlwifi: Remove in_interrupt() usage in halbtc_send_bt_mp_operation()
From: Sebastian Andrzej Siewior @ 2020-10-27 22:54 UTC (permalink / raw)
To: netdev
Cc: Aymen Sghaier, Madalin Bucur, Sebastian Andrzej Siewior,
Zhu Yanjun, Samuel Chessman, Ping-Ke Shih, Herbert Xu,
Horia Geantă, linux-rdma, Rain River, Kalle Valo,
Ulrich Kunitz, Jouni Malinen, Daniel Drake, Jakub Kicinski,
Thomas Gleixner, linux-arm-kernel, Leon Romanovsky, linuxppc-dev,
linux-wireless, Li Yang, linux-crypto, Jon Mason, Saeed Mahameed,
David S. Miller
In-Reply-To: <20201027225454.3492351-1-bigeasy@linutronix.de>
halbtc_send_bt_mp_operation() uses in_interrupt() to determine if it is
safe to invoke wait_for_completion().
The usage of in_interrupt() in drivers is phased out and Linus clearly
requested that code which changes behaviour depending on context should
either be separated or the context be conveyed in an argument passed by the
caller, which usually knows the context.
Aside of that in_interrupt() is not correct as it does not catch preempt
disabled regions which neither can sleep.
halbtc_send_bt_mp_operation() is called from:
rtl_watchdog_wq_callback()
rtl_btc_periodical()
halbtc_get()
case BTC_GET_U4_BT_PATCH_VER:
halbtc_get_bt_patch_version()
which is preemtible context.
rtl_c2h_content_parsing()
btc_ops->btc_btinfo_notify()
rtl_btc_btinfo_notify()
exhalbtc_bt_info_notify()
ex_btc8723b1ant_bt_info_notify()
ex_btc8821a1ant_bt_info_notify()
ex_btc8821a2ant_bt_info_notify()
btcoexist->btc_set_bt_reg()
halbtc_set_bt_reg()
rtl_c2h_content_parsing() is in turn called from:
rtl_c2hcmd_wq_callback()
rtl_c2hcmd_launcher()
which is preemptible context and from:
_rtl_pci_rx_interrupt
rtl_c2hcmd_enqueue()
which is obviously not preemptible but limited to C2H_BT_MP commands
which does invoke rtl_c2h_content_parsing().
Aside of that it can be reached from:
halbtc_get()
case BTC_GET_U4_SUPPORTED_FEATURE:
halbtc_get_bt_coex_supported_feature()
case BTC_GET_U4_BT_FORBIDDEN_SLOT_VAL:
halbtc_get_bt_forbidden_slot_val()
case BTC_GET_U4_BT_DEVICE_INFO:
halbtc_get_bt_device_info()
case BTC_GET_U4_SUPPORTED_VERSION:
halbtc_get_bt_coex_supported_version()
case BTC_GET_U4_SUPPORTED_FEATURE:
halbtc_get_bt_coex_supported_feature()
btcoexist->btc_get_bt_afh_map_from_bt()
halbtc_get_bt_afh_map_from_bt()
btcoexist->btc_get_ble_scan_para_from_bt()
halbtc_get_ble_scan_para_from_bt()
btcoexist->btc_get_ble_scan_type_from_bt()
halbtc_get_ble_scan_type_from_bt()
btcoexist->btc_get_ant_det_val_from_bt()
halbtc_get_ant_det_val_from_bt()
btcoexist->btc_get_bt_coex_supported_version()
halbtc_get_bt_coex_supported_version()
btcoexist->btc_get_bt_coex_supported_feature()
halbtc_get_bt_coex_supported_feature()
None of these have a caller. Welcome to the wonderful world of HALs and
onion layers.
Remove in_interrupt() check.
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Ping-Ke Shih <pkshih@realtek.com>
Cc: Kalle Valo <kvalo@codeaurora.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: linux-wireless@vger.kernel.org
Cc: netdev@vger.kernel.org
---
drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
index 2155a6699ef8d..be4c0e60d44d1 100644
--- a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
+++ b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
@@ -240,9 +240,6 @@ bool halbtc_send_bt_mp_operation(struct btc_coexist *btcoexist, u8 op_code,
rtl_dbg(rtlpriv, COMP_BT_COEXIST, DBG_LOUD,
"btmpinfo wait req_num=%d wait=%ld\n", req_num, wait_ms);
- if (in_interrupt())
- return false;
-
if (wait_for_completion_timeout(&btcoexist->bt_mp_comp,
msecs_to_jiffies(wait_ms)) == 0) {
rtl_dbg(rtlpriv, COMP_BT_COEXIST, DBG_DMESG,
--
2.28.0
^ permalink raw reply related
* [PATCH net-next 15/15] crypto: caam: Replace in_irq() usage.
From: Sebastian Andrzej Siewior @ 2020-10-27 22:54 UTC (permalink / raw)
To: netdev
Cc: Aymen Sghaier, Madalin Bucur, Sebastian Andrzej Siewior,
Zhu Yanjun, Samuel Chessman, Ping-Ke Shih, Herbert Xu,
Horia Geantă, linux-rdma, Rain River, Kalle Valo,
Ulrich Kunitz, Jouni Malinen, Daniel Drake, Jakub Kicinski,
Thomas Gleixner, linux-arm-kernel, Leon Romanovsky, linuxppc-dev,
linux-wireless, Li Yang, linux-crypto, Jon Mason, Saeed Mahameed,
David S. Miller
In-Reply-To: <20201027225454.3492351-1-bigeasy@linutronix.de>
The driver uses in_irq() + in_serving_softirq() magic to decide if NAPI
scheduling is required or packet processing.
The usage of in_*() in drivers is phased out and Linus clearly requested
that code which changes behaviour depending on context should either be
seperated or the context be conveyed in an argument passed by the caller,
which usually knows the context.
Use the `napi' argument passed by the callback. It is set true if
called from the interrupt handler and NAPI should be scheduled.
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: "Horia Geantă" <horia.geanta@nxp.com>
Cc: Aymen Sghaier <aymen.sghaier@nxp.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Madalin Bucur <madalin.bucur@nxp.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Li Yang <leoyang.li@nxp.com>
Cc: linux-crypto@vger.kernel.org
Cc: netdev@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-arm-kernel@lists.infradead.org
---
drivers/crypto/caam/qi.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/crypto/caam/qi.c b/drivers/crypto/caam/qi.c
index 09ea398304c8b..79dbd90887f8a 100644
--- a/drivers/crypto/caam/qi.c
+++ b/drivers/crypto/caam/qi.c
@@ -545,14 +545,10 @@ static void cgr_cb(struct qman_portal *qm, struct qman_cgr *cgr, int congested)
}
}
-static int caam_qi_napi_schedule(struct qman_portal *p, struct caam_napi *np)
+static int caam_qi_napi_schedule(struct qman_portal *p, struct caam_napi *np,
+ bool napi)
{
- /*
- * In case of threaded ISR, for RT kernels in_irq() does not return
- * appropriate value, so use in_serving_softirq to distinguish between
- * softirq and irq contexts.
- */
- if (unlikely(in_irq() || !in_serving_softirq())) {
+ if (napi) {
/* Disable QMan IRQ source and invoke NAPI */
qman_p_irqsource_remove(p, QM_PIRQ_DQRI);
np->p = p;
@@ -574,7 +570,7 @@ static enum qman_cb_dqrr_result caam_rsp_fq_dqrr_cb(struct qman_portal *p,
struct caam_drv_private *priv = dev_get_drvdata(qidev);
u32 status;
- if (caam_qi_napi_schedule(p, caam_napi))
+ if (caam_qi_napi_schedule(p, caam_napi, napi))
return qman_cb_dqrr_stop;
fd = &dqrr->fd;
--
2.28.0
^ permalink raw reply related
* [PATCH net-next 08/15] net: airo: Replace in_atomic() usage.
From: Sebastian Andrzej Siewior @ 2020-10-27 22:54 UTC (permalink / raw)
To: netdev
Cc: Aymen Sghaier, Madalin Bucur, Sebastian Andrzej Siewior,
Zhu Yanjun, Samuel Chessman, Ping-Ke Shih, Herbert Xu,
Horia Geantă, linux-rdma, Rain River, Kalle Valo,
Ulrich Kunitz, Jouni Malinen, Daniel Drake, Jakub Kicinski,
Thomas Gleixner, linux-arm-kernel, Leon Romanovsky, linuxppc-dev,
linux-wireless, Li Yang, linux-crypto, Jon Mason, Saeed Mahameed,
David S. Miller
In-Reply-To: <20201027225454.3492351-1-bigeasy@linutronix.de>
issuecommand() is using in_atomic() to decide if it is safe to invoke
schedule() while waiting for the command to be accepted.
Usage of in_atomic() for this is only half correct as it can not detect all
condition where it is not allowed to schedule(). Also Linus clearly
requested that code which changes behaviour depending on context should
either be seperated or the context be conveyed in an argument passed by the
caller, which usually knows the context.
Add an may_sleep argument to issuecommand() indicating when it is save to
sleep and change schedule() to cond_resched() because it's pointless to
invoke schedule() if there is no request to reschedule.
Pass the may_sleep condition through the various call chains leading to
issuecommand().
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Kalle Valo <kvalo@codeaurora.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: linux-wireless@vger.kernel.org
Cc: netdev@vger.kernel.org
---
drivers/net/wireless/cisco/airo.c | 86 +++++++++++++++++--------------
1 file changed, 47 insertions(+), 39 deletions(-)
diff --git a/drivers/net/wireless/cisco/airo.c b/drivers/net/wireless/cisco/airo.c
index 369a6ca44d1ff..74acf9af2adb1 100644
--- a/drivers/net/wireless/cisco/airo.c
+++ b/drivers/net/wireless/cisco/airo.c
@@ -1115,7 +1115,8 @@ static int enable_MAC(struct airo_info *ai, int lock);
static void disable_MAC(struct airo_info *ai, int lock);
static void enable_interrupts(struct airo_info*);
static void disable_interrupts(struct airo_info*);
-static u16 issuecommand(struct airo_info*, Cmd *pCmd, Resp *pRsp);
+static u16 issuecommand(struct airo_info*, Cmd *pCmd, Resp *pRsp,
+ bool may_sleep);
static int bap_setup(struct airo_info*, u16 rid, u16 offset, int whichbap);
static int aux_bap_read(struct airo_info*, __le16 *pu16Dst, int bytelen,
int whichbap);
@@ -1130,8 +1131,10 @@ static int PC4500_writerid(struct airo_info*, u16 rid, const void
static int do_writerid(struct airo_info*, u16 rid, const void *rid_data,
int len, int dummy);
static u16 transmit_allocate(struct airo_info*, int lenPayload, int raw);
-static int transmit_802_3_packet(struct airo_info*, int len, char *pPacket);
-static int transmit_802_11_packet(struct airo_info*, int len, char *pPacket);
+static int transmit_802_3_packet(struct airo_info*, int len, char *pPacket,
+ bool may_sleep);
+static int transmit_802_11_packet(struct airo_info*, int len, char *pPacket,
+ bool may_sleep);
static int mpi_send_packet(struct net_device *dev);
static void mpi_unmap_card(struct pci_dev *pci);
@@ -1753,7 +1756,7 @@ static int readBSSListRid(struct airo_info *ai, int first,
if (down_interruptible(&ai->sem))
return -ERESTARTSYS;
ai->list_bss_task = current;
- issuecommand(ai, &cmd, &rsp);
+ issuecommand(ai, &cmd, &rsp, true);
up(&ai->sem);
/* Let the command take effect */
schedule_timeout_uninterruptible(3 * HZ);
@@ -2096,7 +2099,7 @@ static void get_tx_error(struct airo_info *ai, s32 fid)
}
}
-static void airo_end_xmit(struct net_device *dev)
+static void airo_end_xmit(struct net_device *dev, bool may_sleep)
{
u16 status;
int i;
@@ -2107,7 +2110,7 @@ static void airo_end_xmit(struct net_device *dev)
clear_bit(JOB_XMIT, &priv->jobs);
clear_bit(FLAG_PENDING_XMIT, &priv->flags);
- status = transmit_802_3_packet (priv, fids[fid], skb->data);
+ status = transmit_802_3_packet(priv, fids[fid], skb->data, may_sleep);
up(&priv->sem);
i = 0;
@@ -2164,11 +2167,11 @@ static netdev_tx_t airo_start_xmit(struct sk_buff *skb,
set_bit(JOB_XMIT, &priv->jobs);
wake_up_interruptible(&priv->thr_wait);
} else
- airo_end_xmit(dev);
+ airo_end_xmit(dev, false);
return NETDEV_TX_OK;
}
-static void airo_end_xmit11(struct net_device *dev)
+static void airo_end_xmit11(struct net_device *dev, bool may_sleep)
{
u16 status;
int i;
@@ -2179,7 +2182,7 @@ static void airo_end_xmit11(struct net_device *dev)
clear_bit(JOB_XMIT11, &priv->jobs);
clear_bit(FLAG_PENDING_XMIT11, &priv->flags);
- status = transmit_802_11_packet (priv, fids[fid], skb->data);
+ status = transmit_802_11_packet(priv, fids[fid], skb->data, may_sleep);
up(&priv->sem);
i = MAX_FIDS / 2;
@@ -2243,7 +2246,7 @@ static netdev_tx_t airo_start_xmit11(struct sk_buff *skb,
set_bit(JOB_XMIT11, &priv->jobs);
wake_up_interruptible(&priv->thr_wait);
} else
- airo_end_xmit11(dev);
+ airo_end_xmit11(dev, false);
return NETDEV_TX_OK;
}
@@ -2293,7 +2296,7 @@ static struct net_device_stats *airo_get_stats(struct net_device *dev)
return &dev->stats;
}
-static void airo_set_promisc(struct airo_info *ai)
+static void airo_set_promisc(struct airo_info *ai, bool may_sleep)
{
Cmd cmd;
Resp rsp;
@@ -2302,7 +2305,7 @@ static void airo_set_promisc(struct airo_info *ai)
cmd.cmd = CMD_SETMODE;
clear_bit(JOB_PROMISC, &ai->jobs);
cmd.parm0=(ai->flags&IFF_PROMISC) ? PROMISC : NOPROMISC;
- issuecommand(ai, &cmd, &rsp);
+ issuecommand(ai, &cmd, &rsp, may_sleep);
up(&ai->sem);
}
@@ -2316,7 +2319,7 @@ static void airo_set_multicast_list(struct net_device *dev)
set_bit(JOB_PROMISC, &ai->jobs);
wake_up_interruptible(&ai->thr_wait);
} else
- airo_set_promisc(ai);
+ airo_set_promisc(ai, false);
}
if ((dev->flags&IFF_ALLMULTI) || !netdev_mc_empty(dev)) {
@@ -2476,7 +2479,7 @@ static int mpi_init_descriptors (struct airo_info *ai)
cmd.parm0 = FID_RX;
cmd.parm1 = (ai->rxfids[0].card_ram_off - ai->pciaux);
cmd.parm2 = MPI_MAX_FIDS;
- rc = issuecommand(ai, &cmd, &rsp);
+ rc = issuecommand(ai, &cmd, &rsp, true);
if (rc != SUCCESS) {
airo_print_err(ai->dev->name, "Couldn't allocate RX FID");
return rc;
@@ -2504,7 +2507,7 @@ static int mpi_init_descriptors (struct airo_info *ai)
}
ai->txfids[i-1].tx_desc.eoc = 1; /* Last descriptor has EOC set */
- rc = issuecommand(ai, &cmd, &rsp);
+ rc = issuecommand(ai, &cmd, &rsp, true);
if (rc != SUCCESS) {
airo_print_err(ai->dev->name, "Couldn't allocate TX FID");
return rc;
@@ -2518,7 +2521,7 @@ static int mpi_init_descriptors (struct airo_info *ai)
cmd.parm0 = RID_RW;
cmd.parm1 = (ai->config_desc.card_ram_off - ai->pciaux);
cmd.parm2 = 1; /* Magic number... */
- rc = issuecommand(ai, &cmd, &rsp);
+ rc = issuecommand(ai, &cmd, &rsp, true);
if (rc != SUCCESS) {
airo_print_err(ai->dev->name, "Couldn't allocate RID");
return rc;
@@ -3144,13 +3147,13 @@ static int airo_thread(void *data)
}
if (test_bit(JOB_XMIT, &ai->jobs))
- airo_end_xmit(dev);
+ airo_end_xmit(dev, true);
else if (test_bit(JOB_XMIT11, &ai->jobs))
- airo_end_xmit11(dev);
+ airo_end_xmit11(dev, true);
else if (test_bit(JOB_STATS, &ai->jobs))
airo_read_stats(dev);
else if (test_bit(JOB_PROMISC, &ai->jobs))
- airo_set_promisc(ai);
+ airo_set_promisc(ai, true);
else if (test_bit(JOB_MIC, &ai->jobs))
micinit(ai);
else if (test_bit(JOB_EVENT, &ai->jobs))
@@ -3599,7 +3602,7 @@ static int enable_MAC(struct airo_info *ai, int lock)
if (!test_bit(FLAG_ENABLED, &ai->flags)) {
memset(&cmd, 0, sizeof(cmd));
cmd.cmd = MAC_ENABLE;
- rc = issuecommand(ai, &cmd, &rsp);
+ rc = issuecommand(ai, &cmd, &rsp, true);
if (rc == SUCCESS)
set_bit(FLAG_ENABLED, &ai->flags);
} else
@@ -3631,7 +3634,7 @@ static void disable_MAC(struct airo_info *ai, int lock)
netif_carrier_off(ai->dev);
memset(&cmd, 0, sizeof(cmd));
cmd.cmd = MAC_DISABLE; // disable in case already enabled
- issuecommand(ai, &cmd, &rsp);
+ issuecommand(ai, &cmd, &rsp, true);
clear_bit(FLAG_ENABLED, &ai->flags);
}
if (lock == 1)
@@ -3834,7 +3837,7 @@ static u16 setup_card(struct airo_info *ai, u8 *mac, int lock)
cmd.parm0 = cmd.parm1 = cmd.parm2 = 0;
if (lock && down_interruptible(&ai->sem))
return ERROR;
- if (issuecommand(ai, &cmd, &rsp) != SUCCESS) {
+ if (issuecommand(ai, &cmd, &rsp, true) != SUCCESS) {
if (lock)
up(&ai->sem);
return ERROR;
@@ -3844,7 +3847,7 @@ static u16 setup_card(struct airo_info *ai, u8 *mac, int lock)
// Let's figure out if we need to use the AUX port
if (!test_bit(FLAG_MPI,&ai->flags)) {
cmd.cmd = CMD_ENABLEAUX;
- if (issuecommand(ai, &cmd, &rsp) != SUCCESS) {
+ if (issuecommand(ai, &cmd, &rsp, true) != SUCCESS) {
if (lock)
up(&ai->sem);
airo_print_err(ai->dev->name, "Error checking for AUX port");
@@ -3956,7 +3959,8 @@ static u16 setup_card(struct airo_info *ai, u8 *mac, int lock)
return SUCCESS;
}
-static u16 issuecommand(struct airo_info *ai, Cmd *pCmd, Resp *pRsp)
+static u16 issuecommand(struct airo_info *ai, Cmd *pCmd, Resp *pRsp,
+ bool may_sleep)
{
// Im really paranoid about letting it run forever!
int max_tries = 600000;
@@ -3973,8 +3977,8 @@ static u16 issuecommand(struct airo_info *ai, Cmd *pCmd, Resp *pRsp)
if ((IN4500(ai, COMMAND)) == pCmd->cmd)
// PC4500 didn't notice command, try again
OUT4500(ai, COMMAND, pCmd->cmd);
- if (!in_atomic() && (max_tries & 255) == 0)
- schedule();
+ if (may_sleep && (max_tries & 255) == 0)
+ cond_resched();
}
if (max_tries == -1) {
@@ -4131,7 +4135,7 @@ static int PC4500_accessrid(struct airo_info *ai, u16 rid, u16 accmd)
memset(&cmd, 0, sizeof(cmd));
cmd.cmd = accmd;
cmd.parm0 = rid;
- status = issuecommand(ai, &cmd, &rsp);
+ status = issuecommand(ai, &cmd, &rsp, true);
if (status != 0) return status;
if ((rsp.status & 0x7F00) != 0) {
return (accmd << 8) + (rsp.rsp0 & 0xFF);
@@ -4167,7 +4171,7 @@ static int PC4500_readrid(struct airo_info *ai, u16 rid, void *pBuf, int len, in
memcpy_toio(ai->config_desc.card_ram_off,
&ai->config_desc.rid_desc, sizeof(Rid));
- rc = issuecommand(ai, &cmd, &rsp);
+ rc = issuecommand(ai, &cmd, &rsp, true);
if (rsp.status & 0x7f00)
rc = rsp.rsp0;
@@ -4246,7 +4250,7 @@ static int PC4500_writerid(struct airo_info *ai, u16 rid,
memcpy(ai->config_desc.virtual_host_addr,
pBuf, len);
- rc = issuecommand(ai, &cmd, &rsp);
+ rc = issuecommand(ai, &cmd, &rsp, true);
if ((rc & 0xff00) != 0) {
airo_print_err(ai->dev->name, "%s: Write rid Error %d",
__func__, rc);
@@ -4292,7 +4296,7 @@ static u16 transmit_allocate(struct airo_info *ai, int lenPayload, int raw)
cmd.parm0 = lenPayload;
if (down_interruptible(&ai->sem))
return ERROR;
- if (issuecommand(ai, &cmd, &rsp) != SUCCESS) {
+ if (issuecommand(ai, &cmd, &rsp, true) != SUCCESS) {
txFid = ERROR;
goto done;
}
@@ -4338,7 +4342,8 @@ static u16 transmit_allocate(struct airo_info *ai, int lenPayload, int raw)
/* In general BAP1 is dedicated to transmiting packets. However,
since we need a BAP when accessing RIDs, we also use BAP1 for that.
Make sure the BAP1 spinlock is held when this is called. */
-static int transmit_802_3_packet(struct airo_info *ai, int len, char *pPacket)
+static int transmit_802_3_packet(struct airo_info *ai, int len, char *pPacket,
+ bool may_sleep)
{
__le16 payloadLen;
Cmd cmd;
@@ -4376,12 +4381,14 @@ static int transmit_802_3_packet(struct airo_info *ai, int len, char *pPacket)
memset(&cmd, 0, sizeof(cmd));
cmd.cmd = CMD_TRANSMIT;
cmd.parm0 = txFid;
- if (issuecommand(ai, &cmd, &rsp) != SUCCESS) return ERROR;
+ if (issuecommand(ai, &cmd, &rsp, may_sleep) != SUCCESS)
+ return ERROR;
if ((rsp.status & 0xFF00) != 0) return ERROR;
return SUCCESS;
}
-static int transmit_802_11_packet(struct airo_info *ai, int len, char *pPacket)
+static int transmit_802_11_packet(struct airo_info *ai, int len, char *pPacket,
+ bool may_sleep)
{
__le16 fc, payloadLen;
Cmd cmd;
@@ -4416,7 +4423,8 @@ static int transmit_802_11_packet(struct airo_info *ai, int len, char *pPacket)
memset(&cmd, 0, sizeof(cmd));
cmd.cmd = CMD_TRANSMIT;
cmd.parm0 = txFid;
- if (issuecommand(ai, &cmd, &rsp) != SUCCESS) return ERROR;
+ if (issuecommand(ai, &cmd, &rsp, may_sleep) != SUCCESS)
+ return ERROR;
if ((rsp.status & 0xFF00) != 0) return ERROR;
return SUCCESS;
}
@@ -5480,7 +5488,7 @@ static int proc_BSSList_open(struct inode *inode, struct file *file)
kfree(file->private_data);
return -ERESTARTSYS;
}
- issuecommand(ai, &cmd, &rsp);
+ issuecommand(ai, &cmd, &rsp, true);
up(&ai->sem);
data->readlen = 0;
return 0;
@@ -5617,7 +5625,7 @@ static int __maybe_unused airo_pci_suspend(struct device *dev_d)
netif_device_detach(dev);
ai->power = PMSG_SUSPEND;
cmd.cmd = HOSTSLEEP;
- issuecommand(ai, &cmd, &rsp);
+ issuecommand(ai, &cmd, &rsp, true);
device_wakeup_enable(dev_d);
return 0;
@@ -5960,7 +5968,7 @@ static int airo_set_wap(struct net_device *dev,
cmd.cmd = CMD_LOSE_SYNC;
if (down_interruptible(&local->sem))
return -ERESTARTSYS;
- issuecommand(local, &cmd, &rsp);
+ issuecommand(local, &cmd, &rsp, true);
up(&local->sem);
} else {
memset(APList_rid, 0, sizeof(*APList_rid));
@@ -7258,7 +7266,7 @@ static int airo_set_scan(struct net_device *dev,
ai->scan_timeout = RUN_AT(3*HZ);
memset(&cmd, 0, sizeof(cmd));
cmd.cmd = CMD_LISTBSS;
- issuecommand(ai, &cmd, &rsp);
+ issuecommand(ai, &cmd, &rsp, true);
wake = 1;
out:
@@ -7525,7 +7533,7 @@ static int airo_config_commit(struct net_device *dev,
writeConfigRid(local, 0);
enable_MAC(local, 0);
if (test_bit (FLAG_RESET, &local->flags))
- airo_set_promisc(local);
+ airo_set_promisc(local, true);
else
up(&local->sem);
--
2.28.0
^ permalink raw reply related
* [PATCH net-next 11/15] net: rtlwifi: Remove in_interrupt() usage in is_any_client_connect_to_ap().
From: Sebastian Andrzej Siewior @ 2020-10-27 22:54 UTC (permalink / raw)
To: netdev
Cc: Aymen Sghaier, Madalin Bucur, Sebastian Andrzej Siewior,
Zhu Yanjun, Samuel Chessman, Ping-Ke Shih, Herbert Xu,
Horia Geantă, linux-rdma, Rain River, Kalle Valo,
Ulrich Kunitz, Jouni Malinen, Daniel Drake, Jakub Kicinski,
Thomas Gleixner, linux-arm-kernel, Leon Romanovsky, linuxppc-dev,
linux-wireless, Li Yang, linux-crypto, Jon Mason, Saeed Mahameed,
David S. Miller
In-Reply-To: <20201027225454.3492351-1-bigeasy@linutronix.de>
is_any_client_connect_to_ap() is using in_interrupt() to determine whether
it should acquire the lock prior accessing the list.
The usage of in_interrupt() in drivers is phased out and Linus clearly
requested that code which changes behaviour depending on context should
either be separated or the context be conveyed in an argument passed by the
caller, which usually knows the context.
The function is called from:
- halbtc_get()
- halbtc_get()
halbtc_get_wifi_link_status()
- halbtc_display_dbg_msg()
halbtc_display_wifi_status()
halbtc_get_wifi_link_status()
All top level callers are part of the btc_coexist callback inferface and
are never invoked from a context which can hold the lock already.
The contexts which hold the lock are either protecting list add/del
operations or list walks which never call into any of the btc_coexist
interfaces.
In fact the conditional is outright dangerous because if this function
would be invoked from a BH disabled context the check would avoid taking
the lock while on another CPU the list could be manipulated under the lock.
Remove the in_interrupt() check and always acquire the lock.
To simplify the code further use list_empty() instead of walking the list
and counting the entries just to check the count for > 0 at the end.
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Ping-Ke Shih <pkshih@realtek.com>
Cc: Kalle Valo <kvalo@codeaurora.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: linux-wireless@vger.kernel.org
Cc: netdev@vger.kernel.org
---
.../realtek/rtlwifi/btcoexist/halbtcoutsrc.c | 25 +++++--------------
1 file changed, 6 insertions(+), 19 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
index 2c05369b79e4d..2155a6699ef8d 100644
--- a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
+++ b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
@@ -47,30 +47,17 @@ static bool is_any_client_connect_to_ap(struct btc_coexist *btcoexist)
{
struct rtl_priv *rtlpriv = btcoexist->adapter;
struct rtl_mac *mac = rtl_mac(rtlpriv);
- struct rtl_sta_info *drv_priv;
- u8 cnt = 0;
+ bool ret = false;
if (mac->opmode == NL80211_IFTYPE_ADHOC ||
mac->opmode == NL80211_IFTYPE_MESH_POINT ||
mac->opmode == NL80211_IFTYPE_AP) {
- if (in_interrupt() > 0) {
- list_for_each_entry(drv_priv, &rtlpriv->entry_list,
- list) {
- cnt++;
- }
- } else {
- spin_lock_bh(&rtlpriv->locks.entry_list_lock);
- list_for_each_entry(drv_priv, &rtlpriv->entry_list,
- list) {
- cnt++;
- }
- spin_unlock_bh(&rtlpriv->locks.entry_list_lock);
- }
+ spin_lock_bh(&rtlpriv->locks.entry_list_lock);
+ if (!list_empty(&rtlpriv->entry_list))
+ ret = true;
+ spin_unlock_bh(&rtlpriv->locks.entry_list_lock);
}
- if (cnt > 0)
- return true;
- else
- return false;
+ return ret;
}
static bool halbtc_legacy(struct rtl_priv *adapter)
--
2.28.0
^ permalink raw reply related
* [PATCH net-next 09/15] net: hostap: Remove in_atomic() check.
From: Sebastian Andrzej Siewior @ 2020-10-27 22:54 UTC (permalink / raw)
To: netdev
Cc: Aymen Sghaier, Madalin Bucur, Sebastian Andrzej Siewior,
Zhu Yanjun, Samuel Chessman, Ping-Ke Shih, Herbert Xu,
Horia Geantă, linux-rdma, Rain River, Kalle Valo,
Ulrich Kunitz, Jouni Malinen, Daniel Drake, Jakub Kicinski,
Thomas Gleixner, linux-arm-kernel, Leon Romanovsky, linuxppc-dev,
linux-wireless, Li Yang, linux-crypto, Jon Mason, Saeed Mahameed,
David S. Miller
In-Reply-To: <20201027225454.3492351-1-bigeasy@linutronix.de>
hostap_get_wireless_stats() is the iw_handler_if::get_wireless_stats()
callback of this driver. This callback was not allowed to sleep until
commit a160ee69c6a46 ("wext: let get_wireless_stats() sleep") in v2.6.32.
Remove the therefore pointless in_atomic() check.
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Jouni Malinen <j@w1.fi>
Cc: Kalle Valo <kvalo@codeaurora.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: linux-wireless@vger.kernel.org
Cc: netdev@vger.kernel.org
---
drivers/net/wireless/intersil/hostap/hostap_ioctl.c | 13 +------------
1 file changed, 1 insertion(+), 12 deletions(-)
diff --git a/drivers/net/wireless/intersil/hostap/hostap_ioctl.c b/drivers/net/wireless/intersil/hostap/hostap_ioctl.c
index 514c7b01dbf6f..49766b285230c 100644
--- a/drivers/net/wireless/intersil/hostap/hostap_ioctl.c
+++ b/drivers/net/wireless/intersil/hostap/hostap_ioctl.c
@@ -44,19 +44,8 @@ static struct iw_statistics *hostap_get_wireless_stats(struct net_device *dev)
if (local->iw_mode != IW_MODE_MASTER &&
local->iw_mode != IW_MODE_REPEAT) {
- int update = 1;
-#ifdef in_atomic
- /* RID reading might sleep and it must not be called in
- * interrupt context or while atomic. However, this
- * function seems to be called while atomic (at least in Linux
- * 2.5.59). Update signal quality values only if in suitable
- * context. Otherwise, previous values read from tick timer
- * will be used. */
- if (in_atomic())
- update = 0;
-#endif /* in_atomic */
- if (update && prism2_update_comms_qual(dev) == 0)
+ if (prism2_update_comms_qual(dev) == 0)
wstats->qual.updated = IW_QUAL_ALL_UPDATED |
IW_QUAL_DBM;
--
2.28.0
^ permalink raw reply related
* [PATCH net-next 10/15] net: zd1211rw: Remove in_atomic() usage.
From: Sebastian Andrzej Siewior @ 2020-10-27 22:54 UTC (permalink / raw)
To: netdev
Cc: Aymen Sghaier, Madalin Bucur, Sebastian Andrzej Siewior,
Zhu Yanjun, Samuel Chessman, Ping-Ke Shih, Herbert Xu,
Horia Geantă, linux-rdma, Rain River, Kalle Valo,
Ulrich Kunitz, Jouni Malinen, Daniel Drake, Jakub Kicinski,
Thomas Gleixner, linux-arm-kernel, Leon Romanovsky, linuxppc-dev,
linux-wireless, Li Yang, linux-crypto, Jon Mason, Saeed Mahameed,
David S. Miller
In-Reply-To: <20201027225454.3492351-1-bigeasy@linutronix.de>
The usage of in_atomic() in driver code is deprecated as it can not
always detect all states where it is not allowed to sleep.
All callers are in premptible thread context and all functions invoke core
functions which have checks for invalid calling contexts already.
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Daniel Drake <dsd@gentoo.org>
Cc: Ulrich Kunitz <kune@deine-taler.de>
Cc: Kalle Valo <kvalo@codeaurora.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: linux-wireless@vger.kernel.org
Cc: netdev@vger.kernel.org
---
drivers/net/wireless/zydas/zd1211rw/zd_usb.c | 15 ---------------
1 file changed, 15 deletions(-)
diff --git a/drivers/net/wireless/zydas/zd1211rw/zd_usb.c b/drivers/net/wireless/zydas/zd1211rw/zd_usb.c
index 66367ab7e4c1e..5c4cd0e1adebb 100644
--- a/drivers/net/wireless/zydas/zd1211rw/zd_usb.c
+++ b/drivers/net/wireless/zydas/zd1211rw/zd_usb.c
@@ -1711,11 +1711,6 @@ int zd_usb_ioread16v(struct zd_usb *usb, u16 *values,
count, USB_MAX_IOREAD16_COUNT);
return -EINVAL;
}
- if (in_atomic()) {
- dev_dbg_f(zd_usb_dev(usb),
- "error: io in atomic context not supported\n");
- return -EWOULDBLOCK;
- }
if (!usb_int_enabled(usb)) {
dev_dbg_f(zd_usb_dev(usb),
"error: usb interrupt not enabled\n");
@@ -1882,11 +1877,6 @@ int zd_usb_iowrite16v_async(struct zd_usb *usb, const struct zd_ioreq16 *ioreqs,
count, USB_MAX_IOWRITE16_COUNT);
return -EINVAL;
}
- if (in_atomic()) {
- dev_dbg_f(zd_usb_dev(usb),
- "error: io in atomic context not supported\n");
- return -EWOULDBLOCK;
- }
udev = zd_usb_to_usbdev(usb);
@@ -1966,11 +1956,6 @@ int zd_usb_rfwrite(struct zd_usb *usb, u32 value, u8 bits)
int i, req_len, actual_req_len;
u16 bit_value_template;
- if (in_atomic()) {
- dev_dbg_f(zd_usb_dev(usb),
- "error: io in atomic context not supported\n");
- return -EWOULDBLOCK;
- }
if (bits < USB_MIN_RFWRITE_BIT_COUNT) {
dev_dbg_f(zd_usb_dev(usb),
"error: bits %d are smaller than"
--
2.28.0
^ permalink raw reply related
* [PATCH net-next 07/15] net: airo: Always use JOB_STATS and JOB_EVENT
From: Sebastian Andrzej Siewior @ 2020-10-27 22:54 UTC (permalink / raw)
To: netdev
Cc: Aymen Sghaier, Madalin Bucur, Sebastian Andrzej Siewior,
Zhu Yanjun, Samuel Chessman, Ping-Ke Shih, Herbert Xu,
Horia Geantă, linux-rdma, Rain River, Kalle Valo,
Ulrich Kunitz, Jouni Malinen, Daniel Drake, Jakub Kicinski,
Thomas Gleixner, linux-arm-kernel, Leon Romanovsky, linuxppc-dev,
linux-wireless, Li Yang, linux-crypto, Jon Mason, Saeed Mahameed,
David S. Miller
In-Reply-To: <20201027225454.3492351-1-bigeasy@linutronix.de>
issuecommand() is using in_atomic() to decide if it is safe to invoke
schedule() while waiting for the command to be accepted.
Usage of in_atomic() for this is only half correct as it can not detect all
condition where it is not allowed to schedule(). Also Linus clearly
requested that code which changes behaviour depending on context should
either be seperated or the context be conveyed in an argument passed by the
caller, which usually knows the context.
Chasing the call chains leading up to issuecommand() is straight forward,
but airo_link() and airo_get_stats() would require to pass the context
through a quite large amount of functions.
As this is ancient hardware, avoid the churn and enforce the invocation of
those functions through the JOB machinery.
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Kalle Valo <kvalo@codeaurora.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: linux-wireless@vger.kernel.org
Cc: netdev@vger.kernel.org
---
drivers/net/wireless/cisco/airo.c | 16 +++++-----------
1 file changed, 5 insertions(+), 11 deletions(-)
diff --git a/drivers/net/wireless/cisco/airo.c b/drivers/net/wireless/cisco/airo.c
index ca423f3b6b3ea..369a6ca44d1ff 100644
--- a/drivers/net/wireless/cisco/airo.c
+++ b/drivers/net/wireless/cisco/airo.c
@@ -2286,12 +2286,8 @@ static struct net_device_stats *airo_get_stats(struct net_device *dev)
struct airo_info *local = dev->ml_priv;
if (!test_bit(JOB_STATS, &local->jobs)) {
- /* Get stats out of the card if available */
- if (down_trylock(&local->sem) != 0) {
- set_bit(JOB_STATS, &local->jobs);
- wake_up_interruptible(&local->thr_wait);
- } else
- airo_read_stats(dev);
+ set_bit(JOB_STATS, &local->jobs);
+ wake_up_interruptible(&local->thr_wait);
}
return &dev->stats;
@@ -3277,11 +3273,9 @@ static void airo_handle_link(struct airo_info *ai)
set_bit(FLAG_UPDATE_UNI, &ai->flags);
set_bit(FLAG_UPDATE_MULTI, &ai->flags);
- if (down_trylock(&ai->sem) != 0) {
- set_bit(JOB_EVENT, &ai->jobs);
- wake_up_interruptible(&ai->thr_wait);
- } else
- airo_send_event(ai->dev);
+ set_bit(JOB_EVENT, &ai->jobs);
+ wake_up_interruptible(&ai->thr_wait);
+
netif_carrier_on(ai->dev);
} else if (!scan_forceloss) {
if (auto_wep && !ai->expires) {
--
2.28.0
^ permalink raw reply related
* [PATCH net-next 04/15] net: mlx5: Replace in_irq() usage.
From: Sebastian Andrzej Siewior @ 2020-10-27 22:54 UTC (permalink / raw)
To: netdev
Cc: Aymen Sghaier, Madalin Bucur, Sebastian Andrzej Siewior,
Zhu Yanjun, Samuel Chessman, Ping-Ke Shih, Herbert Xu,
Horia Geantă, linux-rdma, Rain River, Kalle Valo,
Ulrich Kunitz, Jouni Malinen, Daniel Drake, Jakub Kicinski,
Thomas Gleixner, linux-arm-kernel, Leon Romanovsky, linuxppc-dev,
linux-wireless, Li Yang, linux-crypto, Jon Mason, Saeed Mahameed,
David S. Miller
In-Reply-To: <20201027225454.3492351-1-bigeasy@linutronix.de>
mlx5_eq_async_int() uses in_irq() to decide whether eq::lock needs to be
acquired and released with spin_[un]lock() or the irq saving/restoring
variants.
The usage of in_*() in drivers is phased out and Linus clearly requested
that code which changes behaviour depending on context should either be
seperated or the context be conveyed in an argument passed by the caller,
which usually knows the context.
mlx5_eq_async_int() knows the context via the action argument already so
using it for the lock variant decision is a straight forward replacement
for in_irq().
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Saeed Mahameed <saeedm@nvidia.com>
Cc: Leon Romanovsky <leon@kernel.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: linux-rdma@vger.kernel.org
---
drivers/net/ethernet/mellanox/mlx5/core/eq.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eq.c b/drivers/net/ethernet/mellanox/mlx5/core/eq.c
index 8ebfe782f95e5..3800e9415158b 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eq.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eq.c
@@ -189,19 +189,21 @@ u32 mlx5_eq_poll_irq_disabled(struct mlx5_eq_comp *eq)
return count_eqe;
}
-static void mlx5_eq_async_int_lock(struct mlx5_eq_async *eq, unsigned long *flags)
+static void mlx5_eq_async_int_lock(struct mlx5_eq_async *eq, bool recovery,
+ unsigned long *flags)
__acquires(&eq->lock)
{
- if (in_irq())
+ if (!recovery)
spin_lock(&eq->lock);
else
spin_lock_irqsave(&eq->lock, *flags);
}
-static void mlx5_eq_async_int_unlock(struct mlx5_eq_async *eq, unsigned long *flags)
+static void mlx5_eq_async_int_unlock(struct mlx5_eq_async *eq, bool recovery,
+ unsigned long *flags)
__releases(&eq->lock)
{
- if (in_irq())
+ if (!recovery)
spin_unlock(&eq->lock);
else
spin_unlock_irqrestore(&eq->lock, *flags);
@@ -222,12 +224,14 @@ static int mlx5_eq_async_int(struct notifier_block *nb,
struct mlx5_core_dev *dev;
struct mlx5_eqe *eqe;
unsigned long flags;
+ bool recovery;
int num_eqes = 0;
dev = eq->dev;
eqt = dev->priv.eq_table;
- mlx5_eq_async_int_lock(eq_async, &flags);
+ recovery = action == ASYNC_EQ_RECOVER;
+ mlx5_eq_async_int_lock(eq_async, recovery, &flags);
eqe = next_eqe_sw(eq);
if (!eqe)
@@ -249,9 +253,9 @@ static int mlx5_eq_async_int(struct notifier_block *nb,
out:
eq_update_ci(eq, 1);
- mlx5_eq_async_int_unlock(eq_async, &flags);
+ mlx5_eq_async_int_unlock(eq_async, recovery, &flags);
- return unlikely(action == ASYNC_EQ_RECOVER) ? num_eqes : 0;
+ return unlikely(recovery) ? num_eqes : 0;
}
void mlx5_cmd_eq_recover(struct mlx5_core_dev *dev)
--
2.28.0
^ permalink raw reply related
* [PATCH net-next 06/15] net: airo: Invoke airo_read_wireless_stats() directly
From: Sebastian Andrzej Siewior @ 2020-10-27 22:54 UTC (permalink / raw)
To: netdev
Cc: Aymen Sghaier, Madalin Bucur, Sebastian Andrzej Siewior,
Zhu Yanjun, Samuel Chessman, Ping-Ke Shih, Herbert Xu,
Horia Geantă, linux-rdma, Rain River, Kalle Valo,
Ulrich Kunitz, Jouni Malinen, Daniel Drake, Jakub Kicinski,
Thomas Gleixner, linux-arm-kernel, Leon Romanovsky, linuxppc-dev,
linux-wireless, Li Yang, linux-crypto, Jon Mason, Saeed Mahameed,
David S. Miller
In-Reply-To: <20201027225454.3492351-1-bigeasy@linutronix.de>
airo_get_wireless_stats() is the iw_handler_if::get_wireless_stats()
callback of this driver. This callback was not allowed to sleep until
commit a160ee69c6a46 ("wext: let get_wireless_stats() sleep") in v2.6.32.
airo still delegates the readout to a thread, which is not longer
necessary.
Invoke airo_read_wireless_stats() directly from the callback and remove
the now unused JOB_WSTATS handling.
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Kalle Valo <kvalo@codeaurora.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: linux-wireless@vger.kernel.org
Cc: netdev@vger.kernel.org
---
drivers/net/wireless/cisco/airo.c | 22 +++++-----------------
1 file changed, 5 insertions(+), 17 deletions(-)
diff --git a/drivers/net/wireless/cisco/airo.c b/drivers/net/wireless/cisco/airo.c
index 87b9398b03fd4..ca423f3b6b3ea 100644
--- a/drivers/net/wireless/cisco/airo.c
+++ b/drivers/net/wireless/cisco/airo.c
@@ -1144,7 +1144,6 @@ static int airo_thread(void *data);
static void timer_func(struct net_device *dev);
static int airo_ioctl(struct net_device *dev, struct ifreq *rq, int cmd);
static struct iw_statistics *airo_get_wireless_stats(struct net_device *dev);
-static void airo_read_wireless_stats(struct airo_info *local);
#ifdef CISCO_EXT
static int readrids(struct net_device *dev, aironet_ioctl *comp);
static int writerids(struct net_device *dev, aironet_ioctl *comp);
@@ -1200,7 +1199,6 @@ struct airo_info {
#define JOB_MIC 5
#define JOB_EVENT 6
#define JOB_AUTOWEP 7
-#define JOB_WSTATS 8
#define JOB_SCAN_RESULTS 9
unsigned long jobs;
int (*bap_read)(struct airo_info*, __le16 *pu16Dst, int bytelen,
@@ -3155,8 +3153,6 @@ static int airo_thread(void *data)
airo_end_xmit11(dev);
else if (test_bit(JOB_STATS, &ai->jobs))
airo_read_stats(dev);
- else if (test_bit(JOB_WSTATS, &ai->jobs))
- airo_read_wireless_stats(ai);
else if (test_bit(JOB_PROMISC, &ai->jobs))
airo_set_promisc(ai);
else if (test_bit(JOB_MIC, &ai->jobs))
@@ -7732,15 +7728,12 @@ static void airo_read_wireless_stats(struct airo_info *local)
__le32 *vals = stats_rid.vals;
/* Get stats out of the card */
- clear_bit(JOB_WSTATS, &local->jobs);
- if (local->power.event) {
- up(&local->sem);
+ if (local->power.event)
return;
- }
+
readCapabilityRid(local, &cap_rid, 0);
readStatusRid(local, &status_rid, 0);
readStatsRid(local, &stats_rid, RID_STATS, 0);
- up(&local->sem);
/* The status */
local->wstats.status = le16_to_cpu(status_rid.mode);
@@ -7783,15 +7776,10 @@ static struct iw_statistics *airo_get_wireless_stats(struct net_device *dev)
{
struct airo_info *local = dev->ml_priv;
- if (!test_bit(JOB_WSTATS, &local->jobs)) {
- /* Get stats out of the card if available */
- if (down_trylock(&local->sem) != 0) {
- set_bit(JOB_WSTATS, &local->jobs);
- wake_up_interruptible(&local->thr_wait);
- } else
- airo_read_wireless_stats(local);
+ if (!down_interruptible(&local->sem)) {
+ airo_read_wireless_stats(local);
+ up(&local->sem);
}
-
return &local->wstats;
}
--
2.28.0
^ permalink raw reply related
* [PATCH net-next 03/15] net: forcedeth: Replace context and lock check with a lockdep_assert()
From: Sebastian Andrzej Siewior @ 2020-10-27 22:54 UTC (permalink / raw)
To: netdev
Cc: Aymen Sghaier, Madalin Bucur, Sebastian Andrzej Siewior,
Zhu Yanjun, Samuel Chessman, Ping-Ke Shih, Herbert Xu,
Horia Geantă, linux-rdma, Rain River, Kalle Valo,
Ulrich Kunitz, Jouni Malinen, Daniel Drake, Jakub Kicinski,
Thomas Gleixner, linux-arm-kernel, Leon Romanovsky, linuxppc-dev,
linux-wireless, Li Yang, linux-crypto, Jon Mason, Saeed Mahameed,
David S. Miller
In-Reply-To: <20201027225454.3492351-1-bigeasy@linutronix.de>
nv_update_stats() triggers a WARN_ON() when invoked from hard interrupt
context because the locks in use are not hard interrupt safe. It also has
an assert_spin_locked() which was the lock check before the lockdep era.
Lockdep has way broader locking correctness checks and covers both issues,
so replace the warning and the lock assert with lockdep_assert_held().
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Rain River <rain.1986.08.12@gmail.com>
Cc: Zhu Yanjun <zyjzyj2000@gmail.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: netdev@vger.kernel.org
---
drivers/net/ethernet/nvidia/forcedeth.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/nvidia/forcedeth.c b/drivers/net/ethernet/nvidia/forcedeth.c
index 2fc10a36afa4a..7e85cf943be11 100644
--- a/drivers/net/ethernet/nvidia/forcedeth.c
+++ b/drivers/net/ethernet/nvidia/forcedeth.c
@@ -1666,11 +1666,7 @@ static void nv_update_stats(struct net_device *dev)
struct fe_priv *np = netdev_priv(dev);
u8 __iomem *base = get_hwbase(dev);
- /* If it happens that this is run in top-half context, then
- * replace the spin_lock of hwstats_lock with
- * spin_lock_irqsave() in calling functions. */
- WARN_ONCE(in_irq(), "forcedeth: estats spin_lock(_bh) from top-half");
- assert_spin_locked(&np->hwstats_lock);
+ lockdep_assert_held(&np->hwstats_lock);
/* query hardware */
np->estats.tx_bytes += readl(base + NvRegTxCnt);
--
2.28.0
^ permalink raw reply related
* [PATCH net-next 05/15] net: tlan: Replace in_irq() usage
From: Sebastian Andrzej Siewior @ 2020-10-27 22:54 UTC (permalink / raw)
To: netdev
Cc: Aymen Sghaier, Madalin Bucur, Sebastian Andrzej Siewior,
Zhu Yanjun, Samuel Chessman, Ping-Ke Shih, Herbert Xu,
Horia Geantă, linux-rdma, Rain River, Kalle Valo,
Ulrich Kunitz, Jouni Malinen, Daniel Drake, Jakub Kicinski,
Thomas Gleixner, linux-arm-kernel, Leon Romanovsky, linuxppc-dev,
linux-wireless, Li Yang, linux-crypto, Jon Mason, Saeed Mahameed,
David S. Miller
In-Reply-To: <20201027225454.3492351-1-bigeasy@linutronix.de>
The driver uses in_irq() to determine if the tlan_priv::lock has to be
acquired in tlan_mii_read_reg() and tlan_mii_write_reg().
The interrupt handler acquires the lock outside of these functions so the
in_irq() check is meant to prevent a lock recursion deadlock. But this
check is incorrect when interrupt force threading is enabled because then
the handler runs in thread context and in_irq() correctly returns false.
The usage of in_*() in drivers is phased out and Linus clearly requested
that code which changes behaviour depending on context should either be
seperated or the context be conveyed in an argument passed by the caller,
which usually knows the context.
tlan_set_timer() has this conditional as well, but this function is only
invoked from task context or the timer callback itself. So it always has to
lock and the check can be removed.
tlan_mii_read_reg(), tlan_mii_write_reg() and tlan_phy_print() are invoked
from interrupt and other contexts.
Split out the actual function body into helper variants which are called
from interrupt context and make the original functions wrappers which
acquire tlan_priv::lock unconditionally.
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Samuel Chessman <chessman@tux.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: netdev@vger.kernel.org
---
drivers/net/ethernet/ti/tlan.c | 98 ++++++++++++++++++++--------------
1 file changed, 57 insertions(+), 41 deletions(-)
diff --git a/drivers/net/ethernet/ti/tlan.c b/drivers/net/ethernet/ti/tlan.c
index 267c080ee084b..0b2ce4bdc2c3d 100644
--- a/drivers/net/ethernet/ti/tlan.c
+++ b/drivers/net/ethernet/ti/tlan.c
@@ -186,6 +186,7 @@ static void tlan_reset_adapter(struct net_device *);
static void tlan_finish_reset(struct net_device *);
static void tlan_set_mac(struct net_device *, int areg, char *mac);
+static void __tlan_phy_print(struct net_device *);
static void tlan_phy_print(struct net_device *);
static void tlan_phy_detect(struct net_device *);
static void tlan_phy_power_down(struct net_device *);
@@ -201,9 +202,11 @@ static void tlan_phy_finish_auto_neg(struct net_device *);
static int tlan_phy_dp83840a_check(struct net_device *);
*/
-static bool tlan_mii_read_reg(struct net_device *, u16, u16, u16 *);
+static bool __tlan_mii_read_reg(struct net_device *, u16, u16, u16 *);
+static void tlan_mii_read_reg(struct net_device *, u16, u16, u16 *);
static void tlan_mii_send_data(u16, u32, unsigned);
static void tlan_mii_sync(u16);
+static void __tlan_mii_write_reg(struct net_device *, u16, u16, u16);
static void tlan_mii_write_reg(struct net_device *, u16, u16, u16);
static void tlan_ee_send_start(u16);
@@ -242,23 +245,20 @@ static u32
tlan_handle_rx_eoc
};
-static inline void
+static void
tlan_set_timer(struct net_device *dev, u32 ticks, u32 type)
{
struct tlan_priv *priv = netdev_priv(dev);
unsigned long flags = 0;
- if (!in_irq())
- spin_lock_irqsave(&priv->lock, flags);
+ spin_lock_irqsave(&priv->lock, flags);
if (priv->timer.function != NULL &&
priv->timer_type != TLAN_TIMER_ACTIVITY) {
- if (!in_irq())
- spin_unlock_irqrestore(&priv->lock, flags);
+ spin_unlock_irqrestore(&priv->lock, flags);
return;
}
priv->timer.function = tlan_timer;
- if (!in_irq())
- spin_unlock_irqrestore(&priv->lock, flags);
+ spin_unlock_irqrestore(&priv->lock, flags);
priv->timer_set_at = jiffies;
priv->timer_type = type;
@@ -1703,22 +1703,22 @@ static u32 tlan_handle_status_check(struct net_device *dev, u16 host_int)
dev->name, (unsigned) net_sts);
}
if ((net_sts & TLAN_NET_STS_MIRQ) && (priv->phy_num == 0)) {
- tlan_mii_read_reg(dev, phy, TLAN_TLPHY_STS, &tlphy_sts);
- tlan_mii_read_reg(dev, phy, TLAN_TLPHY_CTL, &tlphy_ctl);
+ __tlan_mii_read_reg(dev, phy, TLAN_TLPHY_STS, &tlphy_sts);
+ __tlan_mii_read_reg(dev, phy, TLAN_TLPHY_CTL, &tlphy_ctl);
if (!(tlphy_sts & TLAN_TS_POLOK) &&
!(tlphy_ctl & TLAN_TC_SWAPOL)) {
tlphy_ctl |= TLAN_TC_SWAPOL;
- tlan_mii_write_reg(dev, phy, TLAN_TLPHY_CTL,
- tlphy_ctl);
+ __tlan_mii_write_reg(dev, phy, TLAN_TLPHY_CTL,
+ tlphy_ctl);
} else if ((tlphy_sts & TLAN_TS_POLOK) &&
(tlphy_ctl & TLAN_TC_SWAPOL)) {
tlphy_ctl &= ~TLAN_TC_SWAPOL;
- tlan_mii_write_reg(dev, phy, TLAN_TLPHY_CTL,
- tlphy_ctl);
+ __tlan_mii_write_reg(dev, phy, TLAN_TLPHY_CTL,
+ tlphy_ctl);
}
if (debug)
- tlan_phy_print(dev);
+ __tlan_phy_print(dev);
}
}
@@ -2379,7 +2379,7 @@ ThunderLAN driver PHY layer routines
/*********************************************************************
- * tlan_phy_print
+ * __tlan_phy_print
*
* Returns:
* Nothing
@@ -2391,11 +2391,13 @@ ThunderLAN driver PHY layer routines
*
********************************************************************/
-static void tlan_phy_print(struct net_device *dev)
+static void __tlan_phy_print(struct net_device *dev)
{
struct tlan_priv *priv = netdev_priv(dev);
u16 i, data0, data1, data2, data3, phy;
+ lockdep_assert_held(&priv->lock);
+
phy = priv->phy[priv->phy_num];
if (priv->adapter->flags & TLAN_ADAPTER_UNMANAGED_PHY) {
@@ -2404,10 +2406,10 @@ static void tlan_phy_print(struct net_device *dev)
netdev_info(dev, "PHY 0x%02x\n", phy);
pr_info(" Off. +0 +1 +2 +3\n");
for (i = 0; i < 0x20; i += 4) {
- tlan_mii_read_reg(dev, phy, i, &data0);
- tlan_mii_read_reg(dev, phy, i + 1, &data1);
- tlan_mii_read_reg(dev, phy, i + 2, &data2);
- tlan_mii_read_reg(dev, phy, i + 3, &data3);
+ __tlan_mii_read_reg(dev, phy, i, &data0);
+ __tlan_mii_read_reg(dev, phy, i + 1, &data1);
+ __tlan_mii_read_reg(dev, phy, i + 2, &data2);
+ __tlan_mii_read_reg(dev, phy, i + 3, &data3);
pr_info(" 0x%02x 0x%04hx 0x%04hx 0x%04hx 0x%04hx\n",
i, data0, data1, data2, data3);
}
@@ -2417,7 +2419,15 @@ static void tlan_phy_print(struct net_device *dev)
}
+static void tlan_phy_print(struct net_device *dev)
+{
+ struct tlan_priv *priv = netdev_priv(dev);
+ unsigned long flags;
+ spin_lock_irqsave(&priv->lock, flags);
+ __tlan_phy_print(dev);
+ spin_unlock_irqrestore(&priv->lock, flags);
+}
/*********************************************************************
@@ -2795,7 +2805,7 @@ these routines are based on the information in chap. 2 of the
/***************************************************************
- * tlan_mii_read_reg
+ * __tlan_mii_read_reg
*
* Returns:
* false if ack received ok
@@ -2819,7 +2829,7 @@ these routines are based on the information in chap. 2 of the
**************************************************************/
static bool
-tlan_mii_read_reg(struct net_device *dev, u16 phy, u16 reg, u16 *val)
+__tlan_mii_read_reg(struct net_device *dev, u16 phy, u16 reg, u16 *val)
{
u8 nack;
u16 sio, tmp;
@@ -2827,15 +2837,13 @@ tlan_mii_read_reg(struct net_device *dev, u16 phy, u16 reg, u16 *val)
bool err;
int minten;
struct tlan_priv *priv = netdev_priv(dev);
- unsigned long flags = 0;
+
+ lockdep_assert_held(&priv->lock);
err = false;
outw(TLAN_NET_SIO, dev->base_addr + TLAN_DIO_ADR);
sio = dev->base_addr + TLAN_DIO_DATA + TLAN_NET_SIO;
- if (!in_irq())
- spin_lock_irqsave(&priv->lock, flags);
-
tlan_mii_sync(dev->base_addr);
minten = tlan_get_bit(TLAN_NET_SIO_MINTEN, sio);
@@ -2881,15 +2889,19 @@ tlan_mii_read_reg(struct net_device *dev, u16 phy, u16 reg, u16 *val)
*val = tmp;
- if (!in_irq())
- spin_unlock_irqrestore(&priv->lock, flags);
-
return err;
-
}
+static void tlan_mii_read_reg(struct net_device *dev, u16 phy, u16 reg,
+ u16 *val)
+{
+ struct tlan_priv *priv = netdev_priv(dev);
+ unsigned long flags;
-
+ spin_lock_irqsave(&priv->lock, flags);
+ __tlan_mii_read_reg(dev, phy, reg, val);
+ spin_unlock_irqrestore(&priv->lock, flags);
+}
/***************************************************************
* tlan_mii_send_data
@@ -2971,7 +2983,7 @@ static void tlan_mii_sync(u16 base_port)
/***************************************************************
- * tlan_mii_write_reg
+ * __tlan_mii_write_reg
*
* Returns:
* Nothing
@@ -2991,19 +3003,17 @@ static void tlan_mii_sync(u16 base_port)
**************************************************************/
static void
-tlan_mii_write_reg(struct net_device *dev, u16 phy, u16 reg, u16 val)
+__tlan_mii_write_reg(struct net_device *dev, u16 phy, u16 reg, u16 val)
{
u16 sio;
int minten;
- unsigned long flags = 0;
struct tlan_priv *priv = netdev_priv(dev);
+ lockdep_assert_held(&priv->lock);
+
outw(TLAN_NET_SIO, dev->base_addr + TLAN_DIO_ADR);
sio = dev->base_addr + TLAN_DIO_DATA + TLAN_NET_SIO;
- if (!in_irq())
- spin_lock_irqsave(&priv->lock, flags);
-
tlan_mii_sync(dev->base_addr);
minten = tlan_get_bit(TLAN_NET_SIO_MINTEN, sio);
@@ -3024,12 +3034,18 @@ tlan_mii_write_reg(struct net_device *dev, u16 phy, u16 reg, u16 val)
if (minten)
tlan_set_bit(TLAN_NET_SIO_MINTEN, sio);
- if (!in_irq())
- spin_unlock_irqrestore(&priv->lock, flags);
-
}
+static void
+tlan_mii_write_reg(struct net_device *dev, u16 phy, u16 reg, u16 val)
+{
+ struct tlan_priv *priv = netdev_priv(dev);
+ unsigned long flags;
+ spin_lock_irqsave(&priv->lock, flags);
+ __tlan_mii_write_reg(dev, phy, reg, val);
+ spin_unlock_irqrestore(&priv->lock, flags);
+}
/*****************************************************************************
--
2.28.0
^ permalink raw reply related
* [PATCH net-next 02/15] net: neterion: s2io: Replace in_interrupt() for context detection
From: Sebastian Andrzej Siewior @ 2020-10-27 22:54 UTC (permalink / raw)
To: netdev
Cc: Aymen Sghaier, Madalin Bucur, Sebastian Andrzej Siewior,
Zhu Yanjun, Samuel Chessman, Ping-Ke Shih, Herbert Xu,
Horia Geantă, linux-rdma, Rain River, Kalle Valo,
Ulrich Kunitz, Jouni Malinen, Daniel Drake, Jakub Kicinski,
Thomas Gleixner, linux-arm-kernel, Leon Romanovsky, linuxppc-dev,
linux-wireless, Li Yang, linux-crypto, Jon Mason, Saeed Mahameed,
David S. Miller
In-Reply-To: <20201027225454.3492351-1-bigeasy@linutronix.de>
wait_for_cmd_complete() uses in_interrupt() to detect whether it is safe to
sleep or not.
The usage of in_interrupt() in drivers is phased out and Linus clearly
requested that code which changes behaviour depending on context should
either be seperated or the context be conveyed in an argument passed by the
caller, which usually knows the context.
in_interrupt() also is only partially correct because it fails to chose the
correct code path when just preemption or interrupts are disabled.
Add an argument 'may_block' to both functions and adjust the callers to
pass the context information.
The following call chains which end up invoking wait_for_cmd_complete()
were analyzed to be safe to sleep:
s2io_card_up()
s2io_set_multicast()
init_nic()
init_tti()
s2io_close()
do_s2io_delete_unicast_mc()
do_s2io_add_mac()
s2io_set_mac_addr()
do_s2io_prog_unicast()
do_s2io_add_mac()
s2io_reset()
do_s2io_restore_unicast_mc()
do_s2io_add_mc()
do_s2io_add_mac()
s2io_open()
do_s2io_prog_unicast()
do_s2io_add_mac()
The following call chains which end up invoking wait_for_cmd_complete()
were analyzed to be safe to sleep:
__dev_set_rx_mode()
s2io_set_multicast()
s2io_txpic_intr_handle()
s2io_link()
init_tti()
Add a may_sleep argument to wait_for_cmd_complete(), s2io_set_multicast()
and init_tti() and hand the context information in from the call sites.
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Jon Mason <jdmason@kudzu.us>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: netdev@vger.kernel.org
---
drivers/net/ethernet/neterion/s2io.c | 41 ++++++++++++++++------------
drivers/net/ethernet/neterion/s2io.h | 4 +--
2 files changed, 25 insertions(+), 20 deletions(-)
diff --git a/drivers/net/ethernet/neterion/s2io.c b/drivers/net/ethernet/neterion/s2io.c
index d13d92bf74478..8f2f091bce899 100644
--- a/drivers/net/ethernet/neterion/s2io.c
+++ b/drivers/net/ethernet/neterion/s2io.c
@@ -1106,7 +1106,7 @@ static int s2io_print_pci_mode(struct s2io_nic *nic)
* '-1' on failure
*/
-static int init_tti(struct s2io_nic *nic, int link)
+static int init_tti(struct s2io_nic *nic, int link, bool may_sleep)
{
struct XENA_dev_config __iomem *bar0 = nic->bar0;
register u64 val64 = 0;
@@ -1166,7 +1166,7 @@ static int init_tti(struct s2io_nic *nic, int link)
if (wait_for_cmd_complete(&bar0->tti_command_mem,
TTI_CMD_MEM_STROBE_NEW_CMD,
- S2IO_BIT_RESET) != SUCCESS)
+ S2IO_BIT_RESET, may_sleep) != SUCCESS)
return FAILURE;
}
@@ -1659,7 +1659,7 @@ static int init_nic(struct s2io_nic *nic)
*/
/* Initialize TTI */
- if (SUCCESS != init_tti(nic, nic->last_link_state))
+ if (SUCCESS != init_tti(nic, nic->last_link_state, true))
return -ENODEV;
/* RTI Initialization */
@@ -3331,7 +3331,7 @@ static void s2io_updt_xpak_counter(struct net_device *dev)
*/
static int wait_for_cmd_complete(void __iomem *addr, u64 busy_bit,
- int bit_state)
+ int bit_state, bool may_sleep)
{
int ret = FAILURE, cnt = 0, delay = 1;
u64 val64;
@@ -3353,7 +3353,7 @@ static int wait_for_cmd_complete(void __iomem *addr, u64 busy_bit,
}
}
- if (in_interrupt())
+ if (!may_sleep)
mdelay(delay);
else
msleep(delay);
@@ -4877,8 +4877,7 @@ static struct net_device_stats *s2io_get_stats(struct net_device *dev)
* Return value:
* void.
*/
-
-static void s2io_set_multicast(struct net_device *dev)
+static void s2io_set_multicast(struct net_device *dev, bool may_sleep)
{
int i, j, prev_cnt;
struct netdev_hw_addr *ha;
@@ -4903,7 +4902,7 @@ static void s2io_set_multicast(struct net_device *dev)
/* Wait till command completes */
wait_for_cmd_complete(&bar0->rmac_addr_cmd_mem,
RMAC_ADDR_CMD_MEM_STROBE_CMD_EXECUTING,
- S2IO_BIT_RESET);
+ S2IO_BIT_RESET, may_sleep);
sp->m_cast_flg = 1;
sp->all_multi_pos = config->max_mc_addr - 1;
@@ -4920,7 +4919,7 @@ static void s2io_set_multicast(struct net_device *dev)
/* Wait till command completes */
wait_for_cmd_complete(&bar0->rmac_addr_cmd_mem,
RMAC_ADDR_CMD_MEM_STROBE_CMD_EXECUTING,
- S2IO_BIT_RESET);
+ S2IO_BIT_RESET, may_sleep);
sp->m_cast_flg = 0;
sp->all_multi_pos = 0;
@@ -5000,7 +4999,7 @@ static void s2io_set_multicast(struct net_device *dev)
/* Wait for command completes */
if (wait_for_cmd_complete(&bar0->rmac_addr_cmd_mem,
RMAC_ADDR_CMD_MEM_STROBE_CMD_EXECUTING,
- S2IO_BIT_RESET)) {
+ S2IO_BIT_RESET, may_sleep)) {
DBG_PRINT(ERR_DBG,
"%s: Adding Multicasts failed\n",
dev->name);
@@ -5030,7 +5029,7 @@ static void s2io_set_multicast(struct net_device *dev)
/* Wait for command completes */
if (wait_for_cmd_complete(&bar0->rmac_addr_cmd_mem,
RMAC_ADDR_CMD_MEM_STROBE_CMD_EXECUTING,
- S2IO_BIT_RESET)) {
+ S2IO_BIT_RESET, may_sleep)) {
DBG_PRINT(ERR_DBG,
"%s: Adding Multicasts failed\n",
dev->name);
@@ -5041,6 +5040,12 @@ static void s2io_set_multicast(struct net_device *dev)
}
}
+/* NDO wrapper for s2io_set_multicast */
+static void s2io_ndo_set_multicast(struct net_device *dev)
+{
+ s2io_set_multicast(dev, false);
+}
+
/* read from CAM unicast & multicast addresses and store it in
* def_mac_addr structure
*/
@@ -5127,7 +5132,7 @@ static int do_s2io_add_mac(struct s2io_nic *sp, u64 addr, int off)
/* Wait till command completes */
if (wait_for_cmd_complete(&bar0->rmac_addr_cmd_mem,
RMAC_ADDR_CMD_MEM_STROBE_CMD_EXECUTING,
- S2IO_BIT_RESET)) {
+ S2IO_BIT_RESET, true)) {
DBG_PRINT(INFO_DBG, "do_s2io_add_mac failed\n");
return FAILURE;
}
@@ -5171,7 +5176,7 @@ static u64 do_s2io_read_unicast_mc(struct s2io_nic *sp, int offset)
/* Wait till command completes */
if (wait_for_cmd_complete(&bar0->rmac_addr_cmd_mem,
RMAC_ADDR_CMD_MEM_STROBE_CMD_EXECUTING,
- S2IO_BIT_RESET)) {
+ S2IO_BIT_RESET, true)) {
DBG_PRINT(INFO_DBG, "do_s2io_read_unicast_mc failed\n");
return FAILURE;
}
@@ -7141,7 +7146,7 @@ static int s2io_card_up(struct s2io_nic *sp)
}
/* Setting its receive mode */
- s2io_set_multicast(dev);
+ s2io_set_multicast(dev, true);
if (dev->features & NETIF_F_LRO) {
/* Initialize max aggregatable pkts per session based on MTU */
@@ -7447,7 +7452,7 @@ static void s2io_link(struct s2io_nic *sp, int link)
struct swStat *swstats = &sp->mac_control.stats_info->sw_stat;
if (link != sp->last_link_state) {
- init_tti(sp, link);
+ init_tti(sp, link, false);
if (link == LINK_DOWN) {
DBG_PRINT(ERR_DBG, "%s: Link down\n", dev->name);
s2io_stop_all_tx_queue(sp);
@@ -7604,7 +7609,7 @@ static int rts_ds_steer(struct s2io_nic *nic, u8 ds_codepoint, u8 ring)
return wait_for_cmd_complete(&bar0->rts_ds_mem_ctrl,
RTS_DS_MEM_CTRL_STROBE_CMD_BEING_EXECUTED,
- S2IO_BIT_RESET);
+ S2IO_BIT_RESET, true);
}
static const struct net_device_ops s2io_netdev_ops = {
@@ -7613,7 +7618,7 @@ static const struct net_device_ops s2io_netdev_ops = {
.ndo_get_stats = s2io_get_stats,
.ndo_start_xmit = s2io_xmit,
.ndo_validate_addr = eth_validate_addr,
- .ndo_set_rx_mode = s2io_set_multicast,
+ .ndo_set_rx_mode = s2io_ndo_set_multicast,
.ndo_do_ioctl = s2io_ioctl,
.ndo_set_mac_address = s2io_set_mac_addr,
.ndo_change_mtu = s2io_change_mtu,
@@ -7929,7 +7934,7 @@ s2io_init_nic(struct pci_dev *pdev, const struct pci_device_id *pre)
writeq(val64, &bar0->rmac_addr_cmd_mem);
wait_for_cmd_complete(&bar0->rmac_addr_cmd_mem,
RMAC_ADDR_CMD_MEM_STROBE_CMD_EXECUTING,
- S2IO_BIT_RESET);
+ S2IO_BIT_RESET, true);
tmp64 = readq(&bar0->rmac_addr_data0_mem);
mac_down = (u32)tmp64;
mac_up = (u32) (tmp64 >> 32);
diff --git a/drivers/net/ethernet/neterion/s2io.h b/drivers/net/ethernet/neterion/s2io.h
index 6fa3159a977fd..5a6032212c19d 100644
--- a/drivers/net/ethernet/neterion/s2io.h
+++ b/drivers/net/ethernet/neterion/s2io.h
@@ -1066,7 +1066,7 @@ static void tx_intr_handler(struct fifo_info *fifo_data);
static void s2io_handle_errors(void * dev_id);
static void s2io_tx_watchdog(struct net_device *dev, unsigned int txqueue);
-static void s2io_set_multicast(struct net_device *dev);
+static void s2io_set_multicast(struct net_device *dev, bool may_sleep);
static int rx_osm_handler(struct ring_info *ring_data, struct RxD_t * rxdp);
static void s2io_link(struct s2io_nic * sp, int link);
static void s2io_reset(struct s2io_nic * sp);
@@ -1087,7 +1087,7 @@ static int s2io_set_swapper(struct s2io_nic * sp);
static void s2io_card_down(struct s2io_nic *nic);
static int s2io_card_up(struct s2io_nic *nic);
static int wait_for_cmd_complete(void __iomem *addr, u64 busy_bit,
- int bit_state);
+ int bit_state, bool may_sleep);
static int s2io_add_isr(struct s2io_nic * sp);
static void s2io_rem_isr(struct s2io_nic * sp);
--
2.28.0
^ permalink raw reply related
* [PATCH net-next 01/15] net: orinoco: Remove BUG_ON(in_interrupt/irq())
From: Sebastian Andrzej Siewior @ 2020-10-27 22:54 UTC (permalink / raw)
To: netdev
Cc: Aymen Sghaier, Madalin Bucur, Sebastian Andrzej Siewior,
Zhu Yanjun, Samuel Chessman, Ping-Ke Shih, Herbert Xu,
Horia Geantă, linux-rdma, Rain River, Kalle Valo,
Ulrich Kunitz, Jouni Malinen, Daniel Drake, Jakub Kicinski,
Thomas Gleixner, linux-arm-kernel, Leon Romanovsky, linuxppc-dev,
linux-wireless, Li Yang, linux-crypto, Jon Mason, Saeed Mahameed,
David S. Miller
In-Reply-To: <20201027225454.3492351-1-bigeasy@linutronix.de>
The usage of in_irq()/in_interrupt() in drivers is phased out and the
BUG_ON()'s based on those are not covering all contexts in which these
functions cannot be called.
Aside of that BUG_ON() should only be used as last resort, which is clearly
not the case here.
A broad variety of checks in the invoked functions (always enabled or debug
option dependent) cover these conditions already, so the BUG_ON()'s do not
really provide additional value.
Just remove them.
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Kalle Valo <kvalo@codeaurora.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: linux-wireless@vger.kernel.org
Cc: netdev@vger.kernel.org
---
drivers/net/wireless/intersil/orinoco/orinoco_usb.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/net/wireless/intersil/orinoco/orinoco_usb.c b/drivers/net/wireless/intersil/orinoco/orinoco_usb.c
index b849d27bd741e..046f2453ad5d9 100644
--- a/drivers/net/wireless/intersil/orinoco/orinoco_usb.c
+++ b/drivers/net/wireless/intersil/orinoco/orinoco_usb.c
@@ -859,8 +859,6 @@ static int ezusb_access_ltv(struct ezusb_priv *upriv,
int retval = 0;
enum ezusb_state state;
- BUG_ON(in_irq());
-
if (!upriv->udev) {
retval = -ENODEV;
goto exit;
@@ -1349,7 +1347,6 @@ static int ezusb_init(struct hermes *hw)
struct ezusb_priv *upriv = hw->priv;
int retval;
- BUG_ON(in_interrupt());
if (!upriv)
return -EINVAL;
@@ -1448,7 +1445,6 @@ static inline void ezusb_delete(struct ezusb_priv *upriv)
struct list_head *tmp_item;
unsigned long flags;
- BUG_ON(in_interrupt());
BUG_ON(!upriv);
mutex_lock(&upriv->mtx);
--
2.28.0
^ permalink raw reply related
* [PATCH net-next 00/15] in_interrupt() cleanup, part 2
From: Sebastian Andrzej Siewior @ 2020-10-27 22:54 UTC (permalink / raw)
To: netdev
Cc: Aymen Sghaier, Madalin Bucur, Sebastian Andrzej Siewior,
Zhu Yanjun, Samuel Chessman, Ping-Ke Shih, Herbert Xu,
Horia Geantă, linux-rdma, Rain River, Kalle Valo,
Ulrich Kunitz, Jouni Malinen, Daniel Drake, Jakub Kicinski,
Thomas Gleixner, linux-arm-kernel, Leon Romanovsky, linuxppc-dev,
linux-wireless, Li Yang, linux-crypto, Jon Mason, Saeed Mahameed,
David S. Miller
Folks,
in the discussion about preempt count consistency across kernel configurations:
https://lore.kernel.org/r/20200914204209.256266093@linutronix.de/
Linus clearly requested that code in drivers and libraries which changes
behaviour based on execution context should either be split up so that
e.g. task context invocations and BH invocations have different interfaces
or if that's not possible the context information has to be provided by the
caller which knows in which context it is executing.
This includes conditional locking, allocation mode (GFP_*) decisions and
avoidance of code paths which might sleep.
In the long run, usage of 'preemptible, in_*irq etc.' should be banned from
driver code completely.
This is part two addressing remaining drivers except for orinoco-usb.
Sebastian
^ permalink raw reply
* Re: [PATCH 2/4] PM: hibernate: improve robustness of mapping pages in the direct map
From: Edgecombe, Rick P @ 2020-10-27 22:44 UTC (permalink / raw)
To: rppt@kernel.org
Cc: david@redhat.com, peterz@infradead.org, catalin.marinas@arm.com,
dave.hansen@linux.intel.com, linux-mm@kvack.org, paulus@samba.org,
pavel@ucw.cz, hpa@zytor.com, sparclinux@vger.kernel.org,
cl@linux.com, will@kernel.org, linux-riscv@lists.infradead.org,
linux-s390@vger.kernel.org, x86@kernel.org, rppt@linux.ibm.com,
borntraeger@de.ibm.com, mingo@redhat.com, rientjes@google.com,
Brown, Len, aou@eecs.berkeley.edu, gor@linux.ibm.com,
linux-pm@vger.kernel.org, hca@linux.ibm.com, bp@alien8.de,
luto@kernel.org, paul.walmsley@sifive.com, kirill@shutemov.name,
tglx@linutronix.de, iamjoonsoo.kim@lge.com,
linux-arm-kernel@lists.infradead.org, rjw@rjwysocki.net,
linux-kernel@vger.kernel.org, penberg@kernel.org,
palmer@dabbelt.com, akpm@linux-foundation.org,
linuxppc-dev@lists.ozlabs.org, davem@davemloft.net
In-Reply-To: <20201027084902.GH1154158@kernel.org>
On Tue, 2020-10-27 at 10:49 +0200, Mike Rapoport wrote:
> On Mon, Oct 26, 2020 at 06:57:32PM +0000, Edgecombe, Rick P wrote:
> > On Mon, 2020-10-26 at 11:15 +0200, Mike Rapoport wrote:
> > > On Mon, Oct 26, 2020 at 12:38:32AM +0000, Edgecombe, Rick P
> > > wrote:
> > > > On Sun, 2020-10-25 at 12:15 +0200, Mike Rapoport wrote:
> > > > > From: Mike Rapoport <rppt@linux.ibm.com>
> > > > >
> > > > > When DEBUG_PAGEALLOC or ARCH_HAS_SET_DIRECT_MAP is enabled a
> > > > > page
> > > > > may
> > > > > be
> > > > > not present in the direct map and has to be explicitly mapped
> > > > > before
> > > > > it
> > > > > could be copied.
> > > > >
> > > > > On arm64 it is possible that a page would be removed from the
> > > > > direct
> > > > > map
> > > > > using set_direct_map_invalid_noflush() but
> > > > > __kernel_map_pages()
> > > > > will
> > > > > refuse
> > > > > to map this page back if DEBUG_PAGEALLOC is disabled.
> > > >
> > > > It looks to me that arm64 __kernel_map_pages() will still
> > > > attempt
> > > > to
> > > > map it if rodata_full is true, how does this happen?
> > >
> > > Unless I misread the code, arm64 requires both rodata_full and
> > > debug_pagealloc_enabled() to be true for __kernel_map_pages() to
> > > do
> > > anything.
> > > But rodata_full condition applies to set_direct_map_*_noflush()
> > > as
> > > well,
> > > so with !rodata_full the linear map won't be ever changed.
> >
> > Hmm, looks to me that __kernel_map_pages() will only skip it if
> > both
> > debug pagealloc and rodata_full are false.
> >
> > But now I'm wondering if maybe we could simplify things by just
> > moving
> > the hibernate unmapped page logic off of the direct map. On x86,
> > text_poke() used to use this reserved fixmap pte thing that it
> > could
> > rely on to remap memory with. If hibernate had some separate pte
> > for
> > remapping like that, then we could not have any direct map
> > restrictions
> > caused by it/kernel_map_pages(), and it wouldn't have to worry
> > about
> > relying on anything else.
>
> Well, there is map_kernel_range() that can be used by hibernation as
> there is no requirement for particular virtual address, but that
> would
> be quite costly if done for every page.
>
> Maybe we can do somthing like
>
> if (kernel_page_present(s_page)) {
> do_copy_page(dst, page_address(s_page));
> } else {
> map_kernel_range_noflush(page_address(page), PAGE_SIZE,
> PROT_READ, &page);
> do_copy_page(dst, page_address(s_page));
> unmap_kernel_range_noflush(page_address(page),
> PAGE_SIZE);
> }
>
> But it seems that a prerequisite for changing the way a page is
> mapped
> in safe_copy_page() would be to teach hibernation that a mapping here
> may fail.
>
Yea that is what I meant, the direct map could still be used for mapped
pages.
But for the unmapped case it could have a pre-setup 4k pte for some non
direct map address. Then just change the pte to point to any unmapped
direct map page that was encountered. The point would be to give
hibernate some 4k pte of its own to manipulate so that it can't fail.
Yet another option would be have hibernate_map_page() just map large
pages if it finds them.
So we could teach hibernate to handle mapping failures, OR we could
change it so it doesn't rely on direct map page sizes in order to
succeed. The latter seems better to me since there isn't a reason why
it should have to fail and the resulting logic might be simpler. Both
seem like improvements in robustness though.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox