On 2/9/24 08:46, Yuan Yao wrote:
> Hi Bernd,
>
> Thank you for taking a look at this patch! I appreciate it a lot for
> adding this patch to the next version. However, I just found that
> there’s a bug with my patch. The *BUG_ON(!d_unhashed(dentry));* in
> d_splice_alias() function will be triggered when doing the parallel
> lookup on a non-exist file.
>
>> struct dentry *d_splice_alias(struct inode *inode, struct dentry *dentry)
>> {
>> if (IS_ERR(inode))
>> return ERR_CAST(inode);
>>
>> BUG_ON(!d_unhashed(dentry));
>
> This bug can be easily reproduced by adding 3 seconds sleep in fuse
> server’s atomic_open handler, and execute the following commands in
> fuse filesystem:
> cat non-exist-file &
> cat non-exist-file &
>
> I think this bug can be addressed by following two approaches:
>
> 1. adding check for d_in_lookup(entry)
> -----------------------------------------------------------------------
> if (outentry.entry_valid) {
> + if (d_in_lookup(entry)) {
> inode = NULL;
> d_splice_alias(inode, entry);
> fuse_change_entry_timeout(entry, &outentry);
> + }
> goto free_and_no_open;
> }
>
> 2. adding d_drop(entry)
> -----------------------------------------------------------------------
> if (outentry.entry_valid) {
> inode = NULL;
> + d_drop(entry)
> d_splice_alias(inode, entry);
> fuse_change_entry_timeout(entry, &outentry);
> goto free_and_no_open;
> }
>
> But I'm not so sure which one is preferable, or maybe the handling
> should be elsewhere?
>
Sorry for my terribly late reply, will look into it in the evening.
Thanks,
Bernd
