From: Vinod Koul <vkoul@kernel.org>
To: "Rafał Hibner" <rafal.hibner@secom.com.pl>
Cc: Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com>,
Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>,
Harini Katakam <harini.katakam@xilinx.com>,
Dan Williams <dan.j.williams@intel.com>,
Michal Simek <michal.simek@xilinx.com>,
"open list:DMA GENERIC OFFLOAD ENGINE SUBSYSTEM"
<dmaengine@vger.kernel.org>,
"moderated list:ARM/ZYNQ ARCHITECTURE"
<linux-arm-kernel@lists.infradead.org>,
open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] dma: zynqmp_dma: Initialize descriptor list after freeing during reset
Date: Mon, 4 May 2020 10:46:23 +0530 [thread overview]
Message-ID: <20200504051623.GE1375924@vkoul-mobl> (raw)
In-Reply-To: <1330934e-342e-1e16-6451-d8952463119c@secom.com.pl>
On 02-05-20, 15:00, Rafał Hibner wrote:
> Hello Vinod,
>
> On 02.05.2020 14:32, Vinod Koul wrote:
> > Would it not be better to use list_del_init() where we delete it rather
> > than do the init here?
> >
>
> It is not a problem of list element itself not being initialized.
> The problem is that during fault conditions (zynqmp_dma_reset) all
> elements are moved to free list. List head however is not reinitialized.
>
> In normal flow elements are removed by list_del and resubmitted to
> free list with zynqmp_dma_free_descriptor.
>
> static void zynqmp_dma_chan_desc_cleanup(struct zynqmp_dma_chan *chan)
> {
> ...
> list_for_each_entry_safe(desc, next, &chan->done_list, node) {
> ...
> list_del(&desc->node);
> ...
> zynqmp_dma_free_descriptor(chan, desc);
> }
> }
>
> The zynqmp_dma_free_descriptor does not delete elements from the
> list by itself.
> I am not he author of this driver so I fixed it by
> doing non intrusive changes.
>
> Anyways, I do not see how using list_del_init would fix the bug.
Looking at this, i think it would make sense to do list_splice_init()
before we send the list to be freed.
Radhey/Appana are cced, they should test this.
--
~Vinod
WARNING: multiple messages have this Message-ID (diff)
From: Vinod Koul <vkoul@kernel.org>
To: "Rafał Hibner" <rafal.hibner@secom.com.pl>
Cc: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>,
Michal Simek <michal.simek@xilinx.com>,
open list <linux-kernel@vger.kernel.org>,
Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com>,
"open list:DMA GENERIC OFFLOAD ENGINE SUBSYSTEM"
<dmaengine@vger.kernel.org>,
Harini Katakam <harini.katakam@xilinx.com>,
Dan Williams <dan.j.williams@intel.com>,
"moderated list:ARM/ZYNQ ARCHITECTURE"
<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH] dma: zynqmp_dma: Initialize descriptor list after freeing during reset
Date: Mon, 4 May 2020 10:46:23 +0530 [thread overview]
Message-ID: <20200504051623.GE1375924@vkoul-mobl> (raw)
In-Reply-To: <1330934e-342e-1e16-6451-d8952463119c@secom.com.pl>
On 02-05-20, 15:00, Rafał Hibner wrote:
> Hello Vinod,
>
> On 02.05.2020 14:32, Vinod Koul wrote:
> > Would it not be better to use list_del_init() where we delete it rather
> > than do the init here?
> >
>
> It is not a problem of list element itself not being initialized.
> The problem is that during fault conditions (zynqmp_dma_reset) all
> elements are moved to free list. List head however is not reinitialized.
>
> In normal flow elements are removed by list_del and resubmitted to
> free list with zynqmp_dma_free_descriptor.
>
> static void zynqmp_dma_chan_desc_cleanup(struct zynqmp_dma_chan *chan)
> {
> ...
> list_for_each_entry_safe(desc, next, &chan->done_list, node) {
> ...
> list_del(&desc->node);
> ...
> zynqmp_dma_free_descriptor(chan, desc);
> }
> }
>
> The zynqmp_dma_free_descriptor does not delete elements from the
> list by itself.
> I am not he author of this driver so I fixed it by
> doing non intrusive changes.
>
> Anyways, I do not see how using list_del_init would fix the bug.
Looking at this, i think it would make sense to do list_splice_init()
before we send the list to be freed.
Radhey/Appana are cced, they should test this.
--
~Vinod
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-05-04 5:16 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-28 14:32 [PATCH] dma: zynqmp_dma: Initialize descriptor list after freeing during reset Rafał Hibner
2020-04-28 14:32 ` Rafał Hibner
2020-05-02 12:32 ` Vinod Koul
2020-05-02 12:32 ` Vinod Koul
2020-05-02 13:00 ` Rafał Hibner
2020-05-02 13:00 ` Rafał Hibner
2020-05-04 5:16 ` Vinod Koul [this message]
2020-05-04 5:16 ` Vinod Koul
2020-05-04 5:33 ` Harini Katakam
2020-05-04 5:33 ` Harini Katakam
2020-05-06 10:28 ` [PATCH V2] dma: zynqmp_dma: Move list_del inside zynqmp_dma_free_descriptor Rafał Hibner
2020-05-06 10:28 ` Rafał Hibner
2020-05-15 5:50 ` Vinod Koul
2020-05-15 5:50 ` Vinod Koul
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=20200504051623.GE1375924@vkoul-mobl \
--to=vkoul@kernel.org \
--cc=appana.durga.rao@xilinx.com \
--cc=dan.j.williams@intel.com \
--cc=dmaengine@vger.kernel.org \
--cc=harini.katakam@xilinx.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=michal.simek@xilinx.com \
--cc=radhey.shyam.pandey@xilinx.com \
--cc=rafal.hibner@secom.com.pl \
/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.