On 08/21, Andy Lutomirski wrote: > > On Wed, Aug 21, 2013 at 11:20 AM, Oleg Nesterov <oleg@xxxxxxxxxx> wrote: > > Can't really comment the patch, just a nit: > > > > On 08/21, Andy Lutomirski wrote: > >> > >> +static bool may_flink(const struct path *path) > >> +{ > >> + bool ret; > >> + struct inode *inode = path->dentry->d_inode; > >> + > >> + /* > >> + * This is racy: I_LINKABLE could be cleared between this check > >> + * and the actual link operation. > > > > OK, > > > >> + spin_lock(&inode->i_lock); > >> + ret = !!(inode->i_state & I_LINKABLE); > >> + spin_unlock(&inode->i_lock); > > > > so why do we need to take a lock ? > > > > We probably don't. But other accesses to this field take that lock, Not if you only need to check ->i_lock, > (In principle, someone could take the lock, write I_LINKABLE, clear > it, and unlock, and we'd get confused if we didn't take the lock > ourselves.) Or it can be cleared right after we drop i_lock, I do not think there is any difference. But OK, I won't argue. Oleg. -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html