From: Richard Palethorpe <rpalethorpe@suse.de>
To: Dario Binacchi <dario.binacchi@amarulasolutions.com>
Cc: kernel test robot <oliver.sang@intel.com>,
lkp@intel.com, lkp@lists.01.org,
Jeroen Hofstee <jhofstee@victronenergy.com>,
LKML <linux-kernel@vger.kernel.org>,
linux-can@vger.kernel.org,
Linux Memory Management List <linux-mm@kvack.org>,
Marc Kleine-Budde <mkl@pengutronix.de>,
ltp@lists.linux.it
Subject: Re: [LTP] [can] c4e54b063f: BUG:sleeping_function_called_from_invalid_context_at_kernel/workqueue.c
Date: Wed, 13 Jul 2022 09:23:28 +0100 [thread overview]
Message-ID: <87v8s1l8a3.fsf@suse.de> (raw)
In-Reply-To: <CABGWkvqF9f4vOYUQeYuaDGT7yuB=8=h=yPpuG04VwicNP=wgMA@mail.gmail.com>
Hello,
Dario Binacchi <dario.binacchi@amarulasolutions.com> writes:
> Hello,
>
> On Mon, Jul 11, 2022 at 10:05 AM Richard Palethorpe <rpalethorpe@suse.de> wrote:
>>
>> Hello,
>>
>> kernel test robot <oliver.sang@intel.com> 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.
WARNING: multiple messages have this Message-ID (diff)
From: Richard Palethorpe <rpalethorpe@suse.de>
To: Dario Binacchi <dario.binacchi@amarulasolutions.com>
Cc: lkp@intel.com, Linux Memory Management List <linux-mm@kvack.org>,
Jeroen Hofstee <jhofstee@victronenergy.com>,
LKML <linux-kernel@vger.kernel.org>,
linux-can@vger.kernel.org, lkp@lists.01.org,
kernel test robot <oliver.sang@intel.com>,
Marc Kleine-Budde <mkl@pengutronix.de>,
ltp@lists.linux.it
Subject: Re: [LTP] [can] c4e54b063f: BUG:sleeping_function_called_from_invalid_context_at_kernel/workqueue.c
Date: Wed, 13 Jul 2022 09:23:28 +0100 [thread overview]
Message-ID: <87v8s1l8a3.fsf@suse.de> (raw)
In-Reply-To: <CABGWkvqF9f4vOYUQeYuaDGT7yuB=8=h=yPpuG04VwicNP=wgMA@mail.gmail.com>
Hello,
Dario Binacchi <dario.binacchi@amarulasolutions.com> writes:
> Hello,
>
> On Mon, Jul 11, 2022 at 10:05 AM Richard Palethorpe <rpalethorpe@suse.de> wrote:
>>
>> Hello,
>>
>> kernel test robot <oliver.sang@intel.com> 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.
--
Mailing list info: https://lists.linux.it/listinfo/ltp
WARNING: multiple messages have this Message-ID (diff)
From: Richard Palethorpe <rpalethorpe@suse.de>
To: lkp@lists.01.org
Subject: Re: [LTP] [can] c4e54b063f: BUG:sleeping_function_called_from_invalid_context_at_kernel/workqueue.c
Date: Wed, 13 Jul 2022 09:23:28 +0100 [thread overview]
Message-ID: <87v8s1l8a3.fsf@suse.de> (raw)
In-Reply-To: <CABGWkvqF9f4vOYUQeYuaDGT7yuB=8=h=yPpuG04VwicNP=wgMA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2027 bytes --]
Hello,
Dario Binacchi <dario.binacchi@amarulasolutions.com> writes:
> Hello,
>
> On Mon, Jul 11, 2022 at 10:05 AM Richard Palethorpe <rpalethorpe@suse.de> wrote:
>>
>> Hello,
>>
>> kernel test robot <oliver.sang@intel.com> 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.
next prev parent reply other threads:[~2022-07-13 8:35 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-10 14:19 [LTP] [can] c4e54b063f: BUG:sleeping_function_called_from_invalid_context_at_kernel/workqueue.c kernel test robot
2022-07-10 14:19 ` kernel test robot
2022-07-10 14:19 ` kernel test robot
2022-07-11 7:57 ` [LTP] " Richard Palethorpe
2022-07-11 7:57 ` Richard Palethorpe
2022-07-11 7:57 ` Richard Palethorpe
2022-07-13 7:39 ` Dario Binacchi
2022-07-13 7:39 ` Dario Binacchi
2022-07-13 7:39 ` Dario Binacchi
2022-07-13 8:23 ` Richard Palethorpe [this message]
2022-07-13 8:23 ` Richard Palethorpe
2022-07-13 8:23 ` Richard Palethorpe
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87v8s1l8a3.fsf@suse.de \
--to=rpalethorpe@suse.de \
--cc=dario.binacchi@amarulasolutions.com \
--cc=jhofstee@victronenergy.com \
--cc=linux-can@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=lkp@intel.com \
--cc=lkp@lists.01.org \
--cc=ltp@lists.linux.it \
--cc=mkl@pengutronix.de \
--cc=oliver.sang@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.