2008-06-26 11:07:21

by Nikanth Karthikesan

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


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 <[email protected]>

---

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;
}


2008-06-27 13:12:12

by Jeff Moyer

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

Nikanth Karthikesan <[email protected]> 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

2008-06-30 05:46:56

by Nikanth Karthikesan

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

On Friday 27 June 2008 18:41:03 Jeff Moyer wrote:
> Nikanth Karthikesan <[email protected]> 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 <[email protected]>

---

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;
}




2008-06-30 17:43:38

by Zach Brown

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

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

2008-07-01 08:26:00

by Nikanth Karthikesan

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

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.




2008-07-01 19:50:53

by Zach Brown

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


> 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

2008-07-02 04:56:43

by Nikanth Karthikesan

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

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(http://www.kvack.org/aio) is
changed to an active link or removed. The link redirects to
http://www.kvack.org/~blah/aio/ which returns 404

Thanks
Nikanth Karthikesan

2008-07-02 16:38:34

by Zach Brown

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


> 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(http://www.kvack.org/aio) is
> changed to an active link or removed. The link redirects to
> http://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

2008-07-02 18:15:09

by Benjamin LaHaise

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

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: <[email protected]>.