Re: [PATCH -next v5 29/32] ext4: add tracepoints for DISKSIZE_GROW_PENDING set, clear, and wait

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Sequence counter deadlock in tracepoint when called from softirq context on 32-bit SMP.
--

--- Patch [29]: [PATCH -next v5 29/32] ext4: add tracepoints for DISKSIZE_GROW_PENDING set, clear, and wait ---
commit f14aeb9661220b55cf0eeb4ff49da9fa6a2a18ca
Author: Zhang Yi <[email protected]>

ext4: add tracepoints for DISKSIZE_GROW_PENDING set, clear, and wait
    
This commit adds trace events ext4_iomap_mark_disksize_pending(),
ext4_iomap_clear_disksize_pending(), and ext4_iomap_wait_disksize_pending().
These tracepoints track disksize-grow-pending state changes and waiting.

> diff --git a/include/trace/events/ext4.h b/include/trace/events/ext4.h
> index 69596a216dcb1..4539ef8e5f86c 100644
> --- a/include/trace/events/ext4.h
> +++ b/include/trace/events/ext4.h
[ ... ]
> +DECLARE_EVENT_CLASS(ext4_iomap_disksize_pending,
> +	TP_PROTO(struct inode *inode),
> +	TP_ARGS(inode),
> +	TP_STRUCT__entry(
> +		__field(dev_t, dev)
> +		__field(u64, ino)
> +		__field(loff_t, i_size)
> +		__field(loff_t, i_disksize)
> +	),
> +	TP_fast_assign(
> +		__entry->dev = inode->i_sb->s_dev;
> +		__entry->ino = inode->i_ino;
> +		__entry->i_size = i_size_read(inode);

[Severity: High]
Could this cause a sequence counter deadlock on 32-bit SMP architectures?

If a process executing i_size_write() is preempted by a block I/O completion
hardware interrupt, and pending softirqs are executed on return from the
interrupt, the softirq could call ext4_iomap_end_bio() in fs/ext4/page-io.c:

fs/ext4/page-io.c:ext4_iomap_end_bio() {
        ...
        if (io_mode == EXT4_IOMAP_IOEND_DISKSIZE_GROW_IO)
                ext4_iomap_clear_disksize_pending(ioend->io_inode);
        ...
}

This would trigger trace_ext4_iomap_clear_disksize_pending() which calls
i_size_read() here in TP_fast_assign.

Since i_size_write() disables preemption but not softirqs, the seqcount
would be odd. This would cause i_size_read() to spin indefinitely waiting
for the interrupted i_size_write() to complete on the same CPU.

> +		__entry->i_disksize = READ_ONCE(EXT4_I(inode)->i_disksize);
> +	),
> +	TP_printk("dev %d:%d ino %llu i_size %lld i_disksize %lld",
> +		  MAJOR(__entry->dev), MINOR(__entry->dev),
> +		  __entry->ino, __entry->i_size, __entry->i_disksize)
> +);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=29