Skip to content

Commit

Permalink
Linux: add zpl_drop_inode
Browse files Browse the repository at this point in the history
This is an attempt to rectify the situation in zfs_zget() on Linux,
where we might be racing with inode reclaim from the kernel.

The code is ugly and I'm not sure it's perfectly OK to do it this way
anyway.

I am by no means sure the refcount check is right, lets develop this
idea in the open, I'm eager to see what the testbots say here.

Signed-off-by: Pavel Snajdr <snajpa@snajpa.net>
  • Loading branch information
snajpa committed Oct 7, 2024
1 parent 437227a commit 08a1cc1
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 39 deletions.
42 changes: 3 additions & 39 deletions module/os/linux/zfs/zfs_znode_os.c
Original file line number Diff line number Diff line change
Expand Up @@ -1053,7 +1053,6 @@ zfs_zget(zfsvfs_t *zfsvfs, uint64_t obj_num, znode_t **zpp)

*zpp = NULL;

again:
zh = zfs_znode_hold_enter(zfsvfs, obj_num);

err = sa_buf_hold(zfsvfs->z_os, obj_num, NULL, &db);
Expand All @@ -1076,55 +1075,20 @@ zfs_zget(zfsvfs_t *zfsvfs, uint64_t obj_num, znode_t **zpp)
if (hdl != NULL) {
zp = sa_get_userdata(hdl);


/*
* Since "SA" does immediate eviction we
* should never find a sa handle that doesn't
* know about the znode.
*/

ASSERT3P(zp, !=, NULL);
VERIFY3P(igrab(ZTOI(zp)), !=, NULL);
*zpp = zp;

mutex_enter(&zp->z_lock);
ASSERT3U(zp->z_id, ==, obj_num);
/*
* If zp->z_unlinked is set, the znode is already marked
* for deletion and should not be discovered. Check this
* after checking igrab() due to fsetxattr() & O_TMPFILE.
*
* If igrab() returns NULL the VFS has independently
* determined the inode should be evicted and has
* called iput_final() to start the eviction process.
* The SA handle is still valid but because the VFS
* requires that the eviction succeed we must drop
* our locks and references to allow the eviction to
* complete. The zfs_zget() may then be retried.
*
* This unlikely case could be optimized by registering
* a sops->drop_inode() callback. The callback would
* need to detect the active SA hold thereby informing
* the VFS that this inode should not be evicted.
*/
if (igrab(ZTOI(zp)) == NULL) {
if (zp->z_unlinked)
err = SET_ERROR(ENOENT);
else
err = SET_ERROR(EAGAIN);
} else {
*zpp = zp;
err = 0;
}

mutex_exit(&zp->z_lock);
sa_buf_rele(db, NULL);
zfs_znode_hold_exit(zfsvfs, zh);

if (err == EAGAIN) {
/* inode might need this to finish evict */
cond_resched();
goto again;
}
return (err);
return 0;

Check failure on line 1091 in module/os/linux/zfs/zfs_znode_os.c

View workflow job for this annotation

GitHub Actions / checkstyle

unparenthesized return expression
}

/*
Expand Down
18 changes: 18 additions & 0 deletions module/os/linux/zfs/zpl_super.c
Original file line number Diff line number Diff line change
Expand Up @@ -378,12 +378,30 @@ zpl_prune_sb(uint64_t nr_to_scan, void *arg)
(void) -zfs_prune(sb, nr_to_scan, &objects);
}

static int
zpl_drop_inode(struct inode *ip)
{
znode_t *zp = ITOZ(ip);
dmu_buf_t *db;
int error;

if (!zp->z_unlinked && zp->z_sa_hdl &&
(db = sa_get_db(zp->z_sa_hdl)) &&
dmu_buf_refcount(db))
return (1);

error = generic_drop_inode(ip);

return (error);
}

const struct super_operations zpl_super_operations = {
.alloc_inode = zpl_inode_alloc,
.destroy_inode = zpl_inode_destroy,
.dirty_inode = zpl_dirty_inode,
.write_inode = NULL,
.evict_inode = zpl_evict_inode,
.drop_inode = zpl_drop_inode,
.put_super = zpl_put_super,
.sync_fs = zpl_sync_fs,
.statfs = zpl_statfs,
Expand Down

0 comments on commit 08a1cc1

Please sign in to comment.