On Tue, May 12, 2015 at 09:36:21PM +0300, Dan Carpenter wrote:
> On Tue, May 12, 2015 at 11:17:16AM -0700, Jaegeuk Kim wrote:
> > > For example, shouldn't we release f2fs_lock_op(sbi) when f2fs_add_link()
> > > fails earlier?
> >
> > The pair of f2fs_lock_op and f2fs_unlock_op here is used to keep FS
> > consistency.
> > And, the handle_failed_inode() should be covered by f2fs_lock_op, so there
> > is
> > no reason to do unlock and lock redundantly to handle the error case.
>
> Yes, you are right, but the code is still not correct unfortunately.
Yeah, it was. Please check the V2 patch.
Thanks,
>
> fs/f2fs/namei.c
> 424
> 425 f2fs_lock_op(sbi);
> 426 err = f2fs_add_link(dentry, inode);
> 427 if (err)
> 428 goto out;
> ^^^^^^^^
> Holding the lock. This is correct as you say.
>
> 429 f2fs_unlock_op(sbi);
> 430
> 431 if (f2fs_encrypted_inode(dir)) {
> 432 struct qstr istr = QSTR_INIT(symname, len);
> 433
> 434 err = f2fs_inherit_context(dir, inode, NULL);
> 435 if (err)
> 436 goto out;
> ^^^^^^^^
> Not holding the lock. This is a double unlock bug.
>
> 437
> 438 err = f2fs_setup_fname_crypto(inode);
> 439 if (err)
> 440 goto out;
>
> regards,
> dan carpenter
------------------------------------------------------------------------------
One dashboard for servers and applications across Physical-Virtual-Cloud
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
