Re: Linux 5.12-rc7

2021-04-13 Thread Michael S. Tsirkin
On Tue, Apr 13, 2021 at 03:42:24PM +0200, Eric Dumazet wrote:
> On Tue, Apr 13, 2021 at 3:38 PM Michael S. Tsirkin  wrote:
> >
> > On Tue, Apr 13, 2021 at 03:33:40PM +0200, Eric Dumazet wrote:
> > > On Tue, Apr 13, 2021 at 3:27 PM Eric Dumazet  wrote:
> > > >
> > > > On Tue, Apr 13, 2021 at 2:57 PM Michael S. Tsirkin  
> > > > wrote:
> > > > >
> > > > > On Mon, Apr 12, 2021 at 06:47:07PM +0200, Eric Dumazet wrote:
> > > > > > On Mon, Apr 12, 2021 at 6:31 PM Eric Dumazet  
> > > > > > wrote:
> > > > > > >
> > > > > > > On Mon, Apr 12, 2021 at 6:28 PM Linus Torvalds
> > > > > > >  wrote:
> > > > > > > >
> > > > > > > > On Sun, Apr 11, 2021 at 10:14 PM Guenter Roeck 
> > > > > > > >  wrote:
> > > > > > > > >
> > > > > > > > > Qemu test results:
> > > > > > > > > total: 460 pass: 459 fail: 1
> > > > > > > > > Failed tests:
> > > > > > > > > sh:rts7751r2dplus_defconfig:ata:net,virtio-net:rootfs
> > > > > > > > >
> > > > > > > > > The failure bisects to commit 0f6925b3e8da ("virtio_net: Do 
> > > > > > > > > not pull payload in
> > > > > > > > > skb->head"). It is a spurious problem - the test passes 
> > > > > > > > > roughly every other
> > > > > > > > > time. When the failure is seen, udhcpc fails to get an IP 
> > > > > > > > > address and aborts
> > > > > > > > > with SIGTERM. So far I have only seen this with the "sh" 
> > > > > > > > > architecture.
> > > > > > > >
> > > > > > > > Hmm. Let's add in some more of the people involved in that 
> > > > > > > > commit, and
> > > > > > > > also netdev.
> > > > > > > >
> > > > > > > > Nothing in there looks like it should have any interaction with
> > > > > > > > architecture, so that "it happens on sh" sounds odd, but maybe 
> > > > > > > > it's
> > > > > > > > some particular interaction with the qemu environment.
> > > > > > >
> > > > > > > Yes, maybe.
> > > > > > >
> > > > > > > I spent few hours on this, and suspect a buggy memcpy() 
> > > > > > > implementation
> > > > > > > on SH, but this was not conclusive.
> > > > > > >
> > > > > > > By pulling one extra byte, the problem goes away.
> > > > > > >
> > > > > > > Strange thing is that the udhcpc process does not go past 
> > > > > > > sendto().
> > > > > >
> > > > > > This is the patch working around the issue. Unfortunately I was not
> > > > > > able to root-cause it (I really suspect something on SH)
> > > > > >
> > > > > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > > > > > index 
> > > > > > 0824e6999e49957f7aaf7c990f6259792d42f32b..fd890a951beea03bdf24406809042666eb972655
> > > > > > 100644
> > > > > > --- a/drivers/net/virtio_net.c
> > > > > > +++ b/drivers/net/virtio_net.c
> > > > > > @@ -408,11 +408,17 @@ static struct sk_buff *page_to_skb(struct
> > > > > > virtnet_info *vi,
> > > > > >
> > > > > > /* Copy all frame if it fits skb->head, otherwise
> > > > > >  * we let virtio_net_hdr_to_skb() and GRO pull headers as 
> > > > > > needed.
> > > > > > +*
> > > > > > +* Apparently, pulling only the Ethernet Header triggers a 
> > > > > > bug
> > > > > > on qemu-system-sh4.
> > > > > > +* Since GRO aggregation really cares of IPv4/IPv6, pull 20 
> > > > > > bytes
> > > > > > +* more to work around this bug : These 20 bytes can not 
> > > > > > belong
> > > > > > +* to UDP/TCP payload.
> > > > > > +* As a bonus, this makes GRO slightly faster for IPv4 (one 
> > > > > > less copy).
> > > > > >  */
> > > > >
> > > > > Question: do we still want to do this for performance reasons?
> > > > > We also have the hdr_len coming from the device which is
> > > > > just skb_headlen on the host.
> > > >
> > > > Well, putting 20 bytes in skb->head will disable frag0 optimization.
> > > >
> > > > The change would only benefit to sh architecture :)
> > > >
> > > > About hdr_len, I suppose we could try it, with appropriate safety 
> > > > checks.
> > >
> > > I have added traces, hdr_len seems to be 0 with the qemu-system-sh4 I am 
> > > using.
> > >
> > > Have I understood you correctly ?
> > >
> > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > > index 
> > > 0824e6999e49957f7aaf7c990f6259792d42f32b..f024860f7dc260d4efbc35a3b8ffd358bd0da894
> > > 100644
> > > --- a/drivers/net/virtio_net.c
> > > +++ b/drivers/net/virtio_net.c
> > > @@ -399,9 +399,10 @@ static struct sk_buff *page_to_skb(struct 
> > > virtnet_info *vi,
> > > hdr_padded_len = sizeof(struct padded_vnet_hdr);
> > >
> > > /* hdr_valid means no XDP, so we can copy the vnet header */
> > > -   if (hdr_valid)
> > > +   if (hdr_valid) {
> > > memcpy(hdr, p, hdr_len);
> > > -
> > > +   pr_err("hdr->hdr_len=%u\n", hdr->hdr.hdr_len);
> > > +   }
> > > len -= hdr_len;
> > > offset += hdr_padded_len;
> > > p += hdr_padded_len;
> >
> >
> > Depends on how you connect qemu on the host. It's filled by host tap,
> > see virtio_net_hdr_from_skb. If 

Re: Linux 5.12-rc7

2021-04-13 Thread Michael S. Tsirkin
On Tue, Apr 13, 2021 at 03:42:24PM +0200, Eric Dumazet wrote:
> On Tue, Apr 13, 2021 at 3:38 PM Michael S. Tsirkin  wrote:
> >
> > On Tue, Apr 13, 2021 at 03:33:40PM +0200, Eric Dumazet wrote:
> > > On Tue, Apr 13, 2021 at 3:27 PM Eric Dumazet  wrote:
> > > >
> > > > On Tue, Apr 13, 2021 at 2:57 PM Michael S. Tsirkin  
> > > > wrote:
> > > > >
> > > > > On Mon, Apr 12, 2021 at 06:47:07PM +0200, Eric Dumazet wrote:
> > > > > > On Mon, Apr 12, 2021 at 6:31 PM Eric Dumazet  
> > > > > > wrote:
> > > > > > >
> > > > > > > On Mon, Apr 12, 2021 at 6:28 PM Linus Torvalds
> > > > > > >  wrote:
> > > > > > > >
> > > > > > > > On Sun, Apr 11, 2021 at 10:14 PM Guenter Roeck 
> > > > > > > >  wrote:
> > > > > > > > >
> > > > > > > > > Qemu test results:
> > > > > > > > > total: 460 pass: 459 fail: 1
> > > > > > > > > Failed tests:
> > > > > > > > > sh:rts7751r2dplus_defconfig:ata:net,virtio-net:rootfs
> > > > > > > > >
> > > > > > > > > The failure bisects to commit 0f6925b3e8da ("virtio_net: Do 
> > > > > > > > > not pull payload in
> > > > > > > > > skb->head"). It is a spurious problem - the test passes 
> > > > > > > > > roughly every other
> > > > > > > > > time. When the failure is seen, udhcpc fails to get an IP 
> > > > > > > > > address and aborts
> > > > > > > > > with SIGTERM. So far I have only seen this with the "sh" 
> > > > > > > > > architecture.
> > > > > > > >
> > > > > > > > Hmm. Let's add in some more of the people involved in that 
> > > > > > > > commit, and
> > > > > > > > also netdev.
> > > > > > > >
> > > > > > > > Nothing in there looks like it should have any interaction with
> > > > > > > > architecture, so that "it happens on sh" sounds odd, but maybe 
> > > > > > > > it's
> > > > > > > > some particular interaction with the qemu environment.
> > > > > > >
> > > > > > > Yes, maybe.
> > > > > > >
> > > > > > > I spent few hours on this, and suspect a buggy memcpy() 
> > > > > > > implementation
> > > > > > > on SH, but this was not conclusive.
> > > > > > >
> > > > > > > By pulling one extra byte, the problem goes away.
> > > > > > >
> > > > > > > Strange thing is that the udhcpc process does not go past 
> > > > > > > sendto().
> > > > > >
> > > > > > This is the patch working around the issue. Unfortunately I was not
> > > > > > able to root-cause it (I really suspect something on SH)
> > > > > >
> > > > > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > > > > > index 
> > > > > > 0824e6999e49957f7aaf7c990f6259792d42f32b..fd890a951beea03bdf24406809042666eb972655
> > > > > > 100644
> > > > > > --- a/drivers/net/virtio_net.c
> > > > > > +++ b/drivers/net/virtio_net.c
> > > > > > @@ -408,11 +408,17 @@ static struct sk_buff *page_to_skb(struct
> > > > > > virtnet_info *vi,
> > > > > >
> > > > > > /* Copy all frame if it fits skb->head, otherwise
> > > > > >  * we let virtio_net_hdr_to_skb() and GRO pull headers as 
> > > > > > needed.
> > > > > > +*
> > > > > > +* Apparently, pulling only the Ethernet Header triggers a 
> > > > > > bug
> > > > > > on qemu-system-sh4.
> > > > > > +* Since GRO aggregation really cares of IPv4/IPv6, pull 20 
> > > > > > bytes
> > > > > > +* more to work around this bug : These 20 bytes can not 
> > > > > > belong
> > > > > > +* to UDP/TCP payload.
> > > > > > +* As a bonus, this makes GRO slightly faster for IPv4 (one 
> > > > > > less copy).
> > > > > >  */
> > > > >
> > > > > Question: do we still want to do this for performance reasons?
> > > > > We also have the hdr_len coming from the device which is
> > > > > just skb_headlen on the host.
> > > >
> > > > Well, putting 20 bytes in skb->head will disable frag0 optimization.
> > > >
> > > > The change would only benefit to sh architecture :)
> > > >
> > > > About hdr_len, I suppose we could try it, with appropriate safety 
> > > > checks.
> > >
> > > I have added traces, hdr_len seems to be 0 with the qemu-system-sh4 I am 
> > > using.
> > >
> > > Have I understood you correctly ?
> > >
> > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > > index 
> > > 0824e6999e49957f7aaf7c990f6259792d42f32b..f024860f7dc260d4efbc35a3b8ffd358bd0da894
> > > 100644
> > > --- a/drivers/net/virtio_net.c
> > > +++ b/drivers/net/virtio_net.c
> > > @@ -399,9 +399,10 @@ static struct sk_buff *page_to_skb(struct 
> > > virtnet_info *vi,
> > > hdr_padded_len = sizeof(struct padded_vnet_hdr);
> > >
> > > /* hdr_valid means no XDP, so we can copy the vnet header */
> > > -   if (hdr_valid)
> > > +   if (hdr_valid) {
> > > memcpy(hdr, p, hdr_len);
> > > -
> > > +   pr_err("hdr->hdr_len=%u\n", hdr->hdr.hdr_len);
> > > +   }
> > > len -= hdr_len;
> > > offset += hdr_padded_len;
> > > p += hdr_padded_len;
> >
> >
> > Depends on how you connect qemu on the host. It's filled by host tap,
> > see virtio_net_hdr_from_skb. If 

Re: Linux 5.12-rc7

2021-04-13 Thread Eric Dumazet
On Tue, Apr 13, 2021 at 3:38 PM Michael S. Tsirkin  wrote:
>
> On Tue, Apr 13, 2021 at 03:33:40PM +0200, Eric Dumazet wrote:
> > On Tue, Apr 13, 2021 at 3:27 PM Eric Dumazet  wrote:
> > >
> > > On Tue, Apr 13, 2021 at 2:57 PM Michael S. Tsirkin  
> > > wrote:
> > > >
> > > > On Mon, Apr 12, 2021 at 06:47:07PM +0200, Eric Dumazet wrote:
> > > > > On Mon, Apr 12, 2021 at 6:31 PM Eric Dumazet  
> > > > > wrote:
> > > > > >
> > > > > > On Mon, Apr 12, 2021 at 6:28 PM Linus Torvalds
> > > > > >  wrote:
> > > > > > >
> > > > > > > On Sun, Apr 11, 2021 at 10:14 PM Guenter Roeck 
> > > > > > >  wrote:
> > > > > > > >
> > > > > > > > Qemu test results:
> > > > > > > > total: 460 pass: 459 fail: 1
> > > > > > > > Failed tests:
> > > > > > > > sh:rts7751r2dplus_defconfig:ata:net,virtio-net:rootfs
> > > > > > > >
> > > > > > > > The failure bisects to commit 0f6925b3e8da ("virtio_net: Do not 
> > > > > > > > pull payload in
> > > > > > > > skb->head"). It is a spurious problem - the test passes roughly 
> > > > > > > > every other
> > > > > > > > time. When the failure is seen, udhcpc fails to get an IP 
> > > > > > > > address and aborts
> > > > > > > > with SIGTERM. So far I have only seen this with the "sh" 
> > > > > > > > architecture.
> > > > > > >
> > > > > > > Hmm. Let's add in some more of the people involved in that 
> > > > > > > commit, and
> > > > > > > also netdev.
> > > > > > >
> > > > > > > Nothing in there looks like it should have any interaction with
> > > > > > > architecture, so that "it happens on sh" sounds odd, but maybe 
> > > > > > > it's
> > > > > > > some particular interaction with the qemu environment.
> > > > > >
> > > > > > Yes, maybe.
> > > > > >
> > > > > > I spent few hours on this, and suspect a buggy memcpy() 
> > > > > > implementation
> > > > > > on SH, but this was not conclusive.
> > > > > >
> > > > > > By pulling one extra byte, the problem goes away.
> > > > > >
> > > > > > Strange thing is that the udhcpc process does not go past sendto().
> > > > >
> > > > > This is the patch working around the issue. Unfortunately I was not
> > > > > able to root-cause it (I really suspect something on SH)
> > > > >
> > > > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > > > > index 
> > > > > 0824e6999e49957f7aaf7c990f6259792d42f32b..fd890a951beea03bdf24406809042666eb972655
> > > > > 100644
> > > > > --- a/drivers/net/virtio_net.c
> > > > > +++ b/drivers/net/virtio_net.c
> > > > > @@ -408,11 +408,17 @@ static struct sk_buff *page_to_skb(struct
> > > > > virtnet_info *vi,
> > > > >
> > > > > /* Copy all frame if it fits skb->head, otherwise
> > > > >  * we let virtio_net_hdr_to_skb() and GRO pull headers as 
> > > > > needed.
> > > > > +*
> > > > > +* Apparently, pulling only the Ethernet Header triggers a bug
> > > > > on qemu-system-sh4.
> > > > > +* Since GRO aggregation really cares of IPv4/IPv6, pull 20 
> > > > > bytes
> > > > > +* more to work around this bug : These 20 bytes can not 
> > > > > belong
> > > > > +* to UDP/TCP payload.
> > > > > +* As a bonus, this makes GRO slightly faster for IPv4 (one 
> > > > > less copy).
> > > > >  */
> > > >
> > > > Question: do we still want to do this for performance reasons?
> > > > We also have the hdr_len coming from the device which is
> > > > just skb_headlen on the host.
> > >
> > > Well, putting 20 bytes in skb->head will disable frag0 optimization.
> > >
> > > The change would only benefit to sh architecture :)
> > >
> > > About hdr_len, I suppose we could try it, with appropriate safety checks.
> >
> > I have added traces, hdr_len seems to be 0 with the qemu-system-sh4 I am 
> > using.
> >
> > Have I understood you correctly ?
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 
> > 0824e6999e49957f7aaf7c990f6259792d42f32b..f024860f7dc260d4efbc35a3b8ffd358bd0da894
> > 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -399,9 +399,10 @@ static struct sk_buff *page_to_skb(struct virtnet_info 
> > *vi,
> > hdr_padded_len = sizeof(struct padded_vnet_hdr);
> >
> > /* hdr_valid means no XDP, so we can copy the vnet header */
> > -   if (hdr_valid)
> > +   if (hdr_valid) {
> > memcpy(hdr, p, hdr_len);
> > -
> > +   pr_err("hdr->hdr_len=%u\n", hdr->hdr.hdr_len);
> > +   }
> > len -= hdr_len;
> > offset += hdr_padded_len;
> > p += hdr_padded_len;
>
>
> Depends on how you connect qemu on the host. It's filled by host tap,
> see virtio_net_hdr_from_skb. If you are using slirp that just zero-fills
> it.

Guenter provided :

qemu-system-sh4 -M r2d -kernel ./arch/sh/boot/zImage -no-reboot \
-snapshot \
-drive file=rootfs.ext2,format=raw,if=ide \
-device virtio-net,netdev=net0 -netdev user,id=net0 \
-append "root=/dev/sda console=ttySC1,115200

Re: Linux 5.12-rc7

2021-04-13 Thread Michael S. Tsirkin
On Tue, Apr 13, 2021 at 03:33:40PM +0200, Eric Dumazet wrote:
> On Tue, Apr 13, 2021 at 3:27 PM Eric Dumazet  wrote:
> >
> > On Tue, Apr 13, 2021 at 2:57 PM Michael S. Tsirkin  wrote:
> > >
> > > On Mon, Apr 12, 2021 at 06:47:07PM +0200, Eric Dumazet wrote:
> > > > On Mon, Apr 12, 2021 at 6:31 PM Eric Dumazet  
> > > > wrote:
> > > > >
> > > > > On Mon, Apr 12, 2021 at 6:28 PM Linus Torvalds
> > > > >  wrote:
> > > > > >
> > > > > > On Sun, Apr 11, 2021 at 10:14 PM Guenter Roeck  
> > > > > > wrote:
> > > > > > >
> > > > > > > Qemu test results:
> > > > > > > total: 460 pass: 459 fail: 1
> > > > > > > Failed tests:
> > > > > > > sh:rts7751r2dplus_defconfig:ata:net,virtio-net:rootfs
> > > > > > >
> > > > > > > The failure bisects to commit 0f6925b3e8da ("virtio_net: Do not 
> > > > > > > pull payload in
> > > > > > > skb->head"). It is a spurious problem - the test passes roughly 
> > > > > > > every other
> > > > > > > time. When the failure is seen, udhcpc fails to get an IP address 
> > > > > > > and aborts
> > > > > > > with SIGTERM. So far I have only seen this with the "sh" 
> > > > > > > architecture.
> > > > > >
> > > > > > Hmm. Let's add in some more of the people involved in that commit, 
> > > > > > and
> > > > > > also netdev.
> > > > > >
> > > > > > Nothing in there looks like it should have any interaction with
> > > > > > architecture, so that "it happens on sh" sounds odd, but maybe it's
> > > > > > some particular interaction with the qemu environment.
> > > > >
> > > > > Yes, maybe.
> > > > >
> > > > > I spent few hours on this, and suspect a buggy memcpy() implementation
> > > > > on SH, but this was not conclusive.
> > > > >
> > > > > By pulling one extra byte, the problem goes away.
> > > > >
> > > > > Strange thing is that the udhcpc process does not go past sendto().
> > > >
> > > > This is the patch working around the issue. Unfortunately I was not
> > > > able to root-cause it (I really suspect something on SH)
> > > >
> > > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > > > index 
> > > > 0824e6999e49957f7aaf7c990f6259792d42f32b..fd890a951beea03bdf24406809042666eb972655
> > > > 100644
> > > > --- a/drivers/net/virtio_net.c
> > > > +++ b/drivers/net/virtio_net.c
> > > > @@ -408,11 +408,17 @@ static struct sk_buff *page_to_skb(struct
> > > > virtnet_info *vi,
> > > >
> > > > /* Copy all frame if it fits skb->head, otherwise
> > > >  * we let virtio_net_hdr_to_skb() and GRO pull headers as 
> > > > needed.
> > > > +*
> > > > +* Apparently, pulling only the Ethernet Header triggers a bug
> > > > on qemu-system-sh4.
> > > > +* Since GRO aggregation really cares of IPv4/IPv6, pull 20 
> > > > bytes
> > > > +* more to work around this bug : These 20 bytes can not belong
> > > > +* to UDP/TCP payload.
> > > > +* As a bonus, this makes GRO slightly faster for IPv4 (one 
> > > > less copy).
> > > >  */
> > >
> > > Question: do we still want to do this for performance reasons?
> > > We also have the hdr_len coming from the device which is
> > > just skb_headlen on the host.
> >
> > Well, putting 20 bytes in skb->head will disable frag0 optimization.
> >
> > The change would only benefit to sh architecture :)
> >
> > About hdr_len, I suppose we could try it, with appropriate safety checks.
> 
> I have added traces, hdr_len seems to be 0 with the qemu-system-sh4 I am 
> using.
> 
> Have I understood you correctly ?
> 
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 
> 0824e6999e49957f7aaf7c990f6259792d42f32b..f024860f7dc260d4efbc35a3b8ffd358bd0da894
> 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -399,9 +399,10 @@ static struct sk_buff *page_to_skb(struct virtnet_info 
> *vi,
> hdr_padded_len = sizeof(struct padded_vnet_hdr);
> 
> /* hdr_valid means no XDP, so we can copy the vnet header */
> -   if (hdr_valid)
> +   if (hdr_valid) {
> memcpy(hdr, p, hdr_len);
> -
> +   pr_err("hdr->hdr_len=%u\n", hdr->hdr.hdr_len);
> +   }
> len -= hdr_len;
> offset += hdr_padded_len;
> p += hdr_padded_len;


Depends on how you connect qemu on the host. It's filled by host tap,
see virtio_net_hdr_from_skb. If you are using slirp that just zero-fills
it.


-- 
MST



Re: Linux 5.12-rc7

2021-04-13 Thread Eric Dumazet
On Tue, Apr 13, 2021 at 3:27 PM Eric Dumazet  wrote:
>
> On Tue, Apr 13, 2021 at 2:57 PM Michael S. Tsirkin  wrote:
> >
> > On Mon, Apr 12, 2021 at 06:47:07PM +0200, Eric Dumazet wrote:
> > > On Mon, Apr 12, 2021 at 6:31 PM Eric Dumazet  wrote:
> > > >
> > > > On Mon, Apr 12, 2021 at 6:28 PM Linus Torvalds
> > > >  wrote:
> > > > >
> > > > > On Sun, Apr 11, 2021 at 10:14 PM Guenter Roeck  
> > > > > wrote:
> > > > > >
> > > > > > Qemu test results:
> > > > > > total: 460 pass: 459 fail: 1
> > > > > > Failed tests:
> > > > > > sh:rts7751r2dplus_defconfig:ata:net,virtio-net:rootfs
> > > > > >
> > > > > > The failure bisects to commit 0f6925b3e8da ("virtio_net: Do not 
> > > > > > pull payload in
> > > > > > skb->head"). It is a spurious problem - the test passes roughly 
> > > > > > every other
> > > > > > time. When the failure is seen, udhcpc fails to get an IP address 
> > > > > > and aborts
> > > > > > with SIGTERM. So far I have only seen this with the "sh" 
> > > > > > architecture.
> > > > >
> > > > > Hmm. Let's add in some more of the people involved in that commit, and
> > > > > also netdev.
> > > > >
> > > > > Nothing in there looks like it should have any interaction with
> > > > > architecture, so that "it happens on sh" sounds odd, but maybe it's
> > > > > some particular interaction with the qemu environment.
> > > >
> > > > Yes, maybe.
> > > >
> > > > I spent few hours on this, and suspect a buggy memcpy() implementation
> > > > on SH, but this was not conclusive.
> > > >
> > > > By pulling one extra byte, the problem goes away.
> > > >
> > > > Strange thing is that the udhcpc process does not go past sendto().
> > >
> > > This is the patch working around the issue. Unfortunately I was not
> > > able to root-cause it (I really suspect something on SH)
> > >
> > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > > index 
> > > 0824e6999e49957f7aaf7c990f6259792d42f32b..fd890a951beea03bdf24406809042666eb972655
> > > 100644
> > > --- a/drivers/net/virtio_net.c
> > > +++ b/drivers/net/virtio_net.c
> > > @@ -408,11 +408,17 @@ static struct sk_buff *page_to_skb(struct
> > > virtnet_info *vi,
> > >
> > > /* Copy all frame if it fits skb->head, otherwise
> > >  * we let virtio_net_hdr_to_skb() and GRO pull headers as needed.
> > > +*
> > > +* Apparently, pulling only the Ethernet Header triggers a bug
> > > on qemu-system-sh4.
> > > +* Since GRO aggregation really cares of IPv4/IPv6, pull 20 bytes
> > > +* more to work around this bug : These 20 bytes can not belong
> > > +* to UDP/TCP payload.
> > > +* As a bonus, this makes GRO slightly faster for IPv4 (one less 
> > > copy).
> > >  */
> >
> > Question: do we still want to do this for performance reasons?
> > We also have the hdr_len coming from the device which is
> > just skb_headlen on the host.
>
> Well, putting 20 bytes in skb->head will disable frag0 optimization.
>
> The change would only benefit to sh architecture :)
>
> About hdr_len, I suppose we could try it, with appropriate safety checks.

I have added traces, hdr_len seems to be 0 with the qemu-system-sh4 I am using.

Have I understood you correctly ?

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 
0824e6999e49957f7aaf7c990f6259792d42f32b..f024860f7dc260d4efbc35a3b8ffd358bd0da894
100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -399,9 +399,10 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
hdr_padded_len = sizeof(struct padded_vnet_hdr);

/* hdr_valid means no XDP, so we can copy the vnet header */
-   if (hdr_valid)
+   if (hdr_valid) {
memcpy(hdr, p, hdr_len);
-
+   pr_err("hdr->hdr_len=%u\n", hdr->hdr.hdr_len);
+   }
len -= hdr_len;
offset += hdr_padded_len;
p += hdr_padded_len;


Re: Linux 5.12-rc7

2021-04-13 Thread Eric Dumazet
On Tue, Apr 13, 2021 at 2:57 PM Michael S. Tsirkin  wrote:
>
> On Mon, Apr 12, 2021 at 06:47:07PM +0200, Eric Dumazet wrote:
> > On Mon, Apr 12, 2021 at 6:31 PM Eric Dumazet  wrote:
> > >
> > > On Mon, Apr 12, 2021 at 6:28 PM Linus Torvalds
> > >  wrote:
> > > >
> > > > On Sun, Apr 11, 2021 at 10:14 PM Guenter Roeck  
> > > > wrote:
> > > > >
> > > > > Qemu test results:
> > > > > total: 460 pass: 459 fail: 1
> > > > > Failed tests:
> > > > > sh:rts7751r2dplus_defconfig:ata:net,virtio-net:rootfs
> > > > >
> > > > > The failure bisects to commit 0f6925b3e8da ("virtio_net: Do not pull 
> > > > > payload in
> > > > > skb->head"). It is a spurious problem - the test passes roughly every 
> > > > > other
> > > > > time. When the failure is seen, udhcpc fails to get an IP address and 
> > > > > aborts
> > > > > with SIGTERM. So far I have only seen this with the "sh" architecture.
> > > >
> > > > Hmm. Let's add in some more of the people involved in that commit, and
> > > > also netdev.
> > > >
> > > > Nothing in there looks like it should have any interaction with
> > > > architecture, so that "it happens on sh" sounds odd, but maybe it's
> > > > some particular interaction with the qemu environment.
> > >
> > > Yes, maybe.
> > >
> > > I spent few hours on this, and suspect a buggy memcpy() implementation
> > > on SH, but this was not conclusive.
> > >
> > > By pulling one extra byte, the problem goes away.
> > >
> > > Strange thing is that the udhcpc process does not go past sendto().
> >
> > This is the patch working around the issue. Unfortunately I was not
> > able to root-cause it (I really suspect something on SH)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 
> > 0824e6999e49957f7aaf7c990f6259792d42f32b..fd890a951beea03bdf24406809042666eb972655
> > 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -408,11 +408,17 @@ static struct sk_buff *page_to_skb(struct
> > virtnet_info *vi,
> >
> > /* Copy all frame if it fits skb->head, otherwise
> >  * we let virtio_net_hdr_to_skb() and GRO pull headers as needed.
> > +*
> > +* Apparently, pulling only the Ethernet Header triggers a bug
> > on qemu-system-sh4.
> > +* Since GRO aggregation really cares of IPv4/IPv6, pull 20 bytes
> > +* more to work around this bug : These 20 bytes can not belong
> > +* to UDP/TCP payload.
> > +* As a bonus, this makes GRO slightly faster for IPv4 (one less 
> > copy).
> >  */
>
> Question: do we still want to do this for performance reasons?
> We also have the hdr_len coming from the device which is
> just skb_headlen on the host.

Well, putting 20 bytes in skb->head will disable frag0 optimization.

The change would only benefit to sh architecture :)

About hdr_len, I suppose we could try it, with appropriate safety checks.

>
> > if (len <= skb_tailroom(skb))
> > copy = len;
> > else
> > -   copy = ETH_HLEN + metasize;
> > +   copy = ETH_HLEN + sizeof(struct iphdr) + metasize;
> > skb_put_data(skb, p, copy);
> >
> > if (metasize) {
>


Re: Linux 5.12-rc7

2021-04-13 Thread Michael S. Tsirkin
On Mon, Apr 12, 2021 at 06:47:07PM +0200, Eric Dumazet wrote:
> On Mon, Apr 12, 2021 at 6:31 PM Eric Dumazet  wrote:
> >
> > On Mon, Apr 12, 2021 at 6:28 PM Linus Torvalds
> >  wrote:
> > >
> > > On Sun, Apr 11, 2021 at 10:14 PM Guenter Roeck  wrote:
> > > >
> > > > Qemu test results:
> > > > total: 460 pass: 459 fail: 1
> > > > Failed tests:
> > > > sh:rts7751r2dplus_defconfig:ata:net,virtio-net:rootfs
> > > >
> > > > The failure bisects to commit 0f6925b3e8da ("virtio_net: Do not pull 
> > > > payload in
> > > > skb->head"). It is a spurious problem - the test passes roughly every 
> > > > other
> > > > time. When the failure is seen, udhcpc fails to get an IP address and 
> > > > aborts
> > > > with SIGTERM. So far I have only seen this with the "sh" architecture.
> > >
> > > Hmm. Let's add in some more of the people involved in that commit, and
> > > also netdev.
> > >
> > > Nothing in there looks like it should have any interaction with
> > > architecture, so that "it happens on sh" sounds odd, but maybe it's
> > > some particular interaction with the qemu environment.
> >
> > Yes, maybe.
> >
> > I spent few hours on this, and suspect a buggy memcpy() implementation
> > on SH, but this was not conclusive.
> >
> > By pulling one extra byte, the problem goes away.
> >
> > Strange thing is that the udhcpc process does not go past sendto().
> 
> This is the patch working around the issue. Unfortunately I was not
> able to root-cause it (I really suspect something on SH)
> 
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 
> 0824e6999e49957f7aaf7c990f6259792d42f32b..fd890a951beea03bdf24406809042666eb972655
> 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -408,11 +408,17 @@ static struct sk_buff *page_to_skb(struct
> virtnet_info *vi,
> 
> /* Copy all frame if it fits skb->head, otherwise
>  * we let virtio_net_hdr_to_skb() and GRO pull headers as needed.
> +*
> +* Apparently, pulling only the Ethernet Header triggers a bug
> on qemu-system-sh4.
> +* Since GRO aggregation really cares of IPv4/IPv6, pull 20 bytes
> +* more to work around this bug : These 20 bytes can not belong
> +* to UDP/TCP payload.
> +* As a bonus, this makes GRO slightly faster for IPv4 (one less 
> copy).
>  */

Question: do we still want to do this for performance reasons?
We also have the hdr_len coming from the device which is
just skb_headlen on the host.

> if (len <= skb_tailroom(skb))
> copy = len;
> else
> -   copy = ETH_HLEN + metasize;
> +   copy = ETH_HLEN + sizeof(struct iphdr) + metasize;
> skb_put_data(skb, p, copy);
> 
> if (metasize) {



Re: Linux 5.12-rc7

2021-04-13 Thread Michael S. Tsirkin
On Tue, Apr 13, 2021 at 02:45:46PM +0200, Eric Dumazet wrote:
> On Tue, Apr 13, 2021 at 12:43 PM Eric Dumazet  wrote:
> >
> > On Tue, Apr 13, 2021 at 11:24 AM Eric Dumazet  wrote:
> > >
> > > On Mon, Apr 12, 2021 at 10:05 PM Guenter Roeck  wrote:
> > > >
> > > > On 4/12/21 10:38 AM, Eric Dumazet wrote:
> > > > [ ... ]
> > > >
> > > > > Yes, I think this is the real issue here. This smells like some memory
> > > > > corruption.
> > > > >
> > > > > In my traces, packet is correctly received in AF_PACKET queue.
> > > > >
> > > > > I have checked the skb is well formed.
> > > > >
> > > > > But the user space seems to never call poll() and recvmsg() on this
> > > > > af_packet socket.
> > > > >
> > > >
> > > > After sprinkling the kernel with debug messages:
> > > >
> > > > 424   00:01:33.674181 sendto(6, 
> > > > "E\0\1H\0\0\0\0@\21y\246\0\0\0\0\377\377\377\377\0D\0C\00148\346\1\1\6\0\246\336\333\v\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0RT\0\
> > > > 424   00:01:33.693873 close(6)  = 0
> > > > 424   00:01:33.694652 fcntl64(5, F_SETFD, FD_CLOEXEC) = 0
> > > > 424   00:01:33.695213 clock_gettime64(CLOCK_MONOTONIC, 0x7be18a18) = -1 
> > > > EFAULT (Bad address)
> > > > 424   00:01:33.695889 write(2, "udhcpc: clock_gettime(MONOTONIC) 
> > > > failed\n", 40) = -1 EFAULT (Bad address)
> > > > 424   00:01:33.697311 exit_group(1) = ?
> > > > 424   00:01:33.698346 +++ exited with 1 +++
> > > >
> > > > I only see that after adding debug messages in the kernel, so I guess 
> > > > there must be
> > > > a heisenbug somehere.
> > > >
> > > > Anyway, indeed, I see (another kernel debug message):
> > > >
> > > > __do_sys_clock_gettime: Returning -EFAULT on address 0x7bacc9a8
> > > >
> > > > So udhcpc doesn't even try to read the reply because it crashes after 
> > > > sendto()
> > > > when trying to read the current time. Unless I am missing something, 
> > > > that means
> > > > that the problem happens somewhere on the send side.
> > > >
> > > > To make things even more interesting, it looks like the failing system 
> > > > call
> > > > isn't always clock_gettime().
> > > >
> > > > Guenter
> > >
> > >
> > > I think GRO fast path has never worked on SUPERH. Probably SUPERH has
> > > never used a fast NIC (10Gbit+)
> > >
> > > The following hack fixes the issue.
> > >
> > >
> > > diff --git a/net/core/dev.c b/net/core/dev.c
> > > index 
> > > af8c1ea040b9364b076e2d72f04dc3de2d7e2f11..91ba89a645ff91d4cd4f3d8dc8a009bcb67da344
> > > 100644
> > > --- a/net/core/dev.c
> > > +++ b/net/core/dev.c
> > > @@ -5916,13 +5916,16 @@ static struct list_head
> > > *gro_list_prepare(struct napi_struct *napi,
> > >
> > >  static void skb_gro_reset_offset(struct sk_buff *skb)
> > >  {
> > > +#if !defined(CONFIG_SUPERH)
> > > const struct skb_shared_info *pinfo = skb_shinfo(skb);
> > > const skb_frag_t *frag0 = >frags[0];
> > > +#endif
> > >
> > > NAPI_GRO_CB(skb)->data_offset = 0;
> > > NAPI_GRO_CB(skb)->frag0 = NULL;
> > > NAPI_GRO_CB(skb)->frag0_len = 0;
> > >
> > > +#if !defined(CONFIG_SUPERH)
> > > if (!skb_headlen(skb) && pinfo->nr_frags &&
> > > !PageHighMem(skb_frag_page(frag0))) {
> > > NAPI_GRO_CB(skb)->frag0 = skb_frag_address(frag0);
> > > @@ -5930,6 +5933,7 @@ static void skb_gro_reset_offset(struct sk_buff 
> > > *skb)
> > > skb_frag_size(frag0),
> > > skb->end - skb->tail);
> > > }
> > > +#endif
> > >  }
> > >
> > >  static void gro_pull_from_frag0(struct sk_buff *skb, int grow)
> >
> > OK ... more sh debugging :
> >
> > diff --git a/arch/sh/mm/alignment.c b/arch/sh/mm/alignment.c
> > index 
> > fb517b82a87b1065cf38c06cb3c178ce86587b00..5d18f9f792991105a8aa05cc6231b7d4532d72c9
> > 100644
> > --- a/arch/sh/mm/alignment.c
> > +++ b/arch/sh/mm/alignment.c
> > @@ -27,7 +27,7 @@ static unsigned long se_multi;
> > valid! */
> >  static int se_usermode = UM_WARN | UM_FIXUP;
> >  /* 0: no warning 1: print a warning message, disabled by default */
> > -static int se_kernmode_warn;
> > +static int se_kernmode_warn = 1;
> >
> >  core_param(alignment, se_usermode, int, 0600);
> >
> > @@ -103,7 +103,7 @@ void unaligned_fixups_notify(struct task_struct
> > *tsk, insn_size_t insn,
> >   (void *)instruction_pointer(regs), insn);
> > else if (se_kernmode_warn)
> > pr_notice_ratelimited("Fixing up unaligned kernel access "
> > - "in \"%s\" pid=%d pc=0x%p ins=0x%04hx\n",
> > + "in \"%s\" pid=%d pc=%px ins=0x%04hx\n",
> >   tsk->comm, task_pid_nr(tsk),
> >   (void *)instruction_pointer(regs), insn);
> >  }
> >
> > I now see something of interest :
> > Fixing up unaligned kernel access in "udhcpc" pid=91 pc=8c43fc2e ins=0x6236
> > Fixing up unaligned kernel access in "udhcpc" pid=91 pc=8c43fc2e ins=0x6236
> 

Re: Linux 5.12-rc7

2021-04-13 Thread Eric Dumazet
On Tue, Apr 13, 2021 at 12:43 PM Eric Dumazet  wrote:
>
> On Tue, Apr 13, 2021 at 11:24 AM Eric Dumazet  wrote:
> >
> > On Mon, Apr 12, 2021 at 10:05 PM Guenter Roeck  wrote:
> > >
> > > On 4/12/21 10:38 AM, Eric Dumazet wrote:
> > > [ ... ]
> > >
> > > > Yes, I think this is the real issue here. This smells like some memory
> > > > corruption.
> > > >
> > > > In my traces, packet is correctly received in AF_PACKET queue.
> > > >
> > > > I have checked the skb is well formed.
> > > >
> > > > But the user space seems to never call poll() and recvmsg() on this
> > > > af_packet socket.
> > > >
> > >
> > > After sprinkling the kernel with debug messages:
> > >
> > > 424   00:01:33.674181 sendto(6, 
> > > "E\0\1H\0\0\0\0@\21y\246\0\0\0\0\377\377\377\377\0D\0C\00148\346\1\1\6\0\246\336\333\v\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0RT\0\
> > > 424   00:01:33.693873 close(6)  = 0
> > > 424   00:01:33.694652 fcntl64(5, F_SETFD, FD_CLOEXEC) = 0
> > > 424   00:01:33.695213 clock_gettime64(CLOCK_MONOTONIC, 0x7be18a18) = -1 
> > > EFAULT (Bad address)
> > > 424   00:01:33.695889 write(2, "udhcpc: clock_gettime(MONOTONIC) 
> > > failed\n", 40) = -1 EFAULT (Bad address)
> > > 424   00:01:33.697311 exit_group(1) = ?
> > > 424   00:01:33.698346 +++ exited with 1 +++
> > >
> > > I only see that after adding debug messages in the kernel, so I guess 
> > > there must be
> > > a heisenbug somehere.
> > >
> > > Anyway, indeed, I see (another kernel debug message):
> > >
> > > __do_sys_clock_gettime: Returning -EFAULT on address 0x7bacc9a8
> > >
> > > So udhcpc doesn't even try to read the reply because it crashes after 
> > > sendto()
> > > when trying to read the current time. Unless I am missing something, that 
> > > means
> > > that the problem happens somewhere on the send side.
> > >
> > > To make things even more interesting, it looks like the failing system 
> > > call
> > > isn't always clock_gettime().
> > >
> > > Guenter
> >
> >
> > I think GRO fast path has never worked on SUPERH. Probably SUPERH has
> > never used a fast NIC (10Gbit+)
> >
> > The following hack fixes the issue.
> >
> >
> > diff --git a/net/core/dev.c b/net/core/dev.c
> > index 
> > af8c1ea040b9364b076e2d72f04dc3de2d7e2f11..91ba89a645ff91d4cd4f3d8dc8a009bcb67da344
> > 100644
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -5916,13 +5916,16 @@ static struct list_head
> > *gro_list_prepare(struct napi_struct *napi,
> >
> >  static void skb_gro_reset_offset(struct sk_buff *skb)
> >  {
> > +#if !defined(CONFIG_SUPERH)
> > const struct skb_shared_info *pinfo = skb_shinfo(skb);
> > const skb_frag_t *frag0 = >frags[0];
> > +#endif
> >
> > NAPI_GRO_CB(skb)->data_offset = 0;
> > NAPI_GRO_CB(skb)->frag0 = NULL;
> > NAPI_GRO_CB(skb)->frag0_len = 0;
> >
> > +#if !defined(CONFIG_SUPERH)
> > if (!skb_headlen(skb) && pinfo->nr_frags &&
> > !PageHighMem(skb_frag_page(frag0))) {
> > NAPI_GRO_CB(skb)->frag0 = skb_frag_address(frag0);
> > @@ -5930,6 +5933,7 @@ static void skb_gro_reset_offset(struct sk_buff *skb)
> > skb_frag_size(frag0),
> > skb->end - skb->tail);
> > }
> > +#endif
> >  }
> >
> >  static void gro_pull_from_frag0(struct sk_buff *skb, int grow)
>
> OK ... more sh debugging :
>
> diff --git a/arch/sh/mm/alignment.c b/arch/sh/mm/alignment.c
> index 
> fb517b82a87b1065cf38c06cb3c178ce86587b00..5d18f9f792991105a8aa05cc6231b7d4532d72c9
> 100644
> --- a/arch/sh/mm/alignment.c
> +++ b/arch/sh/mm/alignment.c
> @@ -27,7 +27,7 @@ static unsigned long se_multi;
> valid! */
>  static int se_usermode = UM_WARN | UM_FIXUP;
>  /* 0: no warning 1: print a warning message, disabled by default */
> -static int se_kernmode_warn;
> +static int se_kernmode_warn = 1;
>
>  core_param(alignment, se_usermode, int, 0600);
>
> @@ -103,7 +103,7 @@ void unaligned_fixups_notify(struct task_struct
> *tsk, insn_size_t insn,
>   (void *)instruction_pointer(regs), insn);
> else if (se_kernmode_warn)
> pr_notice_ratelimited("Fixing up unaligned kernel access "
> - "in \"%s\" pid=%d pc=0x%p ins=0x%04hx\n",
> + "in \"%s\" pid=%d pc=%px ins=0x%04hx\n",
>   tsk->comm, task_pid_nr(tsk),
>   (void *)instruction_pointer(regs), insn);
>  }
>
> I now see something of interest :
> Fixing up unaligned kernel access in "udhcpc" pid=91 pc=8c43fc2e ins=0x6236
> Fixing up unaligned kernel access in "udhcpc" pid=91 pc=8c43fc2e ins=0x6236
> Fixing up unaligned kernel access in "udhcpc" pid=91 pc=8c43fc30 ins=0x6636
> Fixing up unaligned kernel access in "udhcpc" pid=91 pc=8c43fc30 ins=0x6636
> Fixing up unaligned kernel access in "udhcpc" pid=91 pc=8c43fc3a ins=0x6636
> Fixing up unaligned kernel access in "udhcpc" pid=91 pc=8c43fc3a ins=0x6636
> 

Re: Linux 5.12-rc7

2021-04-13 Thread Eric Dumazet
On Tue, Apr 13, 2021 at 11:24 AM Eric Dumazet  wrote:
>
> On Mon, Apr 12, 2021 at 10:05 PM Guenter Roeck  wrote:
> >
> > On 4/12/21 10:38 AM, Eric Dumazet wrote:
> > [ ... ]
> >
> > > Yes, I think this is the real issue here. This smells like some memory
> > > corruption.
> > >
> > > In my traces, packet is correctly received in AF_PACKET queue.
> > >
> > > I have checked the skb is well formed.
> > >
> > > But the user space seems to never call poll() and recvmsg() on this
> > > af_packet socket.
> > >
> >
> > After sprinkling the kernel with debug messages:
> >
> > 424   00:01:33.674181 sendto(6, 
> > "E\0\1H\0\0\0\0@\21y\246\0\0\0\0\377\377\377\377\0D\0C\00148\346\1\1\6\0\246\336\333\v\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0RT\0\
> > 424   00:01:33.693873 close(6)  = 0
> > 424   00:01:33.694652 fcntl64(5, F_SETFD, FD_CLOEXEC) = 0
> > 424   00:01:33.695213 clock_gettime64(CLOCK_MONOTONIC, 0x7be18a18) = -1 
> > EFAULT (Bad address)
> > 424   00:01:33.695889 write(2, "udhcpc: clock_gettime(MONOTONIC) failed\n", 
> > 40) = -1 EFAULT (Bad address)
> > 424   00:01:33.697311 exit_group(1) = ?
> > 424   00:01:33.698346 +++ exited with 1 +++
> >
> > I only see that after adding debug messages in the kernel, so I guess there 
> > must be
> > a heisenbug somehere.
> >
> > Anyway, indeed, I see (another kernel debug message):
> >
> > __do_sys_clock_gettime: Returning -EFAULT on address 0x7bacc9a8
> >
> > So udhcpc doesn't even try to read the reply because it crashes after 
> > sendto()
> > when trying to read the current time. Unless I am missing something, that 
> > means
> > that the problem happens somewhere on the send side.
> >
> > To make things even more interesting, it looks like the failing system call
> > isn't always clock_gettime().
> >
> > Guenter
>
>
> I think GRO fast path has never worked on SUPERH. Probably SUPERH has
> never used a fast NIC (10Gbit+)
>
> The following hack fixes the issue.
>
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 
> af8c1ea040b9364b076e2d72f04dc3de2d7e2f11..91ba89a645ff91d4cd4f3d8dc8a009bcb67da344
> 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -5916,13 +5916,16 @@ static struct list_head
> *gro_list_prepare(struct napi_struct *napi,
>
>  static void skb_gro_reset_offset(struct sk_buff *skb)
>  {
> +#if !defined(CONFIG_SUPERH)
> const struct skb_shared_info *pinfo = skb_shinfo(skb);
> const skb_frag_t *frag0 = >frags[0];
> +#endif
>
> NAPI_GRO_CB(skb)->data_offset = 0;
> NAPI_GRO_CB(skb)->frag0 = NULL;
> NAPI_GRO_CB(skb)->frag0_len = 0;
>
> +#if !defined(CONFIG_SUPERH)
> if (!skb_headlen(skb) && pinfo->nr_frags &&
> !PageHighMem(skb_frag_page(frag0))) {
> NAPI_GRO_CB(skb)->frag0 = skb_frag_address(frag0);
> @@ -5930,6 +5933,7 @@ static void skb_gro_reset_offset(struct sk_buff *skb)
> skb_frag_size(frag0),
> skb->end - skb->tail);
> }
> +#endif
>  }
>
>  static void gro_pull_from_frag0(struct sk_buff *skb, int grow)

OK ... more sh debugging :

diff --git a/arch/sh/mm/alignment.c b/arch/sh/mm/alignment.c
index 
fb517b82a87b1065cf38c06cb3c178ce86587b00..5d18f9f792991105a8aa05cc6231b7d4532d72c9
100644
--- a/arch/sh/mm/alignment.c
+++ b/arch/sh/mm/alignment.c
@@ -27,7 +27,7 @@ static unsigned long se_multi;
valid! */
 static int se_usermode = UM_WARN | UM_FIXUP;
 /* 0: no warning 1: print a warning message, disabled by default */
-static int se_kernmode_warn;
+static int se_kernmode_warn = 1;

 core_param(alignment, se_usermode, int, 0600);

@@ -103,7 +103,7 @@ void unaligned_fixups_notify(struct task_struct
*tsk, insn_size_t insn,
  (void *)instruction_pointer(regs), insn);
else if (se_kernmode_warn)
pr_notice_ratelimited("Fixing up unaligned kernel access "
- "in \"%s\" pid=%d pc=0x%p ins=0x%04hx\n",
+ "in \"%s\" pid=%d pc=%px ins=0x%04hx\n",
  tsk->comm, task_pid_nr(tsk),
  (void *)instruction_pointer(regs), insn);
 }

I now see something of interest :
Fixing up unaligned kernel access in "udhcpc" pid=91 pc=8c43fc2e ins=0x6236
Fixing up unaligned kernel access in "udhcpc" pid=91 pc=8c43fc2e ins=0x6236
Fixing up unaligned kernel access in "udhcpc" pid=91 pc=8c43fc30 ins=0x6636
Fixing up unaligned kernel access in "udhcpc" pid=91 pc=8c43fc30 ins=0x6636
Fixing up unaligned kernel access in "udhcpc" pid=91 pc=8c43fc3a ins=0x6636
Fixing up unaligned kernel access in "udhcpc" pid=91 pc=8c43fc3a ins=0x6636
Fixing up unaligned kernel access in "udhcpc" pid=91 pc=8c43fc3a ins=0x6636
Fixing up unaligned kernel access in "udhcpc" pid=91 pc=8c43fc3a ins=0x6636
Fixing up unaligned kernel access in "udhcpc" pid=91 pc=8c43fc3a ins=0x6636
Fixing up unaligned kernel access in "udhcpc" pid=91 pc=8c43fc3a ins=0x6636

So 

Re: Linux 5.12-rc7

2021-04-13 Thread Eric Dumazet
On Mon, Apr 12, 2021 at 10:05 PM Guenter Roeck  wrote:
>
> On 4/12/21 10:38 AM, Eric Dumazet wrote:
> [ ... ]
>
> > Yes, I think this is the real issue here. This smells like some memory
> > corruption.
> >
> > In my traces, packet is correctly received in AF_PACKET queue.
> >
> > I have checked the skb is well formed.
> >
> > But the user space seems to never call poll() and recvmsg() on this
> > af_packet socket.
> >
>
> After sprinkling the kernel with debug messages:
>
> 424   00:01:33.674181 sendto(6, 
> "E\0\1H\0\0\0\0@\21y\246\0\0\0\0\377\377\377\377\0D\0C\00148\346\1\1\6\0\246\336\333\v\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0RT\0\
> 424   00:01:33.693873 close(6)  = 0
> 424   00:01:33.694652 fcntl64(5, F_SETFD, FD_CLOEXEC) = 0
> 424   00:01:33.695213 clock_gettime64(CLOCK_MONOTONIC, 0x7be18a18) = -1 
> EFAULT (Bad address)
> 424   00:01:33.695889 write(2, "udhcpc: clock_gettime(MONOTONIC) failed\n", 
> 40) = -1 EFAULT (Bad address)
> 424   00:01:33.697311 exit_group(1) = ?
> 424   00:01:33.698346 +++ exited with 1 +++
>
> I only see that after adding debug messages in the kernel, so I guess there 
> must be
> a heisenbug somehere.
>
> Anyway, indeed, I see (another kernel debug message):
>
> __do_sys_clock_gettime: Returning -EFAULT on address 0x7bacc9a8
>
> So udhcpc doesn't even try to read the reply because it crashes after sendto()
> when trying to read the current time. Unless I am missing something, that 
> means
> that the problem happens somewhere on the send side.
>
> To make things even more interesting, it looks like the failing system call
> isn't always clock_gettime().
>
> Guenter


I think GRO fast path has never worked on SUPERH. Probably SUPERH has
never used a fast NIC (10Gbit+)

The following hack fixes the issue.


diff --git a/net/core/dev.c b/net/core/dev.c
index 
af8c1ea040b9364b076e2d72f04dc3de2d7e2f11..91ba89a645ff91d4cd4f3d8dc8a009bcb67da344
100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5916,13 +5916,16 @@ static struct list_head
*gro_list_prepare(struct napi_struct *napi,

 static void skb_gro_reset_offset(struct sk_buff *skb)
 {
+#if !defined(CONFIG_SUPERH)
const struct skb_shared_info *pinfo = skb_shinfo(skb);
const skb_frag_t *frag0 = >frags[0];
+#endif

NAPI_GRO_CB(skb)->data_offset = 0;
NAPI_GRO_CB(skb)->frag0 = NULL;
NAPI_GRO_CB(skb)->frag0_len = 0;

+#if !defined(CONFIG_SUPERH)
if (!skb_headlen(skb) && pinfo->nr_frags &&
!PageHighMem(skb_frag_page(frag0))) {
NAPI_GRO_CB(skb)->frag0 = skb_frag_address(frag0);
@@ -5930,6 +5933,7 @@ static void skb_gro_reset_offset(struct sk_buff *skb)
skb_frag_size(frag0),
skb->end - skb->tail);
}
+#endif
 }

 static void gro_pull_from_frag0(struct sk_buff *skb, int grow)


Re: Linux 5.12-rc7

2021-04-12 Thread Guenter Roeck
On 4/12/21 10:38 AM, Eric Dumazet wrote:
[ ... ]

> Yes, I think this is the real issue here. This smells like some memory
> corruption.
> 
> In my traces, packet is correctly received in AF_PACKET queue.
> 
> I have checked the skb is well formed.
> 
> But the user space seems to never call poll() and recvmsg() on this
> af_packet socket.
> 

After sprinkling the kernel with debug messages:

424   00:01:33.674181 sendto(6, 
"E\0\1H\0\0\0\0@\21y\246\0\0\0\0\377\377\377\377\0D\0C\00148\346\1\1\6\0\246\336\333\v\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0RT\0\
424   00:01:33.693873 close(6)  = 0
424   00:01:33.694652 fcntl64(5, F_SETFD, FD_CLOEXEC) = 0
424   00:01:33.695213 clock_gettime64(CLOCK_MONOTONIC, 0x7be18a18) = -1 EFAULT 
(Bad address)
424   00:01:33.695889 write(2, "udhcpc: clock_gettime(MONOTONIC) failed\n", 40) 
= -1 EFAULT (Bad address)
424   00:01:33.697311 exit_group(1) = ?
424   00:01:33.698346 +++ exited with 1 +++

I only see that after adding debug messages in the kernel, so I guess there 
must be
a heisenbug somehere.

Anyway, indeed, I see (another kernel debug message):

__do_sys_clock_gettime: Returning -EFAULT on address 0x7bacc9a8

So udhcpc doesn't even try to read the reply because it crashes after sendto()
when trying to read the current time. Unless I am missing something, that means
that the problem happens somewhere on the send side.

To make things even more interesting, it looks like the failing system call
isn't always clock_gettime().

Guenter


Re: Linux 5.12-rc7

2021-04-12 Thread Eric Dumazet
On Mon, Apr 12, 2021 at 7:31 PM Guenter Roeck  wrote:
>
> On 4/12/21 9:31 AM, Eric Dumazet wrote:
> > On Mon, Apr 12, 2021 at 6:28 PM Linus Torvalds
> >  wrote:
> >>
> >> On Sun, Apr 11, 2021 at 10:14 PM Guenter Roeck  wrote:
> >>>
> >>> Qemu test results:
> >>> total: 460 pass: 459 fail: 1
> >>> Failed tests:
> >>> sh:rts7751r2dplus_defconfig:ata:net,virtio-net:rootfs
> >>>
> >>> The failure bisects to commit 0f6925b3e8da ("virtio_net: Do not pull 
> >>> payload in
> >>> skb->head"). It is a spurious problem - the test passes roughly every 
> >>> other
> >>> time. When the failure is seen, udhcpc fails to get an IP address and 
> >>> aborts
> >>> with SIGTERM. So far I have only seen this with the "sh" architecture.
> >>
> >> Hmm. Let's add in some more of the people involved in that commit, and
> >> also netdev.
> >>
> >> Nothing in there looks like it should have any interaction with
> >> architecture, so that "it happens on sh" sounds odd, but maybe it's
> >> some particular interaction with the qemu environment.
> >
> > Yes, maybe.
> >
> > I spent few hours on this, and suspect a buggy memcpy() implementation
> > on SH, but this was not conclusive.
> >
>
> I replaced all memcpy() calls in skbuff.h with calls to
>
> static inline void __my_memcpy(unsigned char *to, const unsigned char *from,
>unsigned int len)
> {
>while (len--)
>*to++ = *from++;
> }
>
> That made no difference, so unless you have some other memcpy() in mind that
> seems to be unlikely.


Sure, note I also had :

diff --git a/net/core/dev.c b/net/core/dev.c
index 
af8c1ea040b9364b076e2d72f04dc3de2d7e2f11..4e05a32542dd606ee8038017fea949939c0e
100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5938,7 +5938,7 @@ static void gro_pull_from_frag0(struct sk_buff
*skb, int grow)

BUG_ON(skb->end - skb->tail < grow);

-   memcpy(skb_tail_pointer(skb), NAPI_GRO_CB(skb)->frag0, grow);
+   memmove(skb_tail_pointer(skb), NAPI_GRO_CB(skb)->frag0, grow);

skb->data_len -= grow;
skb->tail += grow;
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 
c421c8f809256f7b13a8b5a1331108449353ee2d..41796dedfa9034f2333cf249a0d81b7250e14d1f
100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -2278,7 +2278,7 @@ int skb_copy_bits(const struct sk_buff *skb, int
offset, void *to, int len)
  skb_frag_off(f) + offset - start,
  copy, p, p_off, p_len, copied) {
vaddr = kmap_atomic(p);
-   memcpy(to + copied, vaddr + p_off, p_len);
+   memmove(to + copied, vaddr + p_off, p_len);
kunmap_atomic(vaddr);
}


>
> > By pulling one extra byte, the problem goes away.
> >
> > Strange thing is that the udhcpc process does not go past sendto().
> >
>
> I have been trying to debug that one. Unfortunately gdb doesn't work with sh,
> so I can't use it to debug the problem. I'll spend some more time on this 
> today.

Yes, I think this is the real issue here. This smells like some memory
corruption.

In my traces, packet is correctly received in AF_PACKET queue.

I have checked the skb is well formed.

But the user space seems to never call poll() and recvmsg() on this
af_packet socket.


Re: Linux 5.12-rc7

2021-04-12 Thread Guenter Roeck
On 4/12/21 9:31 AM, Eric Dumazet wrote:
> On Mon, Apr 12, 2021 at 6:28 PM Linus Torvalds
>  wrote:
>>
>> On Sun, Apr 11, 2021 at 10:14 PM Guenter Roeck  wrote:
>>>
>>> Qemu test results:
>>> total: 460 pass: 459 fail: 1
>>> Failed tests:
>>> sh:rts7751r2dplus_defconfig:ata:net,virtio-net:rootfs
>>>
>>> The failure bisects to commit 0f6925b3e8da ("virtio_net: Do not pull 
>>> payload in
>>> skb->head"). It is a spurious problem - the test passes roughly every other
>>> time. When the failure is seen, udhcpc fails to get an IP address and aborts
>>> with SIGTERM. So far I have only seen this with the "sh" architecture.
>>
>> Hmm. Let's add in some more of the people involved in that commit, and
>> also netdev.
>>
>> Nothing in there looks like it should have any interaction with
>> architecture, so that "it happens on sh" sounds odd, but maybe it's
>> some particular interaction with the qemu environment.
> 
> Yes, maybe.
> 
> I spent few hours on this, and suspect a buggy memcpy() implementation
> on SH, but this was not conclusive.
> 

I replaced all memcpy() calls in skbuff.h with calls to

static inline void __my_memcpy(unsigned char *to, const unsigned char *from,
   unsigned int len)
{
   while (len--)
   *to++ = *from++;
}

That made no difference, so unless you have some other memcpy() in mind that
seems to be unlikely.

> By pulling one extra byte, the problem goes away.
> 
> Strange thing is that the udhcpc process does not go past sendto().
> 

I have been trying to debug that one. Unfortunately gdb doesn't work with sh,
so I can't use it to debug the problem. I'll spend some more time on this today.

Thanks,
Guenter


Re: Linux 5.12-rc7

2021-04-12 Thread Eric Dumazet
On Mon, Apr 12, 2021 at 6:31 PM Eric Dumazet  wrote:
>
> On Mon, Apr 12, 2021 at 6:28 PM Linus Torvalds
>  wrote:
> >
> > On Sun, Apr 11, 2021 at 10:14 PM Guenter Roeck  wrote:
> > >
> > > Qemu test results:
> > > total: 460 pass: 459 fail: 1
> > > Failed tests:
> > > sh:rts7751r2dplus_defconfig:ata:net,virtio-net:rootfs
> > >
> > > The failure bisects to commit 0f6925b3e8da ("virtio_net: Do not pull 
> > > payload in
> > > skb->head"). It is a spurious problem - the test passes roughly every 
> > > other
> > > time. When the failure is seen, udhcpc fails to get an IP address and 
> > > aborts
> > > with SIGTERM. So far I have only seen this with the "sh" architecture.
> >
> > Hmm. Let's add in some more of the people involved in that commit, and
> > also netdev.
> >
> > Nothing in there looks like it should have any interaction with
> > architecture, so that "it happens on sh" sounds odd, but maybe it's
> > some particular interaction with the qemu environment.
>
> Yes, maybe.
>
> I spent few hours on this, and suspect a buggy memcpy() implementation
> on SH, but this was not conclusive.
>
> By pulling one extra byte, the problem goes away.
>
> Strange thing is that the udhcpc process does not go past sendto().

This is the patch working around the issue. Unfortunately I was not
able to root-cause it (I really suspect something on SH)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 
0824e6999e49957f7aaf7c990f6259792d42f32b..fd890a951beea03bdf24406809042666eb972655
100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -408,11 +408,17 @@ static struct sk_buff *page_to_skb(struct
virtnet_info *vi,

/* Copy all frame if it fits skb->head, otherwise
 * we let virtio_net_hdr_to_skb() and GRO pull headers as needed.
+*
+* Apparently, pulling only the Ethernet Header triggers a bug
on qemu-system-sh4.
+* Since GRO aggregation really cares of IPv4/IPv6, pull 20 bytes
+* more to work around this bug : These 20 bytes can not belong
+* to UDP/TCP payload.
+* As a bonus, this makes GRO slightly faster for IPv4 (one less copy).
 */
if (len <= skb_tailroom(skb))
copy = len;
else
-   copy = ETH_HLEN + metasize;
+   copy = ETH_HLEN + sizeof(struct iphdr) + metasize;
skb_put_data(skb, p, copy);

if (metasize) {


Re: Linux 5.12-rc7

2021-04-12 Thread Eric Dumazet
On Mon, Apr 12, 2021 at 6:28 PM Linus Torvalds
 wrote:
>
> On Sun, Apr 11, 2021 at 10:14 PM Guenter Roeck  wrote:
> >
> > Qemu test results:
> > total: 460 pass: 459 fail: 1
> > Failed tests:
> > sh:rts7751r2dplus_defconfig:ata:net,virtio-net:rootfs
> >
> > The failure bisects to commit 0f6925b3e8da ("virtio_net: Do not pull 
> > payload in
> > skb->head"). It is a spurious problem - the test passes roughly every other
> > time. When the failure is seen, udhcpc fails to get an IP address and aborts
> > with SIGTERM. So far I have only seen this with the "sh" architecture.
>
> Hmm. Let's add in some more of the people involved in that commit, and
> also netdev.
>
> Nothing in there looks like it should have any interaction with
> architecture, so that "it happens on sh" sounds odd, but maybe it's
> some particular interaction with the qemu environment.

Yes, maybe.

I spent few hours on this, and suspect a buggy memcpy() implementation
on SH, but this was not conclusive.

By pulling one extra byte, the problem goes away.

Strange thing is that the udhcpc process does not go past sendto().


Re: Linux 5.12-rc7

2021-04-12 Thread Linus Torvalds
On Sun, Apr 11, 2021 at 10:14 PM Guenter Roeck  wrote:
>
> Qemu test results:
> total: 460 pass: 459 fail: 1
> Failed tests:
> sh:rts7751r2dplus_defconfig:ata:net,virtio-net:rootfs
>
> The failure bisects to commit 0f6925b3e8da ("virtio_net: Do not pull payload 
> in
> skb->head"). It is a spurious problem - the test passes roughly every other
> time. When the failure is seen, udhcpc fails to get an IP address and aborts
> with SIGTERM. So far I have only seen this with the "sh" architecture.

Hmm. Let's add in some more of the people involved in that commit, and
also netdev.

Nothing in there looks like it should have any interaction with
architecture, so that "it happens on sh" sounds odd, but maybe it's
some particular interaction with the qemu environment.

 Linus


Re: Linux 5.12-rc7

2021-04-12 Thread Michael S. Tsirkin
On Mon, Apr 12, 2021 at 09:28:28AM -0700, Linus Torvalds wrote:
> On Sun, Apr 11, 2021 at 10:14 PM Guenter Roeck  wrote:
> >
> > Qemu test results:
> > total: 460 pass: 459 fail: 1
> > Failed tests:
> > sh:rts7751r2dplus_defconfig:ata:net,virtio-net:rootfs
> >
> > The failure bisects to commit 0f6925b3e8da ("virtio_net: Do not pull 
> > payload in
> > skb->head"). It is a spurious problem - the test passes roughly every other
> > time. When the failure is seen, udhcpc fails to get an IP address and aborts
> > with SIGTERM. So far I have only seen this with the "sh" architecture.
> 
> Hmm. Let's add in some more of the people involved in that commit, and
> also netdev.
> 
> Nothing in there looks like it should have any interaction with
> architecture, so that "it happens on sh" sounds odd, but maybe it's
> some particular interaction with the qemu environment.
> 
>  Linus

Yea Eric's been trying to debug this already. Let's give him a bit more time...

-- 
MST



Re: Linux 5.12-rc7

2021-04-11 Thread Guenter Roeck
On Sun, Apr 11, 2021 at 03:41:18PM -0700, Linus Torvalds wrote:
> Oh well. rc5 was big. rc6 was small. And now rc7 is big again. In
> fact, it's the biggest rc7 (at least in number of commits) we've had
> in the 5.x series.
> 
> It's mostly due to networking fixes (of which rc6 had none), and none
> of them should be all that scary, but it's never great when we have
> such a big rc. It's particularly annoying at the end of the release
> window like this.
> 
> End result: I'm still waffling about the final 5.12 release.  The fact
> that we have a big rc7 does make me think that I'll probably do an rc8
> this time around. But it ends up depending a bit on how the upcoming
> week goes, and if things are deathly quiet, I may end up deciding that
> an rc8 doesn't really make sense.
> 
> So we'll see.
> 
> Anyway, networking (both core and drivers) is over half of the rc7
> patch, with the rest being a fairly random collection of fixes all
> over. We've got other driver updates (sound, rdma, scsi, usb..) some
> fs fixes (io_uring, umount, btrfs, cifs, ocfs), minor arch fixes (arc,
> arm, parisc, powerpc, s390, x86), and other misc fixes.
> 
> The shortlog is appended, although it's obviously not as nice and
> small and readable as I'd have liked at this point in the release..
> 
> Please do test,
> 

Build results:
total: 151 pass: 151 fail: 0
Qemu test results:
total: 460 pass: 459 fail: 1
Failed tests:
sh:rts7751r2dplus_defconfig:ata:net,virtio-net:rootfs

The failure bisects to commit 0f6925b3e8da ("virtio_net: Do not pull payload in
skb->head"). It is a spurious problem - the test passes roughly every other
time. When the failure is seen, udhcpc fails to get an IP address and aborts
with SIGTERM. So far I have only seen this with the "sh" architecture.

Guenter


Re: Linux 5.12-rc7

2021-04-11 Thread Ronald Warsow

Hallo

thanks all for the new kernel.

however, I see the following since linux-5.12-rc6 (when I started
testing 5.12)

I'm, as a non-developer, quiet not sure what the warning mean or better
how important they need to be taken, cause the box run's fine, so far.

I'm not sure if the warnings, etc. are only caused by the new compiler,
what is: gcc version 11.0.1 20210405 (Red Hat 11.0.1-0) (GCC) on an
Fedora 34 (beta) box or if it's something else.


during compilation:
===

...
In function ‘poly1305_simd_init’,
inlined from ‘crypto_poly1305_setdctxkey’ at
arch/x86/crypto/poly1305_glue.c:150:4,
inlined from ‘crypto_poly1305_setdctxkey’ at
arch/x86/crypto/poly1305_glue.c:144:21,
inlined from ‘poly1305_update_arch’ at
arch/x86/crypto/poly1305_glue.c:181:8:
arch/x86/crypto/poly1305_glue.c:86:9: warning: ‘poly1305_init_x86_64’
reading 32 bytes from a region of size 16 [-Wstringop-overread]
   86 | poly1305_init_x86_64(ctx, key);
  | ^~
arch/x86/crypto/poly1305_glue.c: In function ‘poly1305_update_arch’:
arch/x86/crypto/poly1305_glue.c:86:9: note: referencing argument 2 of
type ‘const u8 *’ {aka ‘const unsigned char *’}
arch/x86/crypto/poly1305_glue.c:18:17: note: in a call to function
‘poly1305_init_x86_64’
   18 | asmlinkage void poly1305_init_x86_64(void *ctx,
  | ^~~~
...

  CC  security/selinux/ss/sidtab.o
In file included from ./include/linux/string.h:269,
 from ./include/linux/bitmap.h:9,
 from ./include/linux/cpumask.h:12,
 from ./arch/x86/include/asm/cpumask.h:5,
 from ./arch/x86/include/asm/msr.h:11,
 from ./arch/x86/include/asm/processor.h:22,
 from ./arch/x86/include/asm/cpufeature.h:5,
 from ./arch/x86/include/asm/thread_info.h:53,
 from ./include/linux/thread_info.h:59,
 from ./arch/x86/include/asm/preempt.h:7,
 from ./include/linux/preempt.h:78,
 from ./include/linux/rcupdate.h:27,
 from ./include/linux/rbtree.h:22,
 from ./include/linux/iova.h:14,
 from ./include/linux/intel-iommu.h:14,
 from arch/x86/kernel/tboot.c:9:
In function ‘memcmp’,
inlined from ‘tboot_probe’ at arch/x86/kernel/tboot.c:70:6:
./include/linux/fortify-string.h:19:33: warning: ‘__builtin_memcmp_eq’
specified bound 16 exceeds source size 0 [-Wstringop-overread]
   19 | #define __underlying_memcmp __builtin_memcmp
  | ^
./include/linux/fortify-string.h:235:16: note: in expansion of macro
‘__underlying_memcmp’
  235 | return __underlying_memcmp(p, q, size);
  |^~~
...

  CC  net/core/fib_rules.o
In function ‘snb_wm_latency_quirk’,
inlined from ‘ilk_setup_wm_latency’ at
drivers/gpu/drm/i915/intel_pm.c:3108:3,
inlined from ‘intel_init_pm’ at drivers/gpu/drm/i915/intel_pm.c:7628:3:
drivers/gpu/drm/i915/intel_pm.c:3057:9: warning:
‘intel_print_wm_latency’ reading 16 bytes from a region of size 10
[-Wstringop-overread]
 3057 | intel_print_wm_latency(dev_priv, "Primary",
dev_priv->wm.pri_latency);
  |
^
drivers/gpu/drm/i915/intel_pm.c: In function ‘intel_init_pm’:
drivers/gpu/drm/i915/intel_pm.c:3057:9: note: referencing argument 3 of
type ‘const u16 *’ {aka ‘const short unsigned int *’}
drivers/gpu/drm/i915/intel_pm.c:2994:13: note: in a call to function
‘intel_print_wm_latency’
 2994 | static void intel_print_wm_latency(struct drm_i915_private
*dev_priv,
  | ^~
In function ‘snb_wm_latency_quirk’,
inlined from ‘ilk_setup_wm_latency’ at
drivers/gpu/drm/i915/intel_pm.c:3108:3,
inlined from ‘intel_init_pm’ at drivers/gpu/drm/i915/intel_pm.c:7628:3:
drivers/gpu/drm/i915/intel_pm.c:3058:9: warning:
‘intel_print_wm_latency’ reading 16 bytes from a region of size 10
[-Wstringop-overread]
 3058 | intel_print_wm_latency(dev_priv, "Sprite",
dev_priv->wm.spr_latency);
  |
^~~~
drivers/gpu/drm/i915/intel_pm.c: In function ‘intel_init_pm’:
drivers/gpu/drm/i915/intel_pm.c:3058:9: note: referencing argument 3 of
type ‘const u16 *’ {aka ‘const short unsigned int *’}
drivers/gpu/drm/i915/intel_pm.c:2994:13: note: in a call to function
‘intel_print_wm_latency’
 2994 | static void intel_print_wm_latency(struct drm_i915_private
*dev_priv,
  | ^~
In function ‘snb_wm_latency_quirk’,
inlined from ‘ilk_setup_wm_latency’ at
drivers/gpu/drm/i915/intel_pm.c:3108:3,
inlined from ‘intel_init_pm’ at drivers/gpu/drm/i915/intel_pm.c:7628:3:
drivers/gpu/drm/i915/intel_pm.c:3059:9: warning:
‘intel_print_wm_latency’ reading 16 bytes from a region of size 10
[-Wstringop-overread]