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