SYSTEM_SERVICE_EXCEPTION (3b) - ZFSin!zfs_fileobject_cleanup [version 0.21]

[datacore-tlaurent]

System BSODs in IRP_MJ_CLEANUP processing due to the fact that the v_data field in the vnode is NULL. It has been set to NULL during a previous IRP_MJ_CLOSE. This looks like a race condition between the IRP_MJ_CREATE and IRP_MJ_CLOSE whereas the zfs_vnop_lookup() in the CREATE could fetch the vnode that is lingering but got its v_data pointer set to NULL during the CLOSE.
Because the vnode is marked as VNODE_MARKTERM we could use this to ignore using it during lookup and surround both processings with a mutex so it can be atomically checked.

Note: attaching 3 files to support the analysis and will take a crash dump if necessary for further pinpointing of the issue. cbuf.txt analysis.txt stacks.txt

[lundman]

Thank you for the detailed report. My own thoughts are very similar;

ZFSin!zfs_fileobject_cleanup+0x94

Since we have already been dispatched by dispatcher() and entered zfs_fileobject_cleanup(), it should mean vp is ok, as dispatcher grabs an iocount lock before calling down. So it must be v_data / zp that is NULL;

> 3616:         zfsvfs_t *zfsvfs = zp->z_zfsvfs;

5: kd> dt vp
   +0x100 v_data           : (null) 

Nice, that checks out. One thing I was worried about, is a recent change to delete file, so could be related to that:

FFFFD30683DAE040: FFFFD30683473040: delete_entry: deleting '\System Volume Information\WPSettings.dat'

Ah so that is probably it. Comes from;

https://github.com/openzfsonwindows/ZFSin/blob/master/ZFSin/zfs/module/zfs/zfs_vnops.c#L2140

        zp->z_fastpath = B_TRUE;

        zfs_znode_delete(zp, tx);
        vp->v_data = NULL;  // ****
        vp = NULL;
        zp = NULL;

I was concerned that this might break in other places, setting v_data to NULL - but the znode is gone at this point, so we more or less have to.

This code has changed in upstream, where everything gets added to the unlinked_drain list, and when vnop_inactive() is called, removed from the list and zfs_znode_delete() is called.

We don't have (a full) vnop_inactive() framework, at least, I'm not entirely sure about the logic of when it is called. We should eventually do the same here as upstream.

But for now, the zfs_fileobject_cleanup() should be guarded for a NULL zp. The zp (or rather zfsvfs) is only used to send the NOTIFY.

So zfsvfs_t *zfsvfs = zp->z_zfsvfs; should probably be inside if (zccb && zccb->deleteonclose). We can also get zfsvfs from vfs_fsprivate(zmo) which might be the better thing here.

Since we don't want deleteonclose acted on twice, we probably should make sure to clear it inside that if.

Since the file has already been deleted (zp is NULL) the zfs_fileobject_cleanup() is mostly a no-op, the first time through would have sent the events (as shown in cbuf output), so there isn't much to do "this time around".

What do you think?

[datacore-tlaurent]

I'll take a closer look as soon as I finish code for a PR fixing the zvol scsi inquiry and vpds.