Wengang Wang
2018-Nov-01 21:15 UTC
[Ocfs2-devel] [PATCH] 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
Signed-off-by: Wengang Wang <wen.gang.wang at oracle.com>
---
fs/ocfs2/aops.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c
index 302cd7c..a87ca42d 100644
--- a/fs/ocfs2/aops.c
+++ b/fs/ocfs2/aops.c
@@ -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);
--
2.9.5
Changwei Ge
2018-Nov-02 00:51 UTC
[Ocfs2-devel] [PATCH] ocfs2: free up write context when direct IO failed
Hello Wengang, The fix looks sane to me. On 2018/11/2 5:17, Wengang Wang wrote:> 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 > > Signed-off-by: Wengang Wang <wen.gang.wang at oracle.com>Reviewed-by: Changwei Ge <ge.changwei at h3c.com>> --- > fs/ocfs2/aops.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c > index 302cd7c..a87ca42d 100644 > --- a/fs/ocfs2/aops.c > +++ b/fs/ocfs2/aops.c > @@ -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); > >
Joseph Qi
2018-Nov-02 01:35 UTC
[Ocfs2-devel] [PATCH] ocfs2: free up write context when direct IO failed
On 18/11/2 05:15, Wengang Wang wrote:> 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 > > Signed-off-by: Wengang Wang <wen.gang.wang at oracle.com>Looks good. We'd better cc stable as well (4.10+ ?). Acked-by: Joseph Qi <jiangqi903 at gmail.com>> --- > fs/ocfs2/aops.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c > index 302cd7c..a87ca42d 100644 > --- a/fs/ocfs2/aops.c > +++ b/fs/ocfs2/aops.c > @@ -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); > >
Junxiao Bi
2018-Nov-02 02:51 UTC
[Ocfs2-devel] [PATCH] ocfs2: free up write context when direct IO failed
On 11/02/2018 05:15 AM, Wengang Wang wrote:> 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 > > Signed-off-by: Wengang Wang <wen.gang.wang at oracle.com> > --- > fs/ocfs2/aops.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c > index 302cd7c..a87ca42d 100644 > --- a/fs/ocfs2/aops.c > +++ b/fs/ocfs2/aops.c > @@ -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");Is it useful to leave the "bytes" value here? It could be some error code. Thanks, Junxiao.> + ocfs2_dio_free_write_ctx(inode, private); > + } > + } > > ocfs2_iocb_clear_rw_locked(iocb); >