Re: [RFC PATCH v2 2/2] blk-mq: Lockout tagset iter when freeing rqs

2021-02-10 Thread John Garry
+ On 04/01/2021 18:37, John Garry wrote: Hi Bart, Right, what I proposed is unrelated to the use-after-free triggered by disabling I/O scheduling. Regarding the races triggered by disabling I/O scheduling: can these be fixed by quiescing all request queues associated with a tag set before

Re: [RFC PATCH v2 2/2] blk-mq: Lockout tagset iter when freeing rqs

2021-01-04 Thread John Garry
On 04/01/2021 17:22, Bart Van Assche wrote: On 1/4/21 7:33 AM, John Garry wrote: On 23/12/2020 15:47, Bart Van Assche wrote: I propose to change the order in which blk_mq_sched_free_requests(q) and blk_mq_debugfs_unregister(q) are called. Today blk_mq_sched_free_requests(q) is called by

Re: [RFC PATCH v2 2/2] blk-mq: Lockout tagset iter when freeing rqs

2021-01-04 Thread Bart Van Assche
On 1/4/21 7:33 AM, John Garry wrote: > On 23/12/2020 15:47, Bart Van Assche wrote: >> I propose to change the order in which blk_mq_sched_free_requests(q) and >> blk_mq_debugfs_unregister(q) are called. Today blk_mq_sched_free_requests(q) >> is called by blk_cleanup_queue() before blk_put_queue()

Re: [RFC PATCH v2 2/2] blk-mq: Lockout tagset iter when freeing rqs

2021-01-04 Thread John Garry
On 23/12/2020 15:47, Bart Van Assche wrote: On 12/23/20 3:40 AM, John Garry wrote: Sorry, I got the 2x iter functions mixed up. So if we use mutex to solve blk_mq_queue_tag_busy_iter() problem, then we still have this issue in blk_mq_tagset_busy_iter() which I report previously [0]: [ 

Re: [RFC PATCH v2 2/2] blk-mq: Lockout tagset iter when freeing rqs

2020-12-23 Thread Bart Van Assche
On 12/23/20 3:40 AM, John Garry wrote: > Sorry, I got the 2x iter functions mixed up. > > So if we use mutex to solve blk_mq_queue_tag_busy_iter() problem, then we > still have this issue in blk_mq_tagset_busy_iter() which I report previously > [0]: > > [  319.771745] BUG: KASAN: use-after-free

Re: [RFC PATCH v2 2/2] blk-mq: Lockout tagset iter when freeing rqs

2020-12-23 Thread John Garry
- p...@codeaurora.org Are there any blk_mq_tagset_busy_iter() calls that happen from a context where the tag set can disappear while that function is in progress? So isn't the blk_mq_tag_set always a member of the host driver data for those cases, and, since blk_mq_tagset_busy_iter() is

Re: [RFC PATCH v2 2/2] blk-mq: Lockout tagset iter when freeing rqs

2020-12-23 Thread John Garry
On 22/12/2020 16:16, Bart Van Assche wrote: On 12/22/20 3:15 AM, John Garry wrote: So then we could have something like this: ---8<---   -435,9 +444,13 @@ void blk_mq_queue_tag_busy_iter(struct request_queue *q, busy_iter_fn *fn,  if (!blk_mq_hw_queue_mapped(hctx))

Re: [RFC PATCH v2 2/2] blk-mq: Lockout tagset iter when freeing rqs

2020-12-22 Thread Bart Van Assche
On 12/22/20 3:15 AM, John Garry wrote: So then we could have something like this: ---8<---  -435,9 +444,13 @@ void blk_mq_queue_tag_busy_iter(struct request_queue *q, busy_iter_fn *fn, if (!blk_mq_hw_queue_mapped(hctx))     continue; +    while

Re: [RFC PATCH v2 2/2] blk-mq: Lockout tagset iter when freeing rqs

2020-12-22 Thread Ming Lei
On Tue, Dec 22, 2020 at 11:22:19AM +, John Garry wrote: > Resend without p...@codeaurora.org, which bounces for me > > On 22/12/2020 02:13, Bart Van Assche wrote: > > On 12/21/20 10:47 AM, John Garry wrote: > >> Yes, I agree, and I'm not sure what I wrote to give that impression. > >> > >>

Re: [RFC PATCH v2 2/2] blk-mq: Lockout tagset iter when freeing rqs

2020-12-22 Thread John Garry
Resend without p...@codeaurora.org, which bounces for me On 22/12/2020 02:13, Bart Van Assche wrote: > On 12/21/20 10:47 AM, John Garry wrote: >> Yes, I agree, and I'm not sure what I wrote to give that impression. >> >> About "root partition", above, I'm just saying that / is mounted on a >>

Re: [RFC PATCH v2 2/2] blk-mq: Lockout tagset iter when freeing rqs

2020-12-22 Thread John Garry
On 22/12/2020 02:13, Bart Van Assche wrote: On 12/21/20 10:47 AM, John Garry wrote: Yes, I agree, and I'm not sure what I wrote to give that impression. About "root partition", above, I'm just saying that / is mounted on a sda partition: root@ubuntu:/home/john# mount | grep sda /dev/sda2 on /

Re: [RFC PATCH v2 2/2] blk-mq: Lockout tagset iter when freeing rqs

2020-12-21 Thread Bart Van Assche
On 12/21/20 10:47 AM, John Garry wrote: > Yes, I agree, and I'm not sure what I wrote to give that impression. > > About "root partition", above, I'm just saying that / is mounted on a > sda partition: > > root@ubuntu:/home/john# mount | grep sda > /dev/sda2 on / type ext4

Re: [RFC PATCH v2 2/2] blk-mq: Lockout tagset iter when freeing rqs

2020-12-21 Thread John Garry
On 21/12/2020 18:09, Bart Van Assche wrote: On 12/21/20 4:06 AM, John Garry wrote: On 18/12/2020 22:43, Bart Van Assche wrote: Does this mean that we do not yet have a full explanation about why the above call stack can be triggered? We understand it, and I'll describe my experiment in

Re: [RFC PATCH v2 2/2] blk-mq: Lockout tagset iter when freeing rqs

2020-12-21 Thread Bart Van Assche
On 12/21/20 4:06 AM, John Garry wrote: > On 18/12/2020 22:43, Bart Van Assche wrote: >> Does this mean that we do not yet have >> a full explanation about why the above call stack can be triggered? > > We understand it, and I'll describe my experiment in detail: > a. fio runs on 2x disks, sda

Re: [RFC PATCH v2 2/2] blk-mq: Lockout tagset iter when freeing rqs

2020-12-21 Thread John Garry
On 18/12/2020 22:43, Bart Van Assche wrote: On 12/17/20 3:07 AM, John Garry wrote: References to old IO sched requests are currently cleared from the tagset when freeing those requests; switching elevator or changing request queue depth is such a scenario in which this occurs. However, this

Re: [RFC PATCH v2 2/2] blk-mq: Lockout tagset iter when freeing rqs

2020-12-18 Thread Bart Van Assche
On 12/17/20 3:07 AM, John Garry wrote: > References to old IO sched requests are currently cleared from the > tagset when freeing those requests; switching elevator or changing > request queue depth is such a scenario in which this occurs. > > However, this does not stop the potentially racy

Re: [RFC PATCH v2 2/2] blk-mq: Lockout tagset iter when freeing rqs

2020-12-18 Thread John Garry
On 18/12/2020 03:31, Ming Lei wrote: 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c index a6df2d5df88a..853ed5b889aa 100644 --- a/block/blk-mq-tag.c +++ b/block/blk-mq-tag.c @@ -358,10 +358,19 @@ void blk_mq_tagset_busy_iter(struct

Re: [RFC PATCH v2 2/2] blk-mq: Lockout tagset iter when freeing rqs

2020-12-18 Thread John Garry
On 18/12/2020 01:55, Bart Van Assche wrote: On 12/17/20 3:07 AM, John Garry wrote: diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c index a6df2d5df88a..853ed5b889aa 100644 --- a/block/blk-mq-tag.c +++ b/block/blk-mq-tag.c @@ -358,10 +358,19 @@ void blk_mq_tagset_busy_iter(struct

Re: [RFC PATCH v2 2/2] blk-mq: Lockout tagset iter when freeing rqs

2020-12-17 Thread Ming Lei
On Thu, Dec 17, 2020 at 07:07:53PM +0800, John Garry wrote: > References to old IO sched requests are currently cleared from the > tagset when freeing those requests; switching elevator or changing > request queue depth is such a scenario in which this occurs. > > However, this does not stop the

Re: [RFC PATCH v2 2/2] blk-mq: Lockout tagset iter when freeing rqs

2020-12-17 Thread Bart Van Assche
On 12/17/20 3:07 AM, John Garry wrote: > diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c > index a6df2d5df88a..853ed5b889aa 100644 > --- a/block/blk-mq-tag.c > +++ b/block/blk-mq-tag.c > @@ -358,10 +358,19 @@ void blk_mq_tagset_busy_iter(struct blk_mq_tag_set > *tagset, > { > int i; >

[RFC PATCH v2 2/2] blk-mq: Lockout tagset iter when freeing rqs

2020-12-17 Thread John Garry
References to old IO sched requests are currently cleared from the tagset when freeing those requests; switching elevator or changing request queue depth is such a scenario in which this occurs. However, this does not stop the potentially racy behaviour of freeing and clearing a request reference