[PATCH] aio: aio_complete() will never be called in interrupt context

[PATCH] aio: aio_complete() will never be called in interrupt context

[Nikanth Karthikesan]

aio_complete() is never called from interrupt context. It is called from user 
context or worker threads. Remove disabling interrupts and for kmap_atomic use 
KM_USER slots instead of KM_IRQ slots.

Signed-off-by: Nikanth Karthikesan <knikanth@suse.de>

---

diff --git a/fs/aio.c b/fs/aio.c
index 7817e8f..27b7181 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -893,7 +893,6 @@ int aio_complete(struct kiocb *iocb, long res, long res2)
 	struct aio_ring_info	*info;
 	struct aio_ring	*ring;
 	struct io_event	*event;
-	unsigned long	flags;
 	unsigned long	tail;
 	int		ret;
 
@@ -916,11 +915,9 @@ int aio_complete(struct kiocb *iocb, long res, long res2)
 
 	/* add a completion event to the ring buffer.
 	 * must be done holding ctx->ctx_lock to prevent
-	 * other code from messing with the tail
-	 * pointer since we might be called from irq
-	 * context.
+	 * other code from messing with the tail pointer.
 	 */
-	spin_lock_irqsave(&ctx->ctx_lock, flags);
+	spin_lock(&ctx->ctx_lock);
 
 	if (iocb->ki_run_list.prev && !list_empty(&iocb->ki_run_list))
 		list_del_init(&iocb->ki_run_list);
@@ -932,10 +929,10 @@ int aio_complete(struct kiocb *iocb, long res, long 
res2)
 	if (kiocbIsCancelled(iocb))
 		goto put_rq;
 
-	ring = kmap_atomic(info->ring_pages[0], KM_IRQ1);
+	ring = kmap_atomic(info->ring_pages[0], KM_USER1);
 
 	tail = info->tail;
-	event = aio_ring_event(info, tail, KM_IRQ0);
+	event = aio_ring_event(info, tail, KM_USER0);
 	if (++tail >= info->nr)
 		tail = 0;
 
@@ -956,8 +953,8 @@ int aio_complete(struct kiocb *iocb, long res, long res2)
 	info->tail = tail;
 	ring->tail = tail;
 
-	put_aio_ring_event(event, KM_IRQ0);
-	kunmap_atomic(ring, KM_IRQ1);
+	put_aio_ring_event(event, KM_USER0);
+	kunmap_atomic(ring, KM_USER1);
 
 	pr_debug("added to ring %p at [%lu]\n", iocb, tail);
 
@@ -984,7 +981,7 @@ put_rq:
 	if (waitqueue_active(&ctx->wait))
 		wake_up(&ctx->wait);
 
-	spin_unlock_irqrestore(&ctx->ctx_lock, flags);
+	spin_unlock(&ctx->ctx_lock);
 	return ret;
 }
 

Re: [PATCH] aio: aio_complete() will never be called in interrupt context

[Jeff Moyer]

Nikanth Karthikesan <knikanth@suse.de> writes:

> aio_complete() is never called from interrupt context. It is called from user 
> context or worker threads. Remove disabling interrupts and for kmap_atomic use 
> KM_USER slots instead of KM_IRQ slots.

Actually, it can be called from softirq context.  See fs/directio.c.

Cheers,

Jeff

Re: [PATCH] aio: aio_complete() will never be called in interrupt context

[Nikanth Karthikesan]

On Friday 27 June 2008 18:41:03 Jeff Moyer wrote:
> Nikanth Karthikesan <knikanth@suse.de> writes:
> > aio_complete() is never called from interrupt context. It is called from
> > user context or worker threads. Remove disabling interrupts and for
> > kmap_atomic use KM_USER slots instead of KM_IRQ slots.
>
> Actually, it can be called from softirq context.  See fs/directio.c.
>

Oh, sorry for my ignorance. Will using KM_SOFTIRQ slots be fine?

Thanks
Nikanth Karthikesan

aio_complete() is never called from hardware interrupt context. Remove 
disabling interrupts and for kmap_atomic use KM_SOFTIRQ slots instead of 
KM_IRQ slots.

Signed-off-by: Nikanth Karthikesan <knikanth@suse.de>

---

Index: linux-2.6/fs/aio.c
===================================================================
--- linux-2.6.orig/fs/aio.c
+++ linux-2.6/fs/aio.c
@@ -901,7 +901,6 @@ int aio_complete(struct kiocb *iocb, lon
 	struct aio_ring_info	*info;
 	struct aio_ring	*ring;
 	struct io_event	*event;
-	unsigned long	flags;
 	unsigned long	tail;
 	int		ret;
 
@@ -924,11 +923,9 @@ int aio_complete(struct kiocb *iocb, lon
 
 	/* add a completion event to the ring buffer.
 	 * must be done holding ctx->ctx_lock to prevent
-	 * other code from messing with the tail
-	 * pointer since we might be called from irq
-	 * context.
+	 * other code from messing with the tail pointer.
 	 */
-	spin_lock_irqsave(&ctx->ctx_lock, flags);
+	spin_lock(&ctx->ctx_lock);
 
 	if (iocb->ki_run_list.prev && !list_empty(&iocb->ki_run_list))
 		list_del_init(&iocb->ki_run_list);
@@ -940,10 +937,10 @@ int aio_complete(struct kiocb *iocb, lon
 	if (kiocbIsCancelled(iocb))
 		goto put_rq;
 
-	ring = kmap_atomic(info->ring_pages[0], KM_IRQ1);
+	ring = kmap_atomic(info->ring_pages[0], KM_SOFTIRQ1);
 
 	tail = info->tail;
-	event = aio_ring_event(info, tail, KM_IRQ0);
+	event = aio_ring_event(info, tail, KM_SOFTIRQ0);
 	if (++tail >= info->nr)
 		tail = 0;
 
@@ -964,8 +961,8 @@ int aio_complete(struct kiocb *iocb, lon
 	info->tail = tail;
 	ring->tail = tail;
 
-	put_aio_ring_event(event, KM_IRQ0);
-	kunmap_atomic(ring, KM_IRQ1);
+	put_aio_ring_event(event, KM_SOFTIRQ0);
+	kunmap_atomic(ring, KM_SOFTIRQ1);
 
 	pr_debug("added to ring %p at [%lu]\n", iocb, tail);
 
@@ -992,7 +989,7 @@ put_rq:
 	if (waitqueue_active(&ctx->wait))
 		wake_up(&ctx->wait);
 
-	spin_unlock_irqrestore(&ctx->ctx_lock, flags);
+	spin_unlock(&ctx->ctx_lock);
 	return ret;
 }
 

Re: [PATCH] aio: aio_complete() will never be called in interrupt context

[Zach Brown]

Nikanth Karthikesan wrote:
> aio_complete() is never called from interrupt context. It is called from user 
> context or worker threads. Remove disabling interrupts and for kmap_atomic use 
> KM_USER slots instead of KM_IRQ slots.

How are you testing these patches?

- z

Re: [PATCH] aio: aio_complete() will never be called in interrupt context

[Nikanth Karthikesan]

On Monday 30 June 2008 23:13:26 Zach Brown wrote:
> Nikanth Karthikesan wrote:
> > aio_complete() is never called from interrupt context. It is called from
> > user context or worker threads. Remove disabling interrupts and for
> > kmap_atomic use KM_USER slots instead of KM_IRQ slots.
>
> How are you testing these patches?
>

I did not do any specific testing. Just ran multiple instances of a file-copy 
utility that uses aio and a bit of Chris Mason's aio-stress.

Thanks
Nikanth karthikesan

p.s: Is the archives of the linux-aio list available somewhere? The URL in the 
mail footer seems to be unavailable.

Re: [PATCH] aio: aio_complete() will never be called in interrupt context

[Zach Brown]

> I did not do any specific testing. Just ran multiple instances of a file-copy 
> utility that uses aio and a bit of Chris Mason's aio-stress.

Are you running these tests under a kernel with lockdep enabled?

> p.s: Is the archives of the linux-aio list available somewhere? The URL in the 
> mail footer seems to be unavailable.

The very first google hit for 'linux-aio mailing list archive' works:

  http://marc.info/?l=linux-aio

- z

Re: [PATCH] aio: aio_complete() will never be called in interrupt context

[Nikanth Karthikesan]

On Wednesday 02 July 2008 01:20:42 Zach Brown wrote:
> > I did not do any specific testing. Just ran multiple instances of a
> > file-copy utility that uses aio and a bit of Chris Mason's aio-stress.
>
> Are you running these tests under a kernel with lockdep enabled?
>

Yes. I have CONFIG_LOCKDEP_SUPPORT=y and did not get any warning messages.

> > p.s: Is the archives of the linux-aio list available somewhere? The URL
> > in the mail footer seems to be unavailable.
>
> The very first google hit for 'linux-aio mailing list archive' works:
>
>   http://marc.info/?l=linux-aio
>

Thanks and sorry for the noise.

But it would be nice if the link in the mail footer(www.kvack.org/aio) is 
changed to an active link or removed. The link redirects to 
www.kvack.org/~blah/aio/ which returns 404

Thanks
Nikanth Karthikesan

Re: [PATCH] aio: aio_complete() will never be called in interrupt context

[Zach Brown]

> Yes. I have CONFIG_LOCKDEP_SUPPORT=y and did not get any warning messages.

You should mention details like this in the patch descriptions.

> But it would be nice if the link in the mail footer(www.kvack.org/aio) is 
> changed to an active link or removed. The link redirects to 
> www.kvack.org/~blah/aio/ which returns 404

Yeah, it's perhaps not the best that this is still hosted over on Ben's
personal boxes.  I wonder if we should move to linux-aio@vger..

- z

Re: [PATCH] aio: aio_complete() will never be called in interrupt context

[Benjamin LaHaise]

On Wed, Jul 02, 2008 at 09:38:25AM -0700, Zach Brown wrote:
> Yeah, it's perhaps not the best that this is still hosted over on Ben's
> personal boxes.  I wonder if we should move to linux-aio@vger..

Just tell me where to point it to...

		-ben
-- 
"Time is of no importance, Mr. President, only life is important."
Don't Email: <zyntrop@kvack.org>.