Hello,
Dario Binacchi <dario.binacchi@xxxxxxxxxxxxxxxxxxxx> writes:
> Hello,
>
> On Mon, Jul 11, 2022 at 10:05 AM Richard Palethorpe <rpalethorpe@xxxxxxx> wrote:
>>
>> Hello,
>>
>> kernel test robot <oliver.sang@xxxxxxxxx> writes:
>>
>> > Greeting,
>> >
>> > FYI, we noticed the following commit (built with gcc-11):
>> >
>> > commit: c4e54b063f42f20a6b3ad1ffa61c574e631e0216 ("can: slcan: use CAN network device driver API")
>> > https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git
>> > master
>>
>> I guess the problem is this may sleep with soft irqs disabled.
>>
>> static int slc_close(struct net_device *dev)
>> {
>> struct slcan *sl = netdev_priv(dev);
>> int err;
>>
>> spin_lock_bh(&sl->lock); <-- takes lock
>> if (sl->tty) {
>> if (sl->can.bittiming.bitrate &&
>> sl->can.bittiming.bitrate != CAN_BITRATE_UNKNOWN) {
>> spin_unlock_bh(&sl->lock);
>> err = slcan_transmit_cmd(sl, "C\r");
>> spin_lock_bh(&sl->lock);
>> if (err)
>> netdev_warn(dev,
>> "failed to send close command 'C\\r'\n");
>> }
>>
>> /* TTY discipline is running. */
>> clear_bit(TTY_DO_WRITE_WAKEUP, &sl->tty->flags);
>> }
>> netif_stop_queue(dev);
>> close_candev(dev); <-- calls cancel_delayed_work_sync()
>>
>
> I would try (since I am unable to replicate the test) to move the
> spin_unlock_bh()
> before calling close_candev().
I haven't tried, but I think it should replicate every time with
lockdep/lock debugging enabled.
> Can the patch be sent now or do I have to wait until the code is in
> mainline?
IMO it *has* to be fixed before going into mainline :-p. I can't comment on
the correctness of the proposed fix though.
--
Thank you,
Richard.
