* Re: [RFC][PATCH v2 04/31] timers: block: Use del_timer_shutdown() before freeing timer [not found] ` <20221027150925.819019339@goodmis.org> @ 2022-10-27 15:19 ` Steven Rostedt 2022-10-28 8:26 ` Christoph Hellwig 2022-10-28 15:11 ` Guenter Roeck 1 sibling, 1 reply; 8+ messages in thread From: Steven Rostedt @ 2022-10-27 15:19 UTC (permalink / raw) To: linux-kernel Cc: Linus Torvalds, Thomas Gleixner, Stephen Boyd, Guenter Roeck, Philipp Reisner, Lars Ellenberg, Christoph Böhmwalder, Jens Axboe, drbd-dev, Tejun Heo, cgroups, linux-block [ quilt mail --send still can't handle unicode characters. Here's the patch again ] From: "Steven Rostedt (Google)" <rostedt@goodmis.org> Before a timer is freed, del_timer_shutdown() must be called. Link: https://lore.kernel.org/all/20220407161745.7d6754b3@gandalf.local.home/ Cc: Philipp Reisner <philipp.reisner@linbit.com> Cc: Lars Ellenberg <lars.ellenberg@linbit.com> Cc: "Christoph Böhmwalder" <christoph.boehmwalder@linbit.com> Cc: Jens Axboe <axboe@kernel.dk> Cc: drbd-dev@lists.linbit.com Cc: Tejun Heo <tj@kernel.org> Cc: cgroups@vger.kernel.org Cc: linux-block@vger.kernel.org Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org> --- block/blk-iocost.c | 2 +- block/blk-iolatency.c | 2 +- block/blk-stat.c | 2 +- block/blk-throttle.c | 2 +- block/kyber-iosched.c | 2 +- drivers/block/drbd/drbd_main.c | 2 +- drivers/block/loop.c | 2 +- drivers/block/sunvdc.c | 2 +- 8 files changed, 8 insertions(+), 8 deletions(-) diff --git a/block/blk-iocost.c b/block/blk-iocost.c index 495396425bad..e2d4bdd3d135 100644 --- a/block/blk-iocost.c +++ b/block/blk-iocost.c @@ -2814,7 +2814,7 @@ static void ioc_rqos_exit(struct rq_qos *rqos) ioc->running = IOC_STOP; spin_unlock_irq(&ioc->lock); - del_timer_sync(&ioc->timer); + del_timer_shutdown(&ioc->timer); free_percpu(ioc->pcpu_stat); kfree(ioc); } diff --git a/block/blk-iolatency.c b/block/blk-iolatency.c index 571fa95aafe9..7b61f09afedd 100644 --- a/block/blk-iolatency.c +++ b/block/blk-iolatency.c @@ -645,7 +645,7 @@ static void blkcg_iolatency_exit(struct rq_qos *rqos) { struct blk_iolatency *blkiolat = BLKIOLATENCY(rqos); - del_timer_sync(&blkiolat->timer); + del_timer_shutdown(&blkiolat->timer); flush_work(&blkiolat->enable_work); blkcg_deactivate_policy(rqos->q, &blkcg_policy_iolatency); kfree(blkiolat); diff --git a/block/blk-stat.c b/block/blk-stat.c index 2ea01b5c1aca..de51db302c44 100644 --- a/block/blk-stat.c +++ b/block/blk-stat.c @@ -165,7 +165,7 @@ void blk_stat_remove_callback(struct request_queue *q, blk_queue_flag_clear(QUEUE_FLAG_STATS, q); spin_unlock_irqrestore(&q->stats->lock, flags); - del_timer_sync(&cb->timer); + del_timer_shutdown(&cb->timer); } static void blk_stat_free_callback_rcu(struct rcu_head *head) diff --git a/block/blk-throttle.c b/block/blk-throttle.c index 847721dc2b2b..95af99f24137 100644 --- a/block/blk-throttle.c +++ b/block/blk-throttle.c @@ -490,7 +490,7 @@ static void throtl_pd_free(struct blkg_policy_data *pd) { struct throtl_grp *tg = pd_to_tg(pd); - del_timer_sync(&tg->service_queue.pending_timer); + del_timer_shutdown(&tg->service_queue.pending_timer); blkg_rwstat_exit(&tg->stat_bytes); blkg_rwstat_exit(&tg->stat_ios); kfree(tg); diff --git a/block/kyber-iosched.c b/block/kyber-iosched.c index b05357bced99..59a444a47ba3 100644 --- a/block/kyber-iosched.c +++ b/block/kyber-iosched.c @@ -434,7 +434,7 @@ static void kyber_exit_sched(struct elevator_queue *e) struct kyber_queue_data *kqd = e->elevator_data; int i; - del_timer_sync(&kqd->timer); + del_timer_shutdown(&kqd->timer); blk_stat_disable_accounting(kqd->q); for (i = 0; i < KYBER_NUM_DOMAINS; i++) diff --git a/drivers/block/drbd/drbd_main.c b/drivers/block/drbd/drbd_main.c index f3e4db16fd07..3f574f3769c3 100644 --- a/drivers/block/drbd/drbd_main.c +++ b/drivers/block/drbd/drbd_main.c @@ -2184,7 +2184,7 @@ void drbd_destroy_device(struct kref *kref) struct drbd_resource *resource = device->resource; struct drbd_peer_device *peer_device, *tmp_peer_device; - del_timer_sync(&device->request_timer); + del_timer_shutdown(&device->request_timer); /* paranoia asserts */ D_ASSERT(device, device->open_cnt == 0); diff --git a/drivers/block/loop.c b/drivers/block/loop.c index ad92192c7d61..d134a5fd4ae7 100644 --- a/drivers/block/loop.c +++ b/drivers/block/loop.c @@ -1755,7 +1755,7 @@ static void lo_free_disk(struct gendisk *disk) if (lo->workqueue) destroy_workqueue(lo->workqueue); loop_free_idle_workers(lo, true); - del_timer_sync(&lo->timer); + del_timer_shutdown(&lo->timer); mutex_destroy(&lo->lo_mutex); kfree(lo); } diff --git a/drivers/block/sunvdc.c b/drivers/block/sunvdc.c index fb855da971ee..9868937a9602 100644 --- a/drivers/block/sunvdc.c +++ b/drivers/block/sunvdc.c @@ -1067,7 +1067,7 @@ static void vdc_port_remove(struct vio_dev *vdev) flush_work(&port->ldc_reset_work); cancel_delayed_work_sync(&port->ldc_reset_timer_work); - del_timer_sync(&port->vio.timer); + del_timer_shutdown(&port->vio.timer); del_gendisk(port->disk); put_disk(port->disk); -- 2.35.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [RFC][PATCH v2 04/31] timers: block: Use del_timer_shutdown() before freeing timer 2022-10-27 15:19 ` [RFC][PATCH v2 04/31] timers: block: Use del_timer_shutdown() before freeing timer Steven Rostedt @ 2022-10-28 8:26 ` Christoph Hellwig 2022-10-28 10:24 ` Steven Rostedt 0 siblings, 1 reply; 8+ messages in thread From: Christoph Hellwig @ 2022-10-28 8:26 UTC (permalink / raw) To: Steven Rostedt Cc: linux-kernel, Linus Torvalds, Thomas Gleixner, Stephen Boyd, Guenter Roeck, Philipp Reisner, Lars Ellenberg, Christoph Böhmwalder, Jens Axboe, drbd-dev, Tejun Heo, cgroups, linux-block This is just a single patch out of apparently 31, which claims that something that doesn't even exist in mainline must be used without any explanation. How do you expect anyone to be able to review it? ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC][PATCH v2 04/31] timers: block: Use del_timer_shutdown() before freeing timer 2022-10-28 8:26 ` Christoph Hellwig @ 2022-10-28 10:24 ` Steven Rostedt 2022-10-28 13:56 ` Jens Axboe 0 siblings, 1 reply; 8+ messages in thread From: Steven Rostedt @ 2022-10-28 10:24 UTC (permalink / raw) To: Christoph Hellwig Cc: linux-kernel, Linus Torvalds, Thomas Gleixner, Stephen Boyd, Guenter Roeck, Philipp Reisner, Lars Ellenberg, Christoph Böhmwalder, Jens Axboe, drbd-dev, Tejun Heo, cgroups, linux-block On Fri, 28 Oct 2022 01:26:03 -0700 Christoph Hellwig <hch@infradead.org> wrote: > This is just a single patch out of apparently 31, which claims that > something that doesn't even exist in mainline must be used without any > explanation. How do you expect anyone to be able to review it? https://lore.kernel.org/all/20221027150525.753064657@goodmis.org/ Only the first patch is relevant to you. I guess the Cc list would have been too big to Cc everyone that was Cc'd in the series. It not being in mainline is the reason I marked it RFC. As it's more of an FYI than a pull it in request. -- Steve ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC][PATCH v2 04/31] timers: block: Use del_timer_shutdown() before freeing timer 2022-10-28 10:24 ` Steven Rostedt @ 2022-10-28 13:56 ` Jens Axboe 2022-10-28 14:06 ` Steven Rostedt 0 siblings, 1 reply; 8+ messages in thread From: Jens Axboe @ 2022-10-28 13:56 UTC (permalink / raw) To: Steven Rostedt, Christoph Hellwig Cc: linux-kernel, Linus Torvalds, Thomas Gleixner, Stephen Boyd, Guenter Roeck, Philipp Reisner, Lars Ellenberg, Christoph Böhmwalder, drbd-dev, Tejun Heo, cgroups, linux-block On 10/28/22 4:24 AM, Steven Rostedt wrote: > On Fri, 28 Oct 2022 01:26:03 -0700 > Christoph Hellwig <hch@infradead.org> wrote: > >> This is just a single patch out of apparently 31, which claims that >> something that doesn't even exist in mainline must be used without any >> explanation. How do you expect anyone to be able to review it? > > https://lore.kernel.org/all/20221027150525.753064657@goodmis.org/ > > Only the first patch is relevant to you. I guess the Cc list would have > been too big to Cc everyone that was Cc'd in the series. No it's not, because how on earth would anyone know what the change does if you only see the simple s/name/newname change? The patch is useless by itself. -- Jens Axboe ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC][PATCH v2 04/31] timers: block: Use del_timer_shutdown() before freeing timer 2022-10-28 13:56 ` Jens Axboe @ 2022-10-28 14:06 ` Steven Rostedt 2022-10-28 14:11 ` Jens Axboe 0 siblings, 1 reply; 8+ messages in thread From: Steven Rostedt @ 2022-10-28 14:06 UTC (permalink / raw) To: Jens Axboe Cc: Christoph Hellwig, linux-kernel, Linus Torvalds, Thomas Gleixner, Stephen Boyd, Guenter Roeck, Philipp Reisner, Lars Ellenberg, Christoph Böhmwalder, drbd-dev, Tejun Heo, cgroups, linux-block On Fri, 28 Oct 2022 07:56:50 -0600 Jens Axboe <axboe@kernel.dk> wrote: > On 10/28/22 4:24 AM, Steven Rostedt wrote: > > On Fri, 28 Oct 2022 01:26:03 -0700 > > Christoph Hellwig <hch@infradead.org> wrote: > > > >> This is just a single patch out of apparently 31, which claims that > >> something that doesn't even exist in mainline must be used without any > >> explanation. How do you expect anyone to be able to review it? > > > > https://lore.kernel.org/all/20221027150525.753064657@goodmis.org/ > > > > Only the first patch is relevant to you. I guess the Cc list would have > > been too big to Cc everyone that was Cc'd in the series. > > No it's not, because how on earth would anyone know what the change does > if you only see the simple s/name/newname change? The patch is useless > by itself. > I meant this as the first patch: https://lore.kernel.org/all/20221027150925.248421571@goodmis.org/ Which was what the link above was suppose to point to. It's the only patch relevant to the rest of the series, as the rest is just converting over to the shutdown API, and the last patch changes DEBUG_OBJECTS_TIMERS to catch if this was done properly. That is, patch 01/31 and the patch you were Cc'd on is relevant, and for those that want to look deeper, see patch 31 as well. But if I included the Cc list for patch 01 for all those Cc'd in the entire series, it would be a huge Cc list, so I avoided doing so. Also, this is still RFC as the changes may still change. That is, this patch set is a heads up to what is to come. Ideally, I'd like to get just the API possibly in the kernel before the merge window without anyone using it. Then I can ask all the sub systems to pull in these individual patches as well. -- Steve ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC][PATCH v2 04/31] timers: block: Use del_timer_shutdown() before freeing timer 2022-10-28 14:06 ` Steven Rostedt @ 2022-10-28 14:11 ` Jens Axboe 2022-10-28 14:30 ` Steven Rostedt 0 siblings, 1 reply; 8+ messages in thread From: Jens Axboe @ 2022-10-28 14:11 UTC (permalink / raw) To: Steven Rostedt Cc: Christoph Hellwig, linux-kernel, Linus Torvalds, Thomas Gleixner, Stephen Boyd, Guenter Roeck, Philipp Reisner, Lars Ellenberg, Christoph Böhmwalder, drbd-dev, Tejun Heo, cgroups, linux-block On 10/28/22 8:06 AM, Steven Rostedt wrote: > On Fri, 28 Oct 2022 07:56:50 -0600 > Jens Axboe <axboe@kernel.dk> wrote: > >> On 10/28/22 4:24 AM, Steven Rostedt wrote: >>> On Fri, 28 Oct 2022 01:26:03 -0700 >>> Christoph Hellwig <hch@infradead.org> wrote: >>> >>>> This is just a single patch out of apparently 31, which claims that >>>> something that doesn't even exist in mainline must be used without any >>>> explanation. How do you expect anyone to be able to review it? >>> >>> https://lore.kernel.org/all/20221027150525.753064657@goodmis.org/ >>> >>> Only the first patch is relevant to you. I guess the Cc list would have >>> been too big to Cc everyone that was Cc'd in the series. >> >> No it's not, because how on earth would anyone know what the change does >> if you only see the simple s/name/newname change? The patch is useless >> by itself. >> > > I meant this as the first patch: > > https://lore.kernel.org/all/20221027150925.248421571@goodmis.org/ > > Which was what the link above was suppose to point to. > > It's the only patch relevant to the rest of the series, as the rest is just > converting over to the shutdown API, and the last patch changes > DEBUG_OBJECTS_TIMERS to catch if this was done properly. > > That is, patch 01/31 and the patch you were Cc'd on is relevant, and for > those that want to look deeper, see patch 31 as well. So we got half of what was needed to make any kind of sense of judgement on the patch. > But if I included the Cc list for patch 01 for all those Cc'd in the > entire series, it would be a huge Cc list, so I avoided doing so. And my point is that just CC'ing the relevant list for patch 4/31 is useless. Do we need to see the whole series? No. Does everyone need to see patch 1/31? Yes, very much so. Without that, 4/31 means nothing. This is pretty common for tree wide changes. The relevant lists need to see the full context, patch 4/31 by itself is useless and may as well not be sent at this point then. -- Jens Axboe ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC][PATCH v2 04/31] timers: block: Use del_timer_shutdown() before freeing timer 2022-10-28 14:11 ` Jens Axboe @ 2022-10-28 14:30 ` Steven Rostedt 0 siblings, 0 replies; 8+ messages in thread From: Steven Rostedt @ 2022-10-28 14:30 UTC (permalink / raw) To: Jens Axboe Cc: Christoph Hellwig, linux-kernel, Linus Torvalds, Thomas Gleixner, Stephen Boyd, Guenter Roeck, Philipp Reisner, Lars Ellenberg, Christoph Böhmwalder, drbd-dev, Tejun Heo, cgroups, linux-block On Fri, 28 Oct 2022 08:11:27 -0600 Jens Axboe <axboe@kernel.dk> wrote: > This is pretty common for tree wide changes. The relevant lists need > to see the full context, patch 4/31 by itself is useless and may as well > not be sent at this point then. Ah, I didn't think about just including the mailing lists. The Cc lists were auto-generated, and I didn't think about just taking out the lists. Will do that for v2. Thanks, -- Steve ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC][PATCH v2 04/31] timers: block: Use del_timer_shutdown() before freeing timer [not found] ` <20221027150925.819019339@goodmis.org> 2022-10-27 15:19 ` [RFC][PATCH v2 04/31] timers: block: Use del_timer_shutdown() before freeing timer Steven Rostedt @ 2022-10-28 15:11 ` Guenter Roeck 1 sibling, 0 replies; 8+ messages in thread From: Guenter Roeck @ 2022-10-28 15:11 UTC (permalink / raw) To: Steven Rostedt, linux-kernel Cc: Linus Torvalds, Thomas Gleixner, Stephen Boyd, Philipp Reisner, Lars Ellenberg, Christoph Böhmwalder, Jens Axboe, drbd-dev, Tejun Heo, cgroups, linux-block On 10/27/22 08:05, Steven Rostedt wrote: > From: "Steven Rostedt (Google)" <rostedt@goodmis.org> > > Before a timer is freed, del_timer_shutdown() must be called. > I also had to add the following, as you had already suggested. Just changing blk_sync_queue() was insufficient; I had to add the call from blk_release_queue() because otherwise blk_sync_queue() was not always called. Thanks, Guenter --- diff --git a/block/blk-core.c b/block/blk-core.c index 17667159482e..69b1daa2e91a 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -227,7 +227,7 @@ const char *blk_status_to_str(blk_status_t status) */ void blk_sync_queue(struct request_queue *q) { - del_timer_sync(&q->timeout); + del_timer_shutdown(&q->timeout); cancel_work_sync(&q->timeout_work); } EXPORT_SYMBOL(blk_sync_queue); diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c index e71b3b43927c..12a1e46536ed 100644 --- a/block/blk-sysfs.c +++ b/block/blk-sysfs.c @@ -769,6 +769,8 @@ static void blk_release_queue(struct kobject *kobj) percpu_ref_exit(&q->q_usage_counter); + blk_sync_queue(q); + if (q->poll_stat) blk_stat_remove_callback(q, q->poll_cb); blk_stat_free_callback(q->poll_cb); ^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-10-28 15:12 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20221027150525.753064657@goodmis.org>
[not found] ` <20221027150925.819019339@goodmis.org>
2022-10-27 15:19 ` [RFC][PATCH v2 04/31] timers: block: Use del_timer_shutdown() before freeing timer Steven Rostedt
2022-10-28 8:26 ` Christoph Hellwig
2022-10-28 10:24 ` Steven Rostedt
2022-10-28 13:56 ` Jens Axboe
2022-10-28 14:06 ` Steven Rostedt
2022-10-28 14:11 ` Jens Axboe
2022-10-28 14:30 ` Steven Rostedt
2022-10-28 15:11 ` Guenter Roeck
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).