On Thu, Aug 21, 2025 at 04:18:15PM -0400, Josef Bacik wrote:
> If we're holding the inode on one of the writeback lists we need to have
> a reference on that inode. Grab a reference when we add i_wb_list to
> something, drop it when it's removed.
>
> This is potentially dangerous, because we remove the inode from the
> i_wb_list potentially under IRQ via folio_end_writeback(). This will be
> mitigated by making sure all writeback is completed on the final iput,
> before the final iobj_put, preventing a potential free under IRQ.
>
> Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
> ---
> fs/fs-writeback.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> index 001773e6e95c..c2437e3d320a 100644
> --- a/fs/fs-writeback.c
> +++ b/fs/fs-writeback.c
> @@ -1332,6 +1332,7 @@ void sb_mark_inode_writeback(struct inode *inode)
> if (list_empty(&inode->i_wb_list)) {
> spin_lock_irqsave(&sb->s_inode_wblist_lock, flags);
> if (list_empty(&inode->i_wb_list)) {
> + iobj_get(inode);
> list_add_tail(&inode->i_wb_list, &sb->s_inodes_wb);
> trace_sb_mark_inode_writeback(inode);
> }
> @@ -1346,15 +1347,26 @@ void sb_clear_inode_writeback(struct inode *inode)
> {
> struct super_block *sb = inode->i_sb;
> unsigned long flags;
> + bool drop = false;
>
> if (!list_empty(&inode->i_wb_list)) {
> spin_lock_irqsave(&sb->s_inode_wblist_lock, flags);
> if (!list_empty(&inode->i_wb_list)) {
> + drop = true;
> list_del_init(&inode->i_wb_list);
> trace_sb_clear_inode_writeback(inode);
> }
> spin_unlock_irqrestore(&sb->s_inode_wblist_lock, flags);
> }
> +
> + /*
> + * This can be called in IRQ context when we're clearing writeback on
> + * the folio. This should not be the last iobj_put() on the inode, we
> + * run all of the writeback before we free the inode in order to avoid
> + * this possibility.
> + */
> + if (drop)
> + iobj_put(inode);
In that case it might be valuable to have a:
VFS_WARN_ON_ONCE(refcount_read(&inode->i_obj_count) < 2);
before calling iobj_put() here? It'll compile out without
CONFIG_VFS_DEBUG set.
Btw, you should also be able to write this as removing the condition.
diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index 2e10cc2f955f..cfdb2c2793cb 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -1366,13 +1366,13 @@ void sb_mark_inode_writeback(struct inode *inode)
void sb_clear_inode_writeback(struct inode *inode)
{
struct super_block *sb = inode->i_sb;
+ struct inode *drop = NULL;
unsigned long flags;
- bool drop = false;
if (!list_empty(&inode->i_wb_list)) {
spin_lock_irqsave(&sb->s_inode_wblist_lock, flags);
if (!list_empty(&inode->i_wb_list)) {
- drop = true;
+ drop = inode;
list_del_init(&inode->i_wb_list);
trace_sb_clear_inode_writeback(inode);
}
@@ -1385,8 +1385,7 @@ void sb_clear_inode_writeback(struct inode *inode)
* run all of the writeback before we free the inode in order to avoid
* this possibility.
*/
- if (drop)
- iobj_put(inode);
+ iobj_put(drop);
}
> }
>
> /*
> @@ -2683,6 +2695,8 @@ static void wait_sb_inodes(struct super_block *sb)
> * to preserve consistency between i_wb_list and the mapping
> * writeback tag. Writeback completion is responsible to remove
> * the inode from either list once the writeback tag is cleared.
> + * At that point the i_obj_count reference will be dropped for
> + * the i_wb_list reference.
> */
> list_move_tail(&inode->i_wb_list, &sb->s_inodes_wb);
>
> --
> 2.49.0
>
