From: Wei Yongjun <[email protected]>
Since we will remove items off the list using list_del() we need
to use a safe version of the list_for_each() macro aptly named
list_for_each_safe().
This is detected by Coccinelle semantic patch.
Fixes: b8c722ddd548 ("ASoC: Intel: Skylake: Add support for deferred
DSP module bind")
Signed-off-by: Wei Yongjun <[email protected]>
---
sound/soc/intel/skylake/skl-pcm.c | 4 ++--
sound/soc/intel/skylake/skl-topology.c | 4 ++--
2 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/sound/soc/intel/skylake/skl-pcm.c b/sound/soc/intel/skylake/skl-pcm.c
index 600faad..d43d197 100644
--- a/sound/soc/intel/skylake/skl-pcm.c
+++ b/sound/soc/intel/skylake/skl-pcm.c
@@ -1323,10 +1323,10 @@ int skl_platform_unregister(struct device *dev)
{
struct hdac_ext_bus *ebus = dev_get_drvdata(dev);
struct skl *skl = ebus_to_skl(ebus);
- struct skl_module_deferred_bind *modules;
+ struct skl_module_deferred_bind *modules, *tmp;
if (!list_empty(&skl->bind_list)) {
- list_for_each_entry(modules, &skl->bind_list, node) {
+ list_for_each_entry_safe(modules, tmp, &skl->bind_list, node) {
list_del(&modules->node);
kfree(modules);
}
diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c
index 74b3acf..aea7917 100644
--- a/sound/soc/intel/skylake/skl-topology.c
+++ b/sound/soc/intel/skylake/skl-topology.c
@@ -1091,7 +1091,7 @@ static int skl_tplg_mixer_dapm_post_pmd_event(struct snd_soc_dapm_widget *w,
struct skl_module_cfg *src_module = NULL, *dst_module;
struct skl_sst *ctx = skl->skl_sst;
struct skl_pipe *s_pipe = mconfig->pipe;
- struct skl_module_deferred_bind *modules;
+ struct skl_module_deferred_bind *modules, *tmp;
if (s_pipe->state == SKL_PIPE_INVALID)
return -EINVAL;
@@ -1105,7 +1105,7 @@ static int skl_tplg_mixer_dapm_post_pmd_event(struct snd_soc_dapm_widget *w,
src_module = w_module->w->priv;
- list_for_each_entry(modules, &skl->bind_list, node) {
+ list_for_each_entry_safe(modules, tmp, &skl->bind_list, node) {
/*
* When the destination module is deleted, Unbind the
* modules from deferred bind list.
On Tue, Apr 25, 2017 at 03:28:17AM +0000, Wei Yongjun wrote:
> From: Wei Yongjun <[email protected]>
>
> Since we will remove items off the list using list_del() we need
> to use a safe version of the list_for_each() macro aptly named
> list_for_each_safe().
ah yes, god catch
> This is detected by Coccinelle semantic patch.
It is a good practice to include the script you used, do you mind adding
that in the log?
>
> Fixes: b8c722ddd548 ("ASoC: Intel: Skylake: Add support for deferred
> DSP module bind")
> Signed-off-by: Wei Yongjun <[email protected]>
> ---
> sound/soc/intel/skylake/skl-pcm.c | 4 ++--
> sound/soc/intel/skylake/skl-topology.c | 4 ++--
> 2 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/sound/soc/intel/skylake/skl-pcm.c b/sound/soc/intel/skylake/skl-pcm.c
> index 600faad..d43d197 100644
> --- a/sound/soc/intel/skylake/skl-pcm.c
> +++ b/sound/soc/intel/skylake/skl-pcm.c
> @@ -1323,10 +1323,10 @@ int skl_platform_unregister(struct device *dev)
> {
> struct hdac_ext_bus *ebus = dev_get_drvdata(dev);
> struct skl *skl = ebus_to_skl(ebus);
> - struct skl_module_deferred_bind *modules;
> + struct skl_module_deferred_bind *modules, *tmp;
>
> if (!list_empty(&skl->bind_list)) {
> - list_for_each_entry(modules, &skl->bind_list, node) {
> + list_for_each_entry_safe(modules, tmp, &skl->bind_list, node) {
> list_del(&modules->node);
> kfree(modules);
> }
> diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c
> index 74b3acf..aea7917 100644
> --- a/sound/soc/intel/skylake/skl-topology.c
> +++ b/sound/soc/intel/skylake/skl-topology.c
> @@ -1091,7 +1091,7 @@ static int skl_tplg_mixer_dapm_post_pmd_event(struct snd_soc_dapm_widget *w,
> struct skl_module_cfg *src_module = NULL, *dst_module;
> struct skl_sst *ctx = skl->skl_sst;
> struct skl_pipe *s_pipe = mconfig->pipe;
> - struct skl_module_deferred_bind *modules;
> + struct skl_module_deferred_bind *modules, *tmp;
>
> if (s_pipe->state == SKL_PIPE_INVALID)
> return -EINVAL;
> @@ -1105,7 +1105,7 @@ static int skl_tplg_mixer_dapm_post_pmd_event(struct snd_soc_dapm_widget *w,
>
> src_module = w_module->w->priv;
>
> - list_for_each_entry(modules, &skl->bind_list, node) {
> + list_for_each_entry_safe(modules, tmp, &skl->bind_list, node) {
> /*
> * When the destination module is deleted, Unbind the
> * modules from deferred bind list.
>
--
~Vinod
> -----Original Message-----
> From: Vinod Koul [mailto:[email protected]]
> Sent: Tuesday, April 25, 2017 12:01 PM
> To: Wei Yongjun <[email protected]>
> Cc: Liam Girdwood <[email protected]>; Mark Brown
> <[email protected]>; Jaroslav Kysela <[email protected]>; Takashi Iwai
> <[email protected]>; Pardha Saradhi K
> <[email protected]>; G Kranthi
> <[email protected]>; Hardik T Shah
> <[email protected]>; Jeeja KP <[email protected]>; weiyongjun (A)
> <[email protected]>; [email protected]; linux-
> [email protected]
> Subject: Re: [PATCH -next] ASoC: Intel: Skylake: Fix to use
> list_for_each_safe() when delete items
>
> On Tue, Apr 25, 2017 at 03:28:17AM +0000, Wei Yongjun wrote:
> > From: Wei Yongjun <[email protected]>
> >
> > Since we will remove items off the list using list_del() we need
> > to use a safe version of the list_for_each() macro aptly named
> > list_for_each_safe().
>
> ah yes, god catch
>
> > This is detected by Coccinelle semantic patch.
>
> It is a good practice to include the script you used, do you mind adding
> that in the log?
>
Sure.
@@
iterator name list_for_each;
expression E;
expression pos, head;
identifier l;
@@
* list_for_each(pos, head)
{
...
* \(list_del\|list_del_init\|list_move\|list_move_tail\)(pos, ...);
... when != \(goto l; \| break; \| return E; \| return;\)
}
@@
expression E;
expression pos, head, ep;
identifier list;
identifier l;
@@
* list_for_each(pos, head)
{
...
* ep = list_entry(pos, ..., list);
...
* \(list_del\|list_del_init\|list_move\|list_move_tail\)(&ep->list, ...);
... when != \(goto l; \| break; \| return E; \| return;\)
}
@@
iterator name list_for_each_prev;
expression E;
expression pos, head;
identifier l;
@@
* list_for_each_prev(pos, head)
{
...
* \(list_del\|list_del_init\)(pos);
... when != \(goto l; \| break; \| return E; \| return;\)
}
@@
iterator name list_for_each_entry;
expression E;
expression pos, head;
identifier l, member;
@@
* list_for_each_entry(pos, head, member)
{
...
* \(list_del\|list_del_init\)(&pos->member);
... when != \(goto l; \| break; \| return E; \| return;\)
}
@@
iterator name list_for_each_entry_from;
expression E;
expression pos, head;
identifier l, member;
@@
* list_for_each_entry_from(pos, head, member)
{
...
* \(list_del\|list_del_init\)(&pos->member);
... when != \(goto l; \| break; \| return E; \| return;\)
}
@@
iterator name list_for_each_entry_reverse;
expression E;
expression pos, head;
identifier l, member;
@@
* list_for_each_entry_reverse(pos, head, member)
{
...
* \(list_del\|list_del_init\)(&pos->member);
... when != \(goto l; \| break; \| return E; \| return;\)
}
On Tue, Apr 25, 2017 at 07:57:57AM +0000, weiyongjun (A) wrote:
> > >
> > > Since we will remove items off the list using list_del() we need
> > > to use a safe version of the list_for_each() macro aptly named
> > > list_for_each_safe().
> >
> > ah yes, god catch
> >
> > > This is detected by Coccinelle semantic patch.
> >
> > It is a good practice to include the script you used, do you mind adding
> > that in the log?
> >
>
> Sure.
Sorry I meant this should be in changelog of the patch :)
>
> @@
> iterator name list_for_each;
> expression E;
> expression pos, head;
> identifier l;
> @@
> * list_for_each(pos, head)
> {
> ...
> * \(list_del\|list_del_init\|list_move\|list_move_tail\)(pos, ...);
> ... when != \(goto l; \| break; \| return E; \| return;\)
> }
>
> @@
> expression E;
> expression pos, head, ep;
> identifier list;
> identifier l;
> @@
> * list_for_each(pos, head)
> {
> ...
> * ep = list_entry(pos, ..., list);
> ...
> * \(list_del\|list_del_init\|list_move\|list_move_tail\)(&ep->list, ...);
> ... when != \(goto l; \| break; \| return E; \| return;\)
> }
>
> @@
> iterator name list_for_each_prev;
> expression E;
> expression pos, head;
> identifier l;
> @@
> * list_for_each_prev(pos, head)
> {
> ...
> * \(list_del\|list_del_init\)(pos);
> ... when != \(goto l; \| break; \| return E; \| return;\)
> }
>
> @@
> iterator name list_for_each_entry;
> expression E;
> expression pos, head;
> identifier l, member;
> @@
>
> * list_for_each_entry(pos, head, member)
> {
> ...
> * \(list_del\|list_del_init\)(&pos->member);
> ... when != \(goto l; \| break; \| return E; \| return;\)
> }
>
> @@
> iterator name list_for_each_entry_from;
> expression E;
> expression pos, head;
> identifier l, member;
> @@
>
> * list_for_each_entry_from(pos, head, member)
> {
> ...
> * \(list_del\|list_del_init\)(&pos->member);
> ... when != \(goto l; \| break; \| return E; \| return;\)
> }
>
> @@
> iterator name list_for_each_entry_reverse;
> expression E;
> expression pos, head;
> identifier l, member;
> @@
>
> * list_for_each_entry_reverse(pos, head, member)
> {
> ...
> * \(list_del\|list_del_init\)(&pos->member);
> ... when != \(goto l; \| break; \| return E; \| return;\)
> }
--
~Vinod
> -----Original Message-----
> From: Vinod Koul [mailto:[email protected]]
> Sent: Wednesday, April 26, 2017 1:38 PM
> To: weiyongjun (A) <[email protected]>
> Cc: Wei Yongjun <[email protected]>; Liam Girdwood
> <[email protected]>; Mark Brown <[email protected]>; Jaroslav
> Kysela <[email protected]>; Takashi Iwai <[email protected]>; Pardha Saradhi K
> <[email protected]>; G Kranthi
> <[email protected]>; Hardik T Shah
> <[email protected]>; Jeeja KP <[email protected]>; alsa-
> [email protected]; [email protected]
> Subject: Re: [PATCH -next] ASoC: Intel: Skylake: Fix to use
> list_for_each_safe() when delete items
>
> On Tue, Apr 25, 2017 at 07:57:57AM +0000, weiyongjun (A) wrote:
> > > >
> > > > Since we will remove items off the list using list_del() we need
> > > > to use a safe version of the list_for_each() macro aptly named
> > > > list_for_each_safe().
> > >
> > > ah yes, god catch
> > >
> > > > This is detected by Coccinelle semantic patch.
> > >
> > > It is a good practice to include the script you used, do you mind adding
> > > that in the log?
> > >
> >
> > Sure.
>
> Sorry I meant this should be in changelog of the patch :)
OK, I will add this and send the v2 version patch later.
> On Tue, Apr 25, 2017 at 07:57:57AM +0000, weiyongjun (A) wrote:
> > > >
> > > > Since we will remove items off the list using list_del() we need
> > > > to use a safe version of the list_for_each() macro aptly named
> > > > list_for_each_safe().
> > >
> > > ah yes, god catch
> > >
> > > > This is detected by Coccinelle semantic patch.
> > >
> > > It is a good practice to include the script you used, do you mind adding
> > > that in the log?
> > >
> >
> > Sure.
>
> Sorry I meant this should be in changelog of the patch :)
Oh, a similar patch has been applied. Please ignore this one.
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=550b349af0f0e33fedb252aca8dc144299aca308
>
> >
> > @@
> > iterator name list_for_each;
> > expression E;
> > expression pos, head;
> > identifier l;
> > @@
> > * list_for_each(pos, head)
> > {
> > ...
> > * \(list_del\|list_del_init\|list_move\|list_move_tail\)(pos, ...);
> > ... when != \(goto l; \| break; \| return E; \| return;\)
> > }
> >
> > @@
> > expression E;
> > expression pos, head, ep;
> > identifier list;
> > identifier l;
> > @@
> > * list_for_each(pos, head)
> > {
> > ...
> > * ep = list_entry(pos, ..., list);
> > ...
> > * \(list_del\|list_del_init\|list_move\|list_move_tail\)(&ep->list, ...);
> > ... when != \(goto l; \| break; \| return E; \| return;\)
> > }
> >
> > @@
> > iterator name list_for_each_prev;
> > expression E;
> > expression pos, head;
> > identifier l;
> > @@
> > * list_for_each_prev(pos, head)
> > {
> > ...
> > * \(list_del\|list_del_init\)(pos);
> > ... when != \(goto l; \| break; \| return E; \| return;\)
> > }
> >
> > @@
> > iterator name list_for_each_entry;
> > expression E;
> > expression pos, head;
> > identifier l, member;
> > @@
> >
> > * list_for_each_entry(pos, head, member)
> > {
> > ...
> > * \(list_del\|list_del_init\)(&pos->member);
> > ... when != \(goto l; \| break; \| return E; \| return;\)
> > }
> >
> > @@
> > iterator name list_for_each_entry_from;
> > expression E;
> > expression pos, head;
> > identifier l, member;
> > @@
> >
> > * list_for_each_entry_from(pos, head, member)
> > {
> > ...
> > * \(list_del\|list_del_init\)(&pos->member);
> > ... when != \(goto l; \| break; \| return E; \| return;\)
> > }
> >
> > @@
> > iterator name list_for_each_entry_reverse;
> > expression E;
> > expression pos, head;
> > identifier l, member;
> > @@
> >
> > * list_for_each_entry_reverse(pos, head, member)
> > {
> > ...
> > * \(list_del\|list_del_init\)(&pos->member);
> > ... when != \(goto l; \| break; \| return E; \| return;\)
> > }
>
> --
> ~Vinod