From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Subject: dma: cppi41: delete channel from pending list when stop channel From: Bin Liu Message-Id: <20181206145612.GA11314@uda0271908> Date: Thu, 6 Dec 2018 08:56:12 -0600 To: Peter Ujfalusi Cc: dmaengine@vger.kernel.org, linux-usb@vger.kernel.org, Vinod , stable@vger.kernel.org List-ID: UGV0ZXIsCgpPbiBXZWQsIE5vdiAyOCwgMjAxOCBhdCAwMToxNjozMlBNICswMjAwLCBQZXRlciBV amZhbHVzaSB3cm90ZToKPiBIaSwKPiAKPiBPbiAyOC8xMS8yMDE4IDEzLjE1LCBQZXRlciBVamZh bHVzaSB3cm90ZToKPiAKPiBmb3Jnb3QgdG8gZml4IHVwIFZpbm9kJ3MgZW1haWwgYWRkcmVzcy4K PiAKPiA+IAo+ID4gCj4gPiBPbiAxMi8xMS8yMDE4IDE3LjQwLCBCaW4gTGl1IHdyb3RlOgo+ID4g Cj4gPiBDYW4geW91IGZpeCB1cCB0aGUgc3ViamVjdCBsaW5lIHRvOgo+ID4gZG1hZW5naW5lOiB0 aTogY3BwaTQ6IGRlbGV0ZSBjaGFubmVsIGZyb20gcGVuZGluZyBsaXN0IHdoZW4gc3RvcCBjaGFu bmVsCj4gPiAKPiA+PiBUaGUgZHJpdmVyIGRlZmluZXMgdGhyZWUgc3RhdGVzIGZvciBhIGNwcGkg Y2hhbm5lbC4KPiA+PiAtIGlkbGU6IC5jaGFuX2J1c3kgPT0gMCAmJiBub3QgaW4gLnBlbmRpbmcg bGlzdAo+ID4+IC0gcGVuZGluZzogLmNoYW5fYnVzeSA9PSAwICYmIGluIC5wZW5kaW5nIGxpc3QK PiA+PiAtIGJ1c3k6IC5jaGFuX2J1c3kgPT0gMSAmJiBub3QgaW4gLnBlbmRpbmcgbGlzdAo+ID4+ Cj4gPj4gVGhlcmUgYXJlIGNhc2VzIGluIHdoaWNoIHRoZSBjcHBpIGNoYW5uZWwgY291bGQgYmUg aW4gdGhlIHBlbmRpbmcgc3RhdGUKPiA+PiB3aGVuIGNwcGk0MV9kbWFfaXNzdWVfcGVuZGluZygp IGlzIGNhbGxlZCBhZnRlciBjcHBpNDFfcnVudGltZV9zdXNwZW5kKCkKPiA+PiBpcyBjYWxsZWQu Cj4gPj4KPiA+PiBjcHBpNDFfc3RvcF9jaGFuKCkgaGFzIGEgYnVnIGZvciB0aGVzZSBjYXNlcyB0 byBzZXQgY2hhbm5lbHMgdG8gaWRsZSBzdGF0ZS4KPiA+PiBJdCBvbmx5IGNoZWNrcyB0aGUgLmNo YW5fYnVzeSBmbGFnLCBidXQgbm90IHRoZSAucGVuZGluZyBsaXN0LCB0aGVuIGxhdGVyCj4gPj4g d2hlbiBjcHBpNDFfcnVudGltZV9yZXN1bWUoKSBpcyBjYWxsZWQgdGhlIGNoYW5uZWxzIGluIC5w ZW5kaW5nIGxpc3Qgd2lsbAo+ID4+IGJlIHRyYW5zaXRpb25lZCB0byBidXN5IHN0YXRlLgo+ID4+ Cj4gPj4gUmVtb3ZpbmcgY2hhbm5lbHMgZnJvbSB0aGUgLnBlbmRpbmcgbGlzdCBzb2x2ZXMgdGhl IHByb2JsZW0uCj4gPiAKPiA+IFNvLCBsZXQgbWUgc2VlIGlmIEkgdW5kZXJzdGFuZCB0aGlzIGNv cnJlY3RseToKPiA+IC0gY2xpZW50IGlzc3VlZCBhIHRyYW5zZmVyIF9hZnRlcl8gdGhlIGNwcGk0 IGRyaXZlciBpcyBzdXNwZW5kZWQKPiA+IC0gY3BwaTQxX2RtYV9pc3N1ZV9wZW5kaW5nKCkgd2ls bCBwbGFjZSBpdCB0byBwZW5kaW5nIGxpc3QgYW5kIHdpbGwgbm90Cj4gPiBzdGFydCB0aGUgdHJh bnNmZXIgcmlnaHQgYXdheSBhcyBjZGQtPmlzX3N1c3BlbmRlZCBpcyB0cnVlLgo+ID4gLSBvbiBy ZXN1bWUgdGhlIGNwcGk0IHdpbGwgcGljayB1cCB0aGUgcGVuZGluZyB0cmFuc2ZlcnMgZnJvbSB0 aGUKPiA+IHBlbmRpbmcgbGlzdAo+ID4gCj4gPiBUaGlzIGlzIHNvIGZhciBhIHNhbmUgdGhpbmcg dG8gZG8uCj4gPiAKPiA+IElmIEkgZ3Vlc3MgcmlnaHQsIHRoZW4gYWZ0ZXIgdGhlIGlzc3VlX3Bl bmRpbmcgdGhlIGNsaWVudCBkcml2ZXIgd2lsbAo+ID4gY2FsbCB0ZXJtaW5hdGVfYWxsLCBwcmVz dW1hYmx5IGZyb20gaXQncyBzdXNwZW5kIGNhbGxiYWNrPwo+ID4gCj4gPiBBcyBwZXIgdGhlIHB1 cnBvc2Ugb2YgdGVybWluYXRlX2FsbCB3ZSBzaG91bGQgdGVybWluYXRlZCBhbGwgZnV0dXJlCj4g PiB0cmFuc2ZlcnMgb24gdGhlIGNoYW5uZWwsIHNvIGNsZWFyaW5nIHRoZSBwZW5kaW5nIGxpc3Qg aXMgdGhlIGNvcnJlY3QKPiA+IHRoaW5nIHRvIGRvLgo+ID4gCj4gPiBXaXRoIHRoZSBmaXhlZCBz dWJqZWN0Ogo+ID4gUmV2aWV3ZWQtYnk6IFBldGVyIFVqZmFsdXNpIDxwZXRlci51amZhbHVzaUB0 aS5jb20+Cj4gPiAKPiA+IEkgaGF2ZSBvbmUgcXVlc3Rpb246Cj4gPiAKPiA+PiBGaXhlczogOTc1 ZmFhZWI5OTg1ICgiZG1hOiBjcHBpNDE6IHN0YXJ0IHRlYXIgZG93biBvbmx5IGlmIGNoYW5uZWwg aXMgYnVzeSIpCj4gPj4gQ2M6IHN0YWJsZUB2Z2VyLmtlcm5lbC5vcmcgIyB2My4xNSsKPiA+PiBT aWduZWQtb2ZmLWJ5OiBCaW4gTGl1IDxiLWxpdUB0aS5jb20+Cj4gPj4gLS0tCj4gPj4gIGRyaXZl cnMvZG1hL3RpL2NwcGk0MS5jIHwgMTYgKysrKysrKysrKysrKysrLQo+ID4+ICAxIGZpbGUgY2hh bmdlZCwgMTUgaW5zZXJ0aW9ucygrKSwgMSBkZWxldGlvbigtKQo+ID4+Cj4gPj4gZGlmZiAtLWdp dCBhL2RyaXZlcnMvZG1hL3RpL2NwcGk0MS5jIGIvZHJpdmVycy9kbWEvdGkvY3BwaTQxLmMKPiA+ PiBpbmRleCAxNDk3ZGEzNjc3MTAuLmU1MDdlYzM2YzBkMyAxMDA2NDQKPiA+PiAtLS0gYS9kcml2 ZXJzL2RtYS90aS9jcHBpNDEuYwo+ID4+ICsrKyBiL2RyaXZlcnMvZG1hL3RpL2NwcGk0MS5jCj4g Pj4gQEAgLTcyMyw4ICs3MjMsMjIgQEAgc3RhdGljIGludCBjcHBpNDFfc3RvcF9jaGFuKHN0cnVj dCBkbWFfY2hhbiAqY2hhbikKPiA+PiAgCj4gPj4gIAlkZXNjX3BoeXMgPSBsb3dlcl8zMl9iaXRz KGMtPmRlc2NfcGh5cyk7Cj4gPj4gIAlkZXNjX251bSA9IChkZXNjX3BoeXMgLSBjZGQtPmRlc2Nz X3BoeXMpIC8gc2l6ZW9mKHN0cnVjdCBjcHBpNDFfZGVzYyk7Cj4gPj4gLQlpZiAoIWNkZC0+Y2hh bl9idXN5W2Rlc2NfbnVtXSkKPiA+PiArCWlmICghY2RkLT5jaGFuX2J1c3lbZGVzY19udW1dKSB7 Cj4gPj4gKwkJc3RydWN0IGNwcGk0MV9jaGFubmVsICpjYywgKl9jdDsKPiA+PiArCj4gPj4gKwkJ LyoKPiA+PiArCQkgKiBjaGFubmVscyBtaWdodCBzdGlsbCBiZSBpbiB0aGUgcGVuZGxpbmcgbGlz dCBpZgo+ID4+ICsJCSAqIGNwcGk0MV9kbWFfaXNzdWVfcGVuZGluZygpIGlzIGNhbGxlZCBhZnRl cgo+ID4+ICsJCSAqIGNwcGk0MV9ydW50aW1lX3N1c3BlbmQoKSBpcyBjYWxsZWQKPiA+PiArCQkg Ki8KPiA+PiArCQlsaXN0X2Zvcl9lYWNoX2VudHJ5X3NhZmUoY2MsIF9jdCwgJmNkZC0+cGVuZGlu Zywgbm9kZSkgewo+ID4+ICsJCQlpZiAoY2MgIT0gYykKPiA+PiArCQkJCWNvbnRpbnVlOwo+ID4+ ICsJCQlsaXN0X2RlbCgmY2MtPm5vZGUpOwo+ID4gCj4gPiBJZiB3ZSBkZWxldGUgZnJvbSB0aGUg cGVuZGluZyBsaXN0LCBhcmUgd2UgZ29pbmcgdG8gbGVhayBtZW1vcnk/Cj4gPiBJJ20gbm90IGZh bWlsaWFyIHdpdGggdGhlIGNwcGk0LCBpdCBtaWdodCBub3QgYmUgYW4gaXNzdWUgZm9yIGl0LgoK SGVyZSBpcyBubyBtZW1vcnkgbGVhay4KVGhlIGVsZW1lbnRzIGFkZGVkIHRvIHRoZSBwZW5kaW5n IGxpc3QgYXJlIGNwcGk0MSBjaGFubmVscyB3aGljaCBhcmUKYWxsb2NhdGVkIGluIGRyaXZlciBf cHJvYmUoKS4gTm8gZHluYW1pYyBtZW1vcnkgYWxsb2NhdGlvbiBoYXBwZW5pbmcKd2hlbiBvcGVy YXRpbmcgdGhpcyBwZW5kaW5nIGxpc3QuCgpSZWdhcmRzLAotQmluLgo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from fllv0016.ext.ti.com ([198.47.19.142]:59500 "EHLO fllv0016.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726364AbeLFO4O (ORCPT ); Thu, 6 Dec 2018 09:56:14 -0500 Date: Thu, 6 Dec 2018 08:56:12 -0600 From: Bin Liu To: Peter Ujfalusi CC: , , Vinod , Subject: Re: [PATCH] dma: cppi41: delete channel from pending list when stop channel Message-ID: <20181206145612.GA11314@uda0271908> References: <20181112154049.24129-1-b-liu@ti.com> <84a8495b-8c62-f49e-7bbd-318898afa225@ti.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <84a8495b-8c62-f49e-7bbd-318898afa225@ti.com> Sender: stable-owner@vger.kernel.org List-ID: Peter, On Wed, Nov 28, 2018 at 01:16:32PM +0200, Peter Ujfalusi wrote: > Hi, > > On 28/11/2018 13.15, Peter Ujfalusi wrote: > > forgot to fix up Vinod's email address. > > > > > > > On 12/11/2018 17.40, Bin Liu wrote: > > > > Can you fix up the subject line to: > > dmaengine: ti: cppi4: delete channel from pending list when stop channel > > > >> The driver defines three states for a cppi channel. > >> - idle: .chan_busy == 0 && not in .pending list > >> - pending: .chan_busy == 0 && in .pending list > >> - busy: .chan_busy == 1 && not in .pending list > >> > >> There are cases in which the cppi channel could be in the pending state > >> when cppi41_dma_issue_pending() is called after cppi41_runtime_suspend() > >> is called. > >> > >> cppi41_stop_chan() has a bug for these cases to set channels to idle state. > >> It only checks the .chan_busy flag, but not the .pending list, then later > >> when cppi41_runtime_resume() is called the channels in .pending list will > >> be transitioned to busy state. > >> > >> Removing channels from the .pending list solves the problem. > > > > So, let me see if I understand this correctly: > > - client issued a transfer _after_ the cppi4 driver is suspended > > - cppi41_dma_issue_pending() will place it to pending list and will not > > start the transfer right away as cdd->is_suspended is true. > > - on resume the cppi4 will pick up the pending transfers from the > > pending list > > > > This is so far a sane thing to do. > > > > If I guess right, then after the issue_pending the client driver will > > call terminate_all, presumably from it's suspend callback? > > > > As per the purpose of terminate_all we should terminated all future > > transfers on the channel, so clearing the pending list is the correct > > thing to do. > > > > With the fixed subject: > > Reviewed-by: Peter Ujfalusi > > > > I have one question: > > > >> Fixes: 975faaeb9985 ("dma: cppi41: start tear down only if channel is busy") > >> Cc: stable@vger.kernel.org # v3.15+ > >> Signed-off-by: Bin Liu > >> --- > >> drivers/dma/ti/cppi41.c | 16 +++++++++++++++- > >> 1 file changed, 15 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/dma/ti/cppi41.c b/drivers/dma/ti/cppi41.c > >> index 1497da367710..e507ec36c0d3 100644 > >> --- a/drivers/dma/ti/cppi41.c > >> +++ b/drivers/dma/ti/cppi41.c > >> @@ -723,8 +723,22 @@ static int cppi41_stop_chan(struct dma_chan *chan) > >> > >> desc_phys = lower_32_bits(c->desc_phys); > >> desc_num = (desc_phys - cdd->descs_phys) / sizeof(struct cppi41_desc); > >> - if (!cdd->chan_busy[desc_num]) > >> + if (!cdd->chan_busy[desc_num]) { > >> + struct cppi41_channel *cc, *_ct; > >> + > >> + /* > >> + * channels might still be in the pendling list if > >> + * cppi41_dma_issue_pending() is called after > >> + * cppi41_runtime_suspend() is called > >> + */ > >> + list_for_each_entry_safe(cc, _ct, &cdd->pending, node) { > >> + if (cc != c) > >> + continue; > >> + list_del(&cc->node); > > > > If we delete from the pending list, are we going to leak memory? > > I'm not familiar with the cppi4, it might not be an issue for it. Here is no memory leak. The elements added to the pending list are cppi41 channels which are allocated in driver _probe(). No dynamic memory allocation happening when operating this pending list. Regards, -Bin.