From: Andrey Grodzovsky <Andrey.Grodzovsky@amd.com> To: "Eric W. Biederman" <ebiederm@xmission.com> Cc: Oleg Nesterov <oleg@redhat.com>, linux-kernel@vger.kernel.org, amd-gfx@lists.freedesktop.org, Alexander.Deucher@amd.com, Christian.Koenig@amd.com, David.Panariti@amd.com, akpm@linux-foundation.org Subject: Re: [PATCH 2/3] drm/scheduler: Don't call wait_event_killable for signaled process. Date: Thu, 26 Apr 2018 16:43:37 -0400 [thread overview] Message-ID: <6e7f0d52-8d31-2ebb-47c3-865d676e8158@amd.com> (raw) In-Reply-To: <87zi1qq7t1.fsf@xmission.com> On 04/26/2018 11:57 AM, Eric W. Biederman wrote: > Andrey Grodzovsky <Andrey.Grodzovsky@amd.com> writes: > >> On 04/26/2018 08:34 AM, Andrey Grodzovsky wrote: >>> >>> On 04/25/2018 08:01 PM, Eric W. Biederman wrote: >>>> Andrey Grodzovsky <Andrey.Grodzovsky@amd.com> writes: >>>> >>>>> On 04/25/2018 01:17 PM, Oleg Nesterov wrote: >>>>>> On 04/25, Andrey Grodzovsky wrote: >>>>>>> here (drm_sched_entity_fini) is also a bad idea, but we still want to be >>>>>>> able to exit immediately >>>>>>> and not wait for GPU jobs completion when the reason for reaching this >>>>>>> code >>>>>>> is because of KILL >>>>>>> signal to the user process who opened the device file. >>>>>> Can you hook f_op->flush method? >>>>> But this one is called for each task releasing a reference to the the file, >>>>> so >>>>> not sure I see how this solves the problem. >>>> The big question is why do you need to wait during the final closing a >>>> file? >>>> >>>> The wait can be terminated so the wait does not appear to be simply a >>>> matter of correctness. >>> Well, as I understand it, it just means that you don't want to abruptly >>> terminate GPU work in progress without a good >>> reason (such as KILL signal). When we exit we are going to release various >>> resources GPU is still using so we either >>> wait for it to complete or terminate the remaining jobs. > At the point of do_exit you might as well be a KILL signal however you > got there. > >> Looked more into code, some correction, drm_sched_entity_fini means the SW job >> queue itself is about to die, so we must >> either wait for completion or terminate any outstanding jobs that are still in >> the SW queue. Anything which already in flight in HW >> will still complete. > It sounds like we don't care if we block the process that had the file > descriptor open, this is just book keeping. Which allows having a piece > of code that cleans up resources when the GPU is done with the queue but > does not make userspace wait. (option 1) > > For it to make sense that we let the process run there has to be > something that cares about the results being completed. If all of the > file descriptors are closed and the process is killed I can't see who > will care that the software queue will continue to be processed. So it > may be reasonable to simply kill the queue (option 2). > > If userspace really needs the wait it is probably better done in > f_op->flush so that every close of the file descriptor blocks > until the queue is flushed (option 3). > > Do you know if userspace cares about the gpu operations completing? I don't have a good answer for that, I would assume it depends on type of jobs still remaining unprocessed and on the general type of work the user process is doing. Some key people who can answer this are currently away for a few days/week so the answer for this will have to wait a bit. Andrey > > My skim of the code suggests that nothing actually cares about those > operations, but I really don't know the gpu well. > > Eric
WARNING: multiple messages have this Message-ID (diff)
From: Andrey Grodzovsky <Andrey.Grodzovsky-5C7GfCeVMHo@public.gmane.org> To: "Eric W. Biederman" <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org> Cc: David.Panariti-5C7GfCeVMHo@public.gmane.org, Oleg Nesterov <oleg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Alexander.Deucher-5C7GfCeVMHo@public.gmane.org, akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org, Christian.Koenig-5C7GfCeVMHo@public.gmane.org Subject: Re: [PATCH 2/3] drm/scheduler: Don't call wait_event_killable for signaled process. Date: Thu, 26 Apr 2018 16:43:37 -0400 [thread overview] Message-ID: <6e7f0d52-8d31-2ebb-47c3-865d676e8158@amd.com> (raw) In-Reply-To: <87zi1qq7t1.fsf-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org> On 04/26/2018 11:57 AM, Eric W. Biederman wrote: > Andrey Grodzovsky <Andrey.Grodzovsky@amd.com> writes: > >> On 04/26/2018 08:34 AM, Andrey Grodzovsky wrote: >>> >>> On 04/25/2018 08:01 PM, Eric W. Biederman wrote: >>>> Andrey Grodzovsky <Andrey.Grodzovsky@amd.com> writes: >>>> >>>>> On 04/25/2018 01:17 PM, Oleg Nesterov wrote: >>>>>> On 04/25, Andrey Grodzovsky wrote: >>>>>>> here (drm_sched_entity_fini) is also a bad idea, but we still want to be >>>>>>> able to exit immediately >>>>>>> and not wait for GPU jobs completion when the reason for reaching this >>>>>>> code >>>>>>> is because of KILL >>>>>>> signal to the user process who opened the device file. >>>>>> Can you hook f_op->flush method? >>>>> But this one is called for each task releasing a reference to the the file, >>>>> so >>>>> not sure I see how this solves the problem. >>>> The big question is why do you need to wait during the final closing a >>>> file? >>>> >>>> The wait can be terminated so the wait does not appear to be simply a >>>> matter of correctness. >>> Well, as I understand it, it just means that you don't want to abruptly >>> terminate GPU work in progress without a good >>> reason (such as KILL signal). When we exit we are going to release various >>> resources GPU is still using so we either >>> wait for it to complete or terminate the remaining jobs. > At the point of do_exit you might as well be a KILL signal however you > got there. > >> Looked more into code, some correction, drm_sched_entity_fini means the SW job >> queue itself is about to die, so we must >> either wait for completion or terminate any outstanding jobs that are still in >> the SW queue. Anything which already in flight in HW >> will still complete. > It sounds like we don't care if we block the process that had the file > descriptor open, this is just book keeping. Which allows having a piece > of code that cleans up resources when the GPU is done with the queue but > does not make userspace wait. (option 1) > > For it to make sense that we let the process run there has to be > something that cares about the results being completed. If all of the > file descriptors are closed and the process is killed I can't see who > will care that the software queue will continue to be processed. So it > may be reasonable to simply kill the queue (option 2). > > If userspace really needs the wait it is probably better done in > f_op->flush so that every close of the file descriptor blocks > until the queue is flushed (option 3). > > Do you know if userspace cares about the gpu operations completing? I don't have a good answer for that, I would assume it depends on type of jobs still remaining unprocessed and on the general type of work the user process is doing. Some key people who can answer this are currently away for a few days/week so the answer for this will have to wait a bit. Andrey > > My skim of the code suggests that nothing actually cares about those > operations, but I really don't know the gpu well. > > Eric _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
next prev parent reply other threads:[~2018-04-26 20:43 UTC|newest] Thread overview: 122+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-04-24 15:30 Avoid uninterruptible sleep during process exit Andrey Grodzovsky 2018-04-24 15:30 ` Andrey Grodzovsky 2018-04-24 15:30 ` [PATCH 1/3] signals: Allow generation of SIGKILL to exiting task Andrey Grodzovsky 2018-04-24 15:30 ` Andrey Grodzovsky 2018-04-24 16:10 ` Eric W. Biederman 2018-04-24 16:10 ` Eric W. Biederman 2018-04-24 16:42 ` Eric W. Biederman 2018-04-24 16:42 ` Eric W. Biederman 2018-04-24 16:51 ` Andrey Grodzovsky 2018-04-24 16:51 ` Andrey Grodzovsky 2018-04-24 17:29 ` Eric W. Biederman 2018-04-25 13:13 ` Oleg Nesterov 2018-04-24 15:30 ` [PATCH 2/3] drm/scheduler: Don't call wait_event_killable for signaled process Andrey Grodzovsky 2018-04-24 15:30 ` Andrey Grodzovsky 2018-04-24 15:46 ` Michel Dänzer 2018-04-24 15:51 ` Andrey Grodzovsky 2018-04-24 15:51 ` Andrey Grodzovsky 2018-04-24 15:52 ` Andrey Grodzovsky 2018-04-24 15:52 ` Andrey Grodzovsky 2018-04-24 19:44 ` Daniel Vetter 2018-04-24 19:44 ` Daniel Vetter 2018-04-24 21:00 ` Eric W. Biederman 2018-04-24 21:02 ` Andrey Grodzovsky 2018-04-24 21:02 ` Andrey Grodzovsky 2018-04-24 21:21 ` Eric W. Biederman 2018-04-24 21:37 ` Andrey Grodzovsky 2018-04-24 21:37 ` Andrey Grodzovsky 2018-04-24 22:11 ` Eric W. Biederman 2018-04-25 7:14 ` Daniel Vetter 2018-04-25 13:08 ` Andrey Grodzovsky 2018-04-25 13:08 ` Andrey Grodzovsky 2018-04-25 15:29 ` Eric W. Biederman 2018-04-25 16:13 ` Andrey Grodzovsky 2018-04-25 16:31 ` Eric W. Biederman 2018-04-24 21:40 ` Daniel Vetter 2018-04-24 21:40 ` Daniel Vetter 2018-04-25 13:22 ` Oleg Nesterov 2018-04-25 13:36 ` Daniel Vetter 2018-04-25 14:18 ` Oleg Nesterov 2018-04-25 14:18 ` Oleg Nesterov 2018-04-25 13:43 ` Andrey Grodzovsky 2018-04-25 13:43 ` Andrey Grodzovsky 2018-04-24 16:23 ` Eric W. Biederman 2018-04-24 16:23 ` Eric W. Biederman 2018-04-24 16:43 ` Andrey Grodzovsky 2018-04-24 16:43 ` Andrey Grodzovsky 2018-04-24 17:12 ` Eric W. Biederman 2018-04-25 13:55 ` Oleg Nesterov 2018-04-25 14:21 ` Andrey Grodzovsky 2018-04-25 14:21 ` Andrey Grodzovsky 2018-04-25 17:17 ` Oleg Nesterov 2018-04-25 18:40 ` Andrey Grodzovsky 2018-04-25 18:40 ` Andrey Grodzovsky 2018-04-26 0:01 ` Eric W. Biederman 2018-04-26 12:34 ` Andrey Grodzovsky 2018-04-26 12:34 ` Andrey Grodzovsky 2018-04-26 12:52 ` Andrey Grodzovsky 2018-04-26 12:52 ` Andrey Grodzovsky 2018-04-26 15:57 ` Eric W. Biederman 2018-04-26 20:43 ` Andrey Grodzovsky [this message] 2018-04-26 20:43 ` Andrey Grodzovsky 2018-04-30 12:08 ` Christian König 2018-04-30 12:08 ` Christian König 2018-04-30 14:32 ` Andrey Grodzovsky 2018-04-30 14:32 ` Andrey Grodzovsky 2018-04-30 15:25 ` Christian König 2018-04-30 15:25 ` Christian König 2018-04-30 16:00 ` Oleg Nesterov 2018-04-30 16:10 ` Andrey Grodzovsky 2018-04-30 16:10 ` Andrey Grodzovsky 2018-04-30 18:29 ` Christian König 2018-04-30 18:29 ` Christian König 2018-04-30 19:28 ` Andrey Grodzovsky 2018-04-30 19:28 ` Andrey Grodzovsky 2018-05-02 11:48 ` Christian König 2018-05-02 11:48 ` Christian König 2018-05-17 11:18 ` Andrey Grodzovsky 2018-05-17 14:48 ` Michel Dänzer 2018-05-17 15:33 ` Andrey Grodzovsky 2018-05-17 15:52 ` Michel Dänzer 2018-05-17 19:05 ` Andrey Grodzovsky 2018-05-18 8:46 ` Michel Dänzer 2018-05-18 9:42 ` Christian König 2018-05-18 14:44 ` Michel Dänzer 2018-05-18 14:50 ` Christian König 2018-05-18 15:02 ` Andrey Grodzovsky 2018-05-22 12:58 ` Christian König 2018-05-22 15:49 ` Andrey Grodzovsky 2018-05-22 16:09 ` Michel Dänzer 2018-05-22 16:30 ` Andrey Grodzovsky 2018-05-22 16:33 ` Michel Dänzer 2018-05-22 16:37 ` Andrey Grodzovsky 2018-05-01 14:35 ` Oleg Nesterov 2018-05-23 15:08 ` Andrey Grodzovsky 2018-05-23 15:08 ` Andrey Grodzovsky 2018-04-30 15:29 ` Oleg Nesterov 2018-04-30 16:25 ` Eric W. Biederman 2018-04-30 17:18 ` Andrey Grodzovsky 2018-04-30 17:18 ` Andrey Grodzovsky 2018-04-25 13:05 ` Oleg Nesterov 2018-04-24 15:30 ` [PATCH 3/3] drm/amdgpu: Switch to interrupted wait to recover from ring hang Andrey Grodzovsky 2018-04-24 15:30 ` Andrey Grodzovsky 2018-04-24 15:52 ` Panariti, David 2018-04-24 15:52 ` Panariti, David 2018-04-24 15:58 ` Andrey Grodzovsky 2018-04-24 15:58 ` Andrey Grodzovsky 2018-04-24 16:20 ` Panariti, David 2018-04-24 16:20 ` Panariti, David 2018-04-24 16:30 ` Eric W. Biederman 2018-04-24 16:30 ` Eric W. Biederman 2018-04-25 17:17 ` Andrey Grodzovsky 2018-04-25 17:17 ` Andrey Grodzovsky 2018-04-25 20:55 ` Eric W. Biederman 2018-04-25 20:55 ` Eric W. Biederman 2018-04-26 12:28 ` Andrey Grodzovsky 2018-04-26 12:28 ` Andrey Grodzovsky 2018-04-24 16:14 ` Eric W. Biederman 2018-04-24 16:14 ` Eric W. Biederman 2018-04-24 16:38 ` Andrey Grodzovsky 2018-04-24 16:38 ` Andrey Grodzovsky 2018-04-30 11:34 ` Christian König 2018-04-30 11:34 ` Christian König
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=6e7f0d52-8d31-2ebb-47c3-865d676e8158@amd.com \ --to=andrey.grodzovsky@amd.com \ --cc=Alexander.Deucher@amd.com \ --cc=Christian.Koenig@amd.com \ --cc=David.Panariti@amd.com \ --cc=akpm@linux-foundation.org \ --cc=amd-gfx@lists.freedesktop.org \ --cc=ebiederm@xmission.com \ --cc=linux-kernel@vger.kernel.org \ --cc=oleg@redhat.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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.