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. 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