Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Kalle Valo <kvalo@kernel.org>
To:     duoming@zju.edu.cn
Cc:     linux-kernel@vger.kernel.org, amitkarwar@gmail.com,
        ganapathi017@gmail.com, sharvari.harisangam@nxp.com,
        huxinming820@gmail.com, davem@davemloft.net, edumazet@google.com,
        kuba@kernel.org, pabeni@redhat.com, linux-wireless@vger.kernel.org,
        netdev@vger.kernel.org
Subject: Re: [PATCH net] net: wireless: marvell: mwifiex: fix sleep in atomic context bugs
References: <20220519101656.44513-1-duoming@zju.edu.cn>
        <87fsl53jic.fsf@kernel.org>
        <257f8e7.216cd.180dc1af4d3.Coremail.duoming@zju.edu.cn>
Date:   Thu, 19 May 2022 17:49:58 +0300
In-Reply-To: <257f8e7.216cd.180dc1af4d3.Coremail.duoming@zju.edu.cn>
        (duoming's message of "Thu, 19 May 2022 19:36:35 +0800 (GMT+08:00)")
Message-ID: <877d6h37c9.fsf@kernel.org>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

duoming@zju.edu.cn writes:

> Hello,
>
> On Thu, 19 May 2022 13:27:07 +0300 Kalle Valo wrote:
>
>> > There are sleep in atomic context bugs when uploading device dump
>> > data on usb interface. The root cause is that the operations that
>> > may sleep are called in fw_dump_timer_fn which is a timer handler.
>> > The call tree shows the execution paths that could lead to bugs:
>> >
>> >    (Interrupt context)
>> > fw_dump_timer_fn
>> >   mwifiex_upload_device_dump
>> >     dev_coredumpv(..., GFP_KERNEL)
>> >       dev_coredumpm()
>> >         kzalloc(sizeof(*devcd), gfp); //may sleep
>> >         dev_set_name
>> >           kobject_set_name_vargs
>> >             kvasprintf_const(GFP_KERNEL, ...); //may sleep
>> >             kstrdup(s, GFP_KERNEL); //may sleep
>> >
>> > This patch moves the operations that may sleep into a work item.
>> > The work item will run in another kernel thread which is in
>> > process context to execute the bottom half of the interrupt.
>> > So it could prevent atomic context from sleeping.
>> >
>> > Fixes: f5ecd02a8b20 ("mwifiex: device dump support for usb interface")
>> > Signed-off-by: Duoming Zhou <duoming@zju.edu.cn>
>> 
>> Have you tested this on real hardware? Or is this just a theoretical
>> fix?
>
> This is a theoretical fix. I don't have the real hardware.

For such patches clearly document that in the commit log, for example
something like "Compile tested only." or similar. But do take into
account that I'm wary about non-trivial fixes which have not been tested
on a real device, it's easy to do more harm than good.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches