Re: [PATCH] block: mq flush: fix race between IPI handler and mq flush worker

2014-05-28 Thread Jens Axboe
On 2014-05-27 23:22, Christoph Hellwig wrote: On Tue, May 27, 2014 at 08:31:18PM -0600, Jens Axboe wrote: Christoph, I'll just run a few tests and then queue it up in the morning. Can you send a properly signed-off patch with a commit message as well? I was writing one up, but I still need the s

Re: [PATCH] block: mq flush: fix race between IPI handler and mq flush worker

2014-05-27 Thread Christoph Hellwig
On Tue, May 27, 2014 at 08:31:18PM -0600, Jens Axboe wrote: > Christoph, I'll just run a few tests and then queue it up in the morning. > Can you send a properly signed-off patch with a commit message as well? I > was writing one up, but I still need the signed-off-by. Attached. >From 125823de3

Re: [PATCH] block: mq flush: fix race between IPI handler and mq flush worker

2014-05-27 Thread Jens Axboe
On 2014-05-27 20:26, Jens Axboe wrote: On 2014-05-27 19:34, Ming Lei wrote: On Wed, May 28, 2014 at 3:35 AM, Jens Axboe wrote: On 05/27/2014 01:21 PM, Christoph Hellwig wrote: On Tue, May 27, 2014 at 01:17:40PM -0600, Jens Axboe wrote: But I think you sent the old one again, not the new vari

Re: [PATCH] block: mq flush: fix race between IPI handler and mq flush worker

2014-05-27 Thread Jens Axboe
On 2014-05-27 19:34, Ming Lei wrote: On Wed, May 28, 2014 at 3:35 AM, Jens Axboe wrote: On 05/27/2014 01:21 PM, Christoph Hellwig wrote: On Tue, May 27, 2014 at 01:17:40PM -0600, Jens Axboe wrote: But I think you sent the old one again, not the new variant :-) Oh well, next try: This look

Re: [PATCH] block: mq flush: fix race between IPI handler and mq flush worker

2014-05-27 Thread Ming Lei
On Wed, May 28, 2014 at 3:35 AM, Jens Axboe wrote: > On 05/27/2014 01:21 PM, Christoph Hellwig wrote: >> On Tue, May 27, 2014 at 01:17:40PM -0600, Jens Axboe wrote: >>> But I think you sent the old one again, not the new variant :-) >> >> Oh well, next try: > > This looks good to me. Was trying to

Re: [PATCH] block: mq flush: fix race between IPI handler and mq flush worker

2014-05-27 Thread Jens Axboe
On 05/27/2014 01:21 PM, Christoph Hellwig wrote: > On Tue, May 27, 2014 at 01:17:40PM -0600, Jens Axboe wrote: >> But I think you sent the old one again, not the new variant :-) > > Oh well, next try: This looks good to me. Was trying to think of ways to reduce that to one list iteration, but I t

Re: [PATCH] block: mq flush: fix race between IPI handler and mq flush worker

2014-05-27 Thread Christoph Hellwig
On Tue, May 27, 2014 at 01:17:40PM -0600, Jens Axboe wrote: > But I think you sent the old one again, not the new variant :-) Oh well, next try: >From ff3685b6052a31fbafc72551cdf7c123cd2b4634 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Wed, 21 May 2014 19:37:11 +0200 Subject: blk-mq: a

Re: [PATCH] block: mq flush: fix race between IPI handler and mq flush worker

2014-05-27 Thread Jens Axboe
On 05/27/2014 01:15 PM, Christoph Hellwig wrote: > On Tue, May 27, 2014 at 12:54:22PM -0600, Jens Axboe wrote: >> Space is plenty big again now... The down side is that the splice trick >> wont work, but if we have more than a few requests on the requeue list, >> we're doing it wrong anyway. > >

Re: [PATCH] block: mq flush: fix race between IPI handler and mq flush worker

2014-05-27 Thread Christoph Hellwig
On Tue, May 27, 2014 at 12:54:22PM -0600, Jens Axboe wrote: > Space is plenty big again now... The down side is that the splice trick > wont work, but if we have more than a few requests on the requeue list, > we're doing it wrong anyway. I don't see a problem with the splice. How about this ve

Re: [PATCH] block: mq flush: fix race between IPI handler and mq flush worker

2014-05-27 Thread Jens Axboe
On 2014-05-27 12:53, Christoph Hellwig wrote: On Tue, May 27, 2014 at 12:38:15PM -0600, Jens Axboe wrote: On 2014-05-27 12:13, Christoph Hellwig wrote: Here is my counter proposal that requeues via two lists and a work struct in the request_queue. I've also tested it with scsi-mq. I like th

Re: [PATCH] block: mq flush: fix race between IPI handler and mq flush worker

2014-05-27 Thread Christoph Hellwig
On Tue, May 27, 2014 at 12:38:15PM -0600, Jens Axboe wrote: > On 2014-05-27 12:13, Christoph Hellwig wrote: >> Here is my counter proposal that requeues via two lists and a work struct >> in the request_queue. I've also tested it with scsi-mq. >> > > I like this, moves the state out of the request

Re: [PATCH] block: mq flush: fix race between IPI handler and mq flush worker

2014-05-27 Thread Jens Axboe
On 2014-05-27 12:13, Christoph Hellwig wrote: Here is my counter proposal that requeues via two lists and a work struct in the request_queue. I've also tested it with scsi-mq. I like this, moves the state out of the request. But how about we consolidate the two requeue requests lists, and ju

Re: [PATCH] block: mq flush: fix race between IPI handler and mq flush worker

2014-05-27 Thread Christoph Hellwig
Here is my counter proposal that requeues via two lists and a work struct in the request_queue. I've also tested it with scsi-mq. >From 95f15eda9a87d8545e1dff1b996d9227dfca1129 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Wed, 21 May 2014 19:37:11 +0200 Subject: blk-mq: add helper to i

Re: [PATCH] block: mq flush: fix race between IPI handler and mq flush worker

2014-05-20 Thread Ming Lei
On Wed, May 21, 2014 at 1:36 PM, Christoph Hellwig wrote: > On Wed, May 21, 2014 at 01:16:14PM +0800, Ming Lei wrote: >> I am wondering if virtio-blk is trivial block driver, :-) > > It's about as simple as it gets. > >> > The scsi-mq work that I plant to submit for the next merge window is >> > t

Re: [PATCH] block: mq flush: fix race between IPI handler and mq flush worker

2014-05-20 Thread Christoph Hellwig
On Wed, May 21, 2014 at 01:16:14PM +0800, Ming Lei wrote: > I am wondering if virtio-blk is trivial block driver, :-) It's about as simple as it gets. > > The scsi-mq work that I plant to submit for the next merge window is > > the prime example. > > It depends if one scsi-mq req has to requeue

Re: [PATCH] block: mq flush: fix race between IPI handler and mq flush worker

2014-05-20 Thread Ming Lei
On Tue, May 20, 2014 at 11:23 PM, Christoph Hellwig wrote: > On Tue, May 20, 2014 at 11:20:25AM +0800, Ming Lei wrote: >> - the conflict on the two structures just happens with flush >> requests because rq->requeue_work is only used to queue >> flush requests > > Once we get non-trivial block driv

Re: [PATCH] block: mq flush: fix race between IPI handler and mq flush worker

2014-05-20 Thread Christoph Hellwig
On Tue, May 20, 2014 at 11:20:25AM +0800, Ming Lei wrote: > - the conflict on the two structures just happens with flush > requests because rq->requeue_work is only used to queue > flush requests Once we get non-trivial block drivers we'll need to be able to requeue arbitrary requests, that's why

Re: [PATCH] block: mq flush: fix race between IPI handler and mq flush worker

2014-05-19 Thread Ming Lei
On Mon, May 19, 2014 at 11:18 PM, Christoph Hellwig wrote: > On Mon, May 19, 2014 at 11:05:50PM +0800, Ming Lei wrote: >> Another simple fix is to disable ipi for flush request, but looks >> this one should be better. > > I think the first thing is to bite the bullet and sort out and document > th

Re: [PATCH] block: mq flush: fix race between IPI handler and mq flush worker

2014-05-19 Thread Ming Lei
On Mon, May 19, 2014 at 11:18 PM, Christoph Hellwig wrote: > On Mon, May 19, 2014 at 11:05:50PM +0800, Ming Lei wrote: >> Another simple fix is to disable ipi for flush request, but looks >> this one should be better. > > I think the first thing is to bite the bullet and sort out and document > th

Re: [PATCH] block: mq flush: fix race between IPI handler and mq flush worker

2014-05-19 Thread Christoph Hellwig
On Mon, May 19, 2014 at 11:05:50PM +0800, Ming Lei wrote: > Another simple fix is to disable ipi for flush request, but looks > this one should be better. I think the first thing is to bite the bullet and sort out and document the various unions in struct request for real. For example the first u

[PATCH] block: mq flush: fix race between IPI handler and mq flush worker

2014-05-19 Thread Ming Lei
In 'struct request', both 'csd' and 'requeue_work' are defined inside one union, unfortunately, the below race can be triggered: - one PREFLUSH request may be queued inside the previous POSTFLUSH request's IPI handler, and both the two requests share one request instance(q-