转发: 回复: 回复: 回复: [PATCH] drm/komeda: drop all currently held locks if deadlock happens

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




-----邮件原件-----
发件人: Liu Lucas/刘保柱 
发送时间: 2023年8月4日 15:47
收件人: 'Liviu Dudau' <liviu.dudau@xxxxxxx>
抄送: Huang Menghui/黄梦辉 <menghui.huang@xxxxxxxxxxxx>; airlied@xxxxxxxxx; daniel@xxxxxxxx
主题: 回复: 回复: 回复: 回复: [PATCH] drm/komeda: drop all currently held locks if deadlock happens

Hi Liviu,

	I have resend that patch and cc to the dri-devel mailing list which public information about it can be looked up now . Please try to deal with this patch when you are available.

Best regards,

-----邮件原件-----
发件人: Liviu Dudau <liviu.dudau@xxxxxxx>
发送时间: 2023年8月4日 6:24
收件人: Liu Lucas/刘保柱 <lucas.liu@xxxxxxxxxxxx>
抄送: Huang Menghui/黄梦辉 <menghui.huang@xxxxxxxxxxxx>; airlied@xxxxxxxxx; daniel@xxxxxxxx
主题: Re: 回复: 回复: 回复: [PATCH] drm/komeda: drop all currently held locks if deadlock happens

On Thu, Aug 03, 2023 at 01:47:08AM +0000, Liu Lucas/刘保柱 wrote:
> Hi Liviu,

Hi Liu,

> 	I resend my patch using the email address of "huangmenghui" and add the Signed-off-by of "huangmenghui". Could you please check whether it meets the requirement?

When trying to apply the patch to drm-misc-next using the dim tool I've realised that you have never Cc-ed dri-devel mailing list.
Because of that I don't have any public record of your patch that I can link into. If you look at the commits in drivers/drm, they all have a "Link" in the commit message that leads to a patchwork page where you can see the patch (and the discussions in the thread).

You can resend the patch Cc-ing dri-devel, however I'm about to go on holiday. I will probably check my emails from time to time but I cannot promise that I'm going to merge the patch until I'm back on 28th of August.

Sorry about the delays.

Best regards,
Liviu

> 
> Best regards,
> 
> -----邮件原件-----
> 发件人: Liviu Dudau <liviu.dudau@xxxxxxx>
> 发送时间: 2023年8月2日 0:30
> 收件人: Liu Lucas/刘保柱 <lucas.liu@xxxxxxxxxxxx>
> 抄送: Huang Menghui/黄梦辉 <menghui.huang@xxxxxxxxxxxx>; airlied@xxxxxxxxx; 
> daniel@xxxxxxxx
> 主题: Re: 回复: 回复: [PATCH] drm/komeda: drop all currently held locks if 
> deadlock happens
> 
> On Tue, Aug 01, 2023 at 01:42:58PM +0000, Liu Lucas/刘保柱 wrote:
> > Hi Liviu,
> > 
> > The author of this patch is me, but it needs to add the sign-off-by tag of huang menghui, as follows:
> > Signed-off-by: baozhu.liu <lucas.liu@xxxxxxxxxxxx>
> > Signed-off-by: menghui.huang<menghui.huang@xxxxxxxxxxxx>
> > Can I add two "S-o-b" at the same time?
> 
> Yes.
> 
> > In that case, am I still the author of this patch?
> 
> Yes, you are, as long as you keep the "From: " line at the top.
> 
> Also, the order is correct. First S-o-b is from author (you), next S-o-b is from the sender of the email (Huang).
> 
> Best regards,
> Liviu
> 
> > 
> > Best regards,
> > -----邮件原件-----
> > 发件人: Liviu Dudau <liviu.dudau@xxxxxxx>
> > 发送时间: 2023年8月1日 18:58
> > 收件人: Liu Lucas/刘保柱 <lucas.liu@xxxxxxxxxxxx>
> > 抄送: Huang Menghui/黄梦辉 <menghui.huang@xxxxxxxxxxxx>; 
> > airlied@xxxxxxxxx; daniel@xxxxxxxx
> > 主题: Re: 回复: [PATCH] drm/komeda: drop all currently held locks if 
> > deadlock happens
> > 
> > On Tue, Aug 01, 2023 at 09:39:22AM +0000, Liu Lucas/刘保柱 wrote:
> > > Hi Liviu Dudau ,
> > 
> > Hi Liu,
> > 
> > > 
> > > Since there was some problem with my email account when git 
> > > send-email was sent, I borrowed huang menghui's email address to send it, so there was no need to add his Signed-off-by tag.
> > 
> > I'm afraid that's not how 'git am' works and I also need to put a link to patchwork that will also show a different name for the submitter. If you look into Documentation/process/submitting-patches.rst, it does state under "Sign your work - the Developer's Certificate of Origin":
> > 
> >    Any further SoBs (Signed-off-by:'s) following the author's SoB are from
> >    people handling and transporting the patch, but were not involved in its
> >    development. SoB chains should reflect the **real** route a patch took
> >    as it was propagated to the maintainers and ultimately to Linus, with
> >    the first SoB entry signalling primary authorship of a single author.
> > 
> > 
> > So if you have used Huang's email, you need his S-o-b. Alternatively, resend the patch from your email address.
> > 
> > Best regards,
> > Liviu
> > 
> > > 
> > > Thank you for the recognition of this patch.
> > > 
> > > -----邮件原件-----
> > > 发件人: Liviu Dudau <liviu.dudau@xxxxxxx>
> > > 发送时间: 2023年8月1日 17:31
> > > 收件人: Huang Menghui/黄梦辉 <menghui.huang@xxxxxxxxxxxx>
> > > 抄送: airlied@xxxxxxxxx; daniel@xxxxxxxx; Liu Lucas/刘保柱
> > > <lucas.liu@xxxxxxxxxxxx>
> > > 主题: Re: [PATCH] drm/komeda: drop all currently held locks if 
> > > deadlock happens
> > > 
> > > On Tue, Aug 01, 2023 at 05:03:55PM +0800, menghui.huang wrote:
> > > > From: "baozhu.liu" <lucas.liu@xxxxxxxxxxxx>
> > > > 
> > > > If komeda_pipeline_unbound_components() returns -EDEADLK, it 
> > > > means that a deadlock happened in the locking context.
> > > > Currently, komeda is not dealing with the deadlock 
> > > > properly,producing the following output when CONFIG_DEBUG_WW_MUTEX_SLOWPATH is enabled:
> > > > 
> > > >  ------------[ cut here ]------------
> > > > [   26.103984] WARNING: CPU: 2 PID: 345 at drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c:1248
> > > > 	       komeda_release_unclaimed_resources+0x13c/0x170
> > > > [   26.117453] Modules linked in:
> > > > [   26.120511] CPU: 2 PID: 345 Comm: composer@2.1-se Kdump: loaded Tainted: G   W  5.10.110-SE-SDK1.8-dirty #16
> > > > [   26.131374] Hardware name: Siengine Se1000 Evaluation board (DT)
> > > > [   26.137379] pstate: 20400009 (nzCv daif +PAN -UAO -TCO BTYPE=--)
> > > > [   26.143385] pc : komeda_release_unclaimed_resources+0x13c/0x170
> > > > [   26.149301] lr : komeda_release_unclaimed_resources+0xbc/0x170
> > > > [   26.155130] sp : ffff800017b8b8d0
> > > > [   26.158442] pmr_save: 000000e0
> > > > [   26.161493] x29: ffff800017b8b8d0 x28: ffff000cf2f96200
> > > > [   26.166805] x27: ffff000c8f5a8800 x26: 0000000000000000
> > > > [   26.172116] x25: 0000000000000038 x24: ffff8000116a0140
> > > > [   26.177428] x23: 0000000000000038 x22: ffff000cf2f96200
> > > > [   26.182739] x21: ffff000cfc300300 x20: ffff000c8ab77080
> > > > [   26.188051] x19: 0000000000000003 x18: 0000000000000000
> > > > [   26.193362] x17: 0000000000000000 x16: 0000000000000000
> > > > [   26.198672] x15: b400e638f738ba38 x14: 0000000000000000
> > > > [   26.203983] x13: 0000000106400a00 x12: 0000000000000000
> > > > [   26.209294] x11: 0000000000000000 x10: 0000000000000000
> > > > [   26.214604] x9 : ffff800012f80000 x8 : ffff000ca3308000
> > > > [   26.219915] x7 : 0000000ff3000000 x6 : ffff80001084034c
> > > > [   26.225226] x5 : ffff800017b8bc40 x4 : 000000000000000f
> > > > [   26.230536] x3 : ffff000ca3308000 x2 : 0000000000000000
> > > > [   26.235847] x1 : 0000000000000000 x0 : ffffffffffffffdd
> > > > [   26.241158] Call trace:
> > > > [   26.243604] komeda_release_unclaimed_resources+0x13c/0x170
> > > > [   26.249175] komeda_crtc_atomic_check+0x68/0xf0
> > > > [   26.253706] drm_atomic_helper_check_planes+0x138/0x1f4
> > > > [   26.258929] komeda_kms_check+0x284/0x36c
> > > > [   26.262939] drm_atomic_check_only+0x40c/0x714
> > > > [   26.267381] drm_atomic_nonblocking_commit+0x1c/0x60
> > > > [   26.272344] drm_mode_atomic_ioctl+0xa3c/0xb8c
> > > > [   26.276787] drm_ioctl_kernel+0xc4/0x120
> > > > [   26.280708] drm_ioctl+0x268/0x534
> > > > [   26.284109] __arm64_sys_ioctl+0xa8/0xf0
> > > > [   26.288030] el0_svc_common.constprop.0+0x80/0x240
> > > > [   26.292817] do_el0_svc+0x24/0x90
> > > > [   26.296132] el0_svc+0x20/0x30
> > > > [   26.299185] el0_sync_handler+0xe8/0xf0
> > > > [   26.303018] el0_sync+0x1a4/0x1c0
> > > > [   26.306330] irq event stamp: 0
> > > > [   26.309384] hardirqs last  enabled at (0): [<0000000000000000>] 0x0
> > > > [   26.315650] hardirqs last disabled at (0): [<ffff800010056d34>] copy_process+0x5d0/0x183c
> > > > [   26.323825] softirqs last  enabled at (0): [<ffff800010056d34>] copy_process+0x5d0/0x183c
> > > > [   26.331997] softirqs last disabled at (0): [<0000000000000000>] 0x0
> > > > [   26.338261] ---[ end trace 20ae984fa860184a ]---
> > > > [   26.343021] ------------[ cut here ]------------
> > > > [   26.347646] WARNING: CPU: 3 PID: 345 at drivers/gpu/drm/drm_modeset_lock.c:228 drm_modeset_drop_locks+0x84/0x90
> > > > [   26.357727] Modules linked in:
> > > > [   26.360783] CPU: 3 PID: 345 Comm: composer@2.1-se Kdump: loaded Tainted: G   W  5.10.110-SE-SDK1.8-dirty #16
> > > > [   26.371645] Hardware name: Siengine Se1000 Evaluation board (DT)
> > > > [   26.377647] pstate: 20400009 (nzCv daif +PAN -UAO -TCO BTYPE=--)
> > > > [   26.383649] pc : drm_modeset_drop_locks+0x84/0x90
> > > > [   26.388351] lr : drm_mode_atomic_ioctl+0x860/0xb8c
> > > > [   26.393137] sp : ffff800017b8bb10
> > > > [   26.396447] pmr_save: 000000e0
> > > > [   26.399497] x29: ffff800017b8bb10 x28: 0000000000000001
> > > > [   26.404807] x27: 0000000000000038 x26: 0000000000000002
> > > > [   26.410115] x25: ffff000cecbefa00 x24: ffff000cf2f96200
> > > > [   26.415423] x23: 0000000000000001 x22: 0000000000000018
> > > > [   26.420731] x21: 0000000000000001 x20: ffff800017b8bc10
> > > > [   26.426039] x19: 0000000000000000 x18: 0000000000000000
> > > > [   26.431347] x17: 0000000002e8bf2c x16: 0000000002e94c6b
> > > > [   26.436655] x15: 0000000002ea48b9 x14: ffff8000121f0300
> > > > [   26.441963] x13: 0000000002ee2ca8 x12: ffff80001129cae0
> > > > [   26.447272] x11: ffff800012435000 x10: ffff000ed46b5e88
> > > > [   26.452580] x9 : ffff000c9935e600 x8 : 0000000000000000
> > > > [   26.457888] x7 : 000000008020001e x6 : 000000008020001f
> > > > [   26.463196] x5 : ffff80001085fbe0 x4 : fffffe0033a59f20
> > > > [   26.468504] x3 : 000000008020001e x2 : 0000000000000000
> > > > [   26.473813] x1 : 0000000000000000 x0 : ffff000c8f596090
> > > > [   26.479122] Call trace:
> > > > [   26.481566] drm_modeset_drop_locks+0x84/0x90
> > > > [   26.485918] drm_mode_atomic_ioctl+0x860/0xb8c
> > > > [   26.490359] drm_ioctl_kernel+0xc4/0x120
> > > > [   26.494278] drm_ioctl+0x268/0x534
> > > > [   26.497677] __arm64_sys_ioctl+0xa8/0xf0
> > > > [   26.501598] el0_svc_common.constprop.0+0x80/0x240
> > > > [   26.506384] do_el0_svc+0x24/0x90
> > > > [   26.509697] el0_svc+0x20/0x30
> > > > [   26.512748] el0_sync_handler+0xe8/0xf0
> > > > [   26.516580] el0_sync+0x1a4/0x1c0
> > > > [   26.519891] irq event stamp: 0
> > > > [   26.522943] hardirqs last  enabled at (0): [<0000000000000000>] 0x0
> > > > [   26.529207] hardirqs last disabled at (0): [<ffff800010056d34>] copy_process+0x5d0/0x183c
> > > > [   26.537379] softirqs last  enabled at (0): [<ffff800010056d34>] copy_process+0x5d0/0x183c
> > > > [   26.545550] softirqs last disabled at (0): [<0000000000000000>] 0x0
> > > > [   26.551812] ---[ end trace 20ae984fa860184b ]---
> > > > 
> > > > According to the call trace information, it can be located to be
> > > > WARN_ON(IS_ERR(c_st)) in the komeda_pipeline_unbound_components function; Then follow the function.
> > > > komeda_pipeline_unbound_components
> > > > -> komeda_component_get_state_and_set_user
> > > >   -> komeda_pipeline_get_state_and_set_crtc
> > > >     -> komeda_pipeline_get_state
> > > >       ->drm_atomic_get_private_obj_state
> > > >         -> drm_atomic_get_private_obj_state
> > > >           -> drm_modeset_lock
> > > > 
> > > > komeda_pipeline_unbound_components
> > > > -> komeda_component_get_state_and_set_user
> > > >   -> komeda_component_get_state
> > > >     -> drm_atomic_get_private_obj_state
> > > >      -> drm_modeset_lock
> > > > 
> > > > ret = drm_modeset_lock(&obj->lock, state->acquire_ctx); if (ret)
> > > > 	return ERR_PTR(ret);
> > > > Here it return -EDEADLK.
> > > > 
> > > > deal with the deadlock as suggested by [1], using the function 
> > > > drm_modeset_backoff().
> > > > [1]
> > > > https://docs.kernel.org/gpu/drm-kms.html?highlight=kms#kms-locki
> > > > ng
> > > > 
> > > > Therefore, handling this problem can be solved by adding return 
> > > > -EDEADLK back to the drm_modeset_backoff processing flow in the 
> > > > drm_mode_atomic_ioctl function.
> > > > 
> > > > Signed-off-by: baozhu.liu <lucas.liu@xxxxxxxxxxxx>
> > > 
> > > Hi Menghui,
> > > 
> > > Can you add your Signed-off-by tag as well? With that updated, I will pull the patch into drm-misc-next by Friday.
> > > 
> > > Best regards,
> > > Liviu
> > > 
> > > > Reviewed-by: Liviu Dudau <liviu.dudau@xxxxxxx>
> > > > ---
> > > >  .../gpu/drm/arm/display/komeda/komeda_pipeline_state.c   | 9 ++++++---
> > > >  1 file changed, 6 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git
> > > > a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> > > > b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> > > > index 3276a3e82c62..916f2c36bf2f 100644
> > > > --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> > > > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> > > > @@ -1223,7 +1223,7 @@ int komeda_build_display_data_flow(struct komeda_crtc *kcrtc,
> > > >  	return 0;
> > > >  }
> > > >  
> > > > -static void
> > > > +static int
> > > >  komeda_pipeline_unbound_components(struct komeda_pipeline *pipe,
> > > >  				   struct komeda_pipeline_state *new)  { @@ -1243,8
> > > > +1243,12 @@ komeda_pipeline_unbound_components(struct 
> > > > +komeda_pipeline *pipe,
> > > >  		c = komeda_pipeline_get_component(pipe, id);
> > > >  		c_st = komeda_component_get_state_and_set_user(c,
> > > >  				drm_st, NULL, new->crtc);
> > > > +		if (PTR_ERR(c_st) == -EDEADLK)
> > > > +			return -EDEADLK;
> > > >  		WARN_ON(IS_ERR(c_st));
> > > >  	}
> > > > +
> > > > +	return 0;
> > > >  }
> > > >  
> > > >  /* release unclaimed pipeline resource */ @@ -1266,9 +1270,8 @@ 
> > > > int komeda_release_unclaimed_resources(struct komeda_pipeline *pipe,
> > > >  	if (WARN_ON(IS_ERR_OR_NULL(st)))
> > > >  		return -EINVAL;
> > > >  
> > > > -	komeda_pipeline_unbound_components(pipe, st);
> > > > +	return komeda_pipeline_unbound_components(pipe, st);
> > > >  
> > > > -	return 0;
> > > >  }
> > > >  
> > > >  /* Since standalone disabled components must be disabled 
> > > > separately and in the
> > > > --
> > > > 2.17.1
> > > > 
> > > 
> > > --
> > > ====================
> > > | I would like to |
> > > | fix the world,  |
> > > | but they're not |
> > > | giving me the   |
> > >  \ source code!  /
> > >   ---------------
> > >     ¯\_(ツ)_/¯
> > 
> > --
> > ====================
> > | I would like to |
> > | fix the world,  |
> > | but they're not |
> > | giving me the   |
> >  \ source code!  /
> >   ---------------
> >     ¯\_(ツ)_/¯
> 
> --
> ====================
> | I would like to |
> | fix the world,  |
> | but they're not |
> | giving me the   |
>  \ source code!  /
>   ---------------
>     ¯\_(ツ)_/¯

--
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux