[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAGRyCJEzg2gFCf3svgKGSv5+W4QRsVhbYQ+KZoEfvw_=2Rb+Zg@mail.gmail.com>
Date: Tue, 20 Dec 2022 15:28:16 +0100
From: Daniele Palmas <dnlplm@...il.com>
To: Paolo Abeni <pabeni@...hat.com>
Cc: David Miller <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>,
Eric Dumazet <edumazet@...gle.com>,
Subash Abhinov Kasiviswanathan <quic_subashab@...cinc.com>,
Sean Tranchetti <quic_stranche@...cinc.com>,
Jonathan Corbet <corbet@....net>,
Alexander Lobakin <alexandr.lobakin@...el.com>,
Gal Pressman <gal@...dia.com>,
Bjørn Mork <bjorn@...k.no>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Dave Taht <dave.taht@...il.com>, netdev@...r.kernel.org
Subject: Re: [PATCH net-next v3 2/3] net: qualcomm: rmnet: add tx packets aggregation
Hello Paolo,
Il giorno ven 9 dic 2022 alle ore 08:38 Daniele Palmas
<dnlplm@...il.com> ha scritto:
>
> Il giorno mer 7 dic 2022 alle ore 13:46 Paolo Abeni
> <pabeni@...hat.com> ha scritto:
> > > +static void rmnet_map_flush_tx_packet_work(struct work_struct *work)
> > > +{
> > > + struct sk_buff *skb = NULL;
> > > + struct rmnet_port *port;
> > > +
> > > + port = container_of(work, struct rmnet_port, agg_wq);
> > > +
> > > + spin_lock_bh(&port->agg_lock);
> > > + if (likely(port->agg_state == -EINPROGRESS)) {
> > > + /* Buffer may have already been shipped out */
> > > + if (likely(port->skbagg_head)) {
> > > + skb = port->skbagg_head;
> > > + reset_aggr_params(port);
> > > + }
> > > + port->agg_state = 0;
> > > + }
> > > +
> > > + spin_unlock_bh(&port->agg_lock);
> > > + if (skb)
> > > + rmnet_send_skb(port, skb);
> > > +}
> > > +
> > > +static enum hrtimer_restart rmnet_map_flush_tx_packet_queue(struct hrtimer *t)
> > > +{
> > > + struct rmnet_port *port;
> > > +
> > > + port = container_of(t, struct rmnet_port, hrtimer);
> > > +
> > > + schedule_work(&port->agg_wq);
> >
> > Why you need to schedule a work and you can't instead call the core of
> > rmnet_map_flush_tx_packet_work() here? it looks like the latter does
> > not need process context...
> >
>
> Ack.
>
looks like removing the work is not as straightforward as I thought.
Now the timer cb has become:
static enum hrtimer_restart rmnet_map_flush_tx_packet_cb(struct hrtimer *t)
{
struct sk_buff *skb = NULL;
struct rmnet_port *port;
port = container_of(t, struct rmnet_port, hrtimer);
spin_lock_bh(&port->agg_lock);
if (likely(port->agg_state == -EINPROGRESS)) {
/* Buffer may have already been shipped out */
if (likely(port->skbagg_head)) {
skb = port->skbagg_head;
reset_aggr_params(port);
}
port->agg_state = 0;
}
spin_unlock_bh(&port->agg_lock);
if (skb)
rmnet_send_skb(port, skb);
return HRTIMER_NORESTART;
}
but this is causing the following warning:
[ 3106.701296] WARNING: CPU: 15 PID: 0 at kernel/softirq.c:375
__local_bh_enable_ip+0x54/0x70
...
[ 3106.701537] CPU: 15 PID: 0 Comm: swapper/15 Tainted: G OE
6.1.0-rc5-rmnet-v4-warn #1
[ 3106.701543] Hardware name: LENOVO 30DH00H2IX/1048, BIOS S08KT40A 08/23/2021
[ 3106.701546] RIP: 0010:__local_bh_enable_ip+0x54/0x70
[ 3106.701554] Code: a9 00 ff ff 00 74 27 65 ff 0d 08 bb 75 61 65 8b
05 01 bb 75 61 85 c0 74 06 5d c3 cc cc cc cc 0f 1f 44 00 00 5d c3 cc
cc cc cc <0f> 0b eb bf 65 66 8b 05 e0 ca 76 61 66 85 c0 74 cc e8 e6 fd
ff ff
[ 3106.701559] RSP: 0018:ffffb8aa80510ec8 EFLAGS: 00010006
[ 3106.701564] RAX: 0000000080010202 RBX: ffff932d7b687868 RCX: 0000000000000000
[ 3106.701569] RDX: 0000000000000001 RSI: 0000000000000201 RDI: ffffffffc0bd5f7c
[ 3106.701573] RBP: ffffb8aa80510ec8 R08: ffff933bdc3e31a0 R09: 000002d355c2f99d
[ 3106.701576] R10: 0000000000000000 R11: ffffb8aa80510ff8 R12: ffff932d7b687828
[ 3106.701580] R13: ffff932d7b687000 R14: ffff932cc1a76400 R15: ffff933bdc3e3180
[ 3106.701584] FS: 0000000000000000(0000) GS:ffff933bdc3c0000(0000)
knlGS:0000000000000000
[ 3106.701589] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 3106.701593] CR2: 00007ffc26dae080 CR3: 0000000209f04003 CR4: 00000000007706e0
[ 3106.701597] PKRU: 55555554
[ 3106.701599] Call Trace:
[ 3106.701602] <IRQ>
[ 3106.701608] _raw_spin_unlock_bh+0x1d/0x30
[ 3106.701623] rmnet_map_flush_tx_packet_cb+0x4c/0x90 [rmnet]
[ 3106.701640] ? rmnet_send_skb+0x90/0x90 [rmnet]
[ 3106.701655] __hrtimer_run_queues+0x106/0x260
[ 3106.701664] hrtimer_interrupt+0x101/0x220
[ 3106.701671] __sysvec_apic_timer_interrupt+0x61/0x110
[ 3106.701677] sysvec_apic_timer_interrupt+0x7b/0x90
[ 3106.701685] </IRQ>
[ 3106.701687] <TASK>
[ 3106.701689] asm_sysvec_apic_timer_interrupt+0x1b/0x20
[ 3106.701694] RIP: 0010:cpuidle_enter_state+0xde/0x6e0
[ 3106.701704] Code: eb d6 60 e8 74 01 68 ff 8b 53 04 49 89 c7 0f 1f
44 00 00 31 ff e8 b2 23 67 ff 80 7d d0 00 0f 85 da 00 00 00 fb 0f 1f
44 00 00 <45> 85 f6 0f 88 01 02 00 00 4d 63 ee 49 83 fd 09 0f 87 b6 04
00 00
[ 3106.701709] RSP: 0018:ffffb8aa801dbe38 EFLAGS: 00000246
[ 3106.701713] RAX: ffff933bdc3f1380 RBX: ffffd8aa7fbc0700 RCX: 000000000000001f
[ 3106.701717] RDX: 000000000000000f RSI: 0000000000000002 RDI: 0000000000000000
[ 3106.701720] RBP: ffffb8aa801dbe88 R08: 000002d355d34146 R09: 000000000006e988
[ 3106.701723] R10: 0000000000000004 R11: 071c71c71c71c71c R12: ffffffffa04ba5c0
[ 3106.701727] R13: 0000000000000001 R14: 0000000000000001 R15: 000002d355d34146
[ 3106.701735] ? cpuidle_enter_state+0xce/0x6e0
[ 3106.701744] cpuidle_enter+0x2e/0x50
[ 3106.701751] do_idle+0x204/0x290
[ 3106.701758] cpu_startup_entry+0x20/0x30
[ 3106.701763] start_secondary+0x122/0x160
[ 3106.701773] secondary_startup_64_no_verify+0xe5/0xeb
[ 3106.701784] </TASK>
The reason is not obvious to me, so I need to dig further...
Regards,
Daniele
Powered by blists - more mailing lists