From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from szxga03-in.huawei.com (szxga03-in.huawei.com [45.249.212.189]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 91F0BEDB; Sat, 29 Jun 2024 02:00:44 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=45.249.212.189 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1719626447; cv=none; b=jhtbpWMtBGfumpMqfbJaYpGfxxLyglw2yiHjZOkh3wzbSoylKRId2WiXkQOcTm2rhAs1YhwCKN5PHjlqstFANgj4fvauS1PRSPmL0/S0ZPFQGrdpAVPz9/hnY0xlAaLoIOYA2a7LkSs9SMautVW4016ghwtvQBALRRIEgXZR+iQ= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1719626447; c=relaxed/simple; bh=MMIG34b0OD/xUUZyx+BBAXiIWzEG4aurl7mijEzaUo8=; h=Message-ID:Date:MIME-Version:Subject:To:CC:References:From: In-Reply-To:Content-Type; b=DSC7KTSa+LBPh/3UKkf+GeExz4lVmOSHB29B4PhykZDrBZjttgNV6H886Nj8BKtrivOXBw1fQbudoTPF6oqlkZu6cev8ILSpUiMRM/LfSA2Wx18Fj9kVehnhOTPAatb58PXoF7paZ7HPx+Yzxebr4X3jW3Vj97s8xOBtkO1X9XA= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=huawei.com; spf=pass smtp.mailfrom=huawei.com; arc=none smtp.client-ip=45.249.212.189 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=huawei.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=huawei.com Received: from mail.maildlp.com (unknown [172.19.163.252]) by szxga03-in.huawei.com (SkyGuard) with ESMTP id 4W9wQr3nqszMrH7; Sat, 29 Jun 2024 09:56:52 +0800 (CST) Received: from kwepemf500004.china.huawei.com (unknown [7.202.181.242]) by mail.maildlp.com (Postfix) with ESMTPS id D6E0E18006F; Sat, 29 Jun 2024 10:00:35 +0800 (CST) Received: from [10.67.121.175] (10.67.121.175) by kwepemf500004.china.huawei.com (7.202.181.242) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1544.11; Sat, 29 Jun 2024 10:00:35 +0800 Message-ID: <3a09fcf9-b60b-571d-3ec5-e0f7c02cd72f@huawei.com> Date: Sat, 29 Jun 2024 10:00:34 +0800 Precedence: bulk X-Mailing-List: dmaengine@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.9.1 Subject: Re: [PATCH v2] dmaegine: virt-dma : Fix multi-user with vchan To: Frank Li CC: , Paul Walmsley , Samuel Holland , Li Zetao , Guanhua Gao , "open list:DMA GENERIC OFFLOAD ENGINE SUBSYSTEM" , open list , "open list:FREESCALE eDMA DRIVER" , "open list:SIFIVE DRIVERS" References: <20230720114212.51224-1-haijie1@huawei.com> <20240620025400.3300641-1-haijie1@huawei.com> From: Jie Hai In-Reply-To: Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 7bit X-ClientProxiedBy: dggems704-chm.china.huawei.com (10.3.19.181) To kwepemf500004.china.huawei.com (7.202.181.242) On 2024/6/20 22:38, Frank Li wrote: > On Thu, Jun 20, 2024 at 10:53:53AM +0800, Jie Hai wrote: >> List desc_allocated was introduced for the case of a transfer >> submitted multiple times. But elegating descriptors on the list >> causes other problems. >> >> For example, in the multi-thread scenario, which tasks are >> continuously created and submitted by each thread. If one of >> the threads calls dmaengine_terminate_all, for dirvers using >> vchan_get_all_descriptors, all descriptors will be freed. If >> there's another thread submitting a transfer A by >> vchan_tx_submit, the following results may be generated: >> 1. desc A is freeing -> visit wrong address of node prep/next. >> 2. desc A is freed -> visit invalid address of A. >> >> In the above case, calltrace is generated and the system is >> suspended. This can be tested by dmatest. > > What's test steps to reproduce this problem? > > Frank Thanks for your review. The operations are as follows: modprobe hisi_dma modprobe dmatest echo 0 > /sys/module/dmatest/parameters/iterations echo "dma0chan0" > /sys/module/dmatest/parameters/channel echo 20 > /sys/module/dmatest/parameters/threads_per_chan echo 1 > /sys/module/dmatest/parameters/run wait for a while and stop the test by: echo 0 > /sys/module/dmatest/parameters/run >> >> This patch removes desc_allocated from vchan_get_all_descriptors, >> and add new function 'vchan_get_all_allocated_descs' to get all >> descriptors ever allocated. >> >> And apply vchan_get_all_allocated_descs to free chan resource and >> vchan_get_all_descriptors to terminate all transfers, respectively. >> This avoids freeing up descriptors in use by other threads. >> >> Signed-off-by: Jie Hai >> --- >> drivers/dma/fsl-dpaa2-qdma/dpaa2-qdma.c | 2 +- >> drivers/dma/fsl-edma-common.c | 2 +- >> drivers/dma/fsl-qdma.c | 2 +- >> drivers/dma/sf-pdma/sf-pdma.c | 2 +- >> drivers/dma/virt-dma.h | 20 ++++++++++++++++++-- >> 5 files changed, 22 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/dma/fsl-dpaa2-qdma/dpaa2-qdma.c b/drivers/dma/fsl-dpaa2-qdma/dpaa2-qdma.c >> index 36384d019263..efdecf15e1b3 100644 >> --- a/drivers/dma/fsl-dpaa2-qdma/dpaa2-qdma.c >> +++ b/drivers/dma/fsl-dpaa2-qdma/dpaa2-qdma.c >> @@ -71,7 +71,7 @@ static void dpaa2_qdma_free_chan_resources(struct dma_chan *chan) >> LIST_HEAD(head); >> >> spin_lock_irqsave(&dpaa2_chan->vchan.lock, flags); >> - vchan_get_all_descriptors(&dpaa2_chan->vchan, &head); >> + vchan_get_all_allocated_descs(&dpaa2_chan->vchan, &head); >> spin_unlock_irqrestore(&dpaa2_chan->vchan.lock, flags); >> >> vchan_dma_desc_free_list(&dpaa2_chan->vchan, &head); >> diff --git a/drivers/dma/fsl-edma-common.c b/drivers/dma/fsl-edma-common.c >> index 3af430787315..1e0ad87eb7fa 100644 >> --- a/drivers/dma/fsl-edma-common.c >> +++ b/drivers/dma/fsl-edma-common.c >> @@ -828,7 +828,7 @@ void fsl_edma_free_chan_resources(struct dma_chan *chan) >> if (edma->drvdata->dmamuxs) >> fsl_edma_chan_mux(fsl_chan, 0, false); >> fsl_chan->edesc = NULL; >> - vchan_get_all_descriptors(&fsl_chan->vchan, &head); >> + vchan_get_all_allocated_descs(&fsl_chan->vchan, &head); >> fsl_edma_unprep_slave_dma(fsl_chan); >> spin_unlock_irqrestore(&fsl_chan->vchan.lock, flags); >> >> diff --git a/drivers/dma/fsl-qdma.c b/drivers/dma/fsl-qdma.c >> index 5005e138fc23..7af428db404e 100644 >> --- a/drivers/dma/fsl-qdma.c >> +++ b/drivers/dma/fsl-qdma.c >> @@ -316,7 +316,7 @@ static void fsl_qdma_free_chan_resources(struct dma_chan *chan) >> LIST_HEAD(head); >> >> spin_lock_irqsave(&fsl_chan->vchan.lock, flags); >> - vchan_get_all_descriptors(&fsl_chan->vchan, &head); >> + vchan_get_all_allocated_descs(&fsl_chan->vchan, &head); >> spin_unlock_irqrestore(&fsl_chan->vchan.lock, flags); >> >> vchan_dma_desc_free_list(&fsl_chan->vchan, &head); >> diff --git a/drivers/dma/sf-pdma/sf-pdma.c b/drivers/dma/sf-pdma/sf-pdma.c >> index 428473611115..4dc8a8c8ad80 100644 >> --- a/drivers/dma/sf-pdma/sf-pdma.c >> +++ b/drivers/dma/sf-pdma/sf-pdma.c >> @@ -147,7 +147,7 @@ static void sf_pdma_free_chan_resources(struct dma_chan *dchan) >> sf_pdma_disable_request(chan); >> kfree(chan->desc); >> chan->desc = NULL; >> - vchan_get_all_descriptors(&chan->vchan, &head); >> + vchan_get_all_allocated_descs(&chan->vchan, &head); >> sf_pdma_disclaim_chan(chan); >> spin_unlock_irqrestore(&chan->vchan.lock, flags); >> vchan_dma_desc_free_list(&chan->vchan, &head); >> diff --git a/drivers/dma/virt-dma.h b/drivers/dma/virt-dma.h >> index 59d9eabc8b67..4492641b79f6 100644 >> --- a/drivers/dma/virt-dma.h >> +++ b/drivers/dma/virt-dma.h >> @@ -187,13 +187,29 @@ static inline void vchan_get_all_descriptors(struct virt_dma_chan *vc, >> { >> lockdep_assert_held(&vc->lock); >> >> - list_splice_tail_init(&vc->desc_allocated, head); >> list_splice_tail_init(&vc->desc_submitted, head); >> list_splice_tail_init(&vc->desc_issued, head); >> list_splice_tail_init(&vc->desc_completed, head); >> list_splice_tail_init(&vc->desc_terminated, head); >> } >> >> +/** >> + * vchan_get_all_allocated_descs - obtain all descriptors >> + * @vc: virtual channel to get descriptors from >> + * @head: list of descriptors found >> + * >> + * vc.lock must be held by caller >> + * >> + * Removes all descriptors from internal lists, and provides a list of all >> + * descriptors found >> + */ >> +static inline void vchan_get_all_allocated_descs(struct virt_dma_chan *vc, >> + struct list_head *head) >> +{ >> + list_splice_tail_init(&vc->desc_allocated, head); >> + vchan_get_all_descriptors(vc, head); >> +} >> + >> static inline void vchan_free_chan_resources(struct virt_dma_chan *vc) >> { >> struct virt_dma_desc *vd; >> @@ -201,7 +217,7 @@ static inline void vchan_free_chan_resources(struct virt_dma_chan *vc) >> LIST_HEAD(head); >> >> spin_lock_irqsave(&vc->lock, flags); >> - vchan_get_all_descriptors(vc, &head); >> + vchan_get_all_allocated_descs(vc, &head); >> list_for_each_entry(vd, &head, node) >> dmaengine_desc_clear_reuse(&vd->tx); >> spin_unlock_irqrestore(&vc->lock, flags); >> -- >> 2.33.0 >> > .