On 11/30/21 6:40 PM, Dan Carpenter wrote: > The callers assume that "*fsdata" is set on the success path, but > that's not necessarily true on this path. > In ocfs2_page_mkwrite(), since in this case no target page locked, it will finally return VM_FAULT_NOPAGE (better VM_FAULT_RETRY?) and throw to handle_mm_fault(). So no problem as comments described. But things seems changed since append direct io path started to use write_[begin/end]. In this path, the target page is expected as NULL. This needs more discussion. Thanks, Joseph > Fixes: 5cffff9e2986 ("ocfs2: Fix ocfs2_page_mkwrite()") > Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx> > --- > Please review this one EXTRA CAREFULLY. It's from static analysis and > the truth is I'm not 100% sure it's correct. I'm also not sure that > it's a complete fix. > > Especially, please review how this is called from ocfs2_write_begin() > to make sure that this doesn't break anything. > > > fs/ocfs2/aops.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c > index 68d11c295dd3..a74a370f16f0 100644 > --- a/fs/ocfs2/aops.c > +++ b/fs/ocfs2/aops.c > @@ -1813,6 +1813,7 @@ int ocfs2_write_begin_nolock(struct address_space *mapping, > if (ret == -EAGAIN) { > BUG_ON(wc->w_target_page); > ret = 0; > + *fsdata = wc; > goto out_quota; > } > >