[PATCH] virtio_net: Warn if insufficient queue length for transmitting
The transmit queue is stopped when the number of free queue entries is less
than 2+MAX_SKB_FRAGS, in start_xmit(). If the queue length (QUEUE_NUM_MAX)
is less than then this, transmission will immediately trigger a netdev
watchdog timeout. Report this condition earlier and more directly.
Signed-off-by: Darius Rad
---
drivers/net/virtio_net.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 115c3c5414f2..72ee8473b61c 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -4917,6 +4917,9 @@ static int virtnet_probe(struct virtio_device *vdev)
set_bit(guest_offloads[i], &vi->guest_offloads);
vi->guest_offloads_capable = vi->guest_offloads;
+ if (virtqueue_get_vring_size(vi->sq->vq) < 2 + MAX_SKB_FRAGS)
+ netdev_warn_once(dev, "not enough queue entries, expect xmit
timeout\n");
+
pr_debug("virtnet: registered device %s with %d RX and TX vq's\n",
dev->name, max_queue_pairs);
--
2.39.2
[PATCH] include/linux/atomic.h: include asm-generic/atomic-long.h after asm-generic/atomic64.h
Move the include of asm-generic/atomic-long.h to after the conditional include of asm-generic/atomic64.h. This is necessary if CONFIG_GENERIC_ATOMIC64 is y and BITS_PER_LONG == 64, because atomic-long.h uses functions declared in atomic64.h. Signed-off-by: Darius Rad --- It does not appear that this is relevant to architectures that are in the kernel tree (i.e., no architectures use GENERIC_ATOMIC64 on 64-bit). It is relevant, however, to certain combinations of options for the RISC-V architecture currently in development. Patch generated against 3.17-rc1. include/linux/atomic.h |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) --- linux-3.17-rc1.orig/include/linux/atomic.h 2014-08-16 12:40:26.0 -0400 +++ linux-3.17-rc1/include/linux/atomic.h 2014-08-22 16:07:55.297515936 -0400 @@ -160,8 +160,8 @@ static inline void atomic_or(int i, atom } #endif /* #ifndef CONFIG_ARCH_HAS_ATOMIC_OR */ -#include #ifdef CONFIG_GENERIC_ATOMIC64 #include #endif +#include #endif /* _LINUX_ATOMIC_H */ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] include/linux/types.h: avoid duplicate definition of atomic64_t when 64BIT && GENERIC_ATOMIC64
If CONFIG_64BIT and CONFIG_GENERIC_ATOMIC64 are both y, atomic64_t is
defined in both include/linux/types.h and include/asm-generic/atomic64.h.
Allow the definition in include/asm-generic/atomic64.h to prevail, since
that definition is consistent with the rest of CONFIG_GENERIC_ATOMIC64.
Signed-off-by: Darius Rad
---
It does not appear that this is relevant to architectures that are in the
kernel tree (i.e., no architectures use GENERIC_ATOMIC64 on 64-bit). It
is relevant, however, to certain combinations of options for the RISC-V
architecture currently in development.
Patch generated against 3.17-rc1.
include/linux/types.h |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- linux-3.17-rc1.orig/include/linux/types.h 2014-08-16 12:40:26.0
-0400
+++ linux-3.17-rc1/include/linux/types.h2014-08-22 14:27:03.012782185
-0400
@@ -177,7 +177,7 @@ typedef struct {
int counter;
} atomic_t;
-#ifdef CONFIG_64BIT
+#if defined(CONFIG_64BIT) && !defined(CONFIG_GENERIC_ATOMIC64)
typedef struct {
long counter;
} atomic64_t;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] include/asm-generic/cmpxchg-local.h: perform comparison in cmpxchg using appropriate size of data
In the generic implementation of cmpxchg, cast the parameters to the size
of the data prior to comparison. Otherwise, it is possible for the
comparison to be done incorrectly in the case where the data size is
smaller than the architecture register size.
For example, on a 64-bit architecture, a 32-bit value is sign extended
differently if it is typecast from an int to an unsigned long as compared
to being loaded from memory via an unsigned pointer (u32 *). Given that
the primary, or possibly only, use of cmpxchg is with 4 and 8 byte values,
the incorrect comparison could only occur on 64-bit architectures that make
use of the generic cmpxchg.
Signed-off-by: Darius Rad
---
It does not appear that this is relevant to architectures that are in the
kernel tree, since the generic cmpxchg is only ever used by some 32-bit
architectures. This does impact the RISC-V architecture that is currently
in development.
Patch generated against 3.17-rc1.
include/asm-generic/cmpxchg-local.h |8
1 file changed, 4 insertions(+), 4 deletions(-)
--- linux-3.17-rc1.orig/include/asm-generic/cmpxchg-local.h 2014-08-16
12:40:26.0 -0400
+++ linux-3.17-rc1/include/asm-generic/cmpxchg-local.h 2014-08-22
14:26:59.280746090 -0400
@@ -25,19 +25,19 @@ static inline unsigned long __cmpxchg_lo
raw_local_irq_save(flags);
switch (size) {
case 1: prev = *(u8 *)ptr;
- if (prev == old)
+ if ((u8)prev == (u8)old)
*(u8 *)ptr = (u8)new;
break;
case 2: prev = *(u16 *)ptr;
- if (prev == old)
+ if ((u16)prev == (u16)old)
*(u16 *)ptr = (u16)new;
break;
case 4: prev = *(u32 *)ptr;
- if (prev == old)
+ if ((u32)prev == (u32)old)
*(u32 *)ptr = (u32)new;
break;
case 8: prev = *(u64 *)ptr;
- if (prev == old)
+ if ((u64)prev == (u64)old)
*(u64 *)ptr = (u64)new;
break;
default:
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] include/asm-generic/cmpxchg-local.h: perform comparison in cmpxchg using appropriate size of data
On 08/26/2014 08:33 AM, Arnd Bergmann wrote:
> On Monday 25 August 2014 11:33:07 Darius Rad wrote:
>> In the generic implementation of cmpxchg, cast the parameters to the size
>> of the data prior to comparison. Otherwise, it is possible for the
>> comparison to be done incorrectly in the case where the data size is
>> smaller than the architecture register size.
>>
>> For example, on a 64-bit architecture, a 32-bit value is sign extended
>> differently if it is typecast from an int to an unsigned long as compared
>> to being loaded from memory via an unsigned pointer (u32 *).
>
> I don't see the scenario where this applies yet. The function itself
> only deals with unsigned values, so there should never be any sign extension.
>
> Is this a problem with the caller using a signed type? Does the same
> code work on architectures that don't use the generic code?
>
Yes, the problem is when the caller uses a signed type (that is also smaller
than unsigned long). For example, set_last_pid() in kernel/pid.c passes an
int.
And yes, the same code works without using the generic code. In my testing,
simply casting the u32 case comparison makes the code work, whereas
with the generic code everything in user space gets killed.
>> Given that
>> the primary, or possibly only, use of cmpxchg is with 4 and 8 byte values,
>> the incorrect comparison could only occur on 64-bit architectures that make
>> use of the generic cmxchg.
>
> cmpxchg is definitely meant to handle 1 and 2 byte values as well, but it's
> relatively rare. ARMv6 for instance does not handle 2-byte atomics and only
> one driver (xen) has had problems because of this.
>
Understood. I noticed that some architectures (e.g., MIPS, Xtensa) only
implement the 4 and 8 byte varieties, which is why I made that initial
assumption.
>> Signed-off-by: Darius Rad
>>
>> ---
>> It does not appear that this is relevant to architectures that are in the
>> kernel tree, since the generic cmpxchg is only ever used by some 32-bit
>> architectures. This does impact the RISC-V architecture that is currently
>> in development.
>
> I guess that means that RISC-V has no mandatory atomic instructions, right?
>
Correct.
>>
>> include/asm-generic/cmpxchg-local.h |8
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> --- linux-3.17-rc1.orig/include/asm-generic/cmpxchg-local.h 2014-08-16
>> 12:40:26.0 -0400
>> +++ linux-3.17-rc1/include/asm-generic/cmpxchg-local.h 2014-08-22
>> 14:26:59.280746090 -0400
>> @@ -25,19 +25,19 @@ static inline unsigned long __cmpxchg_lo
>> raw_local_irq_save(flags);
>> switch (size) {
>> case 1: prev = *(u8 *)ptr;
>> -if (prev == old)
>> +if ((u8)prev == (u8)old)
>> *(u8 *)ptr = (u8)new;
>> break;
>> case 2: prev = *(u16 *)ptr;
>> -if (prev == old)
>> +if ((u16)prev == (u16)old)
>> *(u16 *)ptr = (u16)new;
>> break;
>> case 4: prev = *(u32 *)ptr;
>> -if (prev == old)
>> +if ((u32)prev == (u32)old)
>> *(u32 *)ptr = (u32)new;
>> break;
>> case 8: prev = *(u64 *)ptr;
>> -if (prev == old)
>> +if ((u64)prev == (u64)old)
>> *(u64 *)ptr = (u64)new;
>> break;
>> default:
>
> I can see how the cast of 'old' to a narrower type makes sense, but
> not 'prev', which we just loaded and zero-extended one line earlier.
>
Indeed, I agree that casting 'prev' is not necessary.
> Arnd
>
// darius
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] irqchip/sifive-plic: add irq_mask and irq_unmask
As per the existing comment, irq_mask and irq_unmask do not need
to do anything for the PLIC. However, the functions must exist
(the pointers cannot be NULL) as they are not optional, based on
the documentation (Documentation/core-api/genericirq.rst) as well
as existing usage (e.g., include/linux/irqchip/chained_irq.h).
Signed-off-by: Darius Rad
---
drivers/irqchip/irq-sifive-plic.c | 13 +
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/irqchip/irq-sifive-plic.c
b/drivers/irqchip/irq-sifive-plic.c
index cf755964f2f8..52d5169f924f 100644
--- a/drivers/irqchip/irq-sifive-plic.c
+++ b/drivers/irqchip/irq-sifive-plic.c
@@ -111,6 +111,13 @@ static void plic_irq_disable(struct irq_data *d)
plic_irq_toggle(cpu_possible_mask, d->hwirq, 0);
}
+/*
+ * There is no need to mask/unmask PLIC interrupts. They are "masked"
+ * by reading claim and "unmasked" when writing it back.
+ */
+static void plic_irq_mask(struct irq_data *d) { }
+static void plic_irq_unmask(struct irq_data *d) { }
+
#ifdef CONFIG_SMP
static int plic_set_affinity(struct irq_data *d,
const struct cpumask *mask_val, bool force)
@@ -138,12 +145,10 @@ static int plic_set_affinity(struct irq_data *d,
static struct irq_chip plic_chip = {
.name = "SiFive PLIC",
- /*
-* There is no need to mask/unmask PLIC interrupts. They are "masked"
-* by reading claim and "unmasked" when writing it back.
-*/
.irq_enable = plic_irq_enable,
.irq_disable= plic_irq_disable,
+ .irq_mask = plic_irq_mask,
+ .irq_unmask = plic_irq_unmask,
#ifdef CONFIG_SMP
.irq_set_affinity = plic_set_affinity,
#endif
--
2.20.1
Re: [PATCH] irqchip/sifive-plic: add irq_mask and irq_unmask
On 9/15/19 2:20 PM, Marc Zyngier wrote:
> On Sun, 15 Sep 2019 18:31:33 +0100,
> Palmer Dabbelt wrote:
>
> Hi Palmer,
>
>>
>> On Sun, 15 Sep 2019 07:24:20 PDT (-0700), m...@kernel.org wrote:
>>> On Thu, 12 Sep 2019 22:40:34 +0100,
>>> Darius Rad wrote:
>>>
>>> Hi Darius,
>>>
>>>>
>>>> As per the existing comment, irq_mask and irq_unmask do not need
>>>> to do anything for the PLIC. However, the functions must exist
>>>> (the pointers cannot be NULL) as they are not optional, based on
>>>> the documentation (Documentation/core-api/genericirq.rst) as well
>>>> as existing usage (e.g., include/linux/irqchip/chained_irq.h).
>>>>
>>>> Signed-off-by: Darius Rad
>>>> ---
>>>> drivers/irqchip/irq-sifive-plic.c | 13 +
>>>> 1 file changed, 9 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/irqchip/irq-sifive-plic.c
>>>> b/drivers/irqchip/irq-sifive-plic.c
>>>> index cf755964f2f8..52d5169f924f 100644
>>>> --- a/drivers/irqchip/irq-sifive-plic.c
>>>> +++ b/drivers/irqchip/irq-sifive-plic.c
>>>> @@ -111,6 +111,13 @@ static void plic_irq_disable(struct irq_data *d)
>>>>plic_irq_toggle(cpu_possible_mask, d->hwirq, 0);
>>>> }
>>>> +/*
>>>> + * There is no need to mask/unmask PLIC interrupts. They are "masked"
>>>> + * by reading claim and "unmasked" when writing it back.
>>>> + */
>>>> +static void plic_irq_mask(struct irq_data *d) { }
>>>> +static void plic_irq_unmask(struct irq_data *d) { }
>>>
>>> This outlines a bigger issue. If your irqchip doesn't require
>>> mask/unmask, you're probably not using the right interrupt
>>> flow. Looking at the code, I see you're using handle_simple_irq, which
>>> is almost universally wrong.
>>>
>>> As per the description above, these interrupts should be using the
>>> fasteoi flow, which is designed for this exact behaviour (the
>>> interrupt controller knows which interrupt is in flight and doesn't
>>> require SW to do anything bar signalling the EOI).
>>>
>>> Another thing is that mask/unmask tends to be a requirement, while
>>> enable/disable tends to be optional. There is no hard line here, but
>>> the expectations are that:
>>>
>>> (a) A disabled line can drop interrupts
>>> (b) A masked line cannot drop interrupts
>>>
>>> Depending what the PLIC architecture mandates, you'll need to
>>> implement one and/or the other. Having just (a) is indicative of a HW
>>> bug, and I'm not assuming that this is the case. (b) only is pretty
>>> common, and (a)+(b) has a few adepts. My bet is that it requires (b)
>>> only.
>>>
>>>> +
>>>> #ifdef CONFIG_SMP
>>>> static int plic_set_affinity(struct irq_data *d,
>>>> const struct cpumask *mask_val, bool force)
>>>> @@ -138,12 +145,10 @@ static int plic_set_affinity(struct irq_data *d,
>>>> static struct irq_chip plic_chip = {
>>>>.name = "SiFive PLIC",
>>>> - /*
>>>> - * There is no need to mask/unmask PLIC interrupts. They are "masked"
>>>> - * by reading claim and "unmasked" when writing it back.
>>>> - */
>>>>.irq_enable = plic_irq_enable,
>>>>.irq_disable= plic_irq_disable,
>>>> + .irq_mask = plic_irq_mask,
>>>> + .irq_unmask = plic_irq_unmask,
>>>> #ifdef CONFIG_SMP
>>>>.irq_set_affinity = plic_set_affinity,
>>>> #endif
>>>
>>> Can you give the following patch a go? It brings the irq flow in line
>>> with what the HW can do. It is of course fully untested (not even
>>> compile tested...).
>>>
>>> Thanks,
>>>
>>> M.
>>>
>>> From c0ce33a992ec18f5d3bac7f70de62b1ba2b42090 Mon Sep 17 00:00:00 2001
>>> From: Marc Zyngier
>>> Date: Sun, 15 Sep 2019 15:17:45 +0100
>>> Subject: [PATCH] irqchip/sifive-plic: Switch to fasteoi flow
>>>
>>> The SiFive PLIC interrupt controller seems to have all the HW
>>> features to support the fasteoi flow, but the driver seems to be
>>> stuck in a distant past. Bring it into the 21st century.
>>
>> Thank
Re: [PATCH] irqchip/sifive-plic: add irq_mask and irq_unmask
On 9/16/19 4:51 PM, Palmer Dabbelt wrote:
> On Mon, 16 Sep 2019 12:04:56 PDT (-0700), Darius Rad wrote:
>> On 9/15/19 2:20 PM, Marc Zyngier wrote:
>>> On Sun, 15 Sep 2019 18:31:33 +0100,
>>> Palmer Dabbelt wrote:
>>>
>>> Hi Palmer,
>>>
>>>>
>>>> On Sun, 15 Sep 2019 07:24:20 PDT (-0700), m...@kernel.org wrote:
>>>>> On Thu, 12 Sep 2019 22:40:34 +0100,
>>>>> Darius Rad wrote:
>>>>>
>>>>> Hi Darius,
>>>>>
>>>>>>
>>>>>> As per the existing comment, irq_mask and irq_unmask do not need
>>>>>> to do anything for the PLIC. However, the functions must exist
>>>>>> (the pointers cannot be NULL) as they are not optional, based on
>>>>>> the documentation (Documentation/core-api/genericirq.rst) as well
>>>>>> as existing usage (e.g., include/linux/irqchip/chained_irq.h).
>>>>>>
>>>>>> Signed-off-by: Darius Rad
>>>>>> ---
>>>>>> drivers/irqchip/irq-sifive-plic.c | 13 +
>>>>>> 1 file changed, 9 insertions(+), 4 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/irqchip/irq-sifive-plic.c
>>>>>> b/drivers/irqchip/irq-sifive-plic.c
>>>>>> index cf755964f2f8..52d5169f924f 100644
>>>>>> --- a/drivers/irqchip/irq-sifive-plic.c
>>>>>> +++ b/drivers/irqchip/irq-sifive-plic.c
>>>>>> @@ -111,6 +111,13 @@ static void plic_irq_disable(struct irq_data *d)
>>>>>> plic_irq_toggle(cpu_possible_mask, d->hwirq, 0);
>>>>>> }
>>>>>> +/*
>>>>>> + * There is no need to mask/unmask PLIC interrupts. They are "masked"
>>>>>> + * by reading claim and "unmasked" when writing it back.
>>>>>> + */
>>>>>> +static void plic_irq_mask(struct irq_data *d) { }
>>>>>> +static void plic_irq_unmask(struct irq_data *d) { }
>>>>>
>>>>> This outlines a bigger issue. If your irqchip doesn't require
>>>>> mask/unmask, you're probably not using the right interrupt
>>>>> flow. Looking at the code, I see you're using handle_simple_irq, which
>>>>> is almost universally wrong.
>>>>>
>>>>> As per the description above, these interrupts should be using the
>>>>> fasteoi flow, which is designed for this exact behaviour (the
>>>>> interrupt controller knows which interrupt is in flight and doesn't
>>>>> require SW to do anything bar signalling the EOI).
>>>>>
>>>>> Another thing is that mask/unmask tends to be a requirement, while
>>>>> enable/disable tends to be optional. There is no hard line here, but
>>>>> the expectations are that:
>>>>>
>>>>> (a) A disabled line can drop interrupts
>>>>> (b) A masked line cannot drop interrupts
>>>>>
>>>>> Depending what the PLIC architecture mandates, you'll need to
>>>>> implement one and/or the other. Having just (a) is indicative of a HW
>>>>> bug, and I'm not assuming that this is the case. (b) only is pretty
>>>>> common, and (a)+(b) has a few adepts. My bet is that it requires (b)
>>>>> only.
>>>>>
>>>>>> +
>>>>>> #ifdef CONFIG_SMP
>>>>>> static int plic_set_affinity(struct irq_data *d,
>>>>>> const struct cpumask *mask_val, bool force)
>>>>>> @@ -138,12 +145,10 @@ static int plic_set_affinity(struct irq_data *d,
>>>>>> static struct irq_chip plic_chip = {
>>>>>> .name = "SiFive PLIC",
>>>>>> - /*
>>>>>> - * There is no need to mask/unmask PLIC interrupts. They are
>>>>>> "masked"
>>>>>> - * by reading claim and "unmasked" when writing it back.
>>>>>> - */
>>>>>> .irq_enable = plic_irq_enable,
>>>>>> .irq_disable = plic_irq_disable,
>>>>>> + .irq_mask = plic_irq_mask,
>>>>>> + .irq_unmask = plic_irq_unmask,
>>>>>> #ifdef CONFIG_SMP
>>>>>> .irq_set_affinity = plic_set_affinity,
>>>>>> #endif
>>>>>
>>>>>
Re: [PATCH] riscv: kbuild: drop CONFIG_RISCV_ISA_C
On 8/12/19 11:03 AM, Christoph Hellwig wrote: On Thu, Aug 08, 2019 at 02:18:53PM +0200, Charles Papon wrote: Please do not drop it. Compressed instruction extension has some specific overhead in small RISC-V FPGA softcore, especialy in the ones which can't implement the register file read in a asynchronous manner because of the FPGA technology. What are reasons to enforce RVC ? Because it it the unix platform baseline as stated in the patch. The same argument could be made for an FPU or MMU, yet there are options to disable those.
