linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Vinod Koul <vkoul@infradead.org>
To: Inderpal Singh <inderpal.singh@linaro.org>
Cc: vinod.koul@intel.com, linux-samsung-soc@vger.kernel.org,
	linux-kernel@vger.kernel.org, jassisinghbrar@gmail.com,
	boojin.kim@samsung.com, patches@linaro.org,
	kgene.kim@samsung.com
Subject: Re: [PATCH v2 3/4] DMA: PL330: Balance module remove function with probe
Date: Mon, 29 Oct 2012 10:15:43 +0530	[thread overview]
Message-ID: <1351485943.7077.30.camel@vkoul-udesk3> (raw)
In-Reply-To: <CAG17yqTx6MnmpYLZCxeJ9gC_ha6i3Bp3Fc+=9L7EJEJoNwCFtQ@mail.gmail.com>

On Sat, 2012-10-27 at 15:50 +0530, Inderpal Singh wrote:
> Hi Vinod,
> 
> On 26 October 2012 10:15, Vinod Koul <vkoul@infradead.org> wrote:
> > On Thu, 2012-10-25 at 16:53 +0530, Inderpal Singh wrote:
> >>
> >> This code will get executed only in case of force removal of the
> >> module which was discussed in the first version of the patch at [1].
> >> Now, if we do not have to think about force removal then this patch
> >> will go back to the first version.
> > But why are you doing force removal of driver even when client is
> > holding a reference to you.
> >
> > What happens when client finally tries to free the channel?
> Since we return EBUSY so forced removal won't succeed. Client can free
> the channel eventually.
And that is my concern. You have forcefully removed the dma module.
What happens then? How will the free calll get executed, wont you hit a
panic.
> 
> >
> > What is the problem you are trying to solve?
> >>
> 
> There was a long discussion about it in the first version of the
> patch. Allow me to explain it to you.
> 
> The existing driver does DMA_TERMINATE_ALL and frees resources for all
> the channels in the _remove function.
Which for starters may not be right thing to do. Shouldn't you first
make sure client has freed all references to your driver and then only
remove. Freeing resources in .remove without keeping client in sync
doesn't sound to be good idea to me.

>  The first version of patch
> removed this flushing and freeing of channel resources because they
> are not getting allocated in the probe. Jassi pointed out that manual
> flushing is needed if a force removal happens and some client is
> queued. Then it was agreed that flushing is not needed, instead we
> should return EBUSY if client is queued on some channel (this will
> happen only in force removal case). Hence this additional check in v2
> version so that force removal does not succeeds if any client is
> queued.
> 
> If you think force removal is not a practical scenario and we should
> not be bothering about it, this check can be removed and the patch
> will go back to first version which just removes flushing and freeing
> of channels beacues they are not getting allocated in probe.
> 
> Let me know your view.
> 
> Regards,
> Inder
> 
> 
> >> Let me know your view.
> >>
> >> [1] https://patchwork.kernel.org/patch/1503171/
> >>
> >
> >
> > --
> > Vinod Koul
> > Intel Corp.
> >


-- 
Vinod Koul
Intel Corp.


  reply	other threads:[~2012-10-29  4:59 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-10-05 10:17 [PATCH v2 0/4] DMA: PL330: Fix mem leaks and balance probe/remove Inderpal Singh
2012-10-05 10:17 ` [PATCH v2 1/4] DMA: PL330: Free memory allocated for peripheral channels Inderpal Singh
2012-10-24  4:05   ` Vinod Koul
2012-10-25 10:59     ` Inderpal Singh
2012-10-05 10:17 ` [PATCH v2 2/4] DMA: PL330: Change allocation method to properly free DMA descriptors Inderpal Singh
2012-10-24  4:10   ` Vinod Koul
2012-10-25 11:02     ` Inderpal Singh
2012-10-05 10:17 ` [PATCH v2 3/4] DMA: PL330: Balance module remove function with probe Inderpal Singh
2012-10-24  4:14   ` Vinod Koul
2012-10-25 11:23     ` Inderpal Singh
2012-10-26  4:45       ` Vinod Koul
2012-10-27 10:20         ` Inderpal Singh
2012-10-29  4:45           ` Vinod Koul [this message]
2012-10-29  9:00             ` Inderpal Singh
2012-10-05 10:17 ` [PATCH v2 4/4] DMA: PL330: unregister dma_device in module's remove function Inderpal Singh
2012-10-24  4:19   ` Vinod Koul
2012-10-25 11:05     ` Inderpal Singh
2012-10-12  4:33 ` [PATCH v2 0/4] DMA: PL330: Fix mem leaks and balance probe/remove Inderpal Singh
2012-10-13 11:03 ` Jassi Brar
2012-10-16 11:32   ` Inderpal Singh

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=1351485943.7077.30.camel@vkoul-udesk3 \
    --to=vkoul@infradead.org \
    --cc=boojin.kim@samsung.com \
    --cc=inderpal.singh@linaro.org \
    --cc=jassisinghbrar@gmail.com \
    --cc=kgene.kim@samsung.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-samsung-soc@vger.kernel.org \
    --cc=patches@linaro.org \
    --cc=vinod.koul@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).