From: Ming Lei <ming.lei@redhat.com> To: Bart Van Assche <bvanassche@acm.org> Cc: Jens Axboe <axboe@kernel.dk>, Christoph Hellwig <hch@lst.de>, djeffery@redhat.com, stefanha@redhat.com, linux-block@vger.kernel.org, linux-scsi@vger.kernel.org, virtualization@lists.linux-foundation.org Subject: Re: [Bug] double ->queue_rq() because of timeout in ->queue_rq() Date: Fri, 21 Oct 2022 08:57:44 +0800 [thread overview] Message-ID: <Y1HuiFXyQ1k+OH92@T590> (raw) In-Reply-To: <7d5eae39-3a56-df7d-eb72-3cb910c2b802@acm.org> On Thu, Oct 20, 2022 at 01:26:48PM -0700, Bart Van Assche wrote: > On 10/20/22 02:10, Ming Lei wrote: > > [ ... ] > > Hi Ming, > > Fixing this in the block layer seems fine to me. A few comments: > > > + /* Before walking tags, we must ensure any submit started before the > > + * current time has finished. Since the submit uses srcu or rcu, wait > > + * for a synchronization point to ensure all running submits have > > + * finished > > + */ > > Should the above comment follow the style of other comments in the block > layer? OK. > > > + blk_mq_wait_quiesce_done(q); > > + > > + blk_mq_queue_tag_busy_iter(q, blk_mq_check_expired, &expired); > > The above doesn't look sufficient to me since .queue_rq() may be called > while blk_mq_queue_tag_busy_iter() is in progress. How about moving the > blk_mq_wait_quiesce_done() call into blk_mq_check_expired() and preventing > new .queue_rq() calls before the timeout handler is called? blk_mq_timeout_work() records the time before calling blk_mq_wait_quiesce_done(), and only handle requests which is timed out before the recorded jiffies, so new queued request won't be covered in this time. Thanks, Ming
WARNING: multiple messages have this Message-ID (diff)
From: Ming Lei <ming.lei@redhat.com> To: Bart Van Assche <bvanassche@acm.org> Cc: Jens Axboe <axboe@kernel.dk>, djeffery@redhat.com, linux-scsi@vger.kernel.org, virtualization@lists.linux-foundation.org, linux-block@vger.kernel.org, stefanha@redhat.com, Christoph Hellwig <hch@lst.de> Subject: Re: [Bug] double ->queue_rq() because of timeout in ->queue_rq() Date: Fri, 21 Oct 2022 08:57:44 +0800 [thread overview] Message-ID: <Y1HuiFXyQ1k+OH92@T590> (raw) In-Reply-To: <7d5eae39-3a56-df7d-eb72-3cb910c2b802@acm.org> On Thu, Oct 20, 2022 at 01:26:48PM -0700, Bart Van Assche wrote: > On 10/20/22 02:10, Ming Lei wrote: > > [ ... ] > > Hi Ming, > > Fixing this in the block layer seems fine to me. A few comments: > > > + /* Before walking tags, we must ensure any submit started before the > > + * current time has finished. Since the submit uses srcu or rcu, wait > > + * for a synchronization point to ensure all running submits have > > + * finished > > + */ > > Should the above comment follow the style of other comments in the block > layer? OK. > > > + blk_mq_wait_quiesce_done(q); > > + > > + blk_mq_queue_tag_busy_iter(q, blk_mq_check_expired, &expired); > > The above doesn't look sufficient to me since .queue_rq() may be called > while blk_mq_queue_tag_busy_iter() is in progress. How about moving the > blk_mq_wait_quiesce_done() call into blk_mq_check_expired() and preventing > new .queue_rq() calls before the timeout handler is called? blk_mq_timeout_work() records the time before calling blk_mq_wait_quiesce_done(), and only handle requests which is timed out before the recorded jiffies, so new queued request won't be covered in this time. Thanks, Ming _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
next prev parent reply other threads:[~2022-10-21 0:58 UTC|newest] Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-10-20 9:10 [Bug] double ->queue_rq() because of timeout in ->queue_rq() Ming Lei 2022-10-20 9:10 ` Ming Lei 2022-10-20 20:01 ` Stefan Hajnoczi 2022-10-20 20:01 ` Stefan Hajnoczi 2022-10-21 2:23 ` Ming Lei 2022-10-21 2:23 ` Ming Lei 2022-10-24 15:30 ` Stefan Hajnoczi 2022-10-24 15:30 ` Stefan Hajnoczi 2022-10-24 15:41 ` Ming Lei 2022-10-24 15:41 ` Ming Lei 2022-10-20 20:26 ` Bart Van Assche 2022-10-20 20:26 ` Bart Van Assche 2022-10-21 0:57 ` Ming Lei [this message] 2022-10-21 0:57 ` Ming Lei 2022-10-21 14:32 ` Keith Busch 2022-10-21 15:22 ` Ming Lei 2022-10-21 15:22 ` Ming Lei 2022-10-21 18:33 ` David Jeffery 2022-10-22 4:27 ` Ming Lei 2022-10-22 4:27 ` Ming Lei 2022-10-21 18:21 ` David Jeffery
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=Y1HuiFXyQ1k+OH92@T590 \ --to=ming.lei@redhat.com \ --cc=axboe@kernel.dk \ --cc=bvanassche@acm.org \ --cc=djeffery@redhat.com \ --cc=hch@lst.de \ --cc=linux-block@vger.kernel.org \ --cc=linux-scsi@vger.kernel.org \ --cc=stefanha@redhat.com \ --cc=virtualization@lists.linux-foundation.org \ /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.