mirror of
https://github.com/versity/scoutfs.git
synced 2026-08-15 19:56:36 +00:00
Fix warning about missing cluster lock during readahead
scoutfs_readahead() will acquire the cluster read lock if the task doesn't already hold it. But it doesn't set the corresponding per-task variable, so the scoutfs_get_block() call won't find it and mistakenly warns that we aren't holding the lock. We need to set the per-task variable in scoutfs_readahead(). And since we can get into this path from scoutfs_fadvise(), and that function acquires the extent_sem, we also need to first acquire the cluster read lock and set the per-task variable there too. This fixes a lock ordering issue that we had not yet seen. Signed-off-by: Chris Kirby <ckirby@versity.com>
This commit is contained in:
+25
-3
@@ -779,6 +779,7 @@ static void scoutfs_readahead(struct readahead_control *rac)
|
||||
{
|
||||
struct inode *inode = rac->file->f_inode;
|
||||
struct scoutfs_inode_info *si = SCOUTFS_I(inode);
|
||||
SCOUTFS_DECLARE_PER_TASK_ENTRY(pt_data_ent);
|
||||
struct super_block *sb = inode->i_sb;
|
||||
struct scoutfs_lock *inode_lock = NULL;
|
||||
bool found_lock;
|
||||
@@ -791,6 +792,7 @@ static void scoutfs_readahead(struct readahead_control *rac)
|
||||
if (ret)
|
||||
return;
|
||||
found_lock = false;
|
||||
scoutfs_per_task_add(&si->pt_data_lock, &pt_data_ent, inode_lock);
|
||||
} else {
|
||||
found_lock = true;
|
||||
}
|
||||
@@ -799,11 +801,14 @@ static void scoutfs_readahead(struct readahead_control *rac)
|
||||
readahead_length(rac), SEF_OFFLINE,
|
||||
SCOUTFS_IOC_DWO_READ, NULL,
|
||||
inode_lock);
|
||||
|
||||
if (ret == 0)
|
||||
mpage_readahead(rac, scoutfs_get_block_read);
|
||||
|
||||
if (!found_lock)
|
||||
if (!found_lock) {
|
||||
scoutfs_per_task_del(&si->pt_data_lock, &pt_data_ent);
|
||||
scoutfs_unlock(sb, inode_lock, SCOUTFS_LOCK_READ);
|
||||
}
|
||||
}
|
||||
|
||||
static int scoutfs_writepage(struct page *page, struct writeback_control *wbc)
|
||||
@@ -1177,21 +1182,35 @@ static int scoutfs_fadvise(struct file *file, loff_t start, loff_t end, int advi
|
||||
{
|
||||
struct inode *inode = file_inode(file);
|
||||
struct scoutfs_inode_info *si = SCOUTFS_I(inode);
|
||||
struct super_block *sb = inode->i_sb;
|
||||
SCOUTFS_DECLARE_PER_TASK_ENTRY(pt_extent_ent);
|
||||
SCOUTFS_DECLARE_PER_TASK_ENTRY(pt_data_ent);
|
||||
struct scoutfs_lock *inode_lock = NULL;
|
||||
bool locked = false;
|
||||
int ret;
|
||||
|
||||
/*
|
||||
* We need to get the extent_sem now, or we'll be out of order with the
|
||||
* mapping.invalidate_lock.
|
||||
* mapping.invalidate_lock. This also means we need to get the cluster
|
||||
* read lock now, or we'll be out of order with it in scoutfs_readahead().
|
||||
*/
|
||||
if (advice == POSIX_FADV_WILLNEED) {
|
||||
ret = scoutfs_lock_inode(sb, SCOUTFS_LOCK_READ, SCOUTFS_LKF_REFRESH_INODE,
|
||||
inode, &inode_lock);
|
||||
if (ret)
|
||||
return ret;
|
||||
|
||||
if (!scoutfs_per_task_add_excl(&si->pt_data_lock, &pt_data_ent,
|
||||
inode_lock))
|
||||
WARN_ON_ONCE(true);
|
||||
|
||||
down_read(&si->extent_sem);
|
||||
locked = true;
|
||||
|
||||
if (!scoutfs_per_task_add_excl(&si->pt_extent_sem, &pt_extent_ent,
|
||||
&pt_extent_ent))
|
||||
WARN_ON_ONCE(true);
|
||||
|
||||
locked = true;
|
||||
}
|
||||
|
||||
ret = generic_fadvise(file, start, end, advice);
|
||||
@@ -1199,6 +1218,9 @@ static int scoutfs_fadvise(struct file *file, loff_t start, loff_t end, int advi
|
||||
if (locked) {
|
||||
scoutfs_per_task_del(&si->pt_extent_sem, &pt_extent_ent);
|
||||
up_read(&si->extent_sem);
|
||||
|
||||
scoutfs_per_task_del(&si->pt_data_lock, &pt_data_ent);
|
||||
scoutfs_unlock(sb, inode_lock, SCOUTFS_LOCK_READ);
|
||||
}
|
||||
|
||||
return ret;
|
||||
|
||||
Reference in New Issue
Block a user