Hello!
On Oct 21, 2014, at 12:02 AM, Al Viro wrote:
> On Tue, Oct 21, 2014 at 03:46:02AM +0000, Drokin, Oleg wrote:
>
>>> a) what protects ->d_name in ll_intent_file_open()? It copies
>>> ->d_name.name and ->d_name.len into local variables and proceeds to
>>> use those; what's to guarantee that dentry won't get hit with d_move()
>>> halfway through that? None of the locks that would give an exclusion
>>> against d_move() appear to be held…
>>
>> You are right. We hit something very similar not too long ago.
>>>
>>> b) what stabilizes *dentryp->d_name in do_statahead_enter()?
>
>> Same as above.
>> All of that would go away once we get rid of the wrapper and unroll the
>> vfs ops.
>>
>> Hopefully this makes at least some sense to you.
>
> OK... I'd gone and ripped some of that stuff out today; see if what's in
> vfs.git#for-oleg makes sense…
I think it makes sense indeed.
The "get rid of duplicate mountpoint checks" patch has a hunk at ll_link
that looks like it belongs to "kill ll_link_generic()" patch,
otherwise the intermediate result is somewhat strange.
> Another question: what's wrong with d_splice_alias() or d_materialise_unique()?
> I.e. why do we need ll_splice_alias()? I have patches in local queue
We used to need it to also find and reuse invalid aliases, so that the dentry
cache does not get out of control with constant dentry invalidations we have.
I checked the code and now that d_splice_alias is calling __d_find_any_alias
that seems to just pick the first alias off the list, we should be fine
with just using generic d_splice_alias.
Just to be sure I guess I need to try a testrun, but it's a bit less convenient
now as I am away from my usual test nodes on a pretty awful internet connection
too.
John, I wonder if you can give this idea a try soonish?
Bye,
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
