V4 changes:
add Acked-by: Joel Becker <[email protected]>
V3 changes:
- add Cc: [email protected] in the patch header to align with stable rules
- add Acked-by: Jeff Moyer <[email protected]>
V2 changes:
- update the patch header of the first patch to make it more clear.
This patch list fixes an issue about ocfs2 aio/dio write process hang.
The call trace is like this:
@ cat /proc/15913/stack
@ [<ffffffffa06e1085>] ocfs2_aiodio_wait+0x85/0xc0 [ocfs2]
@ [<ffffffffa06e3e39>] ocfs2_file_aio_write+0x99/0xa0 [ocfs2]
@ [<ffffffff811ace97>] aio_rw_vect_retry+0x87/0x150
@ [<ffffffff811aebd1>] aio_run_iocb+0x71/0x170
@ [<ffffffff811af59b>] io_submit_one+0x1ab/0x280
@ [<ffffffff811af77c>] do_io_submit+0x10c/0x1c0
@ [<ffffffff811af840>] sys_io_submit+0x10/0x20
@ [<ffffffff81509b42>] system_call_fastpath+0x16/0x1b
@ [<ffffffffffffffff>] 0xffffffffffffffff
Ocfs2 uses kiocb.*private as a flag of unsigned long size. In
commit a11f7e6 ocfs2: serialize unaligned aio, the unaligned
io flag is involved in it to serialize the unaligned aio. As
*private is not initialized in init_sync_kiocb() of do_sync_write(),
this unaligned io flag may be unexpectly set in an aligned dio.
And this will cause OCFS2_I(inode)->ip_unaligned_aio decreased
to -1 in ocfs2_dio_end_io(), thus the following unaligned dio
will hang forever at ocfs2_aiodio_wait() in ocfs2_file_aio_write().
Signed-off-by: Junxiao Bi <[email protected]>
Cc: [email protected]
Acked-by: Jeff Moyer <[email protected]>
Acked-by: Joel Becker <[email protected]>
---
include/linux/aio.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/linux/aio.h b/include/linux/aio.h
index 2314ad8..b1a520e 100644
--- a/include/linux/aio.h
+++ b/include/linux/aio.h
@@ -140,6 +140,7 @@ struct kiocb {
(x)->ki_dtor = NULL; \
(x)->ki_obj.tsk = tsk; \
(x)->ki_user_data = 0; \
+ (x)->private = NULL; \
} while (0)
#define AIO_RING_MAGIC 0xa10a10a1
--
1.7.9.5
The unaligned io flag is set in the kiocb when an unaligned
dio is issued, it should be cleared even when the dio fails,
or it may affect the following io which are using the same
kiocb.
Signed-off-by: Junxiao Bi <[email protected]>
Cc: [email protected]
---
fs/ocfs2/file.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
index 061591a..98513c8 100644
--- a/fs/ocfs2/file.c
+++ b/fs/ocfs2/file.c
@@ -2422,8 +2422,10 @@ out_dio:
unaligned_dio = 0;
}
- if (unaligned_dio)
+ if (unaligned_dio) {
+ ocfs2_iocb_clear_unaligned_aio(iocb);
atomic_dec(&OCFS2_I(inode)->ip_unaligned_aio);
+ }
out:
if (rw_level != -1)
--
1.7.9.5
Btw, do you want me to pull these through the ocfs2 tree, or are you
sending them along yourself?
Joel
On Wed, Jun 27, 2012 at 05:09:53PM +0800, Junxiao Bi wrote:
>
> V4 changes:
> add Acked-by: Joel Becker <[email protected]>
>
> V3 changes:
> - add Cc: [email protected] in the patch header to align with stable rules
> - add Acked-by: Jeff Moyer <[email protected]>
>
> V2 changes:
> - update the patch header of the first patch to make it more clear.
>
>
> This patch list fixes an issue about ocfs2 aio/dio write process hang.
> The call trace is like this:
> @ cat /proc/15913/stack
> @ [<ffffffffa06e1085>] ocfs2_aiodio_wait+0x85/0xc0 [ocfs2]
> @ [<ffffffffa06e3e39>] ocfs2_file_aio_write+0x99/0xa0 [ocfs2]
> @ [<ffffffff811ace97>] aio_rw_vect_retry+0x87/0x150
> @ [<ffffffff811aebd1>] aio_run_iocb+0x71/0x170
> @ [<ffffffff811af59b>] io_submit_one+0x1ab/0x280
> @ [<ffffffff811af77c>] do_io_submit+0x10c/0x1c0
> @ [<ffffffff811af840>] sys_io_submit+0x10/0x20
> @ [<ffffffff81509b42>] system_call_fastpath+0x16/0x1b
> @ [<ffffffffffffffff>] 0xffffffffffffffff
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
Joel's Second Law:
If a code change requires additional user setup, it is wrong.
http://www.jlbec.org/
[email protected]
On 06/28/2012 04:34 PM, Joel Becker wrote:
> Btw, do you want me to pull these through the ocfs2 tree, or are you
> sending them along yourself?
Please help pull these, great thanks.
>
> Joel
>
> On Wed, Jun 27, 2012 at 05:09:53PM +0800, Junxiao Bi wrote:
>> V4 changes:
>> add Acked-by: Joel Becker <[email protected]>
>>
>> V3 changes:
>> - add Cc: [email protected] in the patch header to align with stable rules
>> - add Acked-by: Jeff Moyer <[email protected]>
>>
>> V2 changes:
>> - update the patch header of the first patch to make it more clear.
>>
>>
>> This patch list fixes an issue about ocfs2 aio/dio write process hang.
>> The call trace is like this:
>> @ cat /proc/15913/stack
>> @ [<ffffffffa06e1085>] ocfs2_aiodio_wait+0x85/0xc0 [ocfs2]
>> @ [<ffffffffa06e3e39>] ocfs2_file_aio_write+0x99/0xa0 [ocfs2]
>> @ [<ffffffff811ace97>] aio_rw_vect_retry+0x87/0x150
>> @ [<ffffffff811aebd1>] aio_run_iocb+0x71/0x170
>> @ [<ffffffff811af59b>] io_submit_one+0x1ab/0x280
>> @ [<ffffffff811af77c>] do_io_submit+0x10c/0x1c0
>> @ [<ffffffff811af840>] sys_io_submit+0x10/0x20
>> @ [<ffffffff81509b42>] system_call_fastpath+0x16/0x1b
>> @ [<ffffffffffffffff>] 0xffffffffffffffff
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at http://www.tux.org/lkml/
On Wed, 27 Jun 2012 17:09:54 +0800
Junxiao Bi <[email protected]> wrote:
> Ocfs2 uses kiocb.*private as a flag of unsigned long size. In
> commit a11f7e6 ocfs2: serialize unaligned aio, the unaligned
> io flag is involved in it to serialize the unaligned aio. As
> *private is not initialized in init_sync_kiocb() of do_sync_write(),
> this unaligned io flag may be unexpectly set in an aligned dio.
> And this will cause OCFS2_I(inode)->ip_unaligned_aio decreased
> to -1 in ocfs2_dio_end_io(), thus the following unaligned dio
> will hang forever at ocfs2_aiodio_wait() in ocfs2_file_aio_write().
>
> Signed-off-by: Junxiao Bi <[email protected]>
> Cc: [email protected]
> Acked-by: Jeff Moyer <[email protected]>
> Acked-by: Joel Becker <[email protected]>
> ---
> include/linux/aio.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/include/linux/aio.h b/include/linux/aio.h
> index 2314ad8..b1a520e 100644
> --- a/include/linux/aio.h
> +++ b/include/linux/aio.h
> @@ -140,6 +140,7 @@ struct kiocb {
> (x)->ki_dtor = NULL; \
> (x)->ki_obj.tsk = tsk; \
> (x)->ki_user_data = 0; \
> + (x)->private = NULL; \
> } while (0)
>
> #define AIO_RING_MAGIC 0xa10a10a1
hm, that code is rather cruddy. Pointless macromania.
If we do this:
static inline void init_sync_kiocb(struct kiocb *kiocb, struct file *filp)
{
struct task_struct *tsk = current;
kiocb->ki_flags = 0;
kiocb->ki_users = 1;
kiocb->ki_key = KIOCB_SYNC_KEY;
kiocb->ki_filp = filp;
kiocb->ki_ctx = NULL;
kiocb->ki_cancel = NULL;
kiocb->ki_retry = NULL;
kiocb->ki_dtor = NULL;
kiocb->ki_obj.tsk = tsk;
kiocb->ki_user_data = 0;
}
it is nicer and there is no impact on code size.
If we do this:
static inline void init_sync_kiocb(struct kiocb *kiocb, struct file *filp)
{
*kiocb = (struct kiocb) {
.ki_users = 1,
.ki_key = KIOCB_SYNC_KEY,
.ki_filp = filp,
.ki_obj.tsk = current,
};
}
then fs/read_write.o's .text is shrunk from 9857 bytes to 9714, which
is rather a lot.
But that's all rather irrelevant to your bugfix.
On Thu, Jun 28, 2012 at 03:39:56PM -0700, Andrew Morton wrote:
> On Wed, 27 Jun 2012 17:09:54 +0800
> Junxiao Bi <[email protected]> wrote:
>
> > Ocfs2 uses kiocb.*private as a flag of unsigned long size. In
> > commit a11f7e6 ocfs2: serialize unaligned aio, the unaligned
> > io flag is involved in it to serialize the unaligned aio. As
> > *private is not initialized in init_sync_kiocb() of do_sync_write(),
> > this unaligned io flag may be unexpectly set in an aligned dio.
> > And this will cause OCFS2_I(inode)->ip_unaligned_aio decreased
> > to -1 in ocfs2_dio_end_io(), thus the following unaligned dio
> > will hang forever at ocfs2_aiodio_wait() in ocfs2_file_aio_write().
> >
> > Signed-off-by: Junxiao Bi <[email protected]>
> > Cc: [email protected]
> > Acked-by: Jeff Moyer <[email protected]>
> > Acked-by: Joel Becker <[email protected]>
> > ---
> > include/linux/aio.h | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/include/linux/aio.h b/include/linux/aio.h
> > index 2314ad8..b1a520e 100644
> > --- a/include/linux/aio.h
> > +++ b/include/linux/aio.h
> > @@ -140,6 +140,7 @@ struct kiocb {
> > (x)->ki_dtor = NULL; \
> > (x)->ki_obj.tsk = tsk; \
> > (x)->ki_user_data = 0; \
> > + (x)->private = NULL; \
> > } while (0)
> >
> > #define AIO_RING_MAGIC 0xa10a10a1
>
> hm, that code is rather cruddy. Pointless macromania.
Agreed.
> If we do this:
>
> static inline void init_sync_kiocb(struct kiocb *kiocb, struct file *filp)
> {
> *kiocb = (struct kiocb) {
> .ki_users = 1,
> .ki_key = KIOCB_SYNC_KEY,
> .ki_filp = filp,
> .ki_obj.tsk = current,
> };
> }
>
> then fs/read_write.o's .text is shrunk from 9857 bytes to 9714, which
> is rather a lot.
>
> But that's all rather irrelevant to your bugfix.
I like your solution. Junxiao, if you send me that version, I'd
be happy to take it.
Joel
--
"I'm drifting and drifting
Just like a ship out on the sea.
Cause I ain't got nobody, baby,
In this world to care for me."
http://www.jlbec.org/
[email protected]
On 06/29/2012 05:22 PM, Joel Becker wrote:
> On Thu, Jun 28, 2012 at 03:39:56PM -0700, Andrew Morton wrote:
>> On Wed, 27 Jun 2012 17:09:54 +0800
>> Junxiao Bi <[email protected]> wrote:
>>
>>> Ocfs2 uses kiocb.*private as a flag of unsigned long size. In
>>> commit a11f7e6 ocfs2: serialize unaligned aio, the unaligned
>>> io flag is involved in it to serialize the unaligned aio. As
>>> *private is not initialized in init_sync_kiocb() of do_sync_write(),
>>> this unaligned io flag may be unexpectly set in an aligned dio.
>>> And this will cause OCFS2_I(inode)->ip_unaligned_aio decreased
>>> to -1 in ocfs2_dio_end_io(), thus the following unaligned dio
>>> will hang forever at ocfs2_aiodio_wait() in ocfs2_file_aio_write().
>>>
>>> Signed-off-by: Junxiao Bi <[email protected]>
>>> Cc: [email protected]
>>> Acked-by: Jeff Moyer <[email protected]>
>>> Acked-by: Joel Becker <[email protected]>
>>> ---
>>> include/linux/aio.h | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/include/linux/aio.h b/include/linux/aio.h
>>> index 2314ad8..b1a520e 100644
>>> --- a/include/linux/aio.h
>>> +++ b/include/linux/aio.h
>>> @@ -140,6 +140,7 @@ struct kiocb {
>>> (x)->ki_dtor = NULL; \
>>> (x)->ki_obj.tsk = tsk; \
>>> (x)->ki_user_data = 0; \
>>> + (x)->private = NULL; \
>>> } while (0)
>>>
>>> #define AIO_RING_MAGIC 0xa10a10a1
>> hm, that code is rather cruddy. Pointless macromania.
> Agreed.
>
>> If we do this:
>>
>> static inline void init_sync_kiocb(struct kiocb *kiocb, struct file *filp)
>> {
>> *kiocb = (struct kiocb) {
>> .ki_users = 1,
>> .ki_key = KIOCB_SYNC_KEY,
>> .ki_filp = filp,
>> .ki_obj.tsk = current,
>> };
>> }
>>
>> then fs/read_write.o's .text is shrunk from 9857 bytes to 9714, which
>> is rather a lot.
>>
>> But that's all rather irrelevant to your bugfix.
> I like your solution. Junxiao, if you send me that version, I'd
> be happy to take it.
Hi Joel,
Andrew had merged this patch to his tree. Do you like the second patch "
ocfs2: clear unaligned io flag when dio fails"?
> Joel
>
On Fri, 29 Jun 2012 18:17:11 +0800 Junxiao Bi <[email protected]> wrote:
> Andrew had merged this patch to his tree. Do you like the second patch "
> ocfs2: clear unaligned io flag when dio fails"?
Please go ahead with your patchset for now. I'll feed the cleanup in
later on.
On 06/29/2012 06:29 PM, Andrew Morton wrote:
> On Fri, 29 Jun 2012 18:17:11 +0800 Junxiao Bi <[email protected]> wrote:
>
>> Andrew had merged this patch to his tree. Do you like the second patch "
>> ocfs2: clear unaligned io flag when dio fails"?
> Please go ahead with your patchset for now. I'll feed the cleanup in
> later on.
Ok, thank you.
On 06/29/2012 05:22 PM, Joel Becker wrote:
> On Thu, Jun 28, 2012 at 03:39:56PM -0700, Andrew Morton wrote:
>> On Wed, 27 Jun 2012 17:09:54 +0800
>> Junxiao Bi <[email protected]> wrote:
>>
>>> Ocfs2 uses kiocb.*private as a flag of unsigned long size. In
>>> commit a11f7e6 ocfs2: serialize unaligned aio, the unaligned
>>> io flag is involved in it to serialize the unaligned aio. As
>>> *private is not initialized in init_sync_kiocb() of do_sync_write(),
>>> this unaligned io flag may be unexpectly set in an aligned dio.
>>> And this will cause OCFS2_I(inode)->ip_unaligned_aio decreased
>>> to -1 in ocfs2_dio_end_io(), thus the following unaligned dio
>>> will hang forever at ocfs2_aiodio_wait() in ocfs2_file_aio_write().
>>>
>>> Signed-off-by: Junxiao Bi <[email protected]>
>>> Cc: [email protected]
>>> Acked-by: Jeff Moyer <[email protected]>
>>> Acked-by: Joel Becker <[email protected]>
>>> ---
>>> include/linux/aio.h | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/include/linux/aio.h b/include/linux/aio.h
>>> index 2314ad8..b1a520e 100644
>>> --- a/include/linux/aio.h
>>> +++ b/include/linux/aio.h
>>> @@ -140,6 +140,7 @@ struct kiocb {
>>> (x)->ki_dtor = NULL; \
>>> (x)->ki_obj.tsk = tsk; \
>>> (x)->ki_user_data = 0; \
>>> + (x)->private = NULL; \
>>> } while (0)
>>>
>>> #define AIO_RING_MAGIC 0xa10a10a1
>> hm, that code is rather cruddy. Pointless macromania.
> Agreed.
>
>> If we do this:
>>
>> static inline void init_sync_kiocb(struct kiocb *kiocb, struct file *filp)
>> {
>> *kiocb = (struct kiocb) {
>> .ki_users = 1,
>> .ki_key = KIOCB_SYNC_KEY,
>> .ki_filp = filp,
>> .ki_obj.tsk = current,
>> };
>> }
>>
>> then fs/read_write.o's .text is shrunk from 9857 bytes to 9714, which
>> is rather a lot.
>>
>> But that's all rather irrelevant to your bugfix.
> I like your solution. Junxiao, if you send me that version, I'd
> be happy to take it.
Joel, since Andrew will feed his cleanup patch later on. Please help
merge my patches. Though it is covered by the cleanup patch, but I think
it's useful to merge it, at least we can see from the git log this is
for an ocfs2 hang bug.
>
> Joel
>