Re: [PATCH 2/6] vfs: add d_ancestor()

[OGAWA Hirofumi]

Christoph Hellwig <hch@xxxxxxxxxxxxx> writes:

> This change looks good, but a slightly more detailed changelog would be
> good.

tweaked...

>>  /*
>> - * Helper that returns 1 if p1 is a parent of p2, else 0
>> + * Helper that returns the ancestor dentry of p2 which is a child of
>> + * p1, if p1 is an ancestor of p2, else NULL.
>>   */
>> -static int d_isparent(struct dentry *p1, struct dentry *p2)
>> +struct dentry *d_ancestor(struct dentry *p1, struct dentry *p2)
>
> Given that d_ancestor is non-static a kerneldoc comment describing it
> instead of a plain one would be good.

Yes.

>>  int is_subdir(struct dentry * new_dentry, struct dentry * old_dentry)
>>  {
>>  	int result;
>> -	struct dentry * saved = new_dentry;
>>  	unsigned long seq;
>>  
>> +	if (new_dentry == old_dentry)
>> +		return 1;
>
> Maybe a comment like in your commit description here would be good?

Sure, thanks.

> 	/*
> 	 * Need rcu_readlock to protect against the d_parent trashing due to
> 	 * d_move.
> 	 */
> 	rcu_read_lock();
> 	do {
> 		/* for restarting inner loop in case of seq retry */
>  		seq = read_seqbegin(&rename_lock);
> 		if (d_ancestor(old_dentry, new_dentry))
> 			result = 1;
> 		else
> 			result = 0;
> 	} while (read_seqretry(&rename_lock, seq));
>  	rcu_read_unlock();

I'll borrow your version. Thanks.
-- 
OGAWA Hirofumi <hirofumi@xxxxxxxxxxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/