As suggested by commit [1], now the driver uses the functions and the
data structures provided by the CAN network device driver interface.
Currently the driver doesn't implement a way to set bitrate for SLCAN
based devices via ip tool, so you'll have to do this by slcand or
slcan_attach invocation through the -sX parameter:
- slcan_attach -f -s6 -o /dev/ttyACM0
- slcand -f -s8 -o /dev/ttyUSB0
where -s6 in will set adapter's bitrate to 500 Kbit/s and -s8 to
1Mbit/s.
See the table below for further CAN bitrates:
- s0 -> 10 Kbit/s
- s1 -> 20 Kbit/s
- s2 -> 50 Kbit/s
- s3 -> 100 Kbit/s
- s4 -> 125 Kbit/s
- s5 -> 250 Kbit/s
- s6 -> 500 Kbit/s
- s7 -> 800 Kbit/s
- s8 -> 1000 Kbit/s
In doing so, the struct can_priv::bittiming.bitrate of the driver is not
set and since the open_candev() checks that the bitrate has been set, it
must be a non-zero value, the bitrate is set to a fake value (-1U)
before it is called.
The patch also changes the slcan_devs locking from rtnl to spin_lock. The
change was tested with a kernel with the CONFIG_PROVE_LOCKING option
enabled that did not show any errors.
[1] commit 39549eef3587f ("can: CAN Network device driver and Netlink interface")
Signed-off-by: Dario Binacchi <[email protected]>
---
Changes in v4:
- Update the commit description.
- Use the CAN_BITRATE_UNKNOWN macro.
- Use kfree_skb() instead of can_put_echo_skb() in the slc_xmit().
- Remove the `if (slcan_devs)' check in the slc_dealloc().
Changes in v3:
- Replace (-1) with (-1U) in the commit description.
Changes in v2:
- Move CAN_SLCAN Kconfig option inside CAN_DEV scope.
- Improve the commit message.
drivers/net/can/Kconfig | 40 +++++++--------
drivers/net/can/slcan.c | 107 ++++++++++++++++++++--------------------
2 files changed, 74 insertions(+), 73 deletions(-)
diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
index b2dcc1e5a388..45997d39621c 100644
--- a/drivers/net/can/Kconfig
+++ b/drivers/net/can/Kconfig
@@ -28,26 +28,6 @@ config CAN_VXCAN
This driver can also be built as a module. If so, the module
will be called vxcan.
-config CAN_SLCAN
- tristate "Serial / USB serial CAN Adaptors (slcan)"
- depends on TTY
- help
- CAN driver for several 'low cost' CAN interfaces that are attached
- via serial lines or via USB-to-serial adapters using the LAWICEL
- ASCII protocol. The driver implements the tty linediscipline N_SLCAN.
-
- As only the sending and receiving of CAN frames is implemented, this
- driver should work with the (serial/USB) CAN hardware from:
- http://www.canusb.com / http://www.can232.com / http://www.mictronics.de / http://www.canhack.de
-
- Userspace tools to attach the SLCAN line discipline (slcan_attach,
- slcand) can be found in the can-utils at the linux-can project, see
- https://github.com/linux-can/can-utils for details.
-
- The slcan driver supports up to 10 CAN netdevices by default which
- can be changed by the 'maxdev=xx' module option. This driver can
- also be built as a module. If so, the module will be called slcan.
-
config CAN_DEV
tristate "Platform CAN drivers with Netlink support"
default y
@@ -118,6 +98,26 @@ config CAN_KVASER_PCIEFD
Kvaser Mini PCI Express HS v2
Kvaser Mini PCI Express 2xHS v2
+config CAN_SLCAN
+ tristate "Serial / USB serial CAN Adaptors (slcan)"
+ depends on TTY
+ help
+ CAN driver for several 'low cost' CAN interfaces that are attached
+ via serial lines or via USB-to-serial adapters using the LAWICEL
+ ASCII protocol. The driver implements the tty linediscipline N_SLCAN.
+
+ As only the sending and receiving of CAN frames is implemented, this
+ driver should work with the (serial/USB) CAN hardware from:
+ http://www.canusb.com / http://www.can232.com / http://www.mictronics.de / http://www.canhack.de
+
+ Userspace tools to attach the SLCAN line discipline (slcan_attach,
+ slcand) can be found in the can-utils at the linux-can project, see
+ https://github.com/linux-can/can-utils for details.
+
+ The slcan driver supports up to 10 CAN netdevices by default which
+ can be changed by the 'maxdev=xx' module option. This driver can
+ also be built as a module. If so, the module will be called slcan.
+
config CAN_SUN4I
tristate "Allwinner A10 CAN controller"
depends on MACH_SUN4I || MACH_SUN7I || COMPILE_TEST
diff --git a/drivers/net/can/slcan.c b/drivers/net/can/slcan.c
index c39580b142e0..c7ff11dd2278 100644
--- a/drivers/net/can/slcan.c
+++ b/drivers/net/can/slcan.c
@@ -56,7 +56,6 @@
#include <linux/can.h>
#include <linux/can/dev.h>
#include <linux/can/skb.h>
-#include <linux/can/can-ml.h>
MODULE_ALIAS_LDISC(N_SLCAN);
MODULE_DESCRIPTION("serial line CAN interface");
@@ -79,6 +78,7 @@ MODULE_PARM_DESC(maxdev, "Maximum number of slcan interfaces");
#define SLC_EFF_ID_LEN 8
struct slcan {
+ struct can_priv can;
int magic;
/* Various fields. */
@@ -100,6 +100,7 @@ struct slcan {
};
static struct net_device **slcan_devs;
+static DEFINE_SPINLOCK(slcan_lock);
/************************************************************************
* SLCAN ENCAPSULATION FORMAT *
@@ -394,6 +395,8 @@ static int slc_close(struct net_device *dev)
clear_bit(TTY_DO_WRITE_WAKEUP, &sl->tty->flags);
}
netif_stop_queue(dev);
+ close_candev(dev);
+ sl->can.state = CAN_STATE_STOPPED;
sl->rcount = 0;
sl->xleft = 0;
spin_unlock_bh(&sl->lock);
@@ -405,20 +408,34 @@ static int slc_close(struct net_device *dev)
static int slc_open(struct net_device *dev)
{
struct slcan *sl = netdev_priv(dev);
+ int err;
if (sl->tty == NULL)
return -ENODEV;
+ /* The baud rate is not set with the command
+ * `ip link set <iface> type can bitrate <baud>' and therefore
+ * can.bittiming.bitrate is CAN_BITRATE_UNSET (0), causing
+ * open_candev() to fail. So let's set to a fake value.
+ */
+ sl->can.bittiming.bitrate = CAN_BITRATE_UNKNOWN;
+ err = open_candev(dev);
+ if (err) {
+ netdev_err(dev, "failed to open can device\n");
+ return err;
+ }
+
+ sl->can.state = CAN_STATE_ERROR_ACTIVE;
sl->flags &= BIT(SLF_INUSE);
netif_start_queue(dev);
return 0;
}
-/* Hook the destructor so we can free slcan devs at the right point in time */
-static void slc_free_netdev(struct net_device *dev)
+static void slc_dealloc(struct slcan *sl)
{
- int i = dev->base_addr;
+ int i = sl->dev->base_addr;
+ free_candev(sl->dev);
slcan_devs[i] = NULL;
}
@@ -434,24 +451,6 @@ static const struct net_device_ops slc_netdev_ops = {
.ndo_change_mtu = slcan_change_mtu,
};
-static void slc_setup(struct net_device *dev)
-{
- dev->netdev_ops = &slc_netdev_ops;
- dev->needs_free_netdev = true;
- dev->priv_destructor = slc_free_netdev;
-
- dev->hard_header_len = 0;
- dev->addr_len = 0;
- dev->tx_queue_len = 10;
-
- dev->mtu = CAN_MTU;
- dev->type = ARPHRD_CAN;
-
- /* New-style flags. */
- dev->flags = IFF_NOARP;
- dev->features = NETIF_F_HW_CSUM;
-}
-
/******************************************
Routines looking at TTY side.
******************************************/
@@ -514,11 +513,8 @@ static void slc_sync(void)
static struct slcan *slc_alloc(void)
{
int i;
- char name[IFNAMSIZ];
struct net_device *dev = NULL;
- struct can_ml_priv *can_ml;
struct slcan *sl;
- int size;
for (i = 0; i < maxdev; i++) {
dev = slcan_devs[i];
@@ -531,16 +527,14 @@ static struct slcan *slc_alloc(void)
if (i >= maxdev)
return NULL;
- sprintf(name, "slcan%d", i);
- size = ALIGN(sizeof(*sl), NETDEV_ALIGN) + sizeof(struct can_ml_priv);
- dev = alloc_netdev(size, name, NET_NAME_UNKNOWN, slc_setup);
+ dev = alloc_candev(sizeof(*sl), 1);
if (!dev)
return NULL;
+ snprintf(dev->name, sizeof(dev->name), "slcan%d", i);
+ dev->netdev_ops = &slc_netdev_ops;
dev->base_addr = i;
sl = netdev_priv(dev);
- can_ml = (void *)sl + ALIGN(sizeof(*sl), NETDEV_ALIGN);
- can_set_ml_priv(dev, can_ml);
/* Initialize channel control data */
sl->magic = SLCAN_MAGIC;
@@ -573,11 +567,7 @@ static int slcan_open(struct tty_struct *tty)
if (tty->ops->write == NULL)
return -EOPNOTSUPP;
- /* RTnetlink lock is misused here to serialize concurrent
- opens of slcan channels. There are better ways, but it is
- the simplest one.
- */
- rtnl_lock();
+ spin_lock(&slcan_lock);
/* Collect hanged up channels. */
slc_sync();
@@ -605,13 +595,15 @@ static int slcan_open(struct tty_struct *tty)
set_bit(SLF_INUSE, &sl->flags);
- err = register_netdevice(sl->dev);
- if (err)
+ err = register_candev(sl->dev);
+ if (err) {
+ pr_err("slcan: can't register candev\n");
goto err_free_chan;
+ }
}
/* Done. We have linked the TTY line to a channel. */
- rtnl_unlock();
+ spin_unlock(&slcan_lock);
tty->receive_room = 65536; /* We don't flow control */
/* TTY layer expects 0 on success */
@@ -621,14 +613,10 @@ static int slcan_open(struct tty_struct *tty)
sl->tty = NULL;
tty->disc_data = NULL;
clear_bit(SLF_INUSE, &sl->flags);
- slc_free_netdev(sl->dev);
- /* do not call free_netdev before rtnl_unlock */
- rtnl_unlock();
- free_netdev(sl->dev);
- return err;
+ slc_dealloc(sl);
err_exit:
- rtnl_unlock();
+ spin_unlock(&slcan_lock);
/* Count references from TTY module */
return err;
@@ -658,9 +646,11 @@ static void slcan_close(struct tty_struct *tty)
synchronize_rcu();
flush_work(&sl->tx_work);
- /* Flush network side */
- unregister_netdev(sl->dev);
- /* This will complete via sl_free_netdev */
+ slc_close(sl->dev);
+ unregister_candev(sl->dev);
+ spin_lock(&slcan_lock);
+ slc_dealloc(sl);
+ spin_unlock(&slcan_lock);
}
static void slcan_hangup(struct tty_struct *tty)
@@ -768,18 +758,29 @@ static void __exit slcan_exit(void)
dev = slcan_devs[i];
if (!dev)
continue;
- slcan_devs[i] = NULL;
- sl = netdev_priv(dev);
- if (sl->tty) {
- netdev_err(dev, "tty discipline still running\n");
- }
+ spin_lock(&slcan_lock);
+ dev = slcan_devs[i];
+ if (dev) {
+ slcan_devs[i] = NULL;
+ spin_unlock(&slcan_lock);
+ sl = netdev_priv(dev);
+ if (sl->tty) {
+ netdev_err(dev,
+ "tty discipline still running\n");
+ }
- unregister_netdev(dev);
+ slc_close(dev);
+ unregister_candev(dev);
+ } else {
+ spin_unlock(&slcan_lock);
+ }
}
+ spin_lock(&slcan_lock);
kfree(slcan_devs);
slcan_devs = NULL;
+ spin_unlock(&slcan_lock);
tty_unregister_ldisc(&slc_ldisc);
}
--
2.32.0
Greeting,
FYI, we noticed the following commit (built with gcc-11):
commit: 39bbbe2356a2a13ec26a8765d8a4e6dfe7ce33e9 ("[PATCH v4 05/12] can: slcan: use CAN network device driver API")
url: https://github.com/intel-lab-lkp/linux/commits/Dario-Binacchi/can-slcan-extend-supported-features/20220614-203636
patch link: https://lore.kernel.org/lkml/[email protected]
in testcase: ltp
version: ltp-x86_64-14c1f76-1_20220625
with following parameters:
test: pty
ucode: 0x21
test-description: The LTP testsuite contains a collection of tools for testing the Linux kernel and related features.
test-url: http://linux-test-project.github.io/
on test machine: 4 threads 1 sockets Intel(R) Core(TM) i3-3220 CPU @ 3.30GHz with 8G memory
caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):
If you fix the issue, kindly add following tag
Reported-by: kernel test robot <[email protected]>
[ 164.290583][ T3878] BUG: sleeping function called from invalid context at mm/page_alloc.c:5203
[ 164.290589][ T3878] in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 3878, name: pty03
[ 164.290592][ T3878] preempt_count: 1, expected: 0
[ 164.290595][ T3878] CPU: 1 PID: 3878 Comm: pty03 Tainted: G S 5.19.0-rc2-00005-g39bbbe2356a2 #1
[ 164.290600][ T3878] Hardware name: Hewlett-Packard p6-1451cx/2ADA, BIOS 8.15 02/05/2013
[ 164.290602][ T3878] Call Trace:
[ 164.290604][ T3878] <TASK>
[ 164.290606][ T3878] dump_stack_lvl (lib/dump_stack.c:107 (discriminator 1))
[ 164.290615][ T3878] __might_resched.cold (kernel/sched/core.c:9792)
[ 164.290621][ T3878] prepare_alloc_pages+0x330/0x500
[ 164.290628][ T3878] ? tty_set_ldisc (drivers/tty/tty_ldisc.c:555)
[ 164.290634][ T3878] __alloc_pages (mm/page_alloc.c:5415)
[ 164.290637][ T3878] ? __alloc_pages_slowpath+0x15c0/0x15c0
[ 164.290641][ T3878] ? tty_set_ldisc (drivers/tty/tty_ldisc.c:555)
[ 164.290645][ T3878] ? kasan_save_stack (mm/kasan/common.c:40)
[ 164.290650][ T3878] ? kasan_set_track (mm/kasan/common.c:45)
[ 164.290654][ T3878] ? __kasan_slab_free (mm/kasan/common.c:368 mm/kasan/common.c:328 mm/kasan/common.c:374)
[ 164.290658][ T3878] ? entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:115)
[ 164.290663][ T3878] ? do_syscall_64 (arch/x86/entry/common.c:50 arch/x86/entry/common.c:80)
[ 164.290668][ T3878] ? entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:115)
[ 164.290672][ T3878] kmalloc_large_node (mm/slub.c:4432)
[ 164.290677][ T3878] __kmalloc_node (include/asm-generic/getorder.h:41 mm/slub.c:4450)
[ 164.290681][ T3878] kvmalloc_node (mm/util.c:619)
[ 164.290687][ T3878] alloc_netdev_mqs (net/core/dev.c:10577)
[ 164.290694][ T3878] ? can_get_state_str (drivers/net/can/dev/dev.c:222) can_dev
[ 164.290704][ T3878] ? _raw_spin_lock (arch/x86/include/asm/atomic.h:202 include/linux/atomic/atomic-instrumented.h:543 include/asm-generic/qspinlock.h:111 include/linux/spinlock.h:185 include/linux/spinlock_api_smp.h:134 kernel/locking/spinlock.c:154)
[ 164.290708][ T3878] alloc_candev_mqs (drivers/net/can/dev/dev.c:262) can_dev
[ 164.290717][ T3878] slcan_open (drivers/net/can/slcan.c:531 drivers/net/can/slcan.c:584) slcan
[ 164.290723][ T3878] tty_ldisc_open (drivers/tty/tty_ldisc.c:433)
[ 164.290728][ T3878] tty_set_ldisc (drivers/tty/tty_ldisc.c:558)
[ 164.290733][ T3878] tty_ioctl (drivers/tty/tty_io.c:2714)
[ 164.290736][ T3878] ? do_sys_openat2 (fs/open.c:1288)
[ 164.290740][ T3878] ? tty_release (drivers/tty/tty_io.c:2655)
[ 164.290744][ T3878] ? do_sys_openat2 (fs/open.c:1288)
[ 164.290747][ T3878] ? build_open_flags (fs/open.c:1264)
[ 164.290751][ T3878] ? __ia32_sys_stat (fs/stat.c:396)
[ 164.290755][ T3878] ? userns_owner (kernel/user_namespace.c:371)
[ 164.290760][ T3878] ? __fget_files (arch/x86/include/asm/atomic64_64.h:22 include/linux/atomic/atomic-arch-fallback.h:2363 include/linux/atomic/atomic-arch-fallback.h:2388 include/linux/atomic/atomic-arch-fallback.h:2404 include/linux/atomic/atomic-long.h:497 include/linux/atomic/atomic-instrumented.h:1854 fs/file.c:882 fs/file.c:913)
[ 164.290764][ T3878] __x64_sys_ioctl (fs/ioctl.c:52 fs/ioctl.c:870 fs/ioctl.c:856 fs/ioctl.c:856)
[ 164.290768][ T3878] do_syscall_64 (arch/x86/entry/common.c:50 arch/x86/entry/common.c:80)
[ 164.290773][ T3878] entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:115)
[ 164.290777][ T3878] RIP: 0033:0x7f86f3323cc7
[ 164.290781][ T3878] Code: 00 00 00 48 8b 05 c9 91 0c 00 64 c7 00 26 00 00 00 48 c7 c0 ff ff ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 b8 10 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 99 91 0c 00 f7 d8 64 89 01 48
All code
========
0: 00 00 add %al,(%rax)
2: 00 48 8b add %cl,-0x75(%rax)
5: 05 c9 91 0c 00 add $0xc91c9,%eax
a: 64 c7 00 26 00 00 00 movl $0x26,%fs:(%rax)
11: 48 c7 c0 ff ff ff ff mov $0xffffffffffffffff,%rax
18: c3 retq
19: 66 2e 0f 1f 84 00 00 nopw %cs:0x0(%rax,%rax,1)
20: 00 00 00
23: b8 10 00 00 00 mov $0x10,%eax
28: 0f 05 syscall
2a:* 48 3d 01 f0 ff ff cmp $0xfffffffffffff001,%rax <-- trapping instruction
30: 73 01 jae 0x33
32: c3 retq
33: 48 8b 0d 99 91 0c 00 mov 0xc9199(%rip),%rcx # 0xc91d3
3a: f7 d8 neg %eax
3c: 64 89 01 mov %eax,%fs:(%rcx)
3f: 48 rex.W
Code starting with the faulting instruction
===========================================
0: 48 3d 01 f0 ff ff cmp $0xfffffffffffff001,%rax
6: 73 01 jae 0x9
8: c3 retq
9: 48 8b 0d 99 91 0c 00 mov 0xc9199(%rip),%rcx # 0xc91a9
10: f7 d8 neg %eax
12: 64 89 01 mov %eax,%fs:(%rcx)
15: 48 rex.W
[ 164.290784][ T3878] RSP: 002b:00007fff6a8fb8b8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
[ 164.290789][ T3878] RAX: ffffffffffffffda RBX: 00007f86f322c6c0 RCX: 00007f86f3323cc7
[ 164.290792][ T3878] RDX: 000055911985c700 RSI: 0000000000005423 RDI: 000000000000000d
[ 164.290795][ T3878] RBP: 000055911985c700 R08: 0000000000000000 R09: cccccccccccccccd
[ 164.290797][ T3878] R10: 0000000000000000 R11: 0000000000000246 R12: 000000000000000d
[ 164.290799][ T3878] R13: 0000000000000006 R14: 000055911985c6a0 R15: 0000000000000004
[ 164.290803][ T3878] </TASK>
[ 164.738226][ T3878] BUG: scheduling while atomic: pty03/3878/0x00000002
[ 164.738233][ T3878] Modules linked in: slcan can_dev n_hdlc netconsole btrfs blake2b_generic xor raid6_pq zstd_compress libcrc32c sd_mod t10_pi crc64_rocksoft_generic crc64_rocksoft crc64 sg i915 intel_rapl_msr intel_gtt intel_rapl_common drm_buddy x86_pkg_temp_thermal drm_display_helper intel_powerclamp coretemp crct10dif_pclmul crc32_pclmul crc32c_intel ahci ttm ghash_clmulni_intel rapl libahci intel_cstate ipmi_devintf drm_kms_helper intel_uncore ipmi_msghandler usb_storage mei_me syscopyarea sysfillrect libata sysimgblt mei fb_sys_fops video drm fuse ip_tables
[ 164.738298][ T3878] CPU: 3 PID: 3878 Comm: pty03 Tainted: G S W 5.19.0-rc2-00005-g39bbbe2356a2 #1
[ 164.738302][ T3878] Hardware name: Hewlett-Packard p6-1451cx/2ADA, BIOS 8.15 02/05/2013
[ 164.738305][ T3878] Call Trace:
[ 164.738307][ T3878] <TASK>
[ 164.738309][ T3878] dump_stack_lvl (lib/dump_stack.c:107 (discriminator 1))
[ 164.738318][ T3878] __schedule_bug.cold (kernel/sched/core.c:5661)
[ 164.738323][ T3878] schedule_debug (arch/x86/include/asm/preempt.h:35 kernel/sched/core.c:5688)
[ 164.738329][ T3878] __schedule (arch/x86/include/asm/jump_label.h:27 include/linux/jump_label.h:207 kernel/sched/features.h:40 kernel/sched/core.c:6324)
[ 164.738335][ T3878] ? node_tag_clear (arch/x86/include/asm/bitops.h:94 include/asm-generic/bitops/instrumented-non-atomic.h:43 lib/radix-tree.c:107 lib/radix-tree.c:1000)
[ 164.738340][ T3878] ? io_schedule_timeout (kernel/sched/core.c:6310)
[ 164.738344][ T3878] ? idr_alloc_u32 (lib/idr.c:55)
[ 164.738349][ T3878] ? _raw_spin_lock_irq (arch/x86/include/asm/atomic.h:202 include/linux/atomic/atomic-instrumented.h:543 include/asm-generic/qspinlock.h:111 include/linux/spinlock.h:185 include/linux/spinlock_api_smp.h:120 kernel/locking/spinlock.c:170)
[ 164.738353][ T3878] schedule (include/linux/instrumented.h:71 (discriminator 1) include/asm-generic/bitops/instrumented-non-atomic.h:134 (discriminator 1) include/linux/thread_info.h:118 (discriminator 1) include/linux/sched.h:2196 (discriminator 1) kernel/sched/core.c:6502 (discriminator 1))
[ 164.738358][ T3878] rwsem_down_write_slowpath (arch/x86/include/asm/current.h:15 kernel/locking/rwsem.c:1174)
[ 164.738362][ T3878] ? __down_timeout (kernel/locking/rwsem.c:1099)
[ 164.738365][ T3878] ? _raw_write_lock_irq (kernel/locking/spinlock.c:153)
[ 164.738370][ T3878] ? idr_get_free (lib/radix-tree.c:1533)
[ 164.738373][ T3878] ? kernfs_dir_fop_release (fs/kernfs/dir.c:584)
[ 164.738379][ T3878] down_write (kernel/locking/rwsem.c:1544)
[ 164.738383][ T3878] ? down_write_killable (kernel/locking/rwsem.c:1540)
[ 164.738386][ T3878] ? idr_alloc (lib/idr.c:118)
[ 164.738389][ T3878] ? _raw_spin_lock (arch/x86/include/asm/atomic.h:202 include/linux/atomic/atomic-instrumented.h:543 include/asm-generic/qspinlock.h:111 include/linux/spinlock.h:185 include/linux/spinlock_api_smp.h:134 kernel/locking/spinlock.c:154)
[ 164.738393][ T3878] ? __fprop_add_percpu_max (lib/idr.c:35)
[ 164.738397][ T3878] kernfs_add_one (include/linux/kernfs.h:332 fs/kernfs/dir.c:737)
[ 164.738402][ T3878] ? kernfs_new_node (fs/kernfs/dir.c:659)
[ 164.738406][ T3878] ? device_remove_bin_file (drivers/base/core.c:2089)
[ 164.738411][ T3878] __kernfs_create_file (fs/kernfs/file.c:1016)
[ 164.738415][ T3878] sysfs_add_file_mode_ns (fs/sysfs/file.c:301)
[ 164.738419][ T3878] ? projid_m_show (kernel/user_namespace.c:308)
[ 164.738423][ T3878] ? _raw_write_lock_irq (kernel/locking/spinlock.c:153)
[ 164.738427][ T3878] create_files (fs/sysfs/group.c:66 (discriminator 3))
[ 164.738431][ T3878] ? make_kgid (kernel/user_namespace.c:475)
[ 164.738434][ T3878] internal_create_group (fs/sysfs/group.c:148)
[ 164.738438][ T3878] ? down_write (arch/x86/include/asm/atomic64_64.h:34 include/linux/atomic/atomic-long.h:41 include/linux/atomic/atomic-instrumented.h:1280 kernel/locking/rwsem.c:139 kernel/locking/rwsem.c:256 kernel/locking/rwsem.c:1286 kernel/locking/rwsem.c:1296 kernel/locking/rwsem.c:1543)
[ 164.738441][ T3878] ? create_files (fs/sysfs/group.c:109)
[ 164.738444][ T3878] ? __cond_resched (kernel/sched/core.c:8217)
[ 164.738448][ T3878] ? down_write (arch/x86/include/asm/atomic64_64.h:34 include/linux/atomic/atomic-long.h:41 include/linux/atomic/atomic-instrumented.h:1280 kernel/locking/rwsem.c:139 kernel/locking/rwsem.c:256 kernel/locking/rwsem.c:1286 kernel/locking/rwsem.c:1296 kernel/locking/rwsem.c:1543)
[ 164.738452][ T3878] internal_create_groups+0x7c/0x140
[ 164.738456][ T3878] device_add_attrs (drivers/base/core.c:2621)
[ 164.738460][ T3878] ? klist_children_put (drivers/base/core.c:2614)
[ 164.738464][ T3878] ? kernfs_create_link (fs/kernfs/symlink.c:48)
[ 164.738467][ T3878] ? kernfs_put (arch/x86/include/asm/atomic.h:123 (discriminator 1) include/linux/atomic/atomic-instrumented.h:576 (discriminator 1) fs/kernfs/dir.c:521 (discriminator 1))
[ 164.738471][ T3878] device_add (drivers/base/core.c:3368)
[ 164.738476][ T3878] ? device_initialize (drivers/base/core.c:3200)
[ 164.738479][ T3878] ? __fw_devlink_link_to_suppliers (drivers/base/core.c:3299)
[ 164.738484][ T3878] ? __hrtimer_init (kernel/time/hrtimer.c:1559)
[ 164.738488][ T3878] netdev_register_kobject (net/core/net-sysfs.c:2014)
[ 164.738493][ T3878] register_netdevice (net/core/dev.c:10047)
[ 164.738498][ T3878] ? __cond_resched (kernel/sched/core.c:8217)
[ 164.738502][ T3878] ? netdev_change_features (net/core/dev.c:9942)
[ 164.738507][ T3878] register_netdev (net/core/dev.c:10171)
[ 164.738511][ T3878] register_candev (drivers/net/can/dev/dev.c:460) can_dev
[ 164.738522][ T3878] ? can_set_termination (drivers/net/can/dev/dev.c:460) can_dev
[ 164.738529][ T3878] ? _raw_spin_lock (arch/x86/include/asm/atomic.h:202 include/linux/atomic/atomic-instrumented.h:543 include/asm-generic/qspinlock.h:111 include/linux/spinlock.h:185 include/linux/spinlock_api_smp.h:134 kernel/locking/spinlock.c:154)
[ 164.738533][ T3878] ? alloc_candev_mqs (drivers/net/can/dev/dev.c:284) can_dev
[ 164.738541][ T3878] slcan_open (drivers/net/can/slcan.c:598) slcan
[ 164.738547][ T3878] tty_ldisc_open (drivers/tty/tty_ldisc.c:433)
[ 164.738552][ T3878] tty_set_ldisc (drivers/tty/tty_ldisc.c:558)
[ 164.738557][ T3878] tty_ioctl (drivers/tty/tty_io.c:2714)
[ 164.738561][ T3878] ? tty_release (drivers/tty/tty_io.c:2655)
[ 164.738565][ T3878] ? __switch_to (arch/x86/kernel/process.h:38 arch/x86/kernel/process_64.c:627)
[ 164.738569][ T3878] ? __schedule (kernel/sched/core.c:6310)
[ 164.738574][ T3878] ? io_schedule_timeout (kernel/sched/core.c:6310)
[ 164.738578][ T3878] ? __fget_files (arch/x86/include/asm/atomic64_64.h:22 include/linux/atomic/atomic-arch-fallback.h:2363 include/linux/atomic/atomic-arch-fallback.h:2388 include/linux/atomic/atomic-arch-fallback.h:2404 include/linux/atomic/atomic-long.h:497 include/linux/atomic/atomic-instrumented.h:1854 fs/file.c:882 fs/file.c:913)
[ 164.738583][ T3878] __x64_sys_ioctl (fs/ioctl.c:52 fs/ioctl.c:870 fs/ioctl.c:856 fs/ioctl.c:856)
[ 164.738587][ T3878] do_syscall_64 (arch/x86/entry/common.c:50 arch/x86/entry/common.c:80)
[ 164.738591][ T3878] entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:115)
[ 164.738596][ T3878] RIP: 0033:0x7f86f3323cc7
[ 164.738599][ T3878] Code: 00 00 00 48 8b 05 c9 91 0c 00 64 c7 00 26 00 00 00 48 c7 c0 ff ff ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 b8 10 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 99 91 0c 00 f7 d8 64 89 01 48
All code
========
0: 00 00 add %al,(%rax)
2: 00 48 8b add %cl,-0x75(%rax)
5: 05 c9 91 0c 00 add $0xc91c9,%eax
a: 64 c7 00 26 00 00 00 movl $0x26,%fs:(%rax)
11: 48 c7 c0 ff ff ff ff mov $0xffffffffffffffff,%rax
18: c3 retq
19: 66 2e 0f 1f 84 00 00 nopw %cs:0x0(%rax,%rax,1)
20: 00 00 00
23: b8 10 00 00 00 mov $0x10,%eax
28: 0f 05 syscall
2a:* 48 3d 01 f0 ff ff cmp $0xfffffffffffff001,%rax <-- trapping instruction
30: 73 01 jae 0x33
32: c3 retq
33: 48 8b 0d 99 91 0c 00 mov 0xc9199(%rip),%rcx # 0xc91d3
3a: f7 d8 neg %eax
3c: 64 89 01 mov %eax,%fs:(%rcx)
3f: 48 rex.W
Code starting with the faulting instruction
===========================================
0: 48 3d 01 f0 ff ff cmp $0xfffffffffffff001,%rax
6: 73 01 jae 0x9
8: c3 retq
9: 48 8b 0d 99 91 0c 00 mov 0xc9199(%rip),%rcx # 0xc91a9
10: f7 d8 neg %eax
12: 64 89 01 mov %eax,%fs:(%rcx)
15: 48 rex.W
[ 164.738603][ T3878] RSP: 002b:00007fff6a8fb8b8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
[ 164.738608][ T3878] RAX: ffffffffffffffda RBX: 00007f86f322c6c0 RCX: 00007f86f3323cc7
[ 164.738611][ T3878] RDX: 000055911985c700 RSI: 0000000000005423 RDI: 000000000000000d
[ 164.738613][ T3878] RBP: 000055911985c700 R08: 0000000000000000 R09: cccccccccccccccd
[ 164.738616][ T3878] R10: 0000000000000000 R11: 0000000000000246 R12: 000000000000000d
[ 164.738618][ T3878] R13: 0000000000000006 R14: 000055911985c6a0 R15: 0000000000000004
[ 164.738621][ T3878] </TASK>
To reproduce:
git clone https://github.com/intel/lkp-tests.git
cd lkp-tests
sudo bin/lkp install job.yaml # job file is attached in this email
bin/lkp split-job --compatible job.yaml # generate the yaml file for lkp run
sudo bin/lkp run generated-yaml-file
# if come across any failure that blocks the test,
# please remove ~/.lkp and /lkp dir to run from a clean state.
--
0-DAY CI Kernel Test Service
https://01.org/lkp
On 14.06.2022 14:28:14, Dario Binacchi wrote:
> As suggested by commit [1], now the driver uses the functions and the
> data structures provided by the CAN network device driver interface.
>
> Currently the driver doesn't implement a way to set bitrate for SLCAN
> based devices via ip tool, so you'll have to do this by slcand or
> slcan_attach invocation through the -sX parameter:
>
> - slcan_attach -f -s6 -o /dev/ttyACM0
> - slcand -f -s8 -o /dev/ttyUSB0
>
> where -s6 in will set adapter's bitrate to 500 Kbit/s and -s8 to
> 1Mbit/s.
> See the table below for further CAN bitrates:
> - s0 -> 10 Kbit/s
> - s1 -> 20 Kbit/s
> - s2 -> 50 Kbit/s
> - s3 -> 100 Kbit/s
> - s4 -> 125 Kbit/s
> - s5 -> 250 Kbit/s
> - s6 -> 500 Kbit/s
> - s7 -> 800 Kbit/s
> - s8 -> 1000 Kbit/s
>
> In doing so, the struct can_priv::bittiming.bitrate of the driver is not
> set and since the open_candev() checks that the bitrate has been set, it
> must be a non-zero value, the bitrate is set to a fake value (-1U)
> before it is called.
>
> The patch also changes the slcan_devs locking from rtnl to spin_lock. The
> change was tested with a kernel with the CONFIG_PROVE_LOCKING option
> enabled that did not show any errors.
You're not allowed to call alloc_candev() with a spin_lock held. See
today's kernel test robot mail:
| https://lore.kernel.org/all/YrpqO5jepAvv4zkf@xsang-OptiPlex-9020
I think it's best to keep the rtnl for now.
regards,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung West/Dortmund | Phone: +49-231-2826-924 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Hi Marc,
On Tue, Jun 28, 2022 at 11:28 AM Marc Kleine-Budde <[email protected]> wrote:
>
> On 14.06.2022 14:28:14, Dario Binacchi wrote:
> > As suggested by commit [1], now the driver uses the functions and the
> > data structures provided by the CAN network device driver interface.
> >
> > Currently the driver doesn't implement a way to set bitrate for SLCAN
> > based devices via ip tool, so you'll have to do this by slcand or
> > slcan_attach invocation through the -sX parameter:
> >
> > - slcan_attach -f -s6 -o /dev/ttyACM0
> > - slcand -f -s8 -o /dev/ttyUSB0
> >
> > where -s6 in will set adapter's bitrate to 500 Kbit/s and -s8 to
> > 1Mbit/s.
> > See the table below for further CAN bitrates:
> > - s0 -> 10 Kbit/s
> > - s1 -> 20 Kbit/s
> > - s2 -> 50 Kbit/s
> > - s3 -> 100 Kbit/s
> > - s4 -> 125 Kbit/s
> > - s5 -> 250 Kbit/s
> > - s6 -> 500 Kbit/s
> > - s7 -> 800 Kbit/s
> > - s8 -> 1000 Kbit/s
> >
> > In doing so, the struct can_priv::bittiming.bitrate of the driver is not
> > set and since the open_candev() checks that the bitrate has been set, it
> > must be a non-zero value, the bitrate is set to a fake value (-1U)
> > before it is called.
> >
> > The patch also changes the slcan_devs locking from rtnl to spin_lock. The
> > change was tested with a kernel with the CONFIG_PROVE_LOCKING option
> > enabled that did not show any errors.
>
> You're not allowed to call alloc_candev() with a spin_lock held. See
> today's kernel test robot mail:
>
> | https://lore.kernel.org/all/YrpqO5jepAvv4zkf@xsang-OptiPlex-9020
>
> I think it's best to keep the rtnl for now.
The rtnl_lock() uses a mutex while I used a spin_lock.
static DEFINE_MUTEX(rtnl_mutex);
void rtnl_lock(void)
{
mutex_lock(&rtnl_mutex);
}
EXPORT_SYMBOL(rtnl_lock);
So might it be worth trying with a mutex instead of rtnl_lock(), or do
you think it is
safer to return to rtn_lock () anyway?
Thanks and regards,
Dario
>
> regards,
> Marc
>
> --
> Pengutronix e.K. | Marc Kleine-Budde |
> Embedded Linux | https://www.pengutronix.de |
> Vertretung West/Dortmund | Phone: +49-231-2826-924 |
> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
--
Dario Binacchi
Embedded Linux Developer
[email protected]
__________________________________
Amarula Solutions SRL
Via Le Canevare 30, 31100 Treviso, Veneto, IT
T. +39 042 243 5310
[email protected]
http://www.amarulasolutions.com
On 28.06.2022 11:48:48, Dario Binacchi wrote:
> Hi Marc,
>
> On Tue, Jun 28, 2022 at 11:28 AM Marc Kleine-Budde <[email protected]> wrote:
> >
> > On 14.06.2022 14:28:14, Dario Binacchi wrote:
> > > As suggested by commit [1], now the driver uses the functions and the
> > > data structures provided by the CAN network device driver interface.
> > >
> > > Currently the driver doesn't implement a way to set bitrate for SLCAN
> > > based devices via ip tool, so you'll have to do this by slcand or
> > > slcan_attach invocation through the -sX parameter:
> > >
> > > - slcan_attach -f -s6 -o /dev/ttyACM0
> > > - slcand -f -s8 -o /dev/ttyUSB0
> > >
> > > where -s6 in will set adapter's bitrate to 500 Kbit/s and -s8 to
> > > 1Mbit/s.
> > > See the table below for further CAN bitrates:
> > > - s0 -> 10 Kbit/s
> > > - s1 -> 20 Kbit/s
> > > - s2 -> 50 Kbit/s
> > > - s3 -> 100 Kbit/s
> > > - s4 -> 125 Kbit/s
> > > - s5 -> 250 Kbit/s
> > > - s6 -> 500 Kbit/s
> > > - s7 -> 800 Kbit/s
> > > - s8 -> 1000 Kbit/s
> > >
> > > In doing so, the struct can_priv::bittiming.bitrate of the driver is not
> > > set and since the open_candev() checks that the bitrate has been set, it
> > > must be a non-zero value, the bitrate is set to a fake value (-1U)
> > > before it is called.
> > >
> > > The patch also changes the slcan_devs locking from rtnl to spin_lock. The
> > > change was tested with a kernel with the CONFIG_PROVE_LOCKING option
> > > enabled that did not show any errors.
> >
> > You're not allowed to call alloc_candev() with a spin_lock held. See
> > today's kernel test robot mail:
> >
> > | https://lore.kernel.org/all/YrpqO5jepAvv4zkf@xsang-OptiPlex-9020
> >
> > I think it's best to keep the rtnl for now.
>
> The rtnl_lock() uses a mutex while I used a spin_lock.
>
> static DEFINE_MUTEX(rtnl_mutex);
>
> void rtnl_lock(void)
> {
> mutex_lock(&rtnl_mutex);
> }
> EXPORT_SYMBOL(rtnl_lock);
>
> So might it be worth trying with a mutex instead of rtnl_lock(), or do
> you think it is
> safer to return to rtn_lock () anyway?
As Max pointed out the whole static dev array is not needed at all. Just
keep the rtnl, until removing the array altogether.
regards,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung West/Dortmund | Phone: +49-231-2826-924 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |