* Re: WARNING: ODEBUG bug in netdev_freemem (2)
From: Thomas Gleixner @ 2019-06-24 17:27 UTC (permalink / raw)
To: Dmitry Vyukov
Cc: Eric Dumazet, syzbot, Alexander Duyck, amritha.nambiar,
Andy Shevchenko, David Miller, Dmitry Torokhov, Florian Fainelli,
Greg Kroah-Hartman, Ido Schimmel, LKML, netdev, syzkaller-bugs,
tyhicks, wanghai26, yuehaibing
In-Reply-To: <alpine.DEB.2.21.1906241433020.32342@nanos.tec.linutronix.de>
On Mon, 24 Jun 2019, Thomas Gleixner wrote:
> On Mon, 24 Jun 2019, Dmitry Vyukov wrote:
> > On Mon, Jun 24, 2019 at 2:08 PM Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > > >>> ------------[ cut here ]------------
> > > >>> ODEBUG: free active (active state 0) object type: timer_list hint:
> > > >>> delayed_work_timer_fn+0x0/0x90 arch/x86/include/asm/paravirt.h:767
> > > >>
> > > >> One of the cleaned up devices has left an active timer which belongs to a
> > > >> delayed work. That's all I can decode out of that splat. :(
> > > >
> > > > Hi Thomas,
> > > >
> > > > If ODEBUG would memorize full stack traces for object allocation
> > > > (using lib/stackdepot.c), it would make this splat actionable, right?
> > > > I've fixed https://bugzilla.kernel.org/show_bug.cgi?id=203969 for this.
> > > >
> > >
> > > Not sure this would help in this case as some netdev are allocated through a generic helper.
> > >
> > > The driver specific portion might not show up in the stack trace.
> > >
> > > It would be nice here to get the work queue function pointer,
> > > so that it gives us a clue which driver needs a fix.
>
> Hrm. Let me think about a way to achieve that after I handled that
> regression which is on my desk.
Here is a quick and dirty hack which solves the issue at least for all run
time initialized delayed work objects. Here is the output of a test I
whipped up for this:
OBJ: Init delayed work, arm timer
OBJ: Leak timer
ODEBUG: free active (active state 0) object type: timer_list hint: delayed_work_timer_fn+0x0/0x20 chint: foo_fun+0x0/0x17
chint is the debug object hint of the compound object, i.e. the work
function 'foo_fun'.
Yes, naming sucks and there is still the option to use the existing
debug_obj::astate mechanics, but I was not able to wrap my head around all
the nasty corner cases which the workqueue code provides quickly. Needs
more thought.
Anyway, this should definitely help to diagnose the issue at hand.
Thanks,
tglx
8<--------------------
include/linux/debugobjects.h | 10 ++++++++++
include/linux/workqueue.h | 26 ++++++++++++++++----------
kernel/workqueue.c | 9 ++++++++-
lib/debugobjects.c | 43 +++++++++++++++++++++++++++++++++++++++++--
4 files changed, 75 insertions(+), 13 deletions(-)
--- a/include/linux/debugobjects.h
+++ b/include/linux/debugobjects.h
@@ -24,6 +24,9 @@ struct debug_obj_descr;
* @astate: current active state
* @object: pointer to the real object
* @descr: pointer to an object type specific debug description structure
+ * @comp_addr: pointer to a compound object which is glued with @object
+ * @comp_descr: pointer to a compound object type specific debug description
+ * structure
*/
struct debug_obj {
struct hlist_node node;
@@ -31,6 +34,8 @@ struct debug_obj {
unsigned int astate;
void *object;
struct debug_obj_descr *descr;
+ void *comp_addr;
+ struct debug_obj_descr *comp_descr;
};
/**
@@ -82,6 +87,9 @@ extern void
debug_object_active_state(void *addr, struct debug_obj_descr *descr,
unsigned int expect, unsigned int next);
+extern void debug_object_set_compound(void *addr, void *comp_addr,
+ struct debug_obj_descr *comp_descr);
+
extern void debug_objects_early_init(void);
extern void debug_objects_mem_init(void);
#else
@@ -99,6 +107,8 @@ static inline void
debug_object_free (void *addr, struct debug_obj_descr *descr) { }
static inline void
debug_object_assert_init(void *addr, struct debug_obj_descr *descr) { }
+static inline void
+debug_object_set_compound(void *addr, void *ca, struct debug_obj_descr *cd) { }
static inline void debug_objects_early_init(void) { }
static inline void debug_objects_mem_init(void) { }
--- a/include/linux/workqueue.h
+++ b/include/linux/workqueue.h
@@ -204,7 +204,7 @@ struct execute_work {
struct delayed_work n = __DELAYED_WORK_INITIALIZER(n, f, TIMER_DEFERRABLE)
#ifdef CONFIG_DEBUG_OBJECTS_WORK
-extern void __init_work(struct work_struct *work, int onstack);
+extern void __init_work(struct work_struct *work, int onstack, bool delayed);
extern void destroy_work_on_stack(struct work_struct *work);
extern void destroy_delayed_work_on_stack(struct delayed_work *work);
static inline unsigned int work_static(struct work_struct *work)
@@ -212,7 +212,7 @@ static inline unsigned int work_static(s
return *work_data_bits(work) & WORK_STRUCT_STATIC;
}
#else
-static inline void __init_work(struct work_struct *work, int onstack) { }
+static inline void __init_work(struct work_struct *work, int onstack, bool delayed) { }
static inline void destroy_work_on_stack(struct work_struct *work) { }
static inline void destroy_delayed_work_on_stack(struct delayed_work *work) { }
static inline unsigned int work_static(struct work_struct *work) { return 0; }
@@ -226,20 +226,20 @@ static inline unsigned int work_static(s
* to generate better code.
*/
#ifdef CONFIG_LOCKDEP
-#define __INIT_WORK(_work, _func, _onstack) \
+#define __INIT_WORK(_work, _func, _onstack, _delayed) \
do { \
static struct lock_class_key __key; \
\
- __init_work((_work), _onstack); \
+ __init_work((_work), _onstack, _delayed); \
(_work)->data = (atomic_long_t) WORK_DATA_INIT(); \
lockdep_init_map(&(_work)->lockdep_map, "(work_completion)"#_work, &__key, 0); \
INIT_LIST_HEAD(&(_work)->entry); \
(_work)->func = (_func); \
} while (0)
#else
-#define __INIT_WORK(_work, _func, _onstack) \
+#define __INIT_WORK(_work, _func, _onstack, _delayed) \
do { \
- __init_work((_work), _onstack); \
+ __init_work((_work), _onstack, _delayed); \
(_work)->data = (atomic_long_t) WORK_DATA_INIT(); \
INIT_LIST_HEAD(&(_work)->entry); \
(_work)->func = (_func); \
@@ -247,25 +247,31 @@ static inline unsigned int work_static(s
#endif
#define INIT_WORK(_work, _func) \
- __INIT_WORK((_work), (_func), 0)
+ __INIT_WORK((_work), (_func), 0, 0)
#define INIT_WORK_ONSTACK(_work, _func) \
- __INIT_WORK((_work), (_func), 1)
+ __INIT_WORK((_work), (_func), 1, 0)
+
+#define __INIT_DWORK(_work, _func) \
+ __INIT_WORK((_work), (_func), 0, 1)
+
+#define __INIT_DWORK_ONSTACK(_work, _func) \
+ __INIT_WORK((_work), (_func), 1, 1)
#define __INIT_DELAYED_WORK(_work, _func, _tflags) \
do { \
- INIT_WORK(&(_work)->work, (_func)); \
__init_timer(&(_work)->timer, \
delayed_work_timer_fn, \
(_tflags) | TIMER_IRQSAFE); \
+ __INIT_DWORK(&(_work)->work, (_func)); \
} while (0)
#define __INIT_DELAYED_WORK_ONSTACK(_work, _func, _tflags) \
do { \
- INIT_WORK_ONSTACK(&(_work)->work, (_func)); \
__init_timer_on_stack(&(_work)->timer, \
delayed_work_timer_fn, \
(_tflags) | TIMER_IRQSAFE); \
+ __INIT_DWORK_ONSTACK(&(_work)->work, (_func)); \
} while (0)
#define INIT_DELAYED_WORK(_work, _func) \
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -499,12 +499,19 @@ static inline void debug_work_deactivate
debug_object_deactivate(work, &work_debug_descr);
}
-void __init_work(struct work_struct *work, int onstack)
+void __init_work(struct work_struct *work, int onstack, bool delayed)
{
if (onstack)
debug_object_init_on_stack(work, &work_debug_descr);
else
debug_object_init(work, &work_debug_descr);
+
+ if (delayed) {
+ struct delayed_work *dwork = to_delayed_work(work);
+
+ debug_object_set_compound(&dwork->timer, work,
+ &work_debug_descr);
+ }
}
EXPORT_SYMBOL_GPL(__init_work);
--- a/lib/debugobjects.c
+++ b/lib/debugobjects.c
@@ -179,6 +179,8 @@ alloc_object(void *addr, struct debug_bu
obj->descr = descr;
obj->state = ODEBUG_STATE_NONE;
obj->astate = 0;
+ obj->comp_addr = NULL;
+ obj->comp_descr = NULL;
hlist_del(&obj->node);
hlist_add_head(&obj->node, &b->list);
@@ -321,11 +323,17 @@ static void debug_print_object(struct de
if (limit < 5 && descr != descr_test) {
void *hint = descr->debug_hint ?
descr->debug_hint(obj->object) : NULL;
+ void *chint = NULL;
+
+ /* Get a hint about a compound object */
+ if (obj->comp_descr && obj->comp_descr->debug_hint)
+ chint = obj->comp_descr->debug_hint(obj->comp_addr);
+
limit++;
WARN(1, KERN_ERR "ODEBUG: %s %s (active state %u) "
- "object type: %s hint: %pS\n",
+ "object type: %s hint: %pS chint: %pS\n",
msg, obj_states[obj->state], obj->astate,
- descr->name, hint);
+ descr->name, hint, chint);
}
debug_objects_warnings++;
}
@@ -448,6 +456,37 @@ void debug_object_init_on_stack(void *ad
EXPORT_SYMBOL_GPL(debug_object_init_on_stack);
/**
+ * debug_object_set_compound - Set a pointer to a compund object
+ * @addr: address of the object
+ * @comp_addr: pointer to the compound object related to @addr
+ * @comp_descr: pointer to an object specific debug description structure for
+ * @comp_addr
+ *
+ * Useful for delayed work and similar constructs where the
+ * debug_obj::astate tracking would be complex to achieve.
+ */
+void debug_object_set_compound(void *addr, void *comp_addr,
+ struct debug_obj_descr *comp_descr)
+{
+ struct debug_bucket *db;
+ struct debug_obj *obj;
+ unsigned long flags;
+
+ if (!debug_objects_enabled)
+ return;
+
+ db = get_bucket((unsigned long) addr);
+
+ raw_spin_lock_irqsave(&db->lock, flags);
+ obj = lookup_object(addr, db);
+ if (obj) {
+ obj->comp_addr = comp_addr;
+ obj->comp_descr = comp_descr;
+ }
+ raw_spin_unlock_irqrestore(&db->lock, flags);
+}
+
+/**
* debug_object_activate - debug checks when an object is activated
* @addr: address of the object
* @descr: pointer to an object specific debug description structure
^ permalink raw reply
* Re: [PATCH v7 0/6] Add support for Orange Pi 3
From: David Miller @ 2019-06-24 17:29 UTC (permalink / raw)
To: megous
Cc: linux-sunxi, maxime.ripard, wens, robh+dt, jernej.skrabec,
airlied, daniel, mark.rutland, peppe.cavallaro, alexandre.torgue,
joabreu, mcoquelin.stm32, dri-devel, devicetree, linux-arm-kernel,
linux-kernel, netdev, linux-stm32
In-Reply-To: <20190620134748.17866-1-megous@megous.com>
From: megous@megous.com
Date: Thu, 20 Jun 2019 15:47:42 +0200
> From: Ondrej Jirman <megous@megous.com>
>
> This series implements support for Xunlong Orange Pi 3 board.
>
> - ethernet support (patches 1-3)
> - HDMI support (patches 4-6)
>
> For some people, ethernet doesn't work after reboot (but works on cold
> boot), when the stmmac driver is built into the kernel. It works when
> the driver is built as a module. It's either some timing issue, or power
> supply issue or a combination of both. Module build induces a power
> cycling of the phy.
>
> I encourage people with this issue, to build the driver into the kernel,
> and try to alter the reset timings for the phy in DTS or
> startup-delay-us and report the findings.
This is a mixture of networking and non-networking changes so it really
can't go through my tree.
I wonder how you expect this series to be merged?
Thanks.
^ permalink raw reply
* [PATCH v3 net-next 0/4] cxgb4: Reference count MPS TCAM entries within a PF
From: Raju Rangoju @ 2019-06-24 17:35 UTC (permalink / raw)
To: netdev, davem; +Cc: nirranjan, dt, rajur
Firmware reference counts the MPS TCAM entries by PF and VF,
but it does not do it for usage within a PF or VF. This patch
adds the support to track MPS TCAM entries within a PF.
v2->v3:
Fixed the compiler errors due to incorrect patch
Also, removed the new blank line at EOF
v1->v2:
Use refcount_t type instead of atomic_t for mps reference count
Raju Rangoju (4):
cxgb4: Re-work the logic for mps refcounting
cxgb4: Add MPS TCAM refcounting for raw mac filters
cxgb4: Add MPS TCAM refcounting for cxgb4 change mac
cxgb4: Add MPS refcounting for alloc/free mac filters
drivers/net/ethernet/chelsio/cxgb4/Makefile | 2 +-
drivers/net/ethernet/chelsio/cxgb4/cxgb4.h | 53 ++++-
drivers/net/ethernet/chelsio/cxgb4/cxgb4_filter.c | 8 +-
drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 40 ++--
drivers/net/ethernet/chelsio/cxgb4/cxgb4_mps.c | 241 ++++++++++++++++++++++
5 files changed, 312 insertions(+), 32 deletions(-)
create mode 100644 drivers/net/ethernet/chelsio/cxgb4/cxgb4_mps.c
--
2.12.0
^ permalink raw reply
* [PATCH v3 net-next 1/4] cxgb4: Re-work the logic for mps refcounting
From: Raju Rangoju @ 2019-06-24 17:35 UTC (permalink / raw)
To: netdev, davem; +Cc: nirranjan, dt, rajur
In-Reply-To: <20190624173535.12572-1-rajur@chelsio.com>
Remove existing mps refcounting code which was
added only for encap filters and add necessary
data structures/functions to support mps reference
counting for all the mac filters. Also add wrapper
functions for allocating and freeing encap mac
filters.
Signed-off-by: Raju Rangoju <rajur@chelsio.com>
---
drivers/net/ethernet/chelsio/cxgb4/Makefile | 2 +-
drivers/net/ethernet/chelsio/cxgb4/cxgb4.h | 24 ++++-
drivers/net/ethernet/chelsio/cxgb4/cxgb4_filter.c | 8 +-
drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 14 +--
drivers/net/ethernet/chelsio/cxgb4/cxgb4_mps.c | 108 ++++++++++++++++++++++
5 files changed, 134 insertions(+), 22 deletions(-)
create mode 100644 drivers/net/ethernet/chelsio/cxgb4/cxgb4_mps.c
diff --git a/drivers/net/ethernet/chelsio/cxgb4/Makefile b/drivers/net/ethernet/chelsio/cxgb4/Makefile
index 91d8a885deba..20390f6afbb4 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/Makefile
+++ b/drivers/net/ethernet/chelsio/cxgb4/Makefile
@@ -7,7 +7,7 @@ obj-$(CONFIG_CHELSIO_T4) += cxgb4.o
cxgb4-objs := cxgb4_main.o l2t.o smt.o t4_hw.o sge.o clip_tbl.o cxgb4_ethtool.o \
cxgb4_uld.o srq.o sched.o cxgb4_filter.o cxgb4_tc_u32.o \
- cxgb4_ptp.o cxgb4_tc_flower.o cxgb4_cudbg.o \
+ cxgb4_ptp.o cxgb4_tc_flower.o cxgb4_cudbg.o cxgb4_mps.o \
cudbg_common.o cudbg_lib.o cudbg_zlib.o
cxgb4-$(CONFIG_CHELSIO_T4_DCB) += cxgb4_dcb.o
cxgb4-$(CONFIG_CHELSIO_T4_FCOE) += cxgb4_fcoe.o
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h b/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
index db2ec46ba6b6..39ccd4c64d48 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
@@ -905,10 +905,6 @@ struct mbox_list {
struct list_head list;
};
-struct mps_encap_entry {
- atomic_t refcnt;
-};
-
#if IS_ENABLED(CONFIG_THERMAL)
struct ch_thermal {
struct thermal_zone_device *tzdev;
@@ -917,6 +913,14 @@ struct ch_thermal {
};
#endif
+struct mps_entries_ref {
+ struct list_head list;
+ u8 addr[ETH_ALEN];
+ u8 mask[ETH_ALEN];
+ u16 idx;
+ refcount_t refcnt;
+};
+
struct adapter {
void __iomem *regs;
void __iomem *bar2;
@@ -969,7 +973,6 @@ struct adapter {
unsigned int rawf_start;
unsigned int rawf_cnt;
struct smt_data *smt;
- struct mps_encap_entry *mps_encap;
struct cxgb4_uld_info *uld;
void *uld_handle[CXGB4_ULD_MAX];
unsigned int num_uld;
@@ -977,6 +980,8 @@ struct adapter {
struct list_head list_node;
struct list_head rcu_node;
struct list_head mac_hlist; /* list of MAC addresses in MPS Hash */
+ struct list_head mps_ref;
+ spinlock_t mps_ref_lock; /* lock for syncing mps ref/def activities */
void *iscsi_ppm;
@@ -1906,4 +1911,13 @@ int cxgb4_set_msix_aff(struct adapter *adap, unsigned short vec,
cpumask_var_t *aff_mask, int idx);
void cxgb4_clear_msix_aff(unsigned short vec, cpumask_var_t aff_mask);
+int cxgb4_init_mps_ref_entries(struct adapter *adap);
+void cxgb4_free_mps_ref_entries(struct adapter *adap);
+int cxgb4_alloc_encap_mac_filt(struct adapter *adap, unsigned int viid,
+ const u8 *addr, const u8 *mask,
+ unsigned int vni, unsigned int vni_mask,
+ u8 dip_hit, u8 lookup_type, bool sleep_ok);
+int cxgb4_free_encap_mac_filt(struct adapter *adap, unsigned int viid,
+ int idx, bool sleep_ok);
+
#endif /* __CXGB4_H__ */
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_filter.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_filter.c
index 6232236d7abc..43b0f8c57da7 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_filter.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_filter.c
@@ -727,10 +727,8 @@ void clear_filter(struct adapter *adap, struct filter_entry *f)
cxgb4_smt_release(f->smt);
if (f->fs.val.encap_vld && f->fs.val.ovlan_vld)
- if (atomic_dec_and_test(&adap->mps_encap[f->fs.val.ovlan &
- 0x1ff].refcnt))
- t4_free_encap_mac_filt(adap, pi->viid,
- f->fs.val.ovlan & 0x1ff, 0);
+ t4_free_encap_mac_filt(adap, pi->viid,
+ f->fs.val.ovlan & 0x1ff, 0);
if ((f->fs.hash || is_t6(adap->params.chip)) && f->fs.type)
cxgb4_clip_release(f->dev, (const u32 *)&f->fs.val.lip, 1);
@@ -1177,7 +1175,6 @@ static int cxgb4_set_hash_filter(struct net_device *dev,
if (ret < 0)
goto free_atid;
- atomic_inc(&adapter->mps_encap[ret].refcnt);
f->fs.val.ovlan = ret;
f->fs.mask.ovlan = 0xffff;
f->fs.val.ovlan_vld = 1;
@@ -1420,7 +1417,6 @@ int __cxgb4_set_filter(struct net_device *dev, int filter_id,
if (ret < 0)
goto free_clip;
- atomic_inc(&adapter->mps_encap[ret].refcnt);
f->fs.val.ovlan = ret;
f->fs.mask.ovlan = 0x1ff;
f->fs.val.ovlan_vld = 1;
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
index 54908002c786..4632827f05ba 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
@@ -3273,8 +3273,6 @@ static void cxgb_del_udp_tunnel(struct net_device *netdev,
i);
return;
}
- atomic_dec(&adapter->mps_encap[adapter->rawf_start +
- pi->port_id].refcnt);
}
}
@@ -3363,7 +3361,6 @@ static void cxgb_add_udp_tunnel(struct net_device *netdev,
cxgb_del_udp_tunnel(netdev, ti);
return;
}
- atomic_inc(&adapter->mps_encap[ret].refcnt);
}
}
@@ -5446,7 +5443,6 @@ static void free_some_resources(struct adapter *adapter)
{
unsigned int i;
- kvfree(adapter->mps_encap);
kvfree(adapter->smt);
kvfree(adapter->l2t);
kvfree(adapter->srq);
@@ -5972,12 +5968,6 @@ static int init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
adapter->params.offload = 0;
}
- adapter->mps_encap = kvcalloc(adapter->params.arch.mps_tcam_size,
- sizeof(struct mps_encap_entry),
- GFP_KERNEL);
- if (!adapter->mps_encap)
- dev_warn(&pdev->dev, "could not allocate MPS Encap entries, continuing\n");
-
#if IS_ENABLED(CONFIG_IPV6)
if (chip_ver <= CHELSIO_T5 &&
(!(t4_read_reg(adapter, LE_DB_CONFIG_A) & ASLIPCOMPEN_F))) {
@@ -6053,6 +6043,8 @@ static int init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
/* check for PCI Express bandwidth capabiltites */
pcie_print_link_status(pdev);
+ cxgb4_init_mps_ref_entries(adapter);
+
err = init_rss(adapter);
if (err)
goto out_free_dev;
@@ -6179,6 +6171,8 @@ static void remove_one(struct pci_dev *pdev)
disable_interrupts(adapter);
+ cxgb4_free_mps_ref_entries(adapter);
+
for_each_port(adapter, i)
if (adapter->port[i]->reg_state == NETREG_REGISTERED)
unregister_netdev(adapter->port[i]);
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_mps.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_mps.c
new file mode 100644
index 000000000000..b8a5375bf64d
--- /dev/null
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_mps.c
@@ -0,0 +1,108 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2019 Chelsio Communications, Inc. All rights reserved. */
+
+#include "cxgb4.h"
+
+static int cxgb4_mps_ref_dec(struct adapter *adap, u16 idx)
+{
+ struct mps_entries_ref *mps_entry, *tmp;
+ int ret = -EINVAL;
+
+ spin_lock(&adap->mps_ref_lock);
+ list_for_each_entry_safe(mps_entry, tmp, &adap->mps_ref, list) {
+ if (mps_entry->idx == idx) {
+ if (!refcount_dec_and_test(&mps_entry->refcnt)) {
+ spin_unlock(&adap->mps_ref_lock);
+ return -EBUSY;
+ }
+ list_del(&mps_entry->list);
+ kfree(mps_entry);
+ ret = 0;
+ break;
+ }
+ }
+ spin_unlock(&adap->mps_ref_lock);
+ return ret;
+}
+
+static int cxgb4_mps_ref_inc(struct adapter *adap, const u8 *mac_addr,
+ u16 idx, const u8 *mask)
+{
+ u8 bitmask[] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
+ struct mps_entries_ref *mps_entry;
+ int ret = 0;
+
+ spin_lock_bh(&adap->mps_ref_lock);
+ list_for_each_entry(mps_entry, &adap->mps_ref, list) {
+ if (mps_entry->idx == idx) {
+ refcount_inc(&mps_entry->refcnt);
+ goto unlock;
+ }
+ }
+ mps_entry = kzalloc(sizeof(*mps_entry), GFP_ATOMIC);
+ if (!mps_entry) {
+ ret = -ENOMEM;
+ goto unlock;
+ }
+ ether_addr_copy(mps_entry->mask, mask ? mask : bitmask);
+ ether_addr_copy(mps_entry->addr, mac_addr);
+ mps_entry->idx = idx;
+ refcount_set(&mps_entry->refcnt, 1);
+ list_add_tail(&mps_entry->list, &adap->mps_ref);
+unlock:
+ spin_unlock_bh(&adap->mps_ref_lock);
+ return ret;
+}
+
+int cxgb4_free_encap_mac_filt(struct adapter *adap, unsigned int viid,
+ int idx, bool sleep_ok)
+{
+ int ret = 0;
+
+ if (!cxgb4_mps_ref_dec(adap, idx))
+ ret = t4_free_encap_mac_filt(adap, viid, idx, sleep_ok);
+
+ return ret;
+}
+
+int cxgb4_alloc_encap_mac_filt(struct adapter *adap, unsigned int viid,
+ const u8 *addr, const u8 *mask,
+ unsigned int vni, unsigned int vni_mask,
+ u8 dip_hit, u8 lookup_type, bool sleep_ok)
+{
+ int ret;
+
+ ret = t4_alloc_encap_mac_filt(adap, viid, addr, mask, vni, vni_mask,
+ dip_hit, lookup_type, sleep_ok);
+ if (ret < 0)
+ return ret;
+
+ if (cxgb4_mps_ref_inc(adap, addr, ret, mask)) {
+ ret = -ENOMEM;
+ t4_free_encap_mac_filt(adap, viid, ret, sleep_ok);
+ }
+ return ret;
+}
+
+int cxgb4_init_mps_ref_entries(struct adapter *adap)
+{
+ spin_lock_init(&adap->mps_ref_lock);
+ INIT_LIST_HEAD(&adap->mps_ref);
+
+ return 0;
+}
+
+void cxgb4_free_mps_ref_entries(struct adapter *adap)
+{
+ struct mps_entries_ref *mps_entry, *tmp;
+
+ if (!list_empty(&adap->mps_ref))
+ return;
+
+ spin_lock(&adap->mps_ref_lock);
+ list_for_each_entry_safe(mps_entry, tmp, &adap->mps_ref, list) {
+ list_del(&mps_entry->list);
+ kfree(mps_entry);
+ }
+ spin_unlock(&adap->mps_ref_lock);
+}
--
2.12.0
^ permalink raw reply related
* [PATCH v3 net-next 2/4] cxgb4: Add MPS TCAM refcounting for raw mac filters
From: Raju Rangoju @ 2019-06-24 17:35 UTC (permalink / raw)
To: netdev, davem; +Cc: nirranjan, dt, rajur
In-Reply-To: <20190624173535.12572-1-rajur@chelsio.com>
This patch adds TCAM reference counting
support for raw mac filters.
Signed-off-by: Raju Rangoju <rajur@chelsio.com>
---
drivers/net/ethernet/chelsio/cxgb4/cxgb4.h | 16 +++++++++
drivers/net/ethernet/chelsio/cxgb4/cxgb4_mps.c | 46 ++++++++++++++++++++++++++
2 files changed, 62 insertions(+)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h b/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
index 39ccd4c64d48..c7ab57fd03be 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
@@ -1919,5 +1919,21 @@ int cxgb4_alloc_encap_mac_filt(struct adapter *adap, unsigned int viid,
u8 dip_hit, u8 lookup_type, bool sleep_ok);
int cxgb4_free_encap_mac_filt(struct adapter *adap, unsigned int viid,
int idx, bool sleep_ok);
+int cxgb4_free_raw_mac_filt(struct adapter *adap,
+ unsigned int viid,
+ const u8 *addr,
+ const u8 *mask,
+ unsigned int idx,
+ u8 lookup_type,
+ u8 port_id,
+ bool sleep_ok);
+int cxgb4_alloc_raw_mac_filt(struct adapter *adap,
+ unsigned int viid,
+ const u8 *addr,
+ const u8 *mask,
+ unsigned int idx,
+ u8 lookup_type,
+ u8 port_id,
+ bool sleep_ok);
#endif /* __CXGB4_H__ */
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_mps.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_mps.c
index b8a5375bf64d..b942748c7dfa 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_mps.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_mps.c
@@ -54,6 +54,52 @@ static int cxgb4_mps_ref_inc(struct adapter *adap, const u8 *mac_addr,
return ret;
}
+int cxgb4_free_raw_mac_filt(struct adapter *adap,
+ unsigned int viid,
+ const u8 *addr,
+ const u8 *mask,
+ unsigned int idx,
+ u8 lookup_type,
+ u8 port_id,
+ bool sleep_ok)
+{
+ int ret = 0;
+
+ if (!cxgb4_mps_ref_dec(adap, idx))
+ ret = t4_free_raw_mac_filt(adap, viid, addr,
+ mask, idx, lookup_type,
+ port_id, sleep_ok);
+
+ return ret;
+}
+
+int cxgb4_alloc_raw_mac_filt(struct adapter *adap,
+ unsigned int viid,
+ const u8 *addr,
+ const u8 *mask,
+ unsigned int idx,
+ u8 lookup_type,
+ u8 port_id,
+ bool sleep_ok)
+{
+ int ret;
+
+ ret = t4_alloc_raw_mac_filt(adap, viid, addr,
+ mask, idx, lookup_type,
+ port_id, sleep_ok);
+ if (ret < 0)
+ return ret;
+
+ if (cxgb4_mps_ref_inc(adap, addr, ret, mask)) {
+ ret = -ENOMEM;
+ t4_free_raw_mac_filt(adap, viid, addr,
+ mask, idx, lookup_type,
+ port_id, sleep_ok);
+ }
+
+ return ret;
+}
+
int cxgb4_free_encap_mac_filt(struct adapter *adap, unsigned int viid,
int idx, bool sleep_ok)
{
--
2.12.0
^ permalink raw reply related
* [PATCH v3 net-next 3/4] cxgb4: Add MPS TCAM refcounting for cxgb4 change mac
From: Raju Rangoju @ 2019-06-24 17:35 UTC (permalink / raw)
To: netdev, davem; +Cc: nirranjan, dt, rajur
In-Reply-To: <20190624173535.12572-1-rajur@chelsio.com>
This patch adds TCAM reference counting
support for cxgb4 change mac path
Signed-off-by: Raju Rangoju <rajur@chelsio.com>
---
drivers/net/ethernet/chelsio/cxgb4/cxgb4.h | 7 +++++++
drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 14 +++++++-------
drivers/net/ethernet/chelsio/cxgb4/cxgb4_mps.c | 15 +++++++++++++++
3 files changed, 29 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h b/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
index c7ab57fd03be..6260240743d5 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
@@ -1911,6 +1911,10 @@ int cxgb4_set_msix_aff(struct adapter *adap, unsigned short vec,
cpumask_var_t *aff_mask, int idx);
void cxgb4_clear_msix_aff(unsigned short vec, cpumask_var_t aff_mask);
+int cxgb4_change_mac(struct port_info *pi, unsigned int viid,
+ int *tcam_idx, const u8 *addr,
+ bool persistent, u8 *smt_idx);
+
int cxgb4_init_mps_ref_entries(struct adapter *adap);
void cxgb4_free_mps_ref_entries(struct adapter *adap);
int cxgb4_alloc_encap_mac_filt(struct adapter *adap, unsigned int viid,
@@ -1935,5 +1939,8 @@ int cxgb4_alloc_raw_mac_filt(struct adapter *adap,
u8 lookup_type,
u8 port_id,
bool sleep_ok);
+int cxgb4_update_mac_filt(struct port_info *pi, unsigned int viid,
+ int *tcam_idx, const u8 *addr,
+ bool persistent, u8 *smt_idx);
#endif /* __CXGB4_H__ */
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
index 4632827f05ba..1520e5294289 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
@@ -449,9 +449,9 @@ static int set_rxmode(struct net_device *dev, int mtu, bool sleep_ok)
* Addresses are programmed to hash region, if tcam runs out of entries.
*
*/
-static int cxgb4_change_mac(struct port_info *pi, unsigned int viid,
- int *tcam_idx, const u8 *addr, bool persist,
- u8 *smt_idx)
+int cxgb4_change_mac(struct port_info *pi, unsigned int viid,
+ int *tcam_idx, const u8 *addr, bool persist,
+ u8 *smt_idx)
{
struct adapter *adapter = pi->adapter;
struct hash_mac_addr *entry, *new_entry;
@@ -505,8 +505,8 @@ static int link_start(struct net_device *dev)
ret = t4_set_rxmode(pi->adapter, mb, pi->viid, dev->mtu, -1, -1, -1,
!!(dev->features & NETIF_F_HW_VLAN_CTAG_RX), true);
if (ret == 0)
- ret = cxgb4_change_mac(pi, pi->viid, &pi->xact_addr_filt,
- dev->dev_addr, true, &pi->smt_idx);
+ ret = cxgb4_update_mac_filt(pi, pi->viid, &pi->xact_addr_filt,
+ dev->dev_addr, true, &pi->smt_idx);
if (ret == 0)
ret = t4_link_l1cfg(pi->adapter, mb, pi->tx_chan,
&pi->link_cfg);
@@ -3020,8 +3020,8 @@ static int cxgb_set_mac_addr(struct net_device *dev, void *p)
if (!is_valid_ether_addr(addr->sa_data))
return -EADDRNOTAVAIL;
- ret = cxgb4_change_mac(pi, pi->viid, &pi->xact_addr_filt,
- addr->sa_data, true, &pi->smt_idx);
+ ret = cxgb4_update_mac_filt(pi, pi->viid, &pi->xact_addr_filt,
+ addr->sa_data, true, &pi->smt_idx);
if (ret < 0)
return ret;
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_mps.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_mps.c
index b942748c7dfa..067217c6ca05 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_mps.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_mps.c
@@ -54,6 +54,21 @@ static int cxgb4_mps_ref_inc(struct adapter *adap, const u8 *mac_addr,
return ret;
}
+int cxgb4_update_mac_filt(struct port_info *pi, unsigned int viid,
+ int *tcam_idx, const u8 *addr,
+ bool persistent, u8 *smt_idx)
+{
+ int ret;
+
+ ret = cxgb4_change_mac(pi, viid, tcam_idx,
+ addr, persistent, smt_idx);
+ if (ret < 0)
+ return ret;
+
+ cxgb4_mps_ref_inc(pi->adapter, addr, *tcam_idx, NULL);
+ return ret;
+}
+
int cxgb4_free_raw_mac_filt(struct adapter *adap,
unsigned int viid,
const u8 *addr,
--
2.12.0
^ permalink raw reply related
* [PATCH v3 net-next 4/4] cxgb4: Add MPS refcounting for alloc/free mac filters
From: Raju Rangoju @ 2019-06-24 17:35 UTC (permalink / raw)
To: netdev, davem; +Cc: nirranjan, dt, rajur
In-Reply-To: <20190624173535.12572-1-rajur@chelsio.com>
This patch adds reference counting support for
alloc/free mac filters
Signed-off-by: Raju Rangoju <rajur@chelsio.com>
---
drivers/net/ethernet/chelsio/cxgb4/cxgb4.h | 6 +++
drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 12 +++--
drivers/net/ethernet/chelsio/cxgb4/cxgb4_mps.c | 72 +++++++++++++++++++++++++
3 files changed, 87 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h b/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
index 6260240743d5..1fbb640e896a 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
@@ -1915,6 +1915,12 @@ int cxgb4_change_mac(struct port_info *pi, unsigned int viid,
int *tcam_idx, const u8 *addr,
bool persistent, u8 *smt_idx);
+int cxgb4_alloc_mac_filt(struct adapter *adap, unsigned int viid,
+ bool free, unsigned int naddr,
+ const u8 **addr, u16 *idx,
+ u64 *hash, bool sleep_ok);
+int cxgb4_free_mac_filt(struct adapter *adap, unsigned int viid,
+ unsigned int naddr, const u8 **addr, bool sleep_ok);
int cxgb4_init_mps_ref_entries(struct adapter *adap);
void cxgb4_free_mps_ref_entries(struct adapter *adap);
int cxgb4_alloc_encap_mac_filt(struct adapter *adap, unsigned int viid,
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
index 1520e5294289..b08efc48d42f 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
@@ -366,13 +366,19 @@ static int cxgb4_mac_sync(struct net_device *netdev, const u8 *mac_addr)
int ret;
u64 mhash = 0;
u64 uhash = 0;
+ /* idx stores the index of allocated filters,
+ * its size should be modified based on the number of
+ * MAC addresses that we allocate filters for
+ */
+
+ u16 idx[1] = {};
bool free = false;
bool ucast = is_unicast_ether_addr(mac_addr);
const u8 *maclist[1] = {mac_addr};
struct hash_mac_addr *new_entry;
- ret = t4_alloc_mac_filt(adap, adap->mbox, pi->viid, free, 1, maclist,
- NULL, ucast ? &uhash : &mhash, false);
+ ret = cxgb4_alloc_mac_filt(adap, pi->viid, free, 1, maclist,
+ idx, ucast ? &uhash : &mhash, false);
if (ret < 0)
goto out;
/* if hash != 0, then add the addr to hash addr list
@@ -410,7 +416,7 @@ static int cxgb4_mac_unsync(struct net_device *netdev, const u8 *mac_addr)
}
}
- ret = t4_free_mac_filt(adap, adap->mbox, pi->viid, 1, maclist, false);
+ ret = cxgb4_free_mac_filt(adap, pi->viid, 1, maclist, false);
return ret < 0 ? -EINVAL : 0;
}
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_mps.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_mps.c
index 067217c6ca05..b1a073eea60b 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_mps.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_mps.c
@@ -3,6 +3,31 @@
#include "cxgb4.h"
+static int cxgb4_mps_ref_dec_by_mac(struct adapter *adap,
+ const u8 *addr, const u8 *mask)
+{
+ u8 bitmask[] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
+ struct mps_entries_ref *mps_entry, *tmp;
+ int ret = -EINVAL;
+
+ spin_lock_bh(&adap->mps_ref_lock);
+ list_for_each_entry_safe(mps_entry, tmp, &adap->mps_ref, list) {
+ if (ether_addr_equal(mps_entry->addr, addr) &&
+ ether_addr_equal(mps_entry->mask, mask ? mask : bitmask)) {
+ if (!refcount_dec_and_test(&mps_entry->refcnt)) {
+ spin_unlock_bh(&adap->mps_ref_lock);
+ return -EBUSY;
+ }
+ list_del(&mps_entry->list);
+ kfree(mps_entry);
+ ret = 0;
+ break;
+ }
+ }
+ spin_unlock_bh(&adap->mps_ref_lock);
+ return ret;
+}
+
static int cxgb4_mps_ref_dec(struct adapter *adap, u16 idx)
{
struct mps_entries_ref *mps_entry, *tmp;
@@ -54,6 +79,53 @@ static int cxgb4_mps_ref_inc(struct adapter *adap, const u8 *mac_addr,
return ret;
}
+int cxgb4_free_mac_filt(struct adapter *adap, unsigned int viid,
+ unsigned int naddr, const u8 **addr, bool sleep_ok)
+{
+ int ret, i;
+
+ for (i = 0; i < naddr; i++) {
+ if (!cxgb4_mps_ref_dec_by_mac(adap, addr[i], NULL)) {
+ ret = t4_free_mac_filt(adap, adap->mbox, viid,
+ 1, &addr[i], sleep_ok);
+ if (ret < 0)
+ return ret;
+ }
+ }
+
+ /* return number of filters freed */
+ return naddr;
+}
+
+int cxgb4_alloc_mac_filt(struct adapter *adap, unsigned int viid,
+ bool free, unsigned int naddr, const u8 **addr,
+ u16 *idx, u64 *hash, bool sleep_ok)
+{
+ int ret, i;
+
+ ret = t4_alloc_mac_filt(adap, adap->mbox, viid, free,
+ naddr, addr, idx, hash, sleep_ok);
+ if (ret < 0)
+ return ret;
+
+ for (i = 0; i < naddr; i++) {
+ if (idx[i] != 0xffff) {
+ if (cxgb4_mps_ref_inc(adap, addr[i], idx[i], NULL)) {
+ ret = -ENOMEM;
+ goto error;
+ }
+ }
+ }
+
+ goto out;
+error:
+ cxgb4_free_mac_filt(adap, viid, naddr, addr, sleep_ok);
+
+out:
+ /* Returns a negative error number or the number of filters allocated */
+ return ret;
+}
+
int cxgb4_update_mac_filt(struct port_info *pi, unsigned int viid,
int *tcam_idx, const u8 *addr,
bool persistent, u8 *smt_idx)
--
2.12.0
^ permalink raw reply related
* Re: [PATCH net v2 1/2] ipv6: constify rt6_nexthop()
From: Nick Desaulniers @ 2019-06-24 17:37 UTC (permalink / raw)
To: David Miller, Nicolas Dichtel; +Cc: netdev, kbuild test robot
In-Reply-To: <20190624.102212.4398258272798722.davem@davemloft.net>
On Mon, Jun 24, 2019 at 10:22 AM David Miller <davem@davemloft.net> wrote:
>
> From: Nick Desaulniers <ndesaulniers@google.com>
> Date: Mon, 24 Jun 2019 10:17:03 -0700
>
> > On Mon, Jun 24, 2019 at 10:06 AM David Miller <davem@davemloft.net> wrote:
> >> And you mean just changing to 'const' fixes something, how?
> >
> > See the warning in the above link (assuming now you have access).
> > Assigning a non-const variable the result of a function call that
> > returns const discards the const qualifier.
>
> Ok thanks for clarifying.
>
> However I was speaking in terms of this fixing a functional bug rather
> than a loss of const warning.
The author stated that this patch was no functional change. Nicolas,
it can be helpful to include compiler warnings in the commit message
when sending warning fixes, but it's not a big deal. Thanks for
sending the patches.
--
Thanks,
~Nick Desaulniers
^ permalink raw reply
* RE: [net-next v2] tipc: add loopback device tracking
From: Jon Maloy @ 2019-06-24 17:45 UTC (permalink / raw)
To: David Miller, John Rutherford; +Cc: netdev@vger.kernel.org
In-Reply-To: <20190624.072918.1626161455453937687.davem@davemloft.net>
> -----Original Message-----
> From: netdev-owner@vger.kernel.org <netdev-owner@vger.kernel.org> On
> Behalf Of David Miller
> Sent: 24-Jun-19 10:29
> To: John Rutherford <john.rutherford@dektech.com.au>
> Cc: netdev@vger.kernel.org
> Subject: Re: [net-next v2] tipc: add loopback device tracking
>
> From: john.rutherford@dektech.com.au
> Date: Mon, 24 Jun 2019 16:44:35 +1000
>
> > Since node internal messages are passed directly to socket it is not
> > possible to observe this message exchange via tcpdump or wireshark.
> >
> > We now remedy this by making it possible to clone such messages and
> > send the clones to the loopback interface. The clones are dropped at
> > reception and have no functional role except making the traffic visible.
> >
> > The feature is turned on/off by enabling/disabling the loopback "bearer"
> > "eth:lo".
> >
> > Acked-by: Jon Maloy <jon.maloy@ericsson.com>
> > Signed-off-by: John Rutherford <john.rutherford@dektech.com.au>
>
> What a waste, just clone the packet, attach loopback to it, and go:
>
> if (dev_nit_active(loopback_dev))
> dev_queue_xmit_nit(skb, loopback_dev);
I was never quite happy with this patch, so thank you for the feedback!
///jon
^ permalink raw reply
* Re: [PATCH v7 0/6] Add support for Orange Pi 3
From: Ondřej Jirman @ 2019-06-24 17:46 UTC (permalink / raw)
To: David Miller
Cc: linux-sunxi, maxime.ripard, wens, robh+dt, jernej.skrabec,
airlied, daniel, mark.rutland, peppe.cavallaro, alexandre.torgue,
joabreu, mcoquelin.stm32, dri-devel, devicetree, linux-arm-kernel,
linux-kernel, netdev, linux-stm32
In-Reply-To: <20190624.102927.1268781741493594465.davem@davemloft.net>
On Mon, Jun 24, 2019 at 10:29:27AM -0700, David Miller wrote:
> From: megous@megous.com
> Date: Thu, 20 Jun 2019 15:47:42 +0200
>
> > From: Ondrej Jirman <megous@megous.com>
> >
> > This series implements support for Xunlong Orange Pi 3 board.
> >
> > - ethernet support (patches 1-3)
> > - HDMI support (patches 4-6)
> >
> > For some people, ethernet doesn't work after reboot (but works on cold
> > boot), when the stmmac driver is built into the kernel. It works when
> > the driver is built as a module. It's either some timing issue, or power
> > supply issue or a combination of both. Module build induces a power
> > cycling of the phy.
> >
> > I encourage people with this issue, to build the driver into the kernel,
> > and try to alter the reset timings for the phy in DTS or
> > startup-delay-us and report the findings.
>
> This is a mixture of networking and non-networking changes so it really
> can't go through my tree.
>
> I wonder how you expect this series to be merged?
>
> Thanks.
This series was even longer before, with patches all around for various
maintainers. I'd expect that relevant maintainers pick the range of patches
meant for them. I don't know who's exactly responsible for what, but I think,
this should work:
- 2 stmmac patches should go together via some networking tree (is there
something specific for stmmac?)
- all DTS patches should go via sunxi
- hdmi patches via some drm tree
thank you and regards,
o.
^ permalink raw reply
* Re: [PATCH net-next 1/3] net/sched: Introduce action ct
From: Cong Wang @ 2019-06-24 17:46 UTC (permalink / raw)
To: Paul Blakey
Cc: Marcelo Ricardo Leitner, Toke Høiland-Jørgensen,
Jiri Pirko, Roi Dayan, Yossi Kuperman, Oz Shlomo,
netdev@vger.kernel.org, David Miller, Aaron Conole, Zhike Wang,
Rony Efraim, nst-kernel@redhat.com, John Hurley, Simon Horman,
Justin Pettit, Kevin Darbyshire-Bryant
In-Reply-To: <db10725e-d31a-efda-e57e-9978fd680c92@mellanox.com>
On Thu, Jun 20, 2019 at 12:32 AM Paul Blakey <paulb@mellanox.com> wrote:
>
>
> On 6/18/2019 7:03 PM, Cong Wang wrote:
> > On Fri, Jun 14, 2019 at 12:24 PM Marcelo Ricardo Leitner
> > <marcelo.leitner@gmail.com> wrote:
> >> On Fri, Jun 14, 2019 at 11:07:37AM -0700, Cong Wang wrote:
> >>> On Tue, Jun 11, 2019 at 9:44 AM Marcelo Ricardo Leitner
> >>> <marcelo.leitner@gmail.com> wrote:
> >>>> I had suggested to let act_ct handle the above as well, as there is a
> >>>> big chunk of code on both that is pretty similar. There is quite some
> >>>> boilerplate for interfacing with conntrack which is duplicated.
> >>> Why do you want to mix retrieving conntrack info with executing
> >>> conntrack?
> >> To save on the heavy boilerplate for interfacing with conntrack.
> >>
> >>> They are totally different things to me, act_ctinfo merely retrieves
> >>> information from conntrack, while this one, act_ct, is supposed to
> >>> move packets to conntrack.
> >> Seems we have a different understanding for "move packets to
> >> conntrack": conntrack will not consume the packets after this.
> >> But after act_ct is executed, if not with the clear flag, skb will now
> >> have the skb->_nfct entry available, on which flower then will be able
> >> to match. So in essence, it is also fetching information from
> >> conntrack.
> > Interesting. Is it because cls_flower uses conntrack for flow dissection?
> > What's the reason behind?
> >
> > Again, I am still not convinced to do L3 operations in L2, skb->_nfct
> > belongs to conntrack which is L3, no matter the packet is consumed
> > or not.
> >
> > Thanks.
>
> I'm not sure what you mean, the reason behind what?
Yes, which should be the most important info in changelog.
>
> We use conntrack to track, mark the packet with conntrack info, and
> execute nat, then we push the
>
> headers back to continue processing the next action. This action will
> probably be followed by
>
> goto chain or reclassify and then cls_flower can be used to match on
> conntrack state and metadata via the new flow dissector change.
>
Sounds cool, but again why do we have to do this in L2?
Also, I am not sure if cls_flower really matches packets with any
conntrack state, from my quick glance of its code. Is this feature
merged in upstream?
Is this for ingress only? For egress, packets coming down from L3
so they should already have conntrack state as long as it is enabled?
Sorry for asking many questions here, because your changelog is too
short. :-/
Thanks.
^ permalink raw reply
* Re: [PATCH 2/3] module: Fix up module_notifier return values.
From: Mathieu Desnoyers @ 2019-06-24 17:50 UTC (permalink / raw)
To: Joel Fernandes, Google
Cc: Peter Zijlstra, Frank Ch. Eigler, Jessica Yu, linux-kernel,
Josh Poimboeuf, jikos, mbenes, Petr Mladek, Alexei Starovoitov,
Daniel Borkmann, Andrew Morton, Robert Richter, rostedt,
Ingo Molnar, Martin KaFai Lau, Song Liu, Yonghong Song, paulmck,
Ard Biesheuvel, Thomas Gleixner, oprofile-list, netdev, bpf
In-Reply-To: <20190624155213.GB261936@google.com>
----- On Jun 24, 2019, at 11:52 AM, Joel Fernandes, Google joel@joelfernandes.org wrote:
> On Mon, Jun 24, 2019 at 10:01:04AM -0400, Mathieu Desnoyers wrote:
>> ----- On Jun 24, 2019, at 5:18 AM, Peter Zijlstra peterz@infradead.org wrote:
>>
>> > While auditing all module notifiers I noticed a whole bunch of fail
>> > wrt the return value. Notifiers have a 'special' return semantics.
>> >
>> > Cc: Robert Richter <rric@kernel.org>
>> > Cc: Steven Rostedt <rostedt@goodmis.org>
>> > Cc: Ingo Molnar <mingo@redhat.com>
>> > Cc: Alexei Starovoitov <ast@kernel.org>
>> > Cc: Daniel Borkmann <daniel@iogearbox.net>
>> > Cc: Martin KaFai Lau <kafai@fb.com>
>> > Cc: Song Liu <songliubraving@fb.com>
>> > Cc: Yonghong Song <yhs@fb.com>
>> > Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
>> > Cc: "Paul E. McKenney" <paulmck@linux.ibm.com>
>> > Cc: "Joel Fernandes (Google)" <joel@joelfernandes.org>
>> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> > Cc: Thomas Gleixner <tglx@linutronix.de>
>> > Cc: oprofile-list@lists.sf.net
>> > Cc: linux-kernel@vger.kernel.org
>> > Cc: netdev@vger.kernel.org
>> > Cc: bpf@vger.kernel.org
>> > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
>>
>> Thanks Peter for looking into this, especially considering your
>> endless love for kernel modules! ;)
>>
>> It's not directly related to your changes, but I notice that
>> kernel/trace/trace_printk.c:hold_module_trace_bprintk_format()
>> appears to leak memory. Am I missing something ?
>
> Could you elaborate? Do you mean there is no MODULE_STATE_GOING notifier
> check? If that's what you mean then I agree, there should be some place
> where the format structures are freed when the module is unloaded no?
Yes, the lack of GOING notifier is worrying considering that GOING
performs memory allocation.
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* Re: [PATCH rdma-next v1 10/12] IB/mlx5: Enable subscription for device events over DEVX
From: Jason Gunthorpe @ 2019-06-24 17:56 UTC (permalink / raw)
To: Yishai Hadas
Cc: Leon Romanovsky, Doug Ledford, Leon Romanovsky, RDMA mailing list,
Yishai Hadas, Saeed Mahameed, linux-netdev
In-Reply-To: <33f9402b-ccae-b874-cc72-b6afb1fb8655@dev.mellanox.co.il>
On Mon, Jun 24, 2019 at 07:13:14PM +0300, Yishai Hadas wrote:
> > > + u32 xa_key_level1;
> > > + u32 xa_key_level2;
> > > + struct rcu_head rcu;
> > > + u64 cookie;
> > > + bool is_obj_related;
> > > + struct ib_uobject *fd_uobj;
> > > + void *object; /* May need direct access upon hot unplug */
> >
> > This should be a 'struct file *' and have a better name.
> >
>
> OK, will change.
>
> > And I'm unclear why we need to store both the ib_uobject and the
> > struct file for the same thing?
>
> Post hot unplug/unbind the uobj can't be accessed any more to reach the
> object as it will be set to NULL by ib_core layer [1].
struct file users need to get the uobject from the file->private_data
under a fget.
There is only place place that needed fd_uobj, and it was under the
fget section, so it should simply use private_data.
This is why you should only store the struct file and not the uobject.
> This was the comment that I have just put above in the code, I may improve
> it with more details as pointed here.
>
> [1]
> https://elixir.bootlin.com/linux/latest/source/drivers/infiniband/core/rdma_core.c#L149
I'm wondering if this is a bug to do this for fds?
> > Since uobj->object == flip && filp->private_data == uobj, I have a
> > hard time to understand why we need both things, it seems to me that
> > if we get the fget on the filp then we can rely on the
> > filp->private_data to get back to the devx_async_event_file.
> >
>
> The idea was to not take an extra ref count on the file (i.e. fget) per
> subscription, this will let the release option to be called once the file
> will be closed by the application.
No extra ref is needed, the fget is already obtained in the only place
that needs fd_uobj.
> > > + obj_event = xa_load(&event->object_ids, key_level2);
> > > + if (!obj_event) {
> > > + err = xa_reserve(&event->object_ids, key_level2, GFP_KERNEL);
> > > + if (err)
> > > + goto err_level1;
> > > +
> > > + obj_event = kzalloc(sizeof(*obj_event), GFP_KERNEL);
> > > + if (!obj_event) {
> > > + err = -ENOMEM;
> > > + goto err_level2;
> > > + }
> > > +
> > > + INIT_LIST_HEAD(&obj_event->obj_sub_list);
> > > + *alloc_obj_event = obj_event;
> >
> > This is goofy, just store the empty obj_event in the xa instead of
> > using xa_reserve, and when you go to do the error unwind just delete
> > any level2' devx_obj_event' that has a list_empty(obj_sub_list), get
> > rid of the wonky alloc_obj_event stuff.
> >
>
> Please see my answer above about how level2 is managed by this
> alloc_obj_event, is that really worth a change ? I found current logic to be
> clear. I may put some note here if we can stay with that.
I think it is alot cleaner/simpler than using this extra memory
> > The best configuration would be to use devx_cleanup_subscription to
> > undo the partially ready subscription.
>
> This partially ready subscription might not match the
> devx_cleanup_subscription(), e.g. it wasn't added to xa_list and can't be
> deleted without any specific flag to ignore ..
Maybe, but I suspect it can work out
> > > + event_sub_arr = uverbs_zalloc(attrs,
> > > + MAX_NUM_EVENTS * sizeof(struct devx_event_subscription *));
> > > + event_obj_array_alloc = uverbs_zalloc(attrs,
> > > + MAX_NUM_EVENTS * sizeof(struct devx_obj_event *));
> >
> > There are so many list_heads in the devx_event_subscription, why not
> > use just one of them to store the allocated events instead of this
> > temp array? ie event_list looks good for this purpose.
> >
>
> I'm using the array later on with direct access to the index that should be
> de-allocated. I would prefer staying with this array rather than using the
> 'event_list' which has other purpose down the road, it's used per
> subscription and doesn't look match to hold the devx_obj_event which has no
> list entry for this purpose..
Replace the event_obj_array_alloc by storing that data directly in
the xarray
Replace the event_sub_arr by building them into a linked list - it
always need to iterate over the whole list anyhow.
> > > +
> > > + if (!event_sub_arr || !event_obj_array_alloc)
> > > + return -ENOMEM;
> > > +
> > > + /* Protect from concurrent subscriptions to same XA entries to allow
> > > + * both to succeed
> > > + */
> > > + mutex_lock(&devx_event_table->event_xa_lock);
> > > + for (i = 0; i < num_events; i++) {
> > > + u32 key_level1;
> > > +
> > > + if (obj)
> > > + obj_type = get_dec_obj_type(obj,
> > > + event_type_num_list[i]);
> > > + key_level1 = event_type_num_list[i] | obj_type << 16;
> > > +
> > > + err = subscribe_event_xa_alloc(devx_event_table,
> > > + key_level1,
> > > + obj ? true : false,
> > > + obj_id,
> > > + &event_obj_array_alloc[i]);
> >
> > Usless ?:
>
> What do you suggest instead ?
Nothing is needed, cast to implicit bool is always forced to
true/false
Jason
^ permalink raw reply
* Re: [PATCH iproute2-next v3 1/2] ipaddress: correctly print a VF hw address in the IPoIB case
From: David Ahern @ 2019-06-24 17:59 UTC (permalink / raw)
To: Denis Kirjanov, stephen; +Cc: netdev, linux-rdma, dledford, mkubecek
In-Reply-To: <20190622180035.40245-1-dkirjanov@suse.com>
On 6/22/19 12:00 PM, Denis Kirjanov wrote:
> @@ -365,13 +367,45 @@ static void print_vfinfo(FILE *fp, struct rtattr *vfinfo)
> parse_rtattr_nested(vf, IFLA_VF_MAX, vfinfo);
>
> vf_mac = RTA_DATA(vf[IFLA_VF_MAC]);
> + vf_broadcast = RTA_DATA(vf[IFLA_VF_BROADCAST]);
> vf_tx_rate = RTA_DATA(vf[IFLA_VF_TX_RATE]);
>
> print_string(PRINT_FP, NULL, "%s ", _SL_);
> print_int(PRINT_ANY, "vf", "vf %d ", vf_mac->vf);
> - print_string(PRINT_ANY, "mac", "MAC %s",
> - ll_addr_n2a((unsigned char *) &vf_mac->mac,
> - ETH_ALEN, 0, b1, sizeof(b1)));
> +
> + print_string(PRINT_ANY,
> + "link_type",
> + " link/%s ",
> + ll_type_n2a(ifi->ifi_type, b1, sizeof(b1)));
> +
> + print_color_string(PRINT_ANY,
> + COLOR_MAC,
> + "address",
> + "%s",
> + ll_addr_n2a((unsigned char *) &vf_mac->mac,
> + ifi->ifi_type == ARPHRD_ETHER ?
> + ETH_ALEN : INFINIBAND_ALEN,
> + ifi->ifi_type,
> + b1, sizeof(b1)));
you still have a lot of lines that are not lined up column wise. See how
the COLOR_MAC is offset to the right from PRINT_ANY?
> +
> + if (vf[IFLA_VF_BROADCAST]) {
> + if (ifi->ifi_flags&IFF_POINTOPOINT) {
> + print_string(PRINT_FP, NULL, " peer ", NULL);
> + print_bool(PRINT_JSON,
> + "link_pointtopoint", NULL, true);
> + } else
> + print_string(PRINT_FP, NULL, " brd ", NULL);
> +
> + print_color_string(PRINT_ANY,
> + COLOR_MAC,
> + "broadcast",
> + "%s",
> + ll_addr_n2a((unsigned char *) &vf_broadcast->broadcast,
> + ifi->ifi_type == ARPHRD_ETHER ?
> + ETH_ALEN : INFINIBAND_ALEN,
> + ifi->ifi_type,
> + b1, sizeof(b1)));
And then these lines are offset to the left.
^ permalink raw reply
* Re: [PATCH net-next v2 0/4] net/sched: Introduce tc connection tracking
From: Cong Wang @ 2019-06-24 17:59 UTC (permalink / raw)
To: Paul Blakey
Cc: Jiri Pirko, Roi Dayan, Yossi Kuperman, Oz Shlomo,
Marcelo Ricardo Leitner, Linux Kernel Network Developers,
David Miller, Aaron Conole, Zhike Wang, Rony Efraim, nst-kernel,
John Hurley, Simon Horman, Justin Pettit
In-Reply-To: <1561038141-31370-1-git-send-email-paulb@mellanox.com>
On Thu, Jun 20, 2019 at 6:43 AM Paul Blakey <paulb@mellanox.com> wrote:
>
> Hi,
>
> This patch series add connection tracking capabilities in tc sw datapath.
> It does so via a new tc action, called act_ct, and new tc flower classifier matching
> on conntrack state, mark and label.
Thanks for more detailed description here.
I still don't see why we have to do this in L2, mind to be more specific?
IOW, if you really want to manipulate conntrack info and use it for
matching, why not do it in netfilter layer as it is where conntrack is?
BTW, if the cls_flower ct_state matching is not in upstream yet, please
try to push it first, as it is a justification of this patchset.
Thanks.
^ permalink raw reply
* Re: [PATCH rdma-next v1 11/12] IB/mlx5: Implement DEVX dispatching event
From: Jason Gunthorpe @ 2019-06-24 18:06 UTC (permalink / raw)
To: Yishai Hadas
Cc: Leon Romanovsky, Doug Ledford, Leon Romanovsky, RDMA mailing list,
Yishai Hadas, Saeed Mahameed, linux-netdev
In-Reply-To: <3a2e53f8-e7dd-3e01-c7c7-99d41f711d87@dev.mellanox.co.il>
On Mon, Jun 24, 2019 at 07:55:32PM +0300, Yishai Hadas wrote:
> > > + /* Explicit filtering to kernel events which may occur frequently */
> > > + if (event_type == MLX5_EVENT_TYPE_CMD ||
> > > + event_type == MLX5_EVENT_TYPE_PAGE_REQUEST)
> > > + return NOTIFY_OK;
> > > +
> > > + table = container_of(nb, struct mlx5_devx_event_table, devx_nb.nb);
> > > + dev = container_of(table, struct mlx5_ib_dev, devx_event_table);
> > > + is_unaffiliated = is_unaffiliated_event(dev->mdev, event_type);
> > > +
> > > + if (!is_unaffiliated)
> > > + obj_type = get_event_obj_type(event_type, data);
> > > + event = xa_load(&table->event_xa, event_type | (obj_type << 16));
> > > + if (!event)
> > > + return NOTIFY_DONE;
> >
> > event should be in the rcu as well
>
> Do we really need this ? I didn't see a flow that really requires
> that.
I think there are no frees left? Even so it makes much more sense to
include the event in the rcu as if we ever did need to kfree it would
have to be via rcu
> > > + while (list_empty(&ev_queue->event_list)) {
> > > + spin_unlock_irq(&ev_queue->lock);
> > > +
> > > + if (filp->f_flags & O_NONBLOCK)
> > > + return -EAGAIN;
> > > +
> > > + if (wait_event_interruptible(ev_queue->poll_wait,
> > > + (!list_empty(&ev_queue->event_list) ||
> > > + ev_queue->is_destroyed))) {
> > > + return -ERESTARTSYS;
> > > + }
> > > +
> > > + if (list_empty(&ev_queue->event_list) &&
> > > + ev_queue->is_destroyed)
> > > + return -EIO;
> >
> > All these tests should be under the lock.
>
> We can't call wait_event_interruptible() above which may sleep under the
> lock, correct ? are you referring to the list_empty() and
> is_destroyed ?
yes
> By the way looking in uverb code [1], similar code which is not done under
> the lock as of here..
>
> [1] https://elixir.bootlin.com/linux/latest/source/drivers/infiniband/core/uverbs_main.c#L244
Also not a good idea
> > Why don't we return EIO as soon as is-destroyed happens? What is the
> > point of flushing out the accumulated events?
>
> It follows the above uverb code/logic that returns existing events even in
> that case, also the async command events in this file follows that logic, I
> suggest to stay consistent.
Don't follow broken uverbs stuff...
> > Maybe the event should be re-added on error? Tricky.
>
> What will happen if another copy_to_user may then fail again (loop ?) ...
> not sure that we want to get into this tricky handling ...
>
> As of above, It follows the logic from uverbs at that area.
> https://elixir.bootlin.com/linux/latest/source/drivers/infiniband/core/uverbs_main.c#L267
again it is wrong...
There is no loop if you just stick the item back on the head of the
list and exit, which is probably the right thing to do..
> > > @@ -2374,6 +2705,17 @@ static int devx_hot_unplug_async_cmd_event_file(struct ib_uobject *uobj,
> > > static int devx_hot_unplug_async_event_file(struct ib_uobject *uobj,
> > > enum rdma_remove_reason why)
> > > {
> > > + struct devx_async_event_file *ev_file =
> > > + container_of(uobj, struct devx_async_event_file,
> > > + uobj);
> > > + struct devx_async_event_queue *ev_queue = &ev_file->ev_queue;
> > > +
> > > + spin_lock_irq(&ev_queue->lock);
> > > + ev_queue->is_destroyed = 1;
> > > + spin_unlock_irq(&ev_queue->lock);
> > > +
> > > + if (why == RDMA_REMOVE_DRIVER_REMOVE)
> > > + wake_up_interruptible(&ev_queue->poll_wait);
> >
> > Why isn't this wakeup always done?
>
> Maybe you are right and this can be always done to wake up any readers as
> the 'is_destroyed' was set.
>
> By the way, any idea why it was done as such in uverbs [1] for similar flow
> ? also the command events follows that.
I don't know, it is probably pointless too.
If we don't need it here then we shouldn't have it.
These random pointless ifs bother me as we have to spend time trying
to figure out that they are pointless down the road.
Jason
^ permalink raw reply
* Re: [PATCH rdma-next v1 12/12] IB/mlx5: Add DEVX support for CQ events
From: Jason Gunthorpe @ 2019-06-24 18:06 UTC (permalink / raw)
To: Yishai Hadas
Cc: Leon Romanovsky, Doug Ledford, Leon Romanovsky, RDMA mailing list,
Yishai Hadas, Saeed Mahameed, linux-netdev
In-Reply-To: <a076a050-871b-c468-f62e-95bb4f0ac2c2@dev.mellanox.co.il>
On Mon, Jun 24, 2019 at 08:03:07PM +0300, Yishai Hadas wrote:
> On 6/24/2019 3:04 PM, Jason Gunthorpe wrote:
> > On Tue, Jun 18, 2019 at 08:15:40PM +0300, Leon Romanovsky wrote:
> > > From: Yishai Hadas <yishaih@mellanox.com>
> > >
> > > Add DEVX support for CQ events by creating and destroying the CQ via
> > > mlx5_core and set an handler to manage its completions.
> > >
> > > Signed-off-by: Yishai Hadas <yishaih@mellanox.com>
> > > Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> > > drivers/infiniband/hw/mlx5/devx.c | 40 +++++++++++++++++++++++++++++++
> > > 1 file changed, 40 insertions(+)
> > >
> > > diff --git a/drivers/infiniband/hw/mlx5/devx.c b/drivers/infiniband/hw/mlx5/devx.c
> > > index 49fdce95d6d9..91ccd58ebc05 100644
> > > +++ b/drivers/infiniband/hw/mlx5/devx.c
> > > @@ -19,9 +19,12 @@
> > > #define UVERBS_MODULE_NAME mlx5_ib
> > > #include <rdma/uverbs_named_ioctl.h>
> > > +static void dispatch_event_fd(struct list_head *fd_list, const void *data);
> > > +
> > > enum devx_obj_flags {
> > > DEVX_OBJ_FLAGS_INDIRECT_MKEY = 1 << 0,
> > > DEVX_OBJ_FLAGS_DCT = 1 << 1,
> > > + DEVX_OBJ_FLAGS_CQ = 1 << 2,
> > > };
> > > struct devx_async_data {
> > > @@ -94,6 +97,7 @@ struct devx_async_event_file {
> > > #define MLX5_MAX_DESTROY_INBOX_SIZE_DW MLX5_ST_SZ_DW(delete_fte_in)
> > > struct devx_obj {
> > > struct mlx5_core_dev *mdev;
> > > + struct mlx5_ib_dev *ib_dev;
> >
> > This seems strange, why would we need to store the core_dev and the ib_dev
> > in a struct when ibdev->mdev == core_dev?
> >
>
> We need to add the ib_dev as we can't access it from the core_dev.
> Most of this patch we can probably go and drop the mdev and access it from
> ib_dev, I preferred to not handle that in this patch.
Should add a patch to revise it then
Jason
^ permalink raw reply
* Re: [PATCH v3 net] af_packet: Block execution of tasks waiting for transmit to complete in AF_PACKET
From: Willem de Bruijn @ 2019-06-24 18:08 UTC (permalink / raw)
To: Neil Horman; +Cc: Network Development, Matteo Croce, David S. Miller
In-Reply-To: <20190624004604.25607-1-nhorman@tuxdriver.com>
On Sun, Jun 23, 2019 at 8:46 PM Neil Horman <nhorman@tuxdriver.com> wrote:
>
> When an application is run that:
> a) Sets its scheduler to be SCHED_FIFO
> and
> b) Opens a memory mapped AF_PACKET socket, and sends frames with the
> MSG_DONTWAIT flag cleared, its possible for the application to hang
> forever in the kernel. This occurs because when waiting, the code in
> tpacket_snd calls schedule, which under normal circumstances allows
> other tasks to run, including ksoftirqd, which in some cases is
> responsible for freeing the transmitted skb (which in AF_PACKET calls a
> destructor that flips the status bit of the transmitted frame back to
> available, allowing the transmitting task to complete).
>
> However, when the calling application is SCHED_FIFO, its priority is
> such that the schedule call immediately places the task back on the cpu,
> preventing ksoftirqd from freeing the skb, which in turn prevents the
> transmitting task from detecting that the transmission is complete.
>
> We can fix this by converting the schedule call to a completion
> mechanism. By using a completion queue, we force the calling task, when
> it detects there are no more frames to send, to schedule itself off the
> cpu until such time as the last transmitted skb is freed, allowing
> forward progress to be made.
>
> Tested by myself and the reporter, with good results
>
> Appies to the net tree
>
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> Reported-by: Matteo Croce <mcroce@redhat.com>
> CC: "David S. Miller" <davem@davemloft.net>
> CC: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
>
> Change Notes:
>
> V1->V2:
> Enhance the sleep logic to support being interruptible and
> allowing for honoring to SK_SNDTIMEO (Willem de Bruijn)
>
> V2->V3:
> Rearrage the point at which we wait for the completion queue, to
> avoid needing to check for ph/skb being null at the end of the loop.
> Also move the complete call to the skb destructor to avoid needing to
> modify __packet_set_status. Also gate calling complete on
> packet_read_pending returning zero to avoid multiple calls to complete.
> (Willem de Bruijn)
>
> Move timeo computation within loop, to re-fetch the socket
> timeout since we also use the timeo variable to record the return code
> from the wait_for_complete call (Neil Horman)
> ---
> net/packet/af_packet.c | 59 +++++++++++++++++++++++++++++++++++++-----
> net/packet/internal.h | 2 ++
> 2 files changed, 55 insertions(+), 6 deletions(-)
>
> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index a29d66da7394..5c48bb7a4fa5 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
> @@ -380,7 +380,6 @@ static void __packet_set_status(struct packet_sock *po, void *frame, int status)
> WARN(1, "TPACKET version not supported.\n");
> BUG();
> }
> -
Unrelated to this feature
> smp_wmb();
> }
>
> @@ -1148,6 +1147,14 @@ static void *packet_previous_frame(struct packet_sock *po,
> return packet_lookup_frame(po, rb, previous, status);
> }
>
> +static void *packet_next_frame(struct packet_sock *po,
> + struct packet_ring_buffer *rb,
> + int status)
> +{
> + unsigned int next = rb->head != rb->frame_max ? rb->head+1 : 0;
> + return packet_lookup_frame(po, rb, next, status);
> +}
> +
> static void packet_increment_head(struct packet_ring_buffer *buff)
> {
> buff->head = buff->head != buff->frame_max ? buff->head+1 : 0;
> @@ -2401,6 +2408,9 @@ static void tpacket_destruct_skb(struct sk_buff *skb)
>
> ts = __packet_set_timestamp(po, ph, skb);
> __packet_set_status(po, ph, TP_STATUS_AVAILABLE | ts);
> +
> + if (po->wait_on_complete && !packet_read_pending(&po->tx_ring))
> + complete(&po->skb_completion);
> }
>
> sock_wfree(skb);
> @@ -2600,9 +2610,11 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
> int len_sum = 0;
> int status = TP_STATUS_AVAILABLE;
> int hlen, tlen, copylen = 0;
> + long timeo = 0;
>
> mutex_lock(&po->pg_vec_lock);
>
> +
Same
> if (likely(saddr == NULL)) {
> dev = packet_cached_dev_get(po);
> proto = po->num;
> @@ -2647,16 +2659,16 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
> size_max = dev->mtu + reserve + VLAN_HLEN;
>
> do {
> +
Same
> ph = packet_current_frame(po, &po->tx_ring,
> TP_STATUS_SEND_REQUEST);
> - if (unlikely(ph == NULL)) {
> - if (need_wait && need_resched())
> - schedule();
> - continue;
Why not keep the test whether the process needs to wait exactly here (A)?
Then no need for packet_next_frame.
> - }
> +
> + if (unlikely(ph == NULL))
> + break;
>
> skb = NULL;
> tp_len = tpacket_parse_header(po, ph, size_max, &data);
> +
Again
> if (tp_len < 0)
> goto tpacket_error;
>
> @@ -2720,6 +2732,21 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
>
> skb->destructor = tpacket_destruct_skb;
> __packet_set_status(po, ph, TP_STATUS_SENDING);
> +
> + /*
> + * If we need to wait and we've sent the last frame pending
> + * transmission in the mmaped buffer, flag that we need to wait
> + * on those frames to get freed via tpacket_destruct_skb. This
> + * flag indicates that tpacket_destruct_skb should call complete
> + * when the packet_pending count reaches zero, and that we need
> + * to call wait_on_complete_interruptible_timeout below, to make
> + * sure we pick up the result of that completion
> + */
> + if (need_wait && !packet_next_frame(po, &po->tx_ring, TP_STATUS_SEND_REQUEST)) {
> + po->wait_on_complete = 1;
> + timeo = sock_sndtimeo(&po->sk, msg->msg_flags & MSG_DONTWAIT);
This resets timeout on every loop. should only set above the loop once.
Also, please limit the comments in the code (also below). If every
patch would add this many lines of comment, the file would be
enormous. OTOH, it's great to be this explanatory in the git commit,
which is easily reached for any line with git blame.
> + }
> +
> packet_inc_pending(&po->tx_ring);
>
> status = TP_STATUS_SEND_REQUEST;
> @@ -2728,6 +2755,11 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
> err = net_xmit_errno(err);
> if (err && __packet_get_status(po, ph) ==
> TP_STATUS_AVAILABLE) {
> + /* re-init completion queue to avoid subsequent fallthrough
> + * on a future thread calling wait_on_complete_interruptible_timeout
> + */
> + po->wait_on_complete = 0;
If setting where sleeping, no need for resetting if a failure happens
between those blocks.
> + init_completion(&po->skb_completion);
no need to reinit between each use?
> /* skb was destructed already */
> skb = NULL;
> goto out_status;
> @@ -2740,6 +2772,20 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
> }
> packet_increment_head(&po->tx_ring);
> len_sum += tp_len;
> +
> + if (po->wait_on_complete) {
> + timeo = wait_for_completion_interruptible_timeout(&po->skb_completion, timeo);
> + po->wait_on_complete = 0;
I was going to argue for clearing in tpacket_destruct_skb. But then we
would have to separate clear on timeout instead of signal, too.
po->wait_on_complete = 1;
timeo = wait_for_completion...
po->wait_on_complete = 0;
as the previous version had is fine, as long as the compiler does not
"optimize" away an assignment. The function call will avoid reordering
by the cpu, at least. Probably requires WRITE_ONCE/READ_ONCE.
> + if (!timeo) {
> + /* We timed out, break out and notify userspace */
> + err = -ETIMEDOUT;
> + goto out_status;
goto out_put, there is no active ph or skb here
> + } else if (timeo == -ERESTARTSYS) {
> + err = -ERESTARTSYS;
> + goto out_status;
> + }
> + }
> +
> } while (likely((ph != NULL) ||
> /* Note: packet_read_pending() might be slow if we have
> * to call it as it's per_cpu variable, but in fast-path
> @@ -3207,6 +3253,7 @@ static int packet_create(struct net *net, struct socket *sock, int protocol,
> sock_init_data(sock, sk);
>
> po = pkt_sk(sk);
> + init_completion(&po->skb_completion);
> sk->sk_family = PF_PACKET;
> po->num = proto;
> po->xmit = dev_queue_xmit;
This is basically replacing a busy-wait with schedule() with sleeping
using wait_for_completion_interruptible_timeout. My main question is
does this really need to move control flow around and add
packet_next_frame? If not, especially for net, the shortest, simplest
change is preferable.
^ permalink raw reply
* Re: [PATCH net v2 1/2] ipv6: constify rt6_nexthop()
From: Nicolas Dichtel @ 2019-06-24 18:18 UTC (permalink / raw)
To: Nick Desaulniers, David Miller; +Cc: netdev, kbuild test robot
In-Reply-To: <CAKwvOdkqE_RVosXAe9ULePR8A37CHh6+JtDMaRAghUA41Y_+yg@mail.gmail.com>
Le 24/06/2019 à 19:37, Nick Desaulniers a écrit :
[snip]
>
> The author stated that this patch was no functional change. Nicolas,
> it can be helpful to include compiler warnings in the commit message
> when sending warning fixes, but it's not a big deal. Thanks for
> sending the patches.
>
Yep, but I was not aware of this compilation warning. As explained in the commit
log, the goal of this patch was to prepare the next one.
Regards,
Nicolas
^ permalink raw reply
* Re: [PATCH iproute2] iproute: Pass RTM_F_CLONED on dump to fetch cached routes to be flushed
From: Stefano Brivio @ 2019-06-24 18:20 UTC (permalink / raw)
To: Stephen Hemminger, David Ahern
Cc: David Miller, Jianlin Shi, Wei Wang, Martin KaFai Lau,
Eric Dumazet, Matti Vaittinen, netdev
In-Reply-To: <7ae318a8b632c216df95362524cd4bb5f4f1f537.1560561439.git.sbrivio@redhat.com>
Stephen,
On Sat, 15 Jun 2019 03:33:50 +0200
Stefano Brivio <sbrivio@redhat.com> wrote:
> With a current (5.1) kernel version, IPv6 exception routes can't be listed
> (ip -6 route list cache) or flushed (ip -6 route flush cache). I'm
> re-introducing kernel support for this, but, to allow the kernel to filter
> routes based on the RTM_F_CLONED flag, we need to make sure this flag is
> always passed when we want cached routes to be dumped.
Support for listing IPv6 route exceptions is now back on net-next,
relevant commits:
564c91f7e563 fib_frontend, ip6_fib: Select routes or exceptions dump from RTM_F_CLONED
ef11209d4219 Revert "net/ipv6: Bail early if user only wants cloned entries"
3401bfb1638e ipv6/route: Don't match on fc_nh_id if not set in ip6_route_del()
bf9a8a061ddc ipv6/route: Change return code of rt6_dump_route() for partial node dumps
1e47b4837f3b ipv6: Dump route exceptions if requested
40cb35d5dc04 ip6_fib: Don't discard nodes with valid routing information in fib6_locate_1()
and this iproute2 patch works together with that as it is. Do I need to
re-submit? Thanks.
--
Stefano
^ permalink raw reply
* Re: [PATCH] samples: bpf: make the use of xdp samples consistent
From: Toke Høiland-Jørgensen @ 2019-06-24 18:24 UTC (permalink / raw)
To: Daniel T. Lee, Daniel Borkmann, Alexei Starovoitov; +Cc: netdev
In-Reply-To: <20190624112009.20048-1-danieltimlee@gmail.com>
"Daniel T. Lee" <danieltimlee@gmail.com> writes:
> Currently, each xdp samples are inconsistent in the use.
> Most of the samples fetch the interface with it's name.
> (ex. xdp1, xdp2skb, xdp_redirect, xdp_sample_pkts, etc.)
The xdp_redirect and xdp_redirect_map also only accept ifindexes, not
interface names. Care to fix those while you're at it? :)
-Toke
^ permalink raw reply
* Re: [PATCH v2 net-next 4/4] cxgb4: Add MPS refcounting for alloc/free mac filters
From: kbuild test robot @ 2019-06-24 18:24 UTC (permalink / raw)
To: Raju Rangoju; +Cc: kbuild-all, netdev, davem, nirranjan, dt, rajur
In-Reply-To: <20190624085037.2358-5-rajur@chelsio.com>
[-- Attachment #1: Type: text/plain, Size: 2908 bytes --]
Hi Raju,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
url: https://github.com/0day-ci/linux/commits/Raju-Rangoju/cxgb4-Reference-count-MPS-TCAM-entries-within-a-PF/20190624-230630
config: x86_64-randconfig-b0-06242344 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
drivers/net/ethernet/chelsio/cxgb4/cxgb4_mps.c: In function 'cxgb4_mps_ref_dec_by_mac':
>> drivers/net/ethernet/chelsio/cxgb4/cxgb4_mps.c:17:29: warning: passing argument 1 of 'atomic_dec_and_test' from incompatible pointer type
if (!atomic_dec_and_test(&mps_entry->refcnt)) {
^
In file included from arch/x86/include/asm/atomic.h:265:0,
from arch/x86/include/asm/msr.h:67,
from arch/x86/include/asm/processor.h:21,
from arch/x86/include/asm/cpufeature.h:5,
from arch/x86/include/asm/thread_info.h:53,
from include/linux/thread_info.h:38,
from arch/x86/include/asm/preempt.h:7,
from include/linux/preempt.h:78,
from include/linux/hardirq.h:5,
from include/linux/interrupt.h:11,
from drivers/net/ethernet/chelsio/cxgb4/cxgb4.h:42,
from drivers/net/ethernet/chelsio/cxgb4/cxgb4_mps.c:4:
include/asm-generic/atomic-instrumented.h:745:1: note: expected 'struct atomic_t *' but argument is of type 'struct refcount_t *'
atomic_dec_and_test(atomic_t *v)
^
vim +/atomic_dec_and_test +17 drivers/net/ethernet/chelsio/cxgb4/cxgb4_mps.c
5
6 static int cxgb4_mps_ref_dec_by_mac(struct adapter *adap,
7 const u8 *addr, const u8 *mask)
8 {
9 u8 bitmask[] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
10 struct mps_entries_ref *mps_entry, *tmp;
11 int ret = -EINVAL;
12
13 spin_lock_bh(&adap->mps_ref_lock);
14 list_for_each_entry_safe(mps_entry, tmp, &adap->mps_ref, list) {
15 if (ether_addr_equal(mps_entry->addr, addr) &&
16 ether_addr_equal(mps_entry->mask, mask ? mask : bitmask)) {
> 17 if (!atomic_dec_and_test(&mps_entry->refcnt)) {
18 spin_unlock_bh(&adap->mps_ref_lock);
19 return -EBUSY;
20 }
21 list_del(&mps_entry->list);
22 kfree(mps_entry);
23 ret = 0;
24 break;
25 }
26 }
27 spin_unlock_bh(&adap->mps_ref_lock);
28 return ret;
29 }
30
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 35462 bytes --]
^ permalink raw reply
* Re: [PATCH net-next 4/6] arm64: dts: fsl: ls1028a: Add Felix switch port DT node
From: Alexandre Belloni @ 2019-06-24 18:26 UTC (permalink / raw)
To: Andrew Lunn
Cc: Allan W. Nielsen, Claudiu Manoil, David S . Miller,
devicetree@vger.kernel.org, netdev@vger.kernel.org,
Alexandru Marginean, linux-kernel@vger.kernel.org,
UNGLinuxDriver@microchip.com, Allan Nielsen, Rob Herring,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190624162431.GX31306@lunn.ch>
On 24/06/2019 18:24:31+0200, Andrew Lunn wrote:
> On Mon, Jun 24, 2019 at 05:23:45PM +0200, Allan W. Nielsen wrote:
> > Hi Andrew,
> >
> > The 06/24/2019 16:26, Andrew Lunn wrote:
> > > > > Yeah, there are 2 ethernet controller ports (managed by the enetc driver)
> > > > > connected inside the SoC via SGMII links to 2 of the switch ports, one of
> > > > > these switch ports can be configured as CPU port (with follow-up patches).
> > > > >
> > > > > This configuration may look prettier on DSA, but the main restriction here
> > > > > is that the entire functionality is provided by the ocelot driver which is a
> > > > > switchdev driver. I don't think it would be a good idea to copy-paste code
> > > > > from ocelot to a separate dsa driver.
> > > > >
> > > >
> > > > We should probably make the ocelot driver a DSA driver then...
> > > An important part of DSA is being able to direct frames out specific
> > > ports when they ingress via the CPU port. Does the silicon support
> > > this? At the moment, i think it is using polled IO.
> >
> > That is supported, it requires a bit of initial configuration of the Chip, but
> > nothing big (I believe this configuration is part of Claudiu's change-set).
> >
> > But how do you envision this done?
> >
> > - Let the existing SwitchDev driver and the DSA driver use a set of common
> > functions.
> > - Convert the existing Ocelot driver from SwitchDev to DSA
> > - Fork (copy) the existing driver of Ocelot, and modify it as needed for the
> > Felix driver
> >
> > My guess is the first one, but I would like to understand what you have in mind.
>
> I don't know the various architectures the switch is used in. But it
> does seem like a core library, and then a switchdev wrapper for Ocelot
> and a DSA wrapper for Felix would make sense.
Ocelot could also be used in a DSA setting where one port can be
connected to an external MAC and be used to inject/extract frames
to/from any other ports. In that case, the IFH would serve as the DSA
tag.
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: selftests: bpf: test_libbpf.sh failed at file test_l4lb.o
From: Andrii Nakryiko @ 2019-06-24 18:32 UTC (permalink / raw)
To: Dan Rue
Cc: Naresh Kamboju, open list:KERNEL SELFTEST FRAMEWORK, bpf, Netdev,
open list, xdp-newbies, David S. Miller, Daniel Borkmann,
Alexei Starovoitov, Martin Lau, Yonghong Song, john fastabend,
hawk, Jakub Kicinski, Shuah Khan
In-Reply-To: <20190621161752.d7d7n4m5q67uivys@xps.therub.org>
On Fri, Jun 21, 2019 at 9:17 AM Dan Rue <dan.rue@linaro.org> wrote:
>
> On Thu, Jun 20, 2019 at 10:17:04PM -0700, Andrii Nakryiko wrote:
> > On Thu, Jun 20, 2019 at 1:08 AM Naresh Kamboju
> > <naresh.kamboju@linaro.org> wrote:
> > >
> > > selftests: bpf test_libbpf.sh failed running Linux -next kernel
> > > 20190618 and 20190619.
> > >
> > > Here is the log from x86_64,
> > > # selftests bpf test_libbpf.sh
> > > bpf: test_libbpf.sh_ #
> > > # [0] libbpf BTF is required, but is missing or corrupted.
> >
> > You need at least clang-9.0.0 (not yet released) to run some of these
> > tests successfully, as they rely on Clang's support for
> > BTF_KIND_VAR/BTF_KIND_DATASEC.
>
> Can there be a runtime check for BTF that emits a skip instead of a fail
> in such a case?
I'm not sure how to do this simply and minimally intrusively. The best
I can come up with is setting some envvar from Makefile and checking
for that in each inidividual test, which honestly sounds a bit gross.
How hard is it for you guys to upgrade compiler used to run these test?
>
> Thanks,
> Dan
>
> >
> > > libbpf: BTF_is #
> > > # test_libbpf failed at file test_l4lb.o
> > > failed: at_file #
> > > # selftests test_libbpf [FAILED]
> > > test_libbpf: [FAILED]_ #
> > > [FAIL] 29 selftests bpf test_libbpf.sh
> > > selftests: bpf_test_libbpf.sh [FAIL]
> > >
> > > Full test log,
> > > https://qa-reports.linaro.org/lkft/linux-next-oe/build/next-20190619/testrun/781777/log
> > >
> > > Test results comparison,
> > > https://qa-reports.linaro.org/lkft/linux-next-oe/tests/kselftest/bpf_test_libbpf.sh
> > >
> > > Good linux -next tag: next-20190617
> > > Bad linux -next tag: next-20190618
> > > git branch master
> > > git commit 1c6b40509daf5190b1fd2c758649f7df1da4827b
> > > git repo
> > > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
> > >
> > > Best regards
> > > Naresh Kamboju
>
> --
> Linaro - Kernel Validation
^ permalink raw reply
* Re: [PATCH] perf cs-etm: Improve completeness for kernel address space
From: Arnaldo Carvalho de Melo @ 2019-06-24 19:00 UTC (permalink / raw)
To: Leo Yan
Cc: Mathieu Poirier, Linux Kernel Mailing List, linux-arm-kernel,
netdev, bpf, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Peter Zijlstra, Suzuki Poulouse, Coresight ML
In-Reply-To: <20190620005829.GH24549@leoy-ThinkPad-X240s>
Em Thu, Jun 20, 2019 at 08:58:29AM +0800, Leo Yan escreveu:
> Hi Mathieu,
>
> On Wed, Jun 19, 2019 at 11:49:44AM -0600, Mathieu Poirier wrote:
>
> [...]
>
> > > diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> > > index 51dd00f65709..4776c2c1fb6d 100644
> > > --- a/tools/perf/Makefile.config
> > > +++ b/tools/perf/Makefile.config
> > > @@ -418,6 +418,30 @@ ifdef CORESIGHT
> > > endif
> > > LDFLAGS += $(LIBOPENCSD_LDFLAGS)
> > > EXTLIBS += $(OPENCSDLIBS)
> > > + ifneq ($(wildcard $(srctree)/arch/arm64/kernel/vmlinux.lds),)
> > > + # Extract info from lds:
> > > + # . = ((((((((0xffffffffffffffff)) - (((1)) << (48)) + 1) + (0)) + (0x08000000))) + (0x08000000))) + 0x00080000;
> > > + # ARM64_PRE_START_SIZE := (0x08000000 + 0x08000000 + 0x00080000)
> > > + ARM64_PRE_START_SIZE := $(shell egrep ' \. \= \({8}0x[0-9a-fA-F]+\){2}' \
> > > + $(srctree)/arch/arm64/kernel/vmlinux.lds | \
> > > + sed -e 's/[(|)|.|=|+|<|;|-]//g' -e 's/ \+/ /g' -e 's/^[ \t]*//' | \
> > > + awk -F' ' '{print "("$$6 "+" $$7 "+" $$8")"}' 2>/dev/null)
> > > + else
> > > + ARM64_PRE_START_SIZE := 0
> > > + endif
> > > + CFLAGS += -DARM64_PRE_START_SIZE="$(ARM64_PRE_START_SIZE)"
> > > + ifneq ($(wildcard $(srctree)/arch/arm/kernel/vmlinux.lds),)
> > > + # Extract info from lds:
> > > + # . = ((0xC0000000)) + 0x00208000;
> > > + # ARM_PRE_START_SIZE := 0x00208000
> > > + ARM_PRE_START_SIZE := $(shell egrep ' \. \= \({2}0x[0-9a-fA-F]+\){2}' \
> > > + $(srctree)/arch/arm/kernel/vmlinux.lds | \
> > > + sed -e 's/[(|)|.|=|+|<|;|-]//g' -e 's/ \+/ /g' -e 's/^[ \t]*//' | \
> > > + awk -F' ' '{print "("$$2")"}' 2>/dev/null)
> > > + else
> > > + ARM_PRE_START_SIZE := 0
> > > + endif
> > > + CFLAGS += -DARM_PRE_START_SIZE="$(ARM_PRE_START_SIZE)"
> > > $(call detected,CONFIG_LIBOPENCSD)
> > > ifdef CSTRACE_RAW
> > > CFLAGS += -DCS_DEBUG_RAW
> > > diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> > > index 0c7776b51045..ae831f836c70 100644
> > > --- a/tools/perf/util/cs-etm.c
> > > +++ b/tools/perf/util/cs-etm.c
> > > @@ -613,10 +613,34 @@ static void cs_etm__free(struct perf_session *session)
> > > static u8 cs_etm__cpu_mode(struct cs_etm_queue *etmq, u64 address)
> > > {
> > > struct machine *machine;
> > > + u64 fixup_kernel_start = 0;
> > > + const char *arch;
> > >
> > > machine = etmq->etm->machine;
> > > + arch = perf_env__arch(machine->env);
> > >
> > > - if (address >= etmq->etm->kernel_start) {
> > > + /*
> > > + * Since arm and arm64 specify some memory regions prior to
> > > + * 'kernel_start', kernel addresses can be less than 'kernel_start'.
> > > + *
> > > + * For arm architecture, the 16MB virtual memory space prior to
> > > + * 'kernel_start' is allocated to device modules, a PMD table if
> > > + * CONFIG_HIGHMEM is enabled and a PGD table.
> > > + *
> > > + * For arm64 architecture, the root PGD table, device module memory
> > > + * region and BPF jit region are prior to 'kernel_start'.
> > > + *
> > > + * To reflect the complete kernel address space, compensate these
> > > + * pre-defined regions for kernel start address.
> > > + */
> > > + if (!strcmp(arch, "arm64"))
> > > + fixup_kernel_start = etmq->etm->kernel_start -
> > > + ARM64_PRE_START_SIZE;
> > > + else if (!strcmp(arch, "arm"))
> > > + fixup_kernel_start = etmq->etm->kernel_start -
> > > + ARM_PRE_START_SIZE;
> >
> > I will test your work but from a quick look wouldn't it be better to
> > have a single define name here? From looking at the modifications you
> > did to Makefile.config there doesn't seem to be a reason to have two.
>
> Thanks for suggestion. I changed to use single define
> ARM_PRE_START_SIZE and sent patch v2 [1].
>
> If possible, please test patch v2.
>
> Thanks,
> Leo Yan
So just for the record, I'm waiting for Mathieu on this one, i.e. for
him to test/ack v3.
- Arnaldo
> [1] https://lore.kernel.org/linux-arm-kernel/20190620005428.20883-1-leo.yan@linaro.org/T/#u
>
> > > +
> > > + if (address >= fixup_kernel_start) {
> > > if (machine__is_host(machine))
> > > return PERF_RECORD_MISC_KERNEL;
> > > else
> > > --
> > > 2.17.1
> > >
--
- Arnaldo
^ 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