On Tue 30-11-21 13:46:35, Christoph Hellwig wrote:
> Fold __ioc_clear_queue into ioc_clear_queue, remove the spurious RCU
> criticial section and unify the irq locking style.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> ---
> block/blk-ioc.c | 31 +++++++++----------------------
> 1 file changed, 9 insertions(+), 22 deletions(-)
>
> diff --git a/block/blk-ioc.c b/block/blk-ioc.c
> index 902bca2b273ba..32ae006e1b3e8 100644
> --- a/block/blk-ioc.c
> +++ b/block/blk-ioc.c
> @@ -192,27 +192,6 @@ void exit_io_context(struct task_struct *task)
> }
> }
>
> -static void __ioc_clear_queue(struct list_head *icq_list)
> -{
> - unsigned long flags;
> -
> - rcu_read_lock();
> - while (!list_empty(icq_list)) {
> - struct io_cq *icq = list_entry(icq_list->next,
> - struct io_cq, q_node);
> - struct io_context *ioc = icq->ioc;
> -
> - spin_lock_irqsave(&ioc->lock, flags);
> - if (icq->flags & ICQ_DESTROYED) {
> - spin_unlock_irqrestore(&ioc->lock, flags);
> - continue;
> - }
> - ioc_destroy_icq(icq);
> - spin_unlock_irqrestore(&ioc->lock, flags);
> - }
> - rcu_read_unlock();
> -}
> -
> /**
> * ioc_clear_queue - break any ioc association with the specified queue
> * @q: request_queue being cleared
> @@ -227,7 +206,15 @@ void ioc_clear_queue(struct request_queue *q)
> list_splice_init(&q->icq_list, &icq_list);
> spin_unlock_irq(&q->queue_lock);
>
> - __ioc_clear_queue(&icq_list);
> + while (!list_empty(&icq_list)) {
> + struct io_cq *icq =
> + list_entry(icq_list.next, struct io_cq, q_node);
I'm not quite sure about dropping the rcu protection here. This function
generally runs without any protection so what guards us against icq being
freed just after we've got its pointer from the list?
Honza
> +
> + spin_lock_irq(&icq->ioc->lock);
> + if (!(icq->flags & ICQ_DESTROYED))
> + ioc_destroy_icq(icq);
> + spin_unlock_irq(&icq->ioc->lock);
> + }
> }
>
> static struct io_context *alloc_io_context(gfp_t gfp_flags, int node)
> --
> 2.30.2
>
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
