| Message ID | 20221104054914.085569465@goodmis.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | timers: Use timer_shutdown*() before freeing timers | expand |
[ Once again, quilt fails the MIME coding ] From: "Steven Rostedt (Google)" <rostedt@goodmis.org> Before a timer is freed, timer_shutdown_sync() must be called. Link: https://lore.kernel.org/all/20220407161745.7d6754b3@gandalf.local.home/ Cc: Sumit Semwal <sumit.semwal@linaro.org> Cc: "Christian König" <christian.koenig@amd.com> Cc: linux-media@vger.kernel.org Cc: dri-devel@lists.freedesktop.org Cc: linaro-mm-sig@lists.linaro.org Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org> --- drivers/dma-buf/st-dma-fence.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/dma-buf/st-dma-fence.c b/drivers/dma-buf/st-dma-fence.c index fb6e0a6ae2c9..5d3e7b503501 100644 --- a/drivers/dma-buf/st-dma-fence.c +++ b/drivers/dma-buf/st-dma-fence.c @@ -412,7 +412,7 @@ static int test_wait_timeout(void *arg) err = 0; err_free: - del_timer_sync(&wt.timer); + timer_shutdown_sync(&wt.timer); destroy_timer_on_stack(&wt.timer); dma_fence_signal(wt.f); dma_fence_put(wt.f);
Am 04.11.22 um 06:54 schrieb Steven Rostedt: > [ Once again, quilt fails the MIME coding ] > > From: "Steven Rostedt (Google)" <rostedt@goodmis.org> > > Before a timer is freed, timer_shutdown_sync() must be called. > > Link: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fall%2F20220407161745.7d6754b3%40gandalf.local.home%2F&data=05%7C01%7Cchristian.koenig%40amd.com%7Ca18ff1d0a7e442a1283808dabe29148d%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638031380931371691%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=XZgwOy0u20L1AxOjhUpWICodbSn2VYhh6YGSykjUegQ%3D&reserved=0 > > Cc: Sumit Semwal <sumit.semwal@linaro.org> > Cc: "Christian König" <christian.koenig@amd.com> > Cc: linux-media@vger.kernel.org > Cc: dri-devel@lists.freedesktop.org > Cc: linaro-mm-sig@lists.linaro.org > Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org> > --- > drivers/dma-buf/st-dma-fence.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/dma-buf/st-dma-fence.c b/drivers/dma-buf/st-dma-fence.c > index fb6e0a6ae2c9..5d3e7b503501 100644 > --- a/drivers/dma-buf/st-dma-fence.c > +++ b/drivers/dma-buf/st-dma-fence.c > @@ -412,7 +412,7 @@ static int test_wait_timeout(void *arg) > > err = 0; > err_free: > - del_timer_sync(&wt.timer); > + timer_shutdown_sync(&wt.timer); Mhm, what exactly is the benefit of renaming the function? Not that I'm against the change, but my thinking is more if there are more functions which don't re-arm the time than those which do that then why not forbid it in general? Regards, Christian. > destroy_timer_on_stack(&wt.timer); > dma_fence_signal(wt.f); > dma_fence_put(wt.f);
On Fri, 4 Nov 2022 08:15:53 +0100 Christian König <christian.koenig@amd.com> wrote: > > index fb6e0a6ae2c9..5d3e7b503501 100644 > > --- a/drivers/dma-buf/st-dma-fence.c > > +++ b/drivers/dma-buf/st-dma-fence.c > > @@ -412,7 +412,7 @@ static int test_wait_timeout(void *arg) > > > > err = 0; > > err_free: > > - del_timer_sync(&wt.timer); > > + timer_shutdown_sync(&wt.timer); > > Mhm, what exactly is the benefit of renaming the function? > > Not that I'm against the change, but my thinking is more if there are > more functions which don't re-arm the time than those which do that then > why not forbid it in general? Timers are more often re-armed then not. I had to look for the locations where del_timer*() was called just before freeing, and other locations where they are freed later. I didn't rename del_timer_sync() to timer_shutdown_sync(), this version renamed the new "del_timer_shutdown()" to "timer_shutdown_sync()". Maybe I'm just confused at what you are asking. -- Steve
Am 04.11.22 um 19:58 schrieb Steven Rostedt: > On Fri, 4 Nov 2022 08:15:53 +0100 > Christian König <christian.koenig@amd.com> wrote: > >>> index fb6e0a6ae2c9..5d3e7b503501 100644 >>> --- a/drivers/dma-buf/st-dma-fence.c >>> +++ b/drivers/dma-buf/st-dma-fence.c >>> @@ -412,7 +412,7 @@ static int test_wait_timeout(void *arg) >>> >>> err = 0; >>> err_free: >>> - del_timer_sync(&wt.timer); >>> + timer_shutdown_sync(&wt.timer); >> Mhm, what exactly is the benefit of renaming the function? >> >> Not that I'm against the change, but my thinking is more if there are >> more functions which don't re-arm the time than those which do that then >> why not forbid it in general? > Timers are more often re-armed then not. I had to look for the > locations where del_timer*() was called just before freeing, and other > locations where they are freed later. > > I didn't rename del_timer_sync() to timer_shutdown_sync(), this version > renamed the new "del_timer_shutdown()" to "timer_shutdown_sync()". > > Maybe I'm just confused at what you are asking. No, that explains it a bit better. I was just wondering what exactly the different to del_timer_sync() is. Maybe shorten the summary in the cover letter a bit. The history how this change came to be is not as interesting as why we are changing something. Regards, Christian. > > -- Steve > _______________________________________________ > Linaro-mm-sig mailing list -- linaro-mm-sig@lists.linaro.org > To unsubscribe send an email to linaro-mm-sig-leave@lists.linaro.org
diff --git a/drivers/dma-buf/st-dma-fence.c b/drivers/dma-buf/st-dma-fence.c index fb6e0a6ae2c9..5d3e7b503501 100644 --- a/drivers/dma-buf/st-dma-fence.c +++ b/drivers/dma-buf/st-dma-fence.c @@ -412,7 +412,7 @@ static int test_wait_timeout(void *arg) err = 0; err_free: - del_timer_sync(&wt.timer); + timer_shutdown_sync(&wt.timer); destroy_timer_on_stack(&wt.timer); dma_fence_signal(wt.f); dma_fence_put(wt.f);