ocfs2: free up write context when direct IO failed
PATCH 1/1 ocfs2: free up write context when direct IO failed
The write context should also be freed even when direct IO failed. Otherwise a memory leak is introduced and entries remain in oi->ip_unwritten_list causing the following BUG later in unlink path:
ERROR: bug expression: !list_empty(&oi->ip_unwritten_list)
ERROR: Clear inode of 215043, inode has unwritten extents
...
Call Trace: ? __set_current_blocked+0x42/0x68 ocfs2_evict_inode+0x91/0x6a0 [ocfs2] ? bit_waitqueue+0x40/0x33 evict+0xdb/0x1af iput+0x1a2/0x1f7 do_unlinkat+0x194/0x28f SyS_unlinkat+0x1b/0x2f do_syscall_64+0x79/0x1ae entry_SYSCALL_64_after_hwframe+0x151/0x0
--- fs/ocfs2/aops.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
fs/ocfs2/aops.c +9 -2
@@ -2412,8 +2412,15 @@ static int ocfs2_dio_end_io(struct kiocb *iocb,/* this io's submitter should not have unlocked this before we could */ BUG_ON (!ocfs2_iocb_is_rw_locked (iocb)); -if (bytes >0 && private) - ret =ocfs2_dio_end_io_write (inode, private, offset, bytes); +if (private) { +if (bytes >0 ) { + ret =ocfs2_dio_end_io_write (inode, private, offset, + bytes); + }else { +mlog (ML_ERROR ,"Direct IO failed, free write ctx" ); +ocfs2_dio_free_write_ctx (inode, private); + } + }ocfs2_iocb_clear_rw_locked (iocb);
Signature · 1 line
2.9.5
PATCH 1/1 ocfs2: free up write context when direct IO failed
Hello Wengang,
The fix looks sane to me.
Quoted 18 lines · Wengang Wang
The write context should also be freed even when direct IO failed. Otherwise a memory leak is introduced and entries remain in
oi->ip_unwritten_listcausing the following BUG later in unlink path:ERROR: bug expression: !list_empty(&oi->ip_unwritten_list)
ERROR: Clear inode of 215043, inode has unwritten extents
...Call Trace: ? __set_current_blocked+0x42/0x68 ocfs2_evict_inode+0x91/0x6a0 [ocfs2] ? bit_waitqueue+0x40/0x33 evict+0xdb/0x1af iput+0x1a2/0x1f7 do_unlinkat+0x194/0x28f SyS_unlinkat+0x1b/0x2f do_syscall_64+0x79/0x1ae entry_SYSCALL_64_after_hwframe+0x151/0x0
Quoted 1 line
Signed-off-by Wengang Wang <[email protected]>
Quoted 27 lines
--- fs/ocfs2/aops.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)fs/ocfs2/aops.c +9
-2@@ -2412,8 +2412,15 @@ static int ocfs2_dio_end_io(struct kiocb *iocb,/* this io's submitter should not have unlocked this before we could */ BUG_ON (!ocfs2_iocb_is_rw_locked (iocb)); -if (bytes >0 && private) - ret =ocfs2_dio_end_io_write (inode, private, offset, bytes); +if (private) { +if (bytes >0 ) { + ret =ocfs2_dio_end_io_write (inode, private, offset, + bytes); + }else { +mlog (ML_ERROR ,"Direct IO failed, free write ctx" ); +ocfs2_dio_free_write_ctx (inode, private); + } + }ocfs2_iocb_clear_rw_locked (iocb);
PATCH 1/1 ocfs2: free up write context when direct IO failed
In reply to Wengang WangQuoted 18 lines · Wengang Wang
The write context should also be freed even when direct IO failed. Otherwise a memory leak is introduced and entries remain in
oi->ip_unwritten_listcausing the following BUG later in unlink path:ERROR: bug expression: !list_empty(&oi->ip_unwritten_list)
ERROR: Clear inode of 215043, inode has unwritten extents
...Call Trace: ? __set_current_blocked+0x42/0x68 ocfs2_evict_inode+0x91/0x6a0 [ocfs2] ? bit_waitqueue+0x40/0x33 evict+0xdb/0x1af iput+0x1a2/0x1f7 do_unlinkat+0x194/0x28f SyS_unlinkat+0x1b/0x2f do_syscall_64+0x79/0x1ae entry_SYSCALL_64_after_hwframe+0x151/0x0
Quoted 1 line
Signed-off-by Wengang Wang <[email protected]>
Looks good.
We'd better cc stable as well (4.10+ ?).
Quoted 27 lines
--- fs/ocfs2/aops.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)fs/ocfs2/aops.c +9
-2@@ -2412,8 +2412,15 @@ static int ocfs2_dio_end_io(struct kiocb *iocb,/* this io's submitter should not have unlocked this before we could */ BUG_ON (!ocfs2_iocb_is_rw_locked (iocb)); -if (bytes >0 && private) - ret =ocfs2_dio_end_io_write (inode, private, offset, bytes); +if (private) { +if (bytes >0 ) { + ret =ocfs2_dio_end_io_write (inode, private, offset, + bytes); + }else { +mlog (ML_ERROR ,"Direct IO failed, free write ctx" ); +ocfs2_dio_free_write_ctx (inode, private); + } + }ocfs2_iocb_clear_rw_locked (iocb);
PATCH 1/1 ocfs2: free up write context when direct IO failed
In reply to Wengang WangQuoted 29 lines · Wengang Wang
The write context should also be freed even when direct IO failed. Otherwise a memory leak is introduced and entries remain in
oi->ip_unwritten_listcausing the following BUG later in unlink path:ERROR: bug expression: !list_empty(&oi->ip_unwritten_list)
ERROR: Clear inode of 215043, inode has unwritten extents
...Call Trace: ? __set_current_blocked+0x42/0x68 ocfs2_evict_inode+0x91/0x6a0 [ocfs2] ? bit_waitqueue+0x40/0x33 evict+0xdb/0x1af iput+0x1a2/0x1f7 do_unlinkat+0x194/0x28f SyS_unlinkat+0x1b/0x2f do_syscall_64+0x79/0x1ae entry_SYSCALL_64_after_hwframe+0x151/0x0Signed-off-by Wengang Wang <[email protected]>--- fs/ocfs2/aops.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)fs/ocfs2/aops.c +0
-0@@ -2412,8 +2412,15 @@ static int ocfs2_dio_end_io(struct kiocb *iocb,/* this io's submitter should not have unlocked this before we could */
fs/ocfs2/aops.c static int ocfs2_dio_end_io(struct kiocb *iocb, · line 2420
Quoted 10 lines
fs/ocfs2/aops.c +6
-2BUG_ON (!ocfs2_iocb_is_rw_locked (iocb)); -if (bytes >0 && private) - ret =ocfs2_dio_end_io_write (inode, private, offset, bytes); +if (private) { +if (bytes >0 ) { + ret =ocfs2_dio_end_io_write (inode, private, offset, + bytes); + }else { +mlog (ML_ERROR ,"Direct IO failed, free write ctx" );
Is it useful to leave the "bytes" value here? It could be some error code.
Thanks,
Junxiao.
Quoted 6 lines
fs/ocfs2/aops.c +3
-0+ocfs2_dio_free_write_ctx (inode, private); + } + }ocfs2_iocb_clear_rw_locked (iocb);
PATCH 1/1 ocfs2: free up write context when direct IO failed
Quoted 35 lines · Junxiao Bi
Quoted 33 lines · Wengang Wang
The write context should also be freed even when direct IO failed. Otherwise a memory leak is introduced and entries remain in
oi->ip_unwritten_listcausing the following BUG later in unlink path:ERROR: bug expression: !list_empty(&oi->ip_unwritten_list)
ERROR: Clear inode of 215043, inode has unwritten extents
...Call Trace: ? __set_current_blocked+0x42/0x68 ocfs2_evict_inode+0x91/0x6a0 [ocfs2] ? bit_waitqueue+0x40/0x33 evict+0xdb/0x1af iput+0x1a2/0x1f7 do_unlinkat+0x194/0x28f SyS_unlinkat+0x1b/0x2f do_syscall_64+0x79/0x1ae entry_SYSCALL_64_after_hwframe+0x151/0x0Signed-off-by Wengang Wang <[email protected]>---
?fs/ocfs2/aops.c| 11 +++++++++--
? 1 file changed, 9 insertions(+), 2 deletions(-)fs/ocfs2/aops.c +0
-0@@ -2412,8 +2412,15 @@ static int ocfs2_dio_end_io(struct kiocb *iocb,????? /* this io's submitter should not have unlocked this before we could */
????? BUG_ON(!ocfs2_iocb_is_rw_locked(iocb));
? -??? if (bytes > 0 && private)
-??????? ret =ocfs2_dio_end_io_write(inode, private, offset, bytes);
Quoted 9 lines
Quoted 6 lines
+??? if (private) {
+??????? if (bytes > 0) {
+??????????? ret =ocfs2_dio_end_io_write(inode, private, offset, +???????????????????????????? bytes);
+??????? } else {
+???????????mlog(ML_ERROR, "Direct IO failed, free write ctx");Is it useful to leave the "bytes" value here? It could be some error code.
That would be error come from below file system, but we can log it. Will send v2 to add "bytes".
thanks,
wengang
Quoted 7 lines
Thanks,
Junxiao.Quoted 4 lines
+
ocfs2_dio_free_write_ctx(inode, private);
+??????? }
+??? }
? ?????ocfs2_iocb_clear_rw_locked(iocb);