ASoC:SOF:pm fix DAI config during resume by zhuyingjiang · Pull Request #450 · thesofproject/linux

zhuyingjiang · 2018-12-16T06:43:41Z

the dai_index has to be set in sof_restore_pipelines as in topology.c, or
else can't resume

Signed-off-by: Zhu Yingjiang yingjiang.zhu@linux.intel.com

zhuyingjiang · 2018-12-16T06:45:27Z

this fix #442

plbossart

Not convinced the root cause was found, sorry.

plbossart · 2018-12-16T16:27:30Z

In sof_link_hda_process() the code does this:

config->dai_index = dai->comp_dai.dai_index;

linux/sound/soc/sof/topology.c

Line 2151 in b8db053

config->dai_index = dai->comp_dai.dai_index;

Why would one need to do this again on resume?
And why does this specifically occur on WHL? this looks suspicious.

BTW we should really clean this up, we have two dai_index fields in different structures, it's not really good to track code:

/* DAI */ static const struct sof_topology_token dai_tokens[] = { {SOF_TKN_DAI_DMAC_CONFIG, SND_SOC_TPLG_TUPLE_TYPE_WORD, get_token_u32, offsetof(struct sof_ipc_comp_dai, dmac_config), 0}, {SOF_TKN_DAI_TYPE, SND_SOC_TPLG_TUPLE_TYPE_STRING, get_token_dai_type, offsetof(struct sof_ipc_comp_dai, type), 0}, {SOF_TKN_DAI_INDEX, SND_SOC_TPLG_TUPLE_TYPE_WORD, get_token_u32, offsetof(struct sof_ipc_comp_dai, dai_index), 0}, {SOF_TKN_DAI_DIRECTION, SND_SOC_TPLG_TUPLE_TYPE_WORD, get_token_u32, offsetof(struct sof_ipc_comp_dai, direction), 0}, }; /* BE DAI link */ static const struct sof_topology_token dai_link_tokens[] = { {SOF_TKN_DAI_TYPE, SND_SOC_TPLG_TUPLE_TYPE_STRING, get_token_dai_type, offsetof(struct sof_ipc_dai_config, type), 0}, {SOF_TKN_DAI_INDEX, SND_SOC_TPLG_TUPLE_TYPE_WORD, get_token_u32, offsetof(struct sof_ipc_dai_config, dai_index), 0}, };

@zhuyingjiang if you look at sof_link_hda_load(), we send 2 ipcs for both cases:
if (link->dpcm_capture) and if (link->dpcm_playback) during topology load.

But at resume we only send 1 ipc. thats likely whats missing at resume.

@ranj063 No, at resume we still send 2, but there are two tx messages with the same dai_index, so caused error.

@plbossart

The fix do the same in sof_restore_pipelines as in sof_link_hda_process, which was missing in the previous sof_restore_pipelines code. And I have tested on WHL RVP this can fix issue PM: the dai config message sent by hda_load and sof_restore_pipelines are not the same #442.
The resume use the lists to restore the pipelines, but in sof_link_hda_process, the tx message sent is not the same as stored in the list, the difference is the config->dai_index = dai->comp_dai.dai_index; So when do resume, we need assign it again.

I agree with you we need go deep and find the root cause, maybe this is a issue with topology.c.
As I add a log print before "config->dai_index = dai->comp_dai.dai_index" in sof_link_hda_process, the result is:
[ 3.709139] sof-audio sof-audio: ======config->dai_index: 00000000, dai->comp_dai.dai_index: 00000000=====
[ 3.709192] sof-audio sof-audio: ipc tx: 0x80010000: GLB_DAI_MSG: CONFIG
[ 3.709350] sof-audio sof-audio: ipc tx succeeded: 0x80010000: GLB_DAI_MSG: CONFIG
[ 3.709353] sof-audio sof-audio: ======config->dai_index: 00000000, dai->comp_dai.dai_index: 00000001=====
[ 3.709407] sof-audio sof-audio: ipc tx: 0x80010000: GLB_DAI_MSG: CONFIG
[ 3.709562] sof-audio sof-audio: ipc tx succeeded: 0x80010000: GLB_DAI_MSG: CONFIG
[ 3.709566] sof-audio sof-audio: warn: unexpected DAI config count 0!
[ 3.709569] sof-audio-pci 0000:00:1f.3: link dma channel 1 for playback
[ 3.709571] sof-audio-pci 0000:00:1f.3: link dma channel 1 for capture
[ 3.709572] sof-audio sof-audio: ======config->dai_index: 00000001, dai->comp_dai.dai_index: 00000002=====
[ 3.709626] sof-audio sof-audio: ipc tx: 0x80010000: GLB_DAI_MSG: CONFIG
[ 3.709776] sof-audio sof-audio: ipc tx succeeded: 0x80010000: GLB_DAI_MSG: CONFIG
[ 3.709780] sof-audio sof-audio: ======config->dai_index: 00000002, dai->comp_dai.dai_index: 00000003=====
[ 3.709834] sof-audio sof-audio: ipc tx: 0x80010000: GLB_DAI_MSG: CONFIG
[ 3.709994] sof-audio sof-audio: ipc tx succeeded: 0x80010000: GLB_DAI_MSG: CONFIG
[ 3.710001] sof-audio sof-audio: warn: unexpected DAI config count 0!
[ 3.710005] sof-audio-pci 0000:00:1f.3: link dma channel 2 for playback
[ 3.710007] sof-audio sof-audio: ======config->dai_index: 00000004, dai->comp_dai.dai_index: 00000004=====
[ 3.710039] sof-audio sof-audio: ipc tx: 0x80010000: GLB_DAI_MSG: CONFIG
[ 3.710193] sof-audio sof-audio: ipc tx succeeded: 0x80010000: GLB_DAI_MSG: CONFIG
[ 3.710198] sof-audio sof-audio: warn: unexpected DAI config count 0!
[ 3.710201] sof-audio-pci 0000:00:1f.3: link dma channel 3 for playback
[ 3.710202] sof-audio sof-audio: ======config->dai_index: 00000005, dai->comp_dai.dai_index: 00000005=====
[ 3.710256] sof-audio sof-audio: ipc tx: 0x80010000: GLB_DAI_MSG: CONFIG
[ 3.710411] sof-audio sof-audio: ipc tx succeeded: 0x80010000: GLB_DAI_MSG: CONFIG
[ 3.710416] sof-audio sof-audio: warn: unexpected DAI config count 0!
[ 3.710419] sof-audio-pci 0000:00:1f.3: link dma channel 4 for playback
[ 3.710420] sof-audio sof-audio: ======config->dai_index: 00000006, dai->comp_dai.dai_index: 00000006=====
It seems some dai_index are the same with comp_dai.dai_index, but others are not. The difference make this issue. I will go deep to see if it is a bug of topology or it is intended to do so.

Maybe it is because on other platform the config->dai_index are the same with comp_dai.dai_index, so this will not cause error, and this depend on how the tplg file define the pipeline. I will check on other platform to resolve this.

@zhuyingjiang if we're sengin 2 ipcs then the problem is probably that we're overwriting the config->dai_index with the second one and it fails during resume.

So maybe we should save both playback dai_index and capture dai_index separately for hda dai's or we re-assign the dai_index again upon resume.

@RanderWang can you please help yingjiang with this. You are the one who has most recently worked on the hda dai config

I don't what you mean by historical problem.

@ranj063 I mean this is happened in topology.c, obviously we have in-conformity for some place we have same dai_index playback/capture for same channel, while other place we have different dai_index for playback/capture for same channel.
If fix with root cause, we need to force playback/capture with same channel to use different dai_index.

@zhuyingjiang I dont think we can force the same channel for playback and capture

this is the comment in topology.c that makes the distinction for HDA dai's:
/* for hda link, playback and capture are supported by different
* dai in FW. Here send dai config according to capability of dai.
*/

We handle this during topology parsing. We need to find a way to do the same thing upon resume as well.

@ranj063 @plbossart
If "find a way to do the same thing upon resume as well", this PR is just for that.
It is simply missing the assign "config->dai_index = dai->comp_dai.dai_index;" in the pm code.

In sof_link_hda_process() the code does this:

config->dai_index = dai->comp_dai.dai_index;

linux/sound/soc/sof/topology.c

Line 2151 in b8db053
config->dai_index = dai->comp_dai.dai_index;

Why would one need to do this again on resume?
And why does this specifically occur on WHL? this looks suspicious.

BTW we should really clean this up, we have two dai_index fields in different structures, it's not really good to track code:

/* DAI */ static const struct sof_topology_token dai_tokens[] = { {SOF_TKN_DAI_DMAC_CONFIG, SND_SOC_TPLG_TUPLE_TYPE_WORD, get_token_u32, offsetof(struct sof_ipc_comp_dai, dmac_config), 0}, {SOF_TKN_DAI_TYPE, SND_SOC_TPLG_TUPLE_TYPE_STRING, get_token_dai_type, offsetof(struct sof_ipc_comp_dai, type), 0}, {SOF_TKN_DAI_INDEX, SND_SOC_TPLG_TUPLE_TYPE_WORD, get_token_u32, offsetof(struct sof_ipc_comp_dai, dai_index), 0}, {SOF_TKN_DAI_DIRECTION, SND_SOC_TPLG_TUPLE_TYPE_WORD, get_token_u32, offsetof(struct sof_ipc_comp_dai, direction), 0}, }; /* BE DAI link */ static const struct sof_topology_token dai_link_tokens[] = { {SOF_TKN_DAI_TYPE, SND_SOC_TPLG_TUPLE_TYPE_STRING, get_token_dai_type, offsetof(struct sof_ipc_dai_config, type), 0}, {SOF_TKN_DAI_INDEX, SND_SOC_TPLG_TUPLE_TYPE_WORD, get_token_u32, offsetof(struct sof_ipc_dai_config, dai_index), 0}, };

yes, it is confusing. One is dai component id, another is dai link id.

plbossart · 2018-12-17T16:09:30Z

@RanderWang can you please chime in, it looks like the changes for HDaudio are incomplete and something's fishy here.

plbossart · 2018-12-18T02:48:02Z

On 12/17/18 8:43 PM, Zhu, Yingjiang wrote: ***@***.**** commented on this pull request. ------------------------------------------------------------------------ In sound/soc/sof/pm.c <#450 (comment)>: > @@ -143,6 +143,8 @@ static int sof_restore_pipelines(struct snd_sof_dev *sdev) struct sof_ipc_reply reply; struct sof_ipc_dai_config *config = dai->dai_config; + config->dai_index = dai->comp_dai.dai_index; @ranj063 <https://github.com/ranj063> @plbossart <https://github.com/plbossart> If "find a way to do the same thing upon resume as well", this PR is just for that. It is simply missing the assign "config->dai_index = dai->comp_dai.dai_index;" in the pm code.

Nope. You shouldn't have to force fields on resume, this is a symptom that something more fundamental is broken and your state is not correct on suspend.

zhuyingjiang · 2018-12-18T03:12:27Z

@plbossart OK, I am checking how to fix this during the first topology load and save to list, so when do resume according to the list, we don't need assign it again.
And also will check the logic for dai_config, in principle, we shouldn't have two dai_index, and they are not the same in value.

RanderWang · 2018-12-18T03:23:29Z

@RanderWang can you please chime in, it looks like the changes for HDaudio are incomplete and something's fishy here.

@plbossart @ranj063 @zhuyingjiang
The issue is caused by the commit: 715e274#diff-5e8a1eb50aedd2de9724071bfa7b6ebb

For HDA, there are two be dais in FW for playback and capture in a pcm (for I2S, there is only one be dai in FW), but there is only one dai_link_config for this pcm. So in sof_link_hda_load, sof_set_dai_config should set each dai like sof_link_hda_process which set dai_index based on dai name and direction.

ranj063 · 2018-12-18T03:38:26Z

@RanderWang can you please chime in, it looks like the changes for HDaudio are incomplete and something's fishy here.

@plbossart @ranj063 @zhuyingjiang
The issue is caused by the commit: 715e274#diff-5e8a1eb50aedd2de9724071bfa7b6ebb

For HDA, there are two be dais in FW for playback and capture in a pcm (for I2S, there is only one be dai in FW), but there is only one dai_link_config for this pcm. So in sof_link_hda_load, sof_set_dai_config should set each dai like sof_link_hda_process which set dai_index based on dai name and direction.

Thanks, @RanderWang this is what I suggested to @zhuyingjiang but he seems to think otherwise. Can you please explain it to him?

RanderWang · 2018-12-18T05:24:46Z

@RanderWang can you please chime in, it looks like the changes for HDaudio are incomplete and something's fishy here.

@plbossart @ranj063 @zhuyingjiang
The issue is caused by the commit: 715e274#diff-5e8a1eb50aedd2de9724071bfa7b6ebb
For HDA, there are two be dais in FW for playback and capture in a pcm (for I2S, there is only one be dai in FW), but there is only one dai_link_config for this pcm. So in sof_link_hda_load, sof_set_dai_config should set each dai like sof_link_hda_process which set dai_index based on dai name and direction.

Thanks, @RanderWang this is what I suggested to @zhuyingjiang but he seems to think otherwise. Can you please explain it to him?

I discussed with yingjiang and he understands our idea now

zhuyingjiang · 2018-12-18T07:15:49Z

@plbossart @ranj063 @RanderWang
PR updated, this also works and fix #442, this fix a bug in sof_link_hda_load, that when playback and capture have the same dai name, the first dai_index keep in list will be replaced by the second one. also caused by search name in loop.

RanderWang · 2018-12-18T08:35:59Z

yes, could you add a comment for it ? I know you don't call sof_set_dai_config, but you need to add the comment "set config for the DAI with name and direction matched". Or you could add other comments to emphasize the job of sof_set_dai_config is done at here

Always check for memory alloc issues!

There is no memory leak here now, previous may cause memory leak. The problem is like:
suppose we have a dai list:
{
dai_name="Analog", direction="Playback";
dai_name="Analog", direction="Capture";
dai_name="Digital MIC", direction="Capture";
dai_name="Digital SSP", direction="Playback";
...
}
In the previous code(with the remove part), will do the kmemdup if the name matches, e.g "Analog", when we only set for dai with name "Analog" and direction "Playback", it will kmemdup two times the dai config as only check the name, "Analog Playback" and "Analog Capture" are all set to the dai config for "Analog Playback".
The next time when set for dai with name "Analog" and direction "Capture", still kmemdup two times the capture dai config for both "Analog Playback" and "Analog Capture".
From the tx message to firmware, we can see message for capture send two times, while no tx message for playback.
This is why the bug happens.

I talked about memory alloc and sorry mate but this can fail. you need to test all memory allocation. No discussion allowed on this topic.
Memory leaks happen when you don't free (which is a different problem).

Also I am not clear about your change. We have 2 routines which do something similar:

static int sof_link_hda_process(struct snd_sof_dev *sdev, struct snd_soc_dai_link *link, struct sof_ipc_dai_config *config, int slot, int direction) { struct sof_ipc_reply reply; u32 size = sizeof(*config); struct snd_sof_dai *dai; int found = 0; int ret; /* for hda link, playback and capture are supported by different * dai in FW. Here get the dai_index of each dai and send config * to FW. In FW, each dai sets config by dai_index */ list_for_each_entry(dai, &sdev->dai_list, list) { if (!dai->name) continue; if (strcmp(link->name, dai->name) == 0 && dai->comp_dai.direction == direction) { config->dai_index = dai->comp_dai.dai_index; found = 1; break; } }

and

/* set config for all DAI's with name matching the link name */ static int sof_set_dai_config(struct snd_sof_dev *sdev, u32 size, struct snd_soc_dai_link *link, struct sof_ipc_dai_config *config) { struct snd_sof_dai *dai; list_for_each_entry(dai, &sdev->dai_list, list) { if (!dai->name) continue; if (strcmp(link->name, dai->name) == 0) { dai->dai_config = kmemdup(config, size, GFP_KERNEL); if (!dai->dai_config) return -ENOMEM; } } return 0; }

It seems to me we have two routines doing the same thing and that if the direction needs to be used it could be factored in with a boolean flag. I don't like copy/paste code with differences, it's a sure way to write spaghetti code.

OK, got it. will add check if alloc fails.

@plbossart I discussed with @ranj063, one issue in our restore process is: each dai link is restored by each dai component, but there are two dai components(playback and capture) in one dai link. This makes one dai_link is restored two times not one for I2S. I will refine this algorithm.

but for HDA, this algorithm is correct

Now only dai link with type SSP and DMIC call sof_set_dai_config and use dai name to find the dai.
After remove sof_set_dai_config in sof_link_hda_load, the HDA dai link find the dai with name and direction in sof_link_hda_process, so I think this is not duplicate routine.
The problem is: Is that only HDA will have playback/capture with the same dai name?
If yes, I think this is OK to keep this. If not, that means SSP and other still will have playback/capture with the same dai name, then probably, we need to change sof_set_dai_config to use name and direction to find the dai.
@RanderWang @ranj063 how do you think?

yes, playback/capture in SSP have the same dai name

fix the place to duplicate the dai_config, previous code replace the dai_config of capture with playback, for they are the same dai name. Signed-off-by: Zhu Yingjiang <yingjiang.zhu@linux.intel.com>

plbossart · 2018-12-18T14:14:45Z

yes, it is confusing. One is dai component id, another is dai link id.

care to send a follow-up PR? Thanks!

plbossart

Work needed on this one, the problem statement and solution are far from clear - not to mention that the commit message and PR title aren't valid any longer.

plbossart · 2018-12-18T14:48:53Z


+	/* Set the DAI config here since both DAI name and direction must
+	 * be matched, this is important for playback and capture with the
+	 * same DAI name.


I couldn't figure out what the comment means. You cannot match the DAI direction for playback and capture, the two parts of the sentence are logically opposite.

I mean if the playback and capture have the same DAI name, we need both check the name and the direction to find the right dai we need, and thus set the dai config.

plbossart · 2018-12-18T14:49:06Z

Always check for memory alloc issues!

plbossart · 2018-12-18T14:50:08Z

-			dev_err(sdev->dev, "error: failed to save DAI config for capture HDA%d\n",
-				config->dai_index);
-	}
-


I am lost as to why this is removed? doesn't that function do precisely what you wanted in the first place?

As state before, the HDA dai need match both name and direction, sof_set_dai_config is a common API which only check the name. So the dai config setting part is move into the sof_link_hda_process, where the name and direction both checked.

please look into extending it to cover direction then. No need for duplication of code.

OK, it is OK to add direction check in sof_set_dai_config, and it will also effect sof_link_ssp_load and sof_link_dmic_load.

@zhuyingjiang you could do some other checks in the function to check the type as well....if type is hda, check name and direction, else just name is enough. make sense?

@RanderWang what is your opinion? since you write the sof_set_dai_config, how to refine this is the better way?

sof_set_dai_config is implemented by Ranjiani. I have discussed with Ranjiani, we need to refine pipeline restore. Now dai link is restored by dai component. This is wrong because there maybe two dai components, playback and capture, or one (playback or capture) in one dai link, and dai link would be restored two times for I2S.

I plan to discard all the sof_set_dai_configs and restoring code in pm, and make clear of dai components and dai links. dai links should be recorded and restored by dai_link_list, not by dai components

@zhuyingjiang do you want to do it? Or just close this PR and I will fix it.

@RanderWang OK, please go ahead refine your code, please also consider the two dai_index Pierre mentioned.

ok, I will consider my plan again. I don't want to make a aggressive change without great improvement

zhuyingjiang

OK, I will change the submit message, and fix the confusing dai component id and dai link id.

zhuyingjiang · 2018-12-19T02:50:52Z

-			dev_err(sdev->dev, "error: failed to save DAI config for capture HDA%d\n",
-				config->dai_index);
-	}
-


As state before, the HDA dai need match both name and direction, sof_set_dai_config is a common API which only check the name. So the dai config setting part is move into the sof_link_hda_process, where the name and direction both checked.

zhuyingjiang · 2018-12-19T02:55:29Z


+	/* Set the DAI config here since both DAI name and direction must
+	 * be matched, this is important for playback and capture with the
+	 * same DAI name.


I mean if the playback and capture have the same DAI name, we need both check the name and the direction to find the right dai we need, and thus set the dai config.

RanderWang · 2018-12-19T03:18:11Z

yes, it is confusing. One is dai component id, another is dai link id.
care to send a follow-up PR? Thanks!

Yes, I will

RanderWang · 2018-12-19T08:25:24Z

yes, it is confusing. One is dai component id, another is dai link id.
care to send a follow-up PR? Thanks!

Yes, I will

@plbossart I check kernel code, topology and FW and find that there is no problem of dai_index.
Each dai_index of dai link is stored in dai_index in dai component (playback & capture)

plbossart · 2018-12-19T17:28:20Z

Looks like this has been replaced by #472

zhuyingjiang · 2018-12-20T02:28:38Z

Close the PR, since Rander created #472 to solve this.

rcu_read_lock is needed to protect access to psock inside sock_map_unref when tearing down the map. However, we can't afford to sleep in lock_sock while in RCU read-side critical section. Grab the RCU lock only after we have locked the socket. This fixes RCU warnings triggerable on a VM with 1 vCPU when free'ing a sockmap/sockhash that contains at least one socket: | ============================= | WARNING: suspicious RCU usage | 5.5.0-04005-g8fc91b972b73 #450 Not tainted | ----------------------------- | include/linux/rcupdate.h:272 Illegal context switch in RCU read-side critical section! | | other info that might help us debug this: | | | rcu_scheduler_active = 2, debug_locks = 1 | 4 locks held by kworker/0:1/62: | #0: ffff88813b019748 ((wq_completion)events){+.+.}, at: process_one_work+0x1d7/0x5e0 | #1: ffffc900000abe50 ((work_completion)(&map->work)){+.+.}, at: process_one_work+0x1d7/0x5e0 | #2: ffffffff82065d20 (rcu_read_lock){....}, at: sock_map_free+0x5/0x170 | #3: ffff8881368c5df8 (&stab->lock){+...}, at: sock_map_free+0x64/0x170 | | stack backtrace: | CPU: 0 PID: 62 Comm: kworker/0:1 Not tainted 5.5.0-04005-g8fc91b972b73 #450 | Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS ?-20190727_073836-buildvm-ppc64le-16.ppc.fedoraproject.org-3.fc31 04/01/2014 | Workqueue: events bpf_map_free_deferred | Call Trace: | dump_stack+0x71/0xa0 | ___might_sleep+0x105/0x190 | lock_sock_nested+0x28/0x90 | sock_map_free+0x95/0x170 | bpf_map_free_deferred+0x58/0x80 | process_one_work+0x260/0x5e0 | worker_thread+0x4d/0x3e0 | kthread+0x108/0x140 | ? process_one_work+0x5e0/0x5e0 | ? kthread_park+0x90/0x90 | ret_from_fork+0x3a/0x50 | ============================= | WARNING: suspicious RCU usage | 5.5.0-04005-g8fc91b972b73-dirty #452 Not tainted | ----------------------------- | include/linux/rcupdate.h:272 Illegal context switch in RCU read-side critical section! | | other info that might help us debug this: | | | rcu_scheduler_active = 2, debug_locks = 1 | 4 locks held by kworker/0:1/62: | #0: ffff88813b019748 ((wq_completion)events){+.+.}, at: process_one_work+0x1d7/0x5e0 | #1: ffffc900000abe50 ((work_completion)(&map->work)){+.+.}, at: process_one_work+0x1d7/0x5e0 | #2: ffffffff82065d20 (rcu_read_lock){....}, at: sock_hash_free+0x5/0x1d0 | #3: ffff888139966e00 (&htab->buckets[i].lock){+...}, at: sock_hash_free+0x92/0x1d0 | | stack backtrace: | CPU: 0 PID: 62 Comm: kworker/0:1 Not tainted 5.5.0-04005-g8fc91b972b73-dirty #452 | Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS ?-20190727_073836-buildvm-ppc64le-16.ppc.fedoraproject.org-3.fc31 04/01/2014 | Workqueue: events bpf_map_free_deferred | Call Trace: | dump_stack+0x71/0xa0 | ___might_sleep+0x105/0x190 | lock_sock_nested+0x28/0x90 | sock_hash_free+0xec/0x1d0 | bpf_map_free_deferred+0x58/0x80 | process_one_work+0x260/0x5e0 | worker_thread+0x4d/0x3e0 | kthread+0x108/0x140 | ? process_one_work+0x5e0/0x5e0 | ? kthread_park+0x90/0x90 | ret_from_fork+0x3a/0x50 Fixes: 7e81a35 ("bpf: Sockmap, ensure sock lock held during tear down") Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> Acked-by: John Fastabend <john.fastabend@gmail.com> Link: https://lore.kernel.org/bpf/20200206111652.694507-2-jakub@cloudflare.com

The following warning pops up with enabled UBSAN in tests fstests/generic/303: [23127.529395] UBSAN: Undefined behaviour in fs/read_write.c:1725:7 [23127.529400] signed integer overflow: [23127.529403] 4611686018427322368 + 9223372036854775807 cannot be represented in type 'long long int' [23127.529412] CPU: 4 PID: 26180 Comm: xfs_io Not tainted 5.2.0-rc2-1.ge195904-vanilla+ thesofproject#450 [23127.556999] Hardware name: empty empty/S3993, BIOS PAQEX0-3 02/24/2008 [23127.557001] Call Trace: [23127.557060] dump_stack+0x67/0x9b [23127.557070] ubsan_epilogue+0x9/0x40 [23127.573496] handle_overflow+0xb3/0xc0 [23127.573514] do_clone_file_range+0x28f/0x2a0 [23127.573547] vfs_clone_file_range+0x35/0xb0 [23127.573564] ioctl_file_clone+0x8d/0xc0 [23127.590144] do_vfs_ioctl+0x300/0x700 [23127.590160] ksys_ioctl+0x70/0x80 [23127.590203] ? trace_hardirqs_off_thunk+0x1a/0x1c [23127.590210] __x64_sys_ioctl+0x16/0x20 [23127.590215] do_syscall_64+0x5c/0x1d0 [23127.590224] entry_SYSCALL_64_after_hwframe+0x49/0xbe [23127.590231] RIP: 0033:0x7ff6d7250327 [23127.590241] RSP: 002b:00007ffe3a38f1d8 EFLAGS: 00000206 ORIG_RAX: 0000000000000010 [23127.590246] RAX: ffffffffffffffda RBX: 0000000000000004 RCX: 00007ff6d7250327 [23127.590249] RDX: 00007ffe3a38f220 RSI: 000000004020940d RDI: 0000000000000003 [23127.590252] RBP: 0000000000000000 R08: 00007ffe3a3c80a0 R09: 00007ffe3a3c8080 [23127.590255] R10: 000000000fa99fa0 R11: 0000000000000206 R12: 0000000000000000 [23127.590260] R13: 0000000000000000 R14: 3fffffffffff0000 R15: 00007ff6d750a20c As loff_t is a signed type, we should use the safe overflow checks instead of relying on compiler implementation. The bogus values are intentional and the test is supposed to verify the boundary conditions. Signed-off-by: David Sterba <dsterba@suse.com> Message-Id: <20230523162628.17071-1-dsterba@suse.com> Signed-off-by: Christian Brauner <brauner@kernel.org>

zhuyingjiang requested review from lgirdwood, plbossart and ranj063 December 16, 2018 06:43

zhuyingjiang mentioned this pull request Dec 16, 2018

PM: the dai config message sent by hda_load and sof_restore_pipelines are not the same #442

Closed

plbossart reviewed Dec 16, 2018

View reviewed changes

zhuyingjiang requested a review from keyonjie December 17, 2018 06:34

plbossart requested a review from RanderWang December 17, 2018 16:08

zhuyingjiang force-pushed the topic/sof-dev-pm-fix-dai-config branch from 620c022 to fce8c25 Compare December 18, 2018 07:07

RanderWang reviewed Dec 18, 2018

View reviewed changes

ASoC:SOF:pm fix DAI config during resume

8bc65ca

fix the place to duplicate the dai_config, previous code replace the dai_config of capture with playback, for they are the same dai name. Signed-off-by: Zhu Yingjiang <yingjiang.zhu@linux.intel.com>

zhuyingjiang force-pushed the topic/sof-dev-pm-fix-dai-config branch from fce8c25 to 8bc65ca Compare December 18, 2018 10:46

plbossart requested changes Dec 18, 2018

View reviewed changes

zhuyingjiang commented Dec 19, 2018

View reviewed changes

zhuyingjiang closed this Dec 20, 2018

Conversation

zhuyingjiang commented Dec 16, 2018

Uh oh!

zhuyingjiang commented Dec 16, 2018

Uh oh!

plbossart left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

plbossart commented Dec 17, 2018

Uh oh!

plbossart commented Dec 18, 2018 via email

Uh oh!

zhuyingjiang commented Dec 18, 2018

Uh oh!

RanderWang commented Dec 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ranj063 commented Dec 18, 2018

Uh oh!

RanderWang commented Dec 18, 2018

Uh oh!

zhuyingjiang commented Dec 18, 2018

Uh oh!

RanderWang Dec 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RanderWang Dec 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

plbossart commented Dec 18, 2018 via email

Uh oh!

plbossart left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RanderWang commented Dec 18, 2018 •

edited

Loading

RanderWang Dec 18, 2018 •

edited

Loading

RanderWang Dec 19, 2018 •

edited

Loading

ranj063 Dec 19, 2018 •

edited

Loading