* [RFC PATCH 8/8] xfs: Lift the bs == ps restriction for HW buffered atomic writes
From: Ojaswin Mujoo @ 2025-11-12 11:06 UTC (permalink / raw)
To: Christian Brauner, djwong, ritesh.list, john.g.garry, tytso,
willy, dchinner, hch
Cc: linux-xfs, linux-kernel, linux-ext4, linux-fsdevel, linux-mm,
jack, nilay, martin.petersen, rostedt, axboe, linux-block,
linux-trace-kernel
In-Reply-To: <cover.1762945505.git.ojaswin@linux.ibm.com>
Now that we support bs < ps for HW atomic writes, lift this restirction from XFS
statx reporting
Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
---
fs/xfs/xfs_iops.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index 67d370947d95..5bd31aacf514 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -622,10 +622,9 @@ xfs_get_atomic_write_min(
return bs;
}
/*
- * Buffered IO only supports hw single block atomic writes and bs == ps
- * configurations.
+ * Buffered IO only supports hw single block atomic writes
*/
- if (xfs_inode_can_hw_atomic_write(ip) && bs == PAGE_SIZE)
+ if (xfs_inode_can_hw_atomic_write(ip))
return bs;
return 0;
@@ -661,10 +660,9 @@ xfs_get_atomic_write_max(
return XFS_FSB_TO_B(mp, mp->m_groups[XG_TYPE_AG].awu_max);
}
/*
- * Buffered IO only supports hw single block atomic writes and bs == ps
- * configurations.
+ * Buffered IO only supports hw single block atomic writes
*/
- if (xfs_inode_can_hw_atomic_write(ip) && bs == PAGE_SIZE)
+ if (xfs_inode_can_hw_atomic_write(ip))
return bs;
return 0;
--
2.51.0
^ permalink raw reply related
* [RFC PATCH 7/8] iomap: Add bs<ps buffered atomic writes support
From: Ojaswin Mujoo @ 2025-11-12 11:06 UTC (permalink / raw)
To: Christian Brauner, djwong, ritesh.list, john.g.garry, tytso,
willy, dchinner, hch
Cc: linux-xfs, linux-kernel, linux-ext4, linux-fsdevel, linux-mm,
jack, nilay, martin.petersen, rostedt, axboe, linux-block,
linux-trace-kernel
In-Reply-To: <cover.1762945505.git.ojaswin@linux.ibm.com>
Lift bs == ps restriction for RWF_ATOMIC buffered writes by adding
sub-page support. This is done by adding 2 more bitmaps in folio -> ifs.
One bitmap tracks which blocks marks a start of atomic write and the
other tracks which marks the end. For single block atomic write, both
start and end would be marked on the same block, but the design is kept
as such so we can easily extend to multi block later.
With the help of the 2 bitmaps, we are able to determine which blocks
needs to go atomically together during writeback. This prevents the
issue where write amplification could cause an RWF_ATOMIC write which is
bigger than supported by the underlying device.
As with bs == ps support, if there is a non atomic write that overlaps
an atomic marked block, we will clear the atomic state of that block
in ifs. Similarly, if the folio is mmapd and written to, we will clear
atomic bit from all blocks in the folio.
To illustrate some examples:
A = Dirty, atomic block
D = Dirty, non-atomic block
Let pagesize = 4k, blocksize = 1k
1)
- Initial state of blocks in folio: A A D D
- Non atomic write from block 0 to 3
- New state: D D D D
2)
- Initial state of blocks in folio: A A A A
- Non atomic write from block 1 to 2
- New state: A D D A
3)
- Initial state of blocks in folio: A A _ _
- mmap write to anyblock in folio
- New state: D D D D
Suggested-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
---
fs/iomap/buffered-io.c | 207 ++++++++++++++++++++++++++++++++++---
fs/iomap/ioend.c | 9 +-
fs/iomap/trace.h | 12 ++-
fs/read_write.c | 22 ----
include/linux/iomap.h | 1 +
include/linux/page-flags.h | 2 +-
6 files changed, 207 insertions(+), 46 deletions(-)
diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
index e7dbe9bcb439..d86859728e3b 100644
--- a/fs/iomap/buffered-io.c
+++ b/fs/iomap/buffered-io.c
@@ -26,6 +26,10 @@ struct iomap_folio_state {
* Each block has two bits in this bitmap:
* Bits [0..blocks_per_folio) has the uptodate status.
* Bits [b_p_f...(2*b_p_f)) has the dirty status.
+ * Bits [2*b_p_f..3*b_p_f) has whether block marks the
+ * start of an RWF_ATOMIC write
+ * Bits [3*b_p_f..4*b_p_f) has whether block marks the
+ * end of an RWF_ATOMIC write
*/
unsigned long state[];
};
@@ -76,6 +80,25 @@ static void iomap_set_range_uptodate(struct folio *folio, size_t off,
folio_mark_uptodate(folio);
}
+static inline bool ifs_block_is_atomic_start(struct folio *folio,
+ struct iomap_folio_state *ifs, int block)
+{
+ struct inode *inode = folio->mapping->host;
+ unsigned int blks_per_folio = i_blocks_per_folio(inode, folio);
+
+ return test_bit(block + (blks_per_folio * 2), ifs->state);
+}
+
+static inline bool ifs_block_is_atomic_end(struct folio *folio,
+ struct iomap_folio_state *ifs,
+ int block)
+{
+ struct inode *inode = folio->mapping->host;
+ unsigned int blks_per_folio = i_blocks_per_folio(inode, folio);
+
+ return test_bit(block + (blks_per_folio * 3), ifs->state);
+}
+
static inline bool ifs_block_is_dirty(struct folio *folio,
struct iomap_folio_state *ifs, int block)
{
@@ -85,17 +108,42 @@ static inline bool ifs_block_is_dirty(struct folio *folio,
return test_bit(block + blks_per_folio, ifs->state);
}
+/*
+ * Returns false if the folio has atleast 1 atomic block, else true
+ */
+static inline bool ifs_is_fully_non_atomic(struct folio *folio,
+ struct iomap_folio_state *ifs)
+{
+ struct inode *inode = folio->mapping->host;
+ unsigned int blks_per_folio = i_blocks_per_folio(inode, folio);
+
+ for (int i = 0; i < blks_per_folio; i++) {
+ if (ifs_block_is_atomic_start(folio, ifs, i))
+ return false;
+ }
+
+ return true;
+}
+
static unsigned ifs_find_dirty_range(struct folio *folio,
- struct iomap_folio_state *ifs, u64 *range_start, u64 range_end)
+ struct iomap_folio_state *ifs,
+ u64 *range_start, u64 range_end,
+ bool *is_atomic_range)
{
struct inode *inode = folio->mapping->host;
+ unsigned folio_nblks = i_blocks_per_folio(inode, folio);
unsigned start_blk =
offset_in_folio(folio, *range_start) >> inode->i_blkbits;
unsigned end_blk = min_not_zero(
offset_in_folio(folio, range_end) >> inode->i_blkbits,
- i_blocks_per_folio(inode, folio));
+ folio_nblks);
unsigned nblks = 1;
+ bool is_atomic_folio = folio_test_atomic(folio);
+ /*
+ * We need to be careful in not clubbing together atomic write ranges
+ * with other dirty blocks
+ */
while (!ifs_block_is_dirty(folio, ifs, start_blk))
if (++start_blk == end_blk)
return 0;
@@ -106,12 +154,62 @@ static unsigned ifs_find_dirty_range(struct folio *folio,
nblks++;
}
+ *is_atomic_range = false;
+
+ if (is_atomic_folio) {
+ unsigned int first_atomic;
+ unsigned int last = start_blk + nblks;
+ /*
+ * We now have the dirty range, however if the range has any
+ * RWF_ATOMIC blocks, we need to make sure to not club them with
+ * other dirty blocks.
+ */
+ first_atomic = start_blk;
+ while (!ifs_block_is_atomic_start(folio, ifs, first_atomic)) {
+ if (++first_atomic == start_blk + nblks)
+ break;
+ }
+
+ if (first_atomic != start_blk + nblks) {
+ /* RWF_ATOMIC blocks found in dirty range */
+ if (first_atomic == start_blk) {
+ /*
+ * range start is RWF_ATOMIC. Return only the
+ * atomic range.
+ */
+ nblks = 0;
+ while (first_atomic + nblks < last) {
+ if (ifs_block_is_atomic_end(
+ folio, ifs,
+ first_atomic + nblks++))
+ break;
+ }
+
+ if (first_atomic + nblks > last)
+ /*
+ * RWF_ATOMIC range should
+ * always be contained in the
+ * dirty range
+ */
+ WARN_ON(true);
+
+ *is_atomic_range = true;
+ } else {
+ /*
+ * RWF_ATOMIC range is in middle of dirty range. Return only
+ * the starting non-RWF_ATOMIC range
+ */
+ nblks = first_atomic - start_blk;
+ }
+ }
+ }
+
*range_start = folio_pos(folio) + (start_blk << inode->i_blkbits);
return nblks << inode->i_blkbits;
}
static unsigned iomap_find_dirty_range(struct folio *folio, u64 *range_start,
- u64 range_end)
+ u64 range_end, bool *is_atomic_range)
{
struct iomap_folio_state *ifs = folio->private;
@@ -119,10 +217,33 @@ static unsigned iomap_find_dirty_range(struct folio *folio, u64 *range_start,
return 0;
if (ifs)
- return ifs_find_dirty_range(folio, ifs, range_start, range_end);
+ return ifs_find_dirty_range(folio, ifs, range_start, range_end,
+ is_atomic_range);
+
+ if (folio_test_atomic(folio))
+ *is_atomic_range = true;
+
return range_end - *range_start;
}
+static bool ifs_clear_range_atomic(struct folio *folio,
+ struct iomap_folio_state *ifs, size_t off, size_t len)
+{
+ struct inode *inode = folio->mapping->host;
+ unsigned int blks_per_folio = i_blocks_per_folio(inode, folio);
+ unsigned int first_blk = (off >> inode->i_blkbits);
+ unsigned int last_blk = (off + len - 1) >> inode->i_blkbits;
+ unsigned int nr_blks = last_blk - first_blk + 1;
+ unsigned long flags;
+
+ spin_lock_irqsave(&ifs->state_lock, flags);
+ bitmap_clear(ifs->state, first_blk + (blks_per_folio * 2), nr_blks);
+ bitmap_clear(ifs->state, last_blk + (blks_per_folio * 3), nr_blks);
+ spin_unlock_irqrestore(&ifs->state_lock, flags);
+
+ return ifs_is_fully_non_atomic(folio, ifs);
+}
+
static void ifs_clear_range_dirty(struct folio *folio,
struct iomap_folio_state *ifs, size_t off, size_t len)
{
@@ -138,6 +259,18 @@ static void ifs_clear_range_dirty(struct folio *folio,
spin_unlock_irqrestore(&ifs->state_lock, flags);
}
+static void iomap_clear_range_atomic(struct folio *folio, size_t off, size_t len)
+{
+ struct iomap_folio_state *ifs = folio->private;
+ bool fully_non_atomic = true;
+
+ if (ifs)
+ fully_non_atomic = ifs_clear_range_atomic(folio, ifs, off, len);
+
+ if (fully_non_atomic)
+ folio_clear_atomic(folio);
+}
+
static void iomap_clear_range_dirty(struct folio *folio, size_t off, size_t len)
{
struct iomap_folio_state *ifs = folio->private;
@@ -146,8 +279,34 @@ static void iomap_clear_range_dirty(struct folio *folio, size_t off, size_t len)
ifs_clear_range_dirty(folio, ifs, off, len);
}
-static void ifs_set_range_dirty(struct folio *folio,
+static void ifs_set_range_atomic(struct folio *folio,
struct iomap_folio_state *ifs, size_t off, size_t len)
+{
+ struct inode *inode = folio->mapping->host;
+ unsigned int blks_per_folio = i_blocks_per_folio(inode, folio);
+ unsigned int first_blk = (off >> inode->i_blkbits);
+ unsigned int last_blk = (off + len - 1) >> inode->i_blkbits;
+ unsigned long flags;
+
+ spin_lock_irqsave(&ifs->state_lock, flags);
+ bitmap_set(ifs->state, first_blk + (blks_per_folio * 2), 1);
+ bitmap_set(ifs->state, last_blk + (blks_per_folio * 3), 1);
+ spin_unlock_irqrestore(&ifs->state_lock, flags);
+}
+
+static void iomap_set_range_atomic(struct folio *folio, size_t off, size_t len)
+{
+ struct iomap_folio_state *ifs = folio->private;
+
+ if (ifs)
+ ifs_set_range_atomic(folio, ifs, off, len);
+
+ folio_set_atomic(folio);
+}
+
+static void ifs_set_range_dirty(struct folio *folio,
+ struct iomap_folio_state *ifs, size_t off,
+ size_t len)
{
struct inode *inode = folio->mapping->host;
unsigned int blks_per_folio = i_blocks_per_folio(inode, folio);
@@ -190,8 +349,12 @@ static struct iomap_folio_state *ifs_alloc(struct inode *inode,
* The first state tracks per-block uptodate and the
* second tracks per-block dirty state.
*/
+
+ /*
+ * TODO: How can we only selectively allocate atomic bitmaps for ifs?
+ */
ifs = kzalloc(struct_size(ifs, state,
- BITS_TO_LONGS(2 * nr_blocks)), gfp);
+ BITS_TO_LONGS(4 * nr_blocks)), gfp);
if (!ifs)
return ifs;
@@ -941,6 +1104,8 @@ static int iomap_write_begin(struct iomap_iter *iter,
static bool __iomap_write_end(struct iomap_iter *iter, loff_t pos, size_t len,
size_t copied, struct folio *folio)
{
+ struct inode *inode = iter->inode;
+
flush_dcache_folio(folio);
/*
@@ -975,9 +1140,12 @@ static bool __iomap_write_end(struct iomap_iter *iter, loff_t pos, size_t len,
return false;
}
- folio_set_atomic(folio);
- } else
- folio_clear_atomic(folio);
+ iomap_set_range_atomic(folio, offset_in_folio(folio, pos), len);
+ } else {
+ if (folio_test_atomic(folio))
+ iomap_clear_range_atomic(
+ folio, offset_in_folio(folio, pos), len);
+ }
return true;
}
@@ -1208,7 +1376,11 @@ static int iomap_write_iter(struct iomap_iter *iter, struct iov_iter *i,
written);
iov_iter_revert(i,
iov_iter_count(&atomic_iter));
- }
+ } else
+ iomap_set_range_atomic(
+ folio, offset_in_folio(folio, pos),
+ written);
+
iomap_atomic_write_cleanup(&pages, &pinned_pgs,
&atomic_bvecs);
}
@@ -1743,7 +1915,7 @@ static int iomap_folio_mkwrite_iter(struct iomap_iter *iter,
} else {
WARN_ON_ONCE(!folio_test_uptodate(folio));
folio_mark_dirty(folio);
- folio_clear_atomic(folio);
+ iomap_clear_range_atomic(folio, 0, folio_size(folio));
}
return iomap_iter_advance(iter, length);
@@ -1799,7 +1971,7 @@ void iomap_finish_folio_write(struct inode *inode, struct folio *folio,
WARN_ON_ONCE(ifs && atomic_read(&ifs->write_bytes_pending) <= 0);
if (!ifs || atomic_sub_and_test(len, &ifs->write_bytes_pending)) {
- folio_clear_atomic(folio);
+ iomap_clear_range_atomic(folio, 0, folio_size(folio));
folio_end_writeback(folio);
}
}
@@ -1914,6 +2086,8 @@ int iomap_writeback_folio(struct iomap_writepage_ctx *wpc, struct folio *folio)
if (!ifs) {
ifs = ifs_alloc(inode, folio, 0);
iomap_set_range_dirty(folio, 0, end_pos - pos);
+ if (folio_test_atomic(folio))
+ iomap_set_range_atomic(folio, 0, end_pos - pos);
}
/*
@@ -1936,7 +2110,8 @@ int iomap_writeback_folio(struct iomap_writepage_ctx *wpc, struct folio *folio)
* Walk through the folio to find dirty areas to write back.
*/
end_aligned = round_up(end_pos, i_blocksize(inode));
- while ((rlen = iomap_find_dirty_range(folio, &pos, end_aligned))) {
+ while ((rlen = iomap_find_dirty_range(folio, &pos, end_aligned,
+ &wpc->is_atomic_range))) {
error = iomap_writeback_range(wpc, folio, pos, rlen, end_pos,
&wb_pending);
if (error)
@@ -1962,11 +2137,13 @@ int iomap_writeback_folio(struct iomap_writepage_ctx *wpc, struct folio *folio)
* bit ourselves right after unlocking the page.
*/
if (ifs) {
- if (atomic_dec_and_test(&ifs->write_bytes_pending))
+ if (atomic_dec_and_test(&ifs->write_bytes_pending)) {
+ iomap_clear_range_atomic(folio, 0, folio_size(folio));
folio_end_writeback(folio);
+ }
} else {
if (!wb_pending) {
- folio_clear_atomic(folio);
+ iomap_clear_range_atomic(folio, 0, folio_size(folio));
folio_end_writeback(folio);
}
}
diff --git a/fs/iomap/ioend.c b/fs/iomap/ioend.c
index c129a695ceca..678c052c6443 100644
--- a/fs/iomap/ioend.c
+++ b/fs/iomap/ioend.c
@@ -163,10 +163,10 @@ ssize_t iomap_add_to_ioend(struct iomap_writepage_ctx *wpc, struct folio *folio,
unsigned int ioend_flags = 0;
unsigned int map_len = min_t(u64, dirty_len,
wpc->iomap.offset + wpc->iomap.length - pos);
- bool is_atomic = folio_test_atomic(folio);
int error;
- trace_iomap_add_to_ioend(wpc->inode, pos, dirty_len, &wpc->iomap);
+ trace_iomap_add_to_ioend(wpc->inode, pos, dirty_len, &wpc->iomap,
+ wpc->is_atomic_range);
WARN_ON_ONCE(!folio->private && map_len < dirty_len);
@@ -188,7 +188,7 @@ ssize_t iomap_add_to_ioend(struct iomap_writepage_ctx *wpc, struct folio *folio,
ioend_flags |= IOMAP_IOEND_DONTCACHE;
if (pos == wpc->iomap.offset && (wpc->iomap.flags & IOMAP_F_BOUNDARY))
ioend_flags |= IOMAP_IOEND_BOUNDARY;
- if (is_atomic)
+ if (wpc->is_atomic_range)
ioend_flags |= IOMAP_IOEND_ATOMIC;
if (!ioend || !iomap_can_add_to_ioend(wpc, pos, ioend_flags)) {
@@ -198,7 +198,8 @@ ssize_t iomap_add_to_ioend(struct iomap_writepage_ctx *wpc, struct folio *folio,
if (error)
return error;
}
- wpc->wb_ctx = ioend = iomap_alloc_ioend(wpc, pos, ioend_flags, is_atomic);
+ wpc->wb_ctx = ioend = iomap_alloc_ioend(wpc, pos, ioend_flags,
+ wpc->is_atomic_range);
}
if (!bio_add_folio(&ioend->io_bio, folio, map_len, poff))
diff --git a/fs/iomap/trace.h b/fs/iomap/trace.h
index a61c1dae4742..14ad280c03fe 100644
--- a/fs/iomap/trace.h
+++ b/fs/iomap/trace.h
@@ -172,8 +172,8 @@ DEFINE_IOMAP_EVENT(iomap_iter_srcmap);
TRACE_EVENT(iomap_add_to_ioend,
TP_PROTO(struct inode *inode, u64 pos, unsigned int dirty_len,
- struct iomap *iomap),
- TP_ARGS(inode, pos, dirty_len, iomap),
+ struct iomap *iomap, bool is_atomic),
+ TP_ARGS(inode, pos, dirty_len, iomap, is_atomic),
TP_STRUCT__entry(
__field(dev_t, dev)
__field(u64, ino)
@@ -185,6 +185,7 @@ TRACE_EVENT(iomap_add_to_ioend,
__field(u16, type)
__field(u16, flags)
__field(dev_t, bdev)
+ __field(bool, is_atomic)
),
TP_fast_assign(
__entry->dev = inode->i_sb->s_dev;
@@ -197,9 +198,11 @@ TRACE_EVENT(iomap_add_to_ioend,
__entry->type = iomap->type;
__entry->flags = iomap->flags;
__entry->bdev = iomap->bdev ? iomap->bdev->bd_dev : 0;
+ __entry->is_atomic = is_atomic;
),
TP_printk("dev %d:%d ino 0x%llx bdev %d:%d pos 0x%llx dirty len 0x%llx "
- "addr 0x%llx offset 0x%llx length 0x%llx type %s (0x%x) flags %s (0x%x)",
+ "addr 0x%llx offset 0x%llx length 0x%llx type %s (0x%x) flags %s (0x%x) "
+ "is_atomic=%d",
MAJOR(__entry->dev), MINOR(__entry->dev),
__entry->ino,
MAJOR(__entry->bdev), MINOR(__entry->bdev),
@@ -211,7 +214,8 @@ TRACE_EVENT(iomap_add_to_ioend,
__print_symbolic(__entry->type, IOMAP_TYPE_STRINGS),
__entry->type,
__print_flags(__entry->flags, "|", IOMAP_F_FLAGS_STRINGS),
- __entry->flags)
+ __entry->flags,
+ __entry->is_atomic)
);
TRACE_EVENT(iomap_iter,
diff --git a/fs/read_write.c b/fs/read_write.c
index 7e064561cc4b..ab5d8e17d86d 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -1802,8 +1802,6 @@ int generic_file_rw_checks(struct file *file_in, struct file *file_out)
int generic_atomic_write_valid(struct kiocb *iocb, struct iov_iter *iter)
{
- struct super_block *sb = iocb->ki_filp->f_mapping->host->i_sb;
-
size_t len = iov_iter_count(iter);
if (!iter_is_ubuf(iter))
@@ -1815,26 +1813,6 @@ int generic_atomic_write_valid(struct kiocb *iocb, struct iov_iter *iter)
if (!IS_ALIGNED(iocb->ki_pos, len))
return -EINVAL;
- if (!(iocb->ki_flags & IOCB_DIRECT)) {
- /* Some restrictions to buferred IO */
-
- /*
- * We only support block size == page size
- * right now. This is to avoid the following:
- * 1. 4kb block atomic write marks the complete 64kb folio as
- * atomic.
- * 2. Other writes, dirty the whole 64kb folio.
- * 3. Writeback sees the whole folio dirty and atomic and tries
- * to send a 64kb atomic write, which might exceed the
- * allowed size and fail.
- *
- * Once we support sub-page atomic write tracking, we can remove
- * this restriction.
- */
- if (sb->s_blocksize != PAGE_SIZE)
- return -EOPNOTSUPP;
- }
-
return 0;
}
EXPORT_SYMBOL_GPL(generic_atomic_write_valid);
diff --git a/include/linux/iomap.h b/include/linux/iomap.h
index 693f3e5ad03c..033e0ba49f85 100644
--- a/include/linux/iomap.h
+++ b/include/linux/iomap.h
@@ -460,6 +460,7 @@ struct iomap_writepage_ctx {
const struct iomap_writeback_ops *ops;
u32 nr_folios; /* folios added to the ioend */
void *wb_ctx; /* pending writeback context */
+ bool is_atomic_range;
};
struct iomap_ioend *iomap_init_ioend(struct inode *inode, struct bio *bio,
diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index bdce0f58a77a..542e7db6b21b 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -111,7 +111,7 @@ enum pageflags {
PG_swapbacked, /* Page is backed by RAM/swap */
PG_unevictable, /* Page is "unevictable" */
PG_dropbehind, /* drop pages on IO completion */
- PG_atomic, /* Page is marked atomic for buffered atomic writes */
+ PG_atomic, /* Atlease 1 block in page is marked atomic for buffered atomic writes */
#ifdef CONFIG_MMU
PG_mlocked, /* Page is vma mlocked */
#endif
--
2.51.0
^ permalink raw reply related
* Re: [PATCH v5 6/8] selftests: tracing: Add a basic testcase for wprobe
From: Mark Brown @ 2025-11-12 12:15 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Catalin Marinas, Will Deacon, Steven Rostedt, Peter Zijlstra,
Ingo Molnar, x86, Jinchao Wang, Mathieu Desnoyers,
Thomas Gleixner, Borislav Petkov, Dave Hansen, H . Peter Anvin,
Alexander Shishkin, Ian Rogers, linux-kernel, linux-trace-kernel,
linux-doc, linux-perf-users, linux-arm-kernel, Aishwarya.TCV
In-Reply-To: <20251030090952.befea7f0cecd5518c7fda02c@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 772 bytes --]
On Thu, Oct 30, 2025 at 09:09:52AM +0900, Masami Hiramatsu wrote:
> Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> There is a single step execution code but only for default overflow_handlers.
> This is a bit strange becuase other users can not set it up outside of
> the arch dependent code. Even if it can, it is simply redundant.
> So I made changes below which allow users to set its own custom handler is
> compatible with perf default overflow handlers.
> I confirmed this works on both arm and arm64.
I think everyone who knows this code is really snowed under at the
minute - can I suggest posting this as a fix for now? This is still
broken on arm64, we'll need a fix (or to disable the feature on the
affected arches) one way or another for v6.19.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* RE: [Intel-wired-lan] [PATCH v2 09/21] e1000e: Switch to use %ptSp
From: Loktionov, Aleksandr @ 2025-11-12 13:07 UTC (permalink / raw)
To: Andy Shevchenko, Corey Minyard, Christian König,
Dr. David Alan Gilbert, Alex Deucher, Thomas Zimmermann,
Dmitry Baryshkov, Rob Clark, Brost, Matthew, Ulf Hansson,
Lifshits, Vitaly, Manivannan Sadhasivam, Niklas Cassel,
Calvin Owens, Vadim Fedorenko, Sagi Maimon, Martin K. Petersen,
Karan Tilak Kumar, Hans Verkuil, Casey Schaufler, Steven Rostedt,
Petr Mladek, Viacheslav Dubeyko, Max Kellermann,
linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
openipmi-developer@lists.sourceforge.net,
linux-media@vger.kernel.org, dri-devel@lists.freedesktop.org,
linaro-mm-sig@lists.linaro.org, amd-gfx@lists.freedesktop.org,
linux-arm-msm@vger.kernel.org, freedreno@lists.freedesktop.org,
intel-xe@lists.freedesktop.org, linux-mmc@vger.kernel.org,
netdev@vger.kernel.org, intel-wired-lan@lists.osuosl.org,
linux-pci@vger.kernel.org, linux-s390@vger.kernel.org,
linux-scsi@vger.kernel.org, linux-staging@lists.linux.dev,
ceph-devel@vger.kernel.org, linux-trace-kernel@vger.kernel.org
Cc: Rasmus Villemoes, Sergey Senozhatsky, Jonathan Corbet,
Sumit Semwal, Gustavo Padovan, David Airlie, Simona Vetter,
Maarten Lankhorst, Maxime Ripard, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Sean Paul, Marijn Suijten, Konrad Dybcio,
De Marchi, Lucas, Thomas Hellström, Vivi, Rodrigo,
Vladimir Oltean, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Nguyen, Anthony L,
Kitszel, Przemyslaw, Krzysztof Wilczyński,
Kishon Vijay Abraham I, Bjorn Helgaas, Rodolfo Giometti,
Richard Cochran, Jonathan Lemon, Stefan Haberland, Jan Hoeppner,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Satish Kharat,
Baddela, Sesidhar, James E.J. Bottomley, Mauro Carvalho Chehab,
Greg Kroah-Hartman, Xiubo Li, Ilya Dryomov, Masami Hiramatsu,
Mathieu Desnoyers, Andrew Morton
In-Reply-To: <20251111122735.880607-10-andriy.shevchenko@linux.intel.com>
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> Of Andy Shevchenko
> Sent: Tuesday, November 11, 2025 1:20 PM
> To: Corey Minyard <corey@minyard.net>; Christian König
> <christian.koenig@amd.com>; Dr. David Alan Gilbert
> <linux@treblig.org>; Alex Deucher <alexander.deucher@amd.com>; Thomas
> Zimmermann <tzimmermann@suse.de>; Dmitry Baryshkov
> <dmitry.baryshkov@oss.qualcomm.com>; Rob Clark
> <robin.clark@oss.qualcomm.com>; Brost, Matthew
> <matthew.brost@intel.com>; Ulf Hansson <ulf.hansson@linaro.org>; Andy
> Shevchenko <andriy.shevchenko@linux.intel.com>; Lifshits, Vitaly
> <vitaly.lifshits@intel.com>; Manivannan Sadhasivam <mani@kernel.org>;
> Niklas Cassel <cassel@kernel.org>; Calvin Owens <calvin@wbinvd.org>;
> Vadim Fedorenko <vadim.fedorenko@linux.dev>; Sagi Maimon
> <maimon.sagi@gmail.com>; Martin K. Petersen
> <martin.petersen@oracle.com>; Karan Tilak Kumar <kartilak@cisco.com>;
> Hans Verkuil <hverkuil+cisco@kernel.org>; Casey Schaufler
> <casey@schaufler-ca.com>; Steven Rostedt <rostedt@goodmis.org>; Petr
> Mladek <pmladek@suse.com>; Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>;
> Max Kellermann <max.kellermann@ionos.com>; linux-doc@vger.kernel.org;
> linux-kernel@vger.kernel.org; openipmi-
> developer@lists.sourceforge.net; linux-media@vger.kernel.org; dri-
> devel@lists.freedesktop.org; linaro-mm-sig@lists.linaro.org; amd-
> gfx@lists.freedesktop.org; linux-arm-msm@vger.kernel.org;
> freedreno@lists.freedesktop.org; intel-xe@lists.freedesktop.org;
> linux-mmc@vger.kernel.org; netdev@vger.kernel.org; intel-wired-
> lan@lists.osuosl.org; linux-pci@vger.kernel.org; linux-
> s390@vger.kernel.org; linux-scsi@vger.kernel.org; linux-
> staging@lists.linux.dev; ceph-devel@vger.kernel.org; linux-trace-
> kernel@vger.kernel.org
> Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>; Sergey Senozhatsky
> <senozhatsky@chromium.org>; Jonathan Corbet <corbet@lwn.net>; Sumit
> Semwal <sumit.semwal@linaro.org>; Gustavo Padovan
> <gustavo@padovan.org>; David Airlie <airlied@gmail.com>; Simona Vetter
> <simona@ffwll.ch>; Maarten Lankhorst
> <maarten.lankhorst@linux.intel.com>; Maxime Ripard
> <mripard@kernel.org>; Dmitry Baryshkov <lumag@kernel.org>; Abhinav
> Kumar <abhinav.kumar@linux.dev>; Jessica Zhang
> <jesszhan0024@gmail.com>; Sean Paul <sean@poorly.run>; Marijn Suijten
> <marijn.suijten@somainline.org>; Konrad Dybcio
> <konradybcio@kernel.org>; De Marchi, Lucas <lucas.demarchi@intel.com>;
> Thomas Hellström <thomas.hellstrom@linux.intel.com>; Vivi, Rodrigo
> <rodrigo.vivi@intel.com>; Vladimir Oltean <olteanv@gmail.com>; Andrew
> Lunn <andrew@lunn.ch>; David S. Miller <davem@davemloft.net>; Eric
> Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo
> Abeni <pabeni@redhat.com>; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>; Krzysztof Wilczyński
> <kwilczynski@kernel.org>; Kishon Vijay Abraham I <kishon@kernel.org>;
> Bjorn Helgaas <bhelgaas@google.com>; Rodolfo Giometti
> <giometti@enneenne.com>; Richard Cochran <richardcochran@gmail.com>;
> Jonathan Lemon <jonathan.lemon@gmail.com>; Stefan Haberland
> <sth@linux.ibm.com>; Jan Hoeppner <hoeppner@linux.ibm.com>; Heiko
> Carstens <hca@linux.ibm.com>; Vasily Gorbik <gor@linux.ibm.com>;
> Alexander Gordeev <agordeev@linux.ibm.com>; Christian Borntraeger
> <borntraeger@linux.ibm.com>; Sven Schnelle <svens@linux.ibm.com>;
> Satish Kharat <satishkh@cisco.com>; Baddela, Sesidhar
> <sebaddel@cisco.com>; James E.J. Bottomley
> <James.Bottomley@HansenPartnership.com>; Mauro Carvalho Chehab
> <mchehab@kernel.org>; Greg Kroah-Hartman <gregkh@linuxfoundation.org>;
> Xiubo Li <xiubli@redhat.com>; Ilya Dryomov <idryomov@gmail.com>;
> Masami Hiramatsu <mhiramat@kernel.org>; Mathieu Desnoyers
> <mathieu.desnoyers@efficios.com>; Andrew Morton <akpm@linux-
> foundation.org>
> Subject: [Intel-wired-lan] [PATCH v2 09/21] e1000e: Switch to use
> %ptSp
>
> Use %ptSp instead of open coded variants to print content of struct
> timespec64 in human readable format.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> drivers/net/ethernet/intel/e1000e/ptp.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/e1000e/ptp.c
> b/drivers/net/ethernet/intel/e1000e/ptp.c
> index ea3c3eb2ef20..ec39e35f3857 100644
> --- a/drivers/net/ethernet/intel/e1000e/ptp.c
> +++ b/drivers/net/ethernet/intel/e1000e/ptp.c
> @@ -229,14 +229,11 @@ static void e1000e_systim_overflow_work(struct
> work_struct *work)
>
> systim_overflow_work.work);
> struct e1000_hw *hw = &adapter->hw;
> struct timespec64 ts;
> - u64 ns;
>
> /* Update the timecounter */
> - ns = timecounter_read(&adapter->tc);
> + ts = ns_to_timespec64(timecounter_read(&adapter->tc));
>
> - ts = ns_to_timespec64(ns);
> - e_dbg("SYSTIM overflow check at %lld.%09lu\n",
> - (long long) ts.tv_sec, ts.tv_nsec);
> + e_dbg("SYSTIM overflow check at %ptSp\n", &ts);
>
> schedule_delayed_work(&adapter->systim_overflow_work,
> E1000_SYSTIM_OVERFLOW_PERIOD);
> --
> 2.50.1
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
^ permalink raw reply
* RE: [Intel-wired-lan] [PATCH v2 10/21] igb: Switch to use %ptSp
From: Loktionov, Aleksandr @ 2025-11-12 13:08 UTC (permalink / raw)
To: Andy Shevchenko, Corey Minyard, Christian König,
Dr. David Alan Gilbert, Alex Deucher, Thomas Zimmermann,
Dmitry Baryshkov, Rob Clark, Brost, Matthew, Ulf Hansson,
Lifshits, Vitaly, Manivannan Sadhasivam, Niklas Cassel,
Calvin Owens, Vadim Fedorenko, Sagi Maimon, Martin K. Petersen,
Karan Tilak Kumar, Hans Verkuil, Casey Schaufler, Steven Rostedt,
Petr Mladek, Viacheslav Dubeyko, Max Kellermann,
linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
openipmi-developer@lists.sourceforge.net,
linux-media@vger.kernel.org, dri-devel@lists.freedesktop.org,
linaro-mm-sig@lists.linaro.org, amd-gfx@lists.freedesktop.org,
linux-arm-msm@vger.kernel.org, freedreno@lists.freedesktop.org,
intel-xe@lists.freedesktop.org, linux-mmc@vger.kernel.org,
netdev@vger.kernel.org, intel-wired-lan@lists.osuosl.org,
linux-pci@vger.kernel.org, linux-s390@vger.kernel.org,
linux-scsi@vger.kernel.org, linux-staging@lists.linux.dev,
ceph-devel@vger.kernel.org, linux-trace-kernel@vger.kernel.org
Cc: Rasmus Villemoes, Sergey Senozhatsky, Jonathan Corbet,
Sumit Semwal, Gustavo Padovan, David Airlie, Simona Vetter,
Maarten Lankhorst, Maxime Ripard, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Sean Paul, Marijn Suijten, Konrad Dybcio,
De Marchi, Lucas, Thomas Hellström, Vivi, Rodrigo,
Vladimir Oltean, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Nguyen, Anthony L,
Kitszel, Przemyslaw, Krzysztof Wilczyński,
Kishon Vijay Abraham I, Bjorn Helgaas, Rodolfo Giometti,
Richard Cochran, Jonathan Lemon, Stefan Haberland, Jan Hoeppner,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Satish Kharat,
Baddela, Sesidhar, James E.J. Bottomley, Mauro Carvalho Chehab,
Greg Kroah-Hartman, Xiubo Li, Ilya Dryomov, Masami Hiramatsu,
Mathieu Desnoyers, Andrew Morton
In-Reply-To: <20251111122735.880607-11-andriy.shevchenko@linux.intel.com>
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> Of Andy Shevchenko
> Sent: Tuesday, November 11, 2025 1:20 PM
> To: Corey Minyard <corey@minyard.net>; Christian König
> <christian.koenig@amd.com>; Dr. David Alan Gilbert
> <linux@treblig.org>; Alex Deucher <alexander.deucher@amd.com>; Thomas
> Zimmermann <tzimmermann@suse.de>; Dmitry Baryshkov
> <dmitry.baryshkov@oss.qualcomm.com>; Rob Clark
> <robin.clark@oss.qualcomm.com>; Brost, Matthew
> <matthew.brost@intel.com>; Ulf Hansson <ulf.hansson@linaro.org>; Andy
> Shevchenko <andriy.shevchenko@linux.intel.com>; Lifshits, Vitaly
> <vitaly.lifshits@intel.com>; Manivannan Sadhasivam <mani@kernel.org>;
> Niklas Cassel <cassel@kernel.org>; Calvin Owens <calvin@wbinvd.org>;
> Vadim Fedorenko <vadim.fedorenko@linux.dev>; Sagi Maimon
> <maimon.sagi@gmail.com>; Martin K. Petersen
> <martin.petersen@oracle.com>; Karan Tilak Kumar <kartilak@cisco.com>;
> Hans Verkuil <hverkuil+cisco@kernel.org>; Casey Schaufler
> <casey@schaufler-ca.com>; Steven Rostedt <rostedt@goodmis.org>; Petr
> Mladek <pmladek@suse.com>; Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>;
> Max Kellermann <max.kellermann@ionos.com>; linux-doc@vger.kernel.org;
> linux-kernel@vger.kernel.org; openipmi-
> developer@lists.sourceforge.net; linux-media@vger.kernel.org; dri-
> devel@lists.freedesktop.org; linaro-mm-sig@lists.linaro.org; amd-
> gfx@lists.freedesktop.org; linux-arm-msm@vger.kernel.org;
> freedreno@lists.freedesktop.org; intel-xe@lists.freedesktop.org;
> linux-mmc@vger.kernel.org; netdev@vger.kernel.org; intel-wired-
> lan@lists.osuosl.org; linux-pci@vger.kernel.org; linux-
> s390@vger.kernel.org; linux-scsi@vger.kernel.org; linux-
> staging@lists.linux.dev; ceph-devel@vger.kernel.org; linux-trace-
> kernel@vger.kernel.org
> Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>; Sergey Senozhatsky
> <senozhatsky@chromium.org>; Jonathan Corbet <corbet@lwn.net>; Sumit
> Semwal <sumit.semwal@linaro.org>; Gustavo Padovan
> <gustavo@padovan.org>; David Airlie <airlied@gmail.com>; Simona Vetter
> <simona@ffwll.ch>; Maarten Lankhorst
> <maarten.lankhorst@linux.intel.com>; Maxime Ripard
> <mripard@kernel.org>; Dmitry Baryshkov <lumag@kernel.org>; Abhinav
> Kumar <abhinav.kumar@linux.dev>; Jessica Zhang
> <jesszhan0024@gmail.com>; Sean Paul <sean@poorly.run>; Marijn Suijten
> <marijn.suijten@somainline.org>; Konrad Dybcio
> <konradybcio@kernel.org>; De Marchi, Lucas <lucas.demarchi@intel.com>;
> Thomas Hellström <thomas.hellstrom@linux.intel.com>; Vivi, Rodrigo
> <rodrigo.vivi@intel.com>; Vladimir Oltean <olteanv@gmail.com>; Andrew
> Lunn <andrew@lunn.ch>; David S. Miller <davem@davemloft.net>; Eric
> Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo
> Abeni <pabeni@redhat.com>; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>; Krzysztof Wilczyński
> <kwilczynski@kernel.org>; Kishon Vijay Abraham I <kishon@kernel.org>;
> Bjorn Helgaas <bhelgaas@google.com>; Rodolfo Giometti
> <giometti@enneenne.com>; Richard Cochran <richardcochran@gmail.com>;
> Jonathan Lemon <jonathan.lemon@gmail.com>; Stefan Haberland
> <sth@linux.ibm.com>; Jan Hoeppner <hoeppner@linux.ibm.com>; Heiko
> Carstens <hca@linux.ibm.com>; Vasily Gorbik <gor@linux.ibm.com>;
> Alexander Gordeev <agordeev@linux.ibm.com>; Christian Borntraeger
> <borntraeger@linux.ibm.com>; Sven Schnelle <svens@linux.ibm.com>;
> Satish Kharat <satishkh@cisco.com>; Baddela, Sesidhar
> <sebaddel@cisco.com>; James E.J. Bottomley
> <James.Bottomley@HansenPartnership.com>; Mauro Carvalho Chehab
> <mchehab@kernel.org>; Greg Kroah-Hartman <gregkh@linuxfoundation.org>;
> Xiubo Li <xiubli@redhat.com>; Ilya Dryomov <idryomov@gmail.com>;
> Masami Hiramatsu <mhiramat@kernel.org>; Mathieu Desnoyers
> <mathieu.desnoyers@efficios.com>; Andrew Morton <akpm@linux-
> foundation.org>
> Subject: [Intel-wired-lan] [PATCH v2 10/21] igb: Switch to use %ptSp
>
> Use %ptSp instead of open coded variants to print content of struct
> timespec64 in human readable format.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> drivers/net/ethernet/intel/igb/igb_ptp.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c
> b/drivers/net/ethernet/intel/igb/igb_ptp.c
> index a7876882aeaf..bd85d02ecadd 100644
> --- a/drivers/net/ethernet/intel/igb/igb_ptp.c
> +++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
> @@ -840,14 +840,11 @@ static void igb_ptp_overflow_check(struct
> work_struct *work)
> struct igb_adapter *igb =
> container_of(work, struct igb_adapter,
> ptp_overflow_work.work);
> struct timespec64 ts;
> - u64 ns;
>
> /* Update the timecounter */
> - ns = timecounter_read(&igb->tc);
> + ts = ns_to_timespec64(timecounter_read(&igb->tc));
>
> - ts = ns_to_timespec64(ns);
> - pr_debug("igb overflow check at %lld.%09lu\n",
> - (long long) ts.tv_sec, ts.tv_nsec);
> + pr_debug("igb overflow check at %ptSp\n", &ts);
>
> schedule_delayed_work(&igb->ptp_overflow_work,
> IGB_SYSTIM_OVERFLOW_PERIOD);
> --
> 2.50.1
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
^ permalink raw reply
* RE: [Intel-wired-lan] [PATCH v2 14/21] net: dsa: sja1105: Switch to use %ptSp
From: Loktionov, Aleksandr @ 2025-11-12 13:09 UTC (permalink / raw)
To: Andy Shevchenko, Corey Minyard, Christian König,
Dr. David Alan Gilbert, Alex Deucher, Thomas Zimmermann,
Dmitry Baryshkov, Rob Clark, Brost, Matthew, Ulf Hansson,
Lifshits, Vitaly, Manivannan Sadhasivam, Niklas Cassel,
Calvin Owens, Vadim Fedorenko, Sagi Maimon, Martin K. Petersen,
Karan Tilak Kumar, Hans Verkuil, Casey Schaufler, Steven Rostedt,
Petr Mladek, Viacheslav Dubeyko, Max Kellermann,
linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
openipmi-developer@lists.sourceforge.net,
linux-media@vger.kernel.org, dri-devel@lists.freedesktop.org,
linaro-mm-sig@lists.linaro.org, amd-gfx@lists.freedesktop.org,
linux-arm-msm@vger.kernel.org, freedreno@lists.freedesktop.org,
intel-xe@lists.freedesktop.org, linux-mmc@vger.kernel.org,
netdev@vger.kernel.org, intel-wired-lan@lists.osuosl.org,
linux-pci@vger.kernel.org, linux-s390@vger.kernel.org,
linux-scsi@vger.kernel.org, linux-staging@lists.linux.dev,
ceph-devel@vger.kernel.org, linux-trace-kernel@vger.kernel.org
Cc: Rasmus Villemoes, Sergey Senozhatsky, Jonathan Corbet,
Sumit Semwal, Gustavo Padovan, David Airlie, Simona Vetter,
Maarten Lankhorst, Maxime Ripard, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Sean Paul, Marijn Suijten, Konrad Dybcio,
De Marchi, Lucas, Thomas Hellström, Vivi, Rodrigo,
Vladimir Oltean, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Nguyen, Anthony L,
Kitszel, Przemyslaw, Krzysztof Wilczyński,
Kishon Vijay Abraham I, Bjorn Helgaas, Rodolfo Giometti,
Richard Cochran, Jonathan Lemon, Stefan Haberland, Jan Hoeppner,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Satish Kharat,
Baddela, Sesidhar, James E.J. Bottomley, Mauro Carvalho Chehab,
Greg Kroah-Hartman, Xiubo Li, Ilya Dryomov, Masami Hiramatsu,
Mathieu Desnoyers, Andrew Morton
In-Reply-To: <20251111122735.880607-15-andriy.shevchenko@linux.intel.com>
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> Of Andy Shevchenko
> Sent: Tuesday, November 11, 2025 1:20 PM
> To: Corey Minyard <corey@minyard.net>; Christian König
> <christian.koenig@amd.com>; Dr. David Alan Gilbert
> <linux@treblig.org>; Alex Deucher <alexander.deucher@amd.com>; Thomas
> Zimmermann <tzimmermann@suse.de>; Dmitry Baryshkov
> <dmitry.baryshkov@oss.qualcomm.com>; Rob Clark
> <robin.clark@oss.qualcomm.com>; Brost, Matthew
> <matthew.brost@intel.com>; Ulf Hansson <ulf.hansson@linaro.org>; Andy
> Shevchenko <andriy.shevchenko@linux.intel.com>; Lifshits, Vitaly
> <vitaly.lifshits@intel.com>; Manivannan Sadhasivam <mani@kernel.org>;
> Niklas Cassel <cassel@kernel.org>; Calvin Owens <calvin@wbinvd.org>;
> Vadim Fedorenko <vadim.fedorenko@linux.dev>; Sagi Maimon
> <maimon.sagi@gmail.com>; Martin K. Petersen
> <martin.petersen@oracle.com>; Karan Tilak Kumar <kartilak@cisco.com>;
> Hans Verkuil <hverkuil+cisco@kernel.org>; Casey Schaufler
> <casey@schaufler-ca.com>; Steven Rostedt <rostedt@goodmis.org>; Petr
> Mladek <pmladek@suse.com>; Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>;
> Max Kellermann <max.kellermann@ionos.com>; linux-doc@vger.kernel.org;
> linux-kernel@vger.kernel.org; openipmi-
> developer@lists.sourceforge.net; linux-media@vger.kernel.org; dri-
> devel@lists.freedesktop.org; linaro-mm-sig@lists.linaro.org; amd-
> gfx@lists.freedesktop.org; linux-arm-msm@vger.kernel.org;
> freedreno@lists.freedesktop.org; intel-xe@lists.freedesktop.org;
> linux-mmc@vger.kernel.org; netdev@vger.kernel.org; intel-wired-
> lan@lists.osuosl.org; linux-pci@vger.kernel.org; linux-
> s390@vger.kernel.org; linux-scsi@vger.kernel.org; linux-
> staging@lists.linux.dev; ceph-devel@vger.kernel.org; linux-trace-
> kernel@vger.kernel.org
> Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>; Sergey Senozhatsky
> <senozhatsky@chromium.org>; Jonathan Corbet <corbet@lwn.net>; Sumit
> Semwal <sumit.semwal@linaro.org>; Gustavo Padovan
> <gustavo@padovan.org>; David Airlie <airlied@gmail.com>; Simona Vetter
> <simona@ffwll.ch>; Maarten Lankhorst
> <maarten.lankhorst@linux.intel.com>; Maxime Ripard
> <mripard@kernel.org>; Dmitry Baryshkov <lumag@kernel.org>; Abhinav
> Kumar <abhinav.kumar@linux.dev>; Jessica Zhang
> <jesszhan0024@gmail.com>; Sean Paul <sean@poorly.run>; Marijn Suijten
> <marijn.suijten@somainline.org>; Konrad Dybcio
> <konradybcio@kernel.org>; De Marchi, Lucas <lucas.demarchi@intel.com>;
> Thomas Hellström <thomas.hellstrom@linux.intel.com>; Vivi, Rodrigo
> <rodrigo.vivi@intel.com>; Vladimir Oltean <olteanv@gmail.com>; Andrew
> Lunn <andrew@lunn.ch>; David S. Miller <davem@davemloft.net>; Eric
> Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo
> Abeni <pabeni@redhat.com>; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>; Krzysztof Wilczyński
> <kwilczynski@kernel.org>; Kishon Vijay Abraham I <kishon@kernel.org>;
> Bjorn Helgaas <bhelgaas@google.com>; Rodolfo Giometti
> <giometti@enneenne.com>; Richard Cochran <richardcochran@gmail.com>;
> Jonathan Lemon <jonathan.lemon@gmail.com>; Stefan Haberland
> <sth@linux.ibm.com>; Jan Hoeppner <hoeppner@linux.ibm.com>; Heiko
> Carstens <hca@linux.ibm.com>; Vasily Gorbik <gor@linux.ibm.com>;
> Alexander Gordeev <agordeev@linux.ibm.com>; Christian Borntraeger
> <borntraeger@linux.ibm.com>; Sven Schnelle <svens@linux.ibm.com>;
> Satish Kharat <satishkh@cisco.com>; Baddela, Sesidhar
> <sebaddel@cisco.com>; James E.J. Bottomley
> <James.Bottomley@HansenPartnership.com>; Mauro Carvalho Chehab
> <mchehab@kernel.org>; Greg Kroah-Hartman <gregkh@linuxfoundation.org>;
> Xiubo Li <xiubli@redhat.com>; Ilya Dryomov <idryomov@gmail.com>;
> Masami Hiramatsu <mhiramat@kernel.org>; Mathieu Desnoyers
> <mathieu.desnoyers@efficios.com>; Andrew Morton <akpm@linux-
> foundation.org>
> Subject: [Intel-wired-lan] [PATCH v2 14/21] net: dsa: sja1105: Switch
> to use %ptSp
>
> Use %ptSp instead of open coded variants to print content of struct
> timespec64 in human readable format.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> drivers/net/dsa/sja1105/sja1105_tas.c | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/dsa/sja1105/sja1105_tas.c
> b/drivers/net/dsa/sja1105/sja1105_tas.c
> index d7818710bc02..d5949d2c3e71 100644
> --- a/drivers/net/dsa/sja1105/sja1105_tas.c
> +++ b/drivers/net/dsa/sja1105/sja1105_tas.c
> @@ -775,9 +775,8 @@ static void sja1105_tas_state_machine(struct
> work_struct *work)
> base_time_ts = ns_to_timespec64(base_time);
> now_ts = ns_to_timespec64(now);
>
> - dev_dbg(ds->dev, "OPER base time %lld.%09ld (now
> %lld.%09ld)\n",
> - base_time_ts.tv_sec, base_time_ts.tv_nsec,
> - now_ts.tv_sec, now_ts.tv_nsec);
> + dev_dbg(ds->dev, "OPER base time %ptSp (now %ptSp)\n",
> + &base_time_ts, &now_ts);
>
> break;
>
> @@ -798,8 +797,7 @@ static void sja1105_tas_state_machine(struct
> work_struct *work)
> if (now < tas_data->oper_base_time) {
> /* TAS has not started yet */
> diff = ns_to_timespec64(tas_data->oper_base_time
> - now);
> - dev_dbg(ds->dev, "time to start: [%lld.%09ld]",
> - diff.tv_sec, diff.tv_nsec);
> + dev_dbg(ds->dev, "time to start: [%ptSp]",
> &diff);
> break;
> }
>
> --
> 2.50.1
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
^ permalink raw reply
* [PATCH] selftests: tracing: Update fprobe selftest for ftrace based fprobe
From: Masami Hiramatsu (Google) @ 2025-11-12 13:13 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Shuah Khan, Heiko Carstens
Cc: Mathieu Desnoyers, linux-kernel, linux-trace-kernel,
linux-kselftest
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Since the ftrace fprobe is both fgraph and ftrace based implemented,
the selftest needs to be updated. This does not count the actual
number of lines, but just check the differences.
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
.../ftrace/test.d/dynevent/add_remove_fprobe.tc | 18 ++++--------------
1 file changed, 4 insertions(+), 14 deletions(-)
diff --git a/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_fprobe.tc b/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_fprobe.tc
index 2506f464811b..47067a5e3cb0 100644
--- a/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_fprobe.tc
+++ b/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_fprobe.tc
@@ -28,25 +28,21 @@ test -d events/fprobes/myevent1
test -d events/fprobes/myevent2
echo 1 > events/fprobes/myevent1/enable
-# Make sure the event is attached and is the only one
+# Make sure the event is attached.
grep -q $PLACE enabled_functions
cnt=`cat enabled_functions | wc -l`
-if [ $cnt -ne $((ocnt + 1)) ]; then
+if [ $cnt -eq $ocnt ]; then
exit_fail
fi
echo 1 > events/fprobes/myevent2/enable
-# It should till be the only attached function
-cnt=`cat enabled_functions | wc -l`
-if [ $cnt -ne $((ocnt + 1)) ]; then
- exit_fail
-fi
+cnt2=`cat enabled_functions | wc -l`
echo 1 > events/fprobes/myevent3/enable
# If the function is different, the attached function should be increased
grep -q $PLACE2 enabled_functions
cnt=`cat enabled_functions | wc -l`
-if [ $cnt -ne $((ocnt + 2)) ]; then
+if [ $cnt -eq $cnt2 ]; then
exit_fail
fi
@@ -56,12 +52,6 @@ echo "-:myevent2" >> dynamic_events
grep -q myevent1 dynamic_events
! grep -q myevent2 dynamic_events
-# should still have 2 left
-cnt=`cat enabled_functions | wc -l`
-if [ $cnt -ne $((ocnt + 2)) ]; then
- exit_fail
-fi
-
echo 0 > events/fprobes/enable
echo > dynamic_events
^ permalink raw reply related
* Re: [PATCH v5 6/8] selftests: tracing: Add a basic testcase for wprobe
From: Masami Hiramatsu @ 2025-11-12 13:30 UTC (permalink / raw)
To: Mark Brown
Cc: Catalin Marinas, Will Deacon, Steven Rostedt, Peter Zijlstra,
Ingo Molnar, x86, Jinchao Wang, Mathieu Desnoyers,
Thomas Gleixner, Borislav Petkov, Dave Hansen, H . Peter Anvin,
Alexander Shishkin, Ian Rogers, linux-kernel, linux-trace-kernel,
linux-doc, linux-perf-users, linux-arm-kernel, Aishwarya.TCV
In-Reply-To: <af8f3543-4af6-48d7-a55a-bab24b6c7c7a@sirena.org.uk>
On Wed, 12 Nov 2025 12:15:03 +0000
Mark Brown <broonie@kernel.org> wrote:
> On Thu, Oct 30, 2025 at 09:09:52AM +0900, Masami Hiramatsu wrote:
> > Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
>
> > There is a single step execution code but only for default overflow_handlers.
> > This is a bit strange becuase other users can not set it up outside of
> > the arch dependent code. Even if it can, it is simply redundant.
>
> > So I made changes below which allow users to set its own custom handler is
> > compatible with perf default overflow handlers.
> > I confirmed this works on both arm and arm64.
>
> I think everyone who knows this code is really snowed under at the
> minute - can I suggest posting this as a fix for now? This is still
> broken on arm64, we'll need a fix (or to disable the feature on the
> affected arches) one way or another for v6.19.
Sorry, let me drop it from for-next soon.
Thanks,
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* [PATCH v2 0/7] kallsyms: Prevent invalid access when showing module buildid
From: Petr Mladek @ 2025-11-12 14:19 UTC (permalink / raw)
To: Petr Pavlu, Steven Rostedt, Alexei Starovoitov, Andrew Morton,
Kees Cook
Cc: Aaron Tomlin, Daniel Borkmann, John Fastabend, Masami Hiramatsu,
Mark Rutland, Luis Chamberlain, Daniel Gomez, Sami Tolvanen,
linux-kernel, bpf, linux-modules, linux-trace-kernel, Petr Mladek
This patchset is cleaning up kallsyms code related to module buildid.
It is fixing an invalid access when printing backtraces, see [v1] for
more details:
+ 1st..4th patches are preparatory.
+ 5th and 6th patches are fixing bpf and ftrace related APIs.
+ 7th patch prevents a potential race.
Changes against [v1]:
+ Added existing Reviewed-by tags.
+ Shuffled patches to update the kallsyms_lookup_buildid() initialization
code 1st.
+ Initialized also *modname and *modbuildid in kallsyms_lookup_buildid().
+ Renamed __bpf_address_lookup() to bpf_address_lookup() and used it
in kallsyms_lookup_buildid(). Did this instead of passing @modbuildid
parameter just to clear it.
[v1] https://lore.kernel.org/r/20251105142319.1139183-1-pmladek@suse.com
Petr Mladek (7):
kallsyms: Clean up @namebuf initialization in
kallsyms_lookup_buildid()
kallsyms: Clean up modname and modbuildid initialization in
kallsyms_lookup_buildid()
module: Add helper function for reading module_buildid()
kallsyms: Cleanup code for appending the module buildid
kallsyms/bpf: Rename __bpf_address_lookup() to bpf_address_lookup()
kallsyms/ftrace: Set module buildid in ftrace_mod_address_lookup()
kallsyms: Prevent module removal when printing module name and buildid
arch/arm64/net/bpf_jit_comp.c | 2 +-
arch/powerpc/net/bpf_jit_comp.c | 2 +-
include/linux/filter.h | 26 ++----------
include/linux/ftrace.h | 6 ++-
include/linux/module.h | 9 ++++
kernel/bpf/core.c | 4 +-
kernel/kallsyms.c | 73 ++++++++++++++++++++++++---------
kernel/module/kallsyms.c | 9 +---
kernel/trace/ftrace.c | 5 ++-
9 files changed, 81 insertions(+), 55 deletions(-)
--
2.51.1
^ permalink raw reply
* [PATCH v2 1/7] kallsyms: Clean up @namebuf initialization in kallsyms_lookup_buildid()
From: Petr Mladek @ 2025-11-12 14:19 UTC (permalink / raw)
To: Petr Pavlu, Steven Rostedt, Alexei Starovoitov, Andrew Morton,
Kees Cook
Cc: Aaron Tomlin, Daniel Borkmann, John Fastabend, Masami Hiramatsu,
Mark Rutland, Luis Chamberlain, Daniel Gomez, Sami Tolvanen,
linux-kernel, bpf, linux-modules, linux-trace-kernel, Petr Mladek
In-Reply-To: <20251112142003.182062-1-pmladek@suse.com>
The function kallsyms_lookup_buildid() initializes the given @namebuf
by clearing the first and the last byte. It is not clear why.
The 1st byte makes sense because some callers ignore the return code
and expect that the buffer contains a valid string, for example:
- function_stat_show()
- kallsyms_lookup()
- kallsyms_lookup_buildid()
The initialization of the last byte does not make much sense because it
can later be overwritten. Fortunately, it seems that all called
functions behave correctly:
- kallsyms_expand_symbol() explicitly adds the trailing '\0'
at the end of the function.
- All *__address_lookup() functions either use the safe strscpy()
or they do not touch the buffer at all.
Document the reason for clearing the first byte. And remove the useless
initialization of the last byte.
Reviewed-by: Aaron Tomlin <atomlin@atomlin.com>
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
kernel/kallsyms.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index 1e7635864124..e08c1e57fc0d 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -352,7 +352,12 @@ static int kallsyms_lookup_buildid(unsigned long addr,
{
int ret;
- namebuf[KSYM_NAME_LEN - 1] = 0;
+ /*
+ * kallsyms_lookus() returns pointer to namebuf on success and
+ * NULL on error. But some callers ignore the return value.
+ * Instead they expect @namebuf filled either with valid
+ * or empty string.
+ */
namebuf[0] = 0;
if (is_ksym_addr(addr)) {
--
2.51.1
^ permalink raw reply related
* [PATCH v2 2/7] kallsyms: Clean up modname and modbuildid initialization in kallsyms_lookup_buildid()
From: Petr Mladek @ 2025-11-12 14:19 UTC (permalink / raw)
To: Petr Pavlu, Steven Rostedt, Alexei Starovoitov, Andrew Morton,
Kees Cook
Cc: Aaron Tomlin, Daniel Borkmann, John Fastabend, Masami Hiramatsu,
Mark Rutland, Luis Chamberlain, Daniel Gomez, Sami Tolvanen,
linux-kernel, bpf, linux-modules, linux-trace-kernel, Petr Mladek
In-Reply-To: <20251112142003.182062-1-pmladek@suse.com>
The @modname and @modbuildid optional return parameters are set only
when the symbol is in a module.
Always initialize them so that they do not need to be cleared when
the module is not in a module. It simplifies the logic and makes
the code even slightly more safe.
Note that bpf_address_lookup() function will get updated in a separate
patch.
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
kernel/kallsyms.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index e08c1e57fc0d..ffb64eaa0505 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -359,6 +359,14 @@ static int kallsyms_lookup_buildid(unsigned long addr,
* or empty string.
*/
namebuf[0] = 0;
+ /*
+ * Initialize the module-related return values. They are not set
+ * when the symbol is in vmlinux or it is a bpf address.
+ */
+ if (modname)
+ *modname = NULL;
+ if (modbuildid)
+ *modbuildid = NULL;
if (is_ksym_addr(addr)) {
unsigned long pos;
@@ -367,10 +375,6 @@ static int kallsyms_lookup_buildid(unsigned long addr,
/* Grab name */
kallsyms_expand_symbol(get_symbol_offset(pos),
namebuf, KSYM_NAME_LEN);
- if (modname)
- *modname = NULL;
- if (modbuildid)
- *modbuildid = NULL;
return strlen(namebuf);
}
--
2.51.1
^ permalink raw reply related
* [PATCH v2 3/7] module: Add helper function for reading module_buildid()
From: Petr Mladek @ 2025-11-12 14:19 UTC (permalink / raw)
To: Petr Pavlu, Steven Rostedt, Alexei Starovoitov, Andrew Morton,
Kees Cook
Cc: Aaron Tomlin, Daniel Borkmann, John Fastabend, Masami Hiramatsu,
Mark Rutland, Luis Chamberlain, Daniel Gomez, Sami Tolvanen,
linux-kernel, bpf, linux-modules, linux-trace-kernel, Petr Mladek,
Daniel Gomez
In-Reply-To: <20251112142003.182062-1-pmladek@suse.com>
Add a helper function for reading the optional "build_id" member
of struct module. It is going to be used also in
ftrace_mod_address_lookup().
Use "#ifdef" instead of "#if IS_ENABLED()" to match the declaration
of the optional field in struct module.
Reviewed-by: Daniel Gomez <da.gomez@samsung.com>
Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
include/linux/module.h | 9 +++++++++
kernel/module/kallsyms.c | 9 ++-------
2 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/include/linux/module.h b/include/linux/module.h
index e135cc79acee..4decae2b1675 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -747,6 +747,15 @@ static inline void __module_get(struct module *module)
__mod ? __mod->name : "kernel"; \
})
+static inline const unsigned char *module_buildid(struct module *mod)
+{
+#ifdef CONFIG_STACKTRACE_BUILD_ID
+ return mod->build_id;
+#else
+ return NULL;
+#endif
+}
+
/* Dereference module function descriptor */
void *dereference_module_function_descriptor(struct module *mod, void *ptr);
diff --git a/kernel/module/kallsyms.c b/kernel/module/kallsyms.c
index 00a60796327c..0fc11e45df9b 100644
--- a/kernel/module/kallsyms.c
+++ b/kernel/module/kallsyms.c
@@ -334,13 +334,8 @@ int module_address_lookup(unsigned long addr,
if (mod) {
if (modname)
*modname = mod->name;
- if (modbuildid) {
-#if IS_ENABLED(CONFIG_STACKTRACE_BUILD_ID)
- *modbuildid = mod->build_id;
-#else
- *modbuildid = NULL;
-#endif
- }
+ if (modbuildid)
+ *modbuildid = module_buildid(mod);
sym = find_kallsyms_symbol(mod, addr, size, offset);
--
2.51.1
^ permalink raw reply related
* [PATCH v2 4/7] kallsyms: Cleanup code for appending the module buildid
From: Petr Mladek @ 2025-11-12 14:20 UTC (permalink / raw)
To: Petr Pavlu, Steven Rostedt, Alexei Starovoitov, Andrew Morton,
Kees Cook
Cc: Aaron Tomlin, Daniel Borkmann, John Fastabend, Masami Hiramatsu,
Mark Rutland, Luis Chamberlain, Daniel Gomez, Sami Tolvanen,
linux-kernel, bpf, linux-modules, linux-trace-kernel, Petr Mladek
In-Reply-To: <20251112142003.182062-1-pmladek@suse.com>
Put the code for appending the optional "buildid" into a helper
function, It makes __sprint_symbol() better readable.
Also print a warning when the "modname" is set and the "buildid" isn't.
It might catch a situation when some lookup function in
kallsyms_lookup_buildid() does not handle the "buildid".
Use pr_*_once() to avoid an infinite recursion when the function
is called from printk(). The recursion is rather theoretical but
better be on the safe side.
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
kernel/kallsyms.c | 42 +++++++++++++++++++++++++++++++++---------
1 file changed, 33 insertions(+), 9 deletions(-)
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index ffb64eaa0505..f25b122397ce 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -432,6 +432,37 @@ int lookup_symbol_name(unsigned long addr, char *symname)
return lookup_module_symbol_name(addr, symname);
}
+#ifdef CONFIG_STACKTRACE_BUILD_ID
+
+static int append_buildid(char *buffer, const char *modname,
+ const unsigned char *buildid)
+{
+ if (!modname)
+ return 0;
+
+ if (!buildid) {
+ pr_warn_once("Undefined buildid for the module %s\n", modname);
+ return 0;
+ }
+
+ /* build ID should match length of sprintf */
+#ifdef CONFIG_MODULES
+ static_assert(sizeof(typeof_member(struct module, build_id)) == 20);
+#endif
+
+ return sprintf(buffer, " %20phN", buildid);
+}
+
+#else /* CONFIG_STACKTRACE_BUILD_ID */
+
+static int append_buildid(char *buffer, const char *modname,
+ const unsigned char *buildid)
+{
+ return 0;
+}
+
+#endif /* CONFIG_STACKTRACE_BUILD_ID */
+
/* Look up a kernel symbol and return it in a text buffer. */
static int __sprint_symbol(char *buffer, unsigned long address,
int symbol_offset, int add_offset, int add_buildid)
@@ -454,15 +485,8 @@ static int __sprint_symbol(char *buffer, unsigned long address,
if (modname) {
len += sprintf(buffer + len, " [%s", modname);
-#if IS_ENABLED(CONFIG_STACKTRACE_BUILD_ID)
- if (add_buildid && buildid) {
- /* build ID should match length of sprintf */
-#if IS_ENABLED(CONFIG_MODULES)
- static_assert(sizeof(typeof_member(struct module, build_id)) == 20);
-#endif
- len += sprintf(buffer + len, " %20phN", buildid);
- }
-#endif
+ if (add_buildid)
+ len += append_buildid(buffer + len, modname, buildid);
len += sprintf(buffer + len, "]");
}
--
2.51.1
^ permalink raw reply related
* [PATCH v2 5/7] kallsyms/bpf: Rename __bpf_address_lookup() to bpf_address_lookup()
From: Petr Mladek @ 2025-11-12 14:20 UTC (permalink / raw)
To: Petr Pavlu, Steven Rostedt, Alexei Starovoitov, Andrew Morton,
Kees Cook
Cc: Aaron Tomlin, Daniel Borkmann, John Fastabend, Masami Hiramatsu,
Mark Rutland, Luis Chamberlain, Daniel Gomez, Sami Tolvanen,
linux-kernel, bpf, linux-modules, linux-trace-kernel, Petr Mladek
In-Reply-To: <20251112142003.182062-1-pmladek@suse.com>
bpf_address_lookup() has been used only in kallsyms_lookup_buildid().
It was supposed to set @modname and @modbuildid when the symbol was
in a module.
But it always just cleared @modname because BPF symbols were never in
a module. And it did not clear @modbuildid because the pointer was
not passed.
The wrapper is not longer needed. Both @modname and @modbuildid
are newly always initialized to NULL in kallsyms_lookup_buildid().
Remove the wrapper and rename __bpf_address_lookup() to
bpf_address_lookup() because this variant is used everywhere.
Fixes: 9294523e3768 ("module: add printk formats to add module build ID to stacktraces")
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
arch/arm64/net/bpf_jit_comp.c | 2 +-
arch/powerpc/net/bpf_jit_comp.c | 2 +-
include/linux/filter.h | 26 ++++----------------------
kernel/bpf/core.c | 4 ++--
kernel/kallsyms.c | 5 ++---
5 files changed, 10 insertions(+), 29 deletions(-)
diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
index 0c9a50a1e73e..17e6a041ea4d 100644
--- a/arch/arm64/net/bpf_jit_comp.c
+++ b/arch/arm64/net/bpf_jit_comp.c
@@ -2939,7 +2939,7 @@ int bpf_arch_text_poke(void *ip, enum bpf_text_poke_type poke_type,
u64 plt_target = 0ULL;
bool poking_bpf_entry;
- if (!__bpf_address_lookup((unsigned long)ip, &size, &offset, namebuf))
+ if (!bpf_address_lookup((unsigned long)ip, &size, &offset, namebuf))
/* Only poking bpf text is supported. Since kernel function
* entry is set up by ftrace, we reply on ftrace to poke kernel
* functions.
diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c
index 88ad5ba7b87f..21f7f26a5e2f 100644
--- a/arch/powerpc/net/bpf_jit_comp.c
+++ b/arch/powerpc/net/bpf_jit_comp.c
@@ -1122,7 +1122,7 @@ int bpf_arch_text_poke(void *ip, enum bpf_text_poke_type poke_type,
branch_flags = poke_type == BPF_MOD_CALL ? BRANCH_SET_LINK : 0;
/* We currently only support poking bpf programs */
- if (!__bpf_address_lookup(bpf_func, &size, &offset, name)) {
+ if (!bpf_address_lookup(bpf_func, &size, &offset, name)) {
pr_err("%s (0x%lx): kernel/modules are not supported\n", __func__, bpf_func);
return -EOPNOTSUPP;
}
diff --git a/include/linux/filter.h b/include/linux/filter.h
index f5c859b8131a..d500338af6e0 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -1353,24 +1353,13 @@ static inline bool bpf_jit_kallsyms_enabled(void)
return false;
}
-int __bpf_address_lookup(unsigned long addr, unsigned long *size,
- unsigned long *off, char *sym);
+int bpf_address_lookup(unsigned long addr, unsigned long *size,
+ unsigned long *off, char *sym);
bool is_bpf_text_address(unsigned long addr);
int bpf_get_kallsym(unsigned int symnum, unsigned long *value, char *type,
char *sym);
struct bpf_prog *bpf_prog_ksym_find(unsigned long addr);
-static inline int
-bpf_address_lookup(unsigned long addr, unsigned long *size,
- unsigned long *off, char **modname, char *sym)
-{
- int ret = __bpf_address_lookup(addr, size, off, sym);
-
- if (ret && modname)
- *modname = NULL;
- return ret;
-}
-
void bpf_prog_kallsyms_add(struct bpf_prog *fp);
void bpf_prog_kallsyms_del(struct bpf_prog *fp);
@@ -1409,8 +1398,8 @@ static inline bool bpf_jit_kallsyms_enabled(void)
}
static inline int
-__bpf_address_lookup(unsigned long addr, unsigned long *size,
- unsigned long *off, char *sym)
+bpf_address_lookup(unsigned long addr, unsigned long *size,
+ unsigned long *off, char *sym)
{
return 0;
}
@@ -1431,13 +1420,6 @@ static inline struct bpf_prog *bpf_prog_ksym_find(unsigned long addr)
return NULL;
}
-static inline int
-bpf_address_lookup(unsigned long addr, unsigned long *size,
- unsigned long *off, char **modname, char *sym)
-{
- return 0;
-}
-
static inline void bpf_prog_kallsyms_add(struct bpf_prog *fp)
{
}
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index d595fe512498..c2278f392e93 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -713,8 +713,8 @@ static struct bpf_ksym *bpf_ksym_find(unsigned long addr)
return n ? container_of(n, struct bpf_ksym, tnode) : NULL;
}
-int __bpf_address_lookup(unsigned long addr, unsigned long *size,
- unsigned long *off, char *sym)
+int bpf_address_lookup(unsigned long addr, unsigned long *size,
+ unsigned long *off, char *sym)
{
struct bpf_ksym *ksym;
int ret = 0;
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index f25b122397ce..97b92fc8871d 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -342,7 +342,7 @@ int kallsyms_lookup_size_offset(unsigned long addr, unsigned long *symbolsize,
return 1;
}
return !!module_address_lookup(addr, symbolsize, offset, NULL, NULL, namebuf) ||
- !!__bpf_address_lookup(addr, symbolsize, offset, namebuf);
+ !!bpf_address_lookup(addr, symbolsize, offset, namebuf);
}
static int kallsyms_lookup_buildid(unsigned long addr,
@@ -383,8 +383,7 @@ static int kallsyms_lookup_buildid(unsigned long addr,
ret = module_address_lookup(addr, symbolsize, offset,
modname, modbuildid, namebuf);
if (!ret)
- ret = bpf_address_lookup(addr, symbolsize,
- offset, modname, namebuf);
+ ret = bpf_address_lookup(addr, symbolsize, offset, namebuf);
if (!ret)
ret = ftrace_mod_address_lookup(addr, symbolsize,
--
2.51.1
^ permalink raw reply related
* [PATCH v2 6/7] kallsyms/ftrace: Set module buildid in ftrace_mod_address_lookup()
From: Petr Mladek @ 2025-11-12 14:20 UTC (permalink / raw)
To: Petr Pavlu, Steven Rostedt, Alexei Starovoitov, Andrew Morton,
Kees Cook
Cc: Aaron Tomlin, Daniel Borkmann, John Fastabend, Masami Hiramatsu,
Mark Rutland, Luis Chamberlain, Daniel Gomez, Sami Tolvanen,
linux-kernel, bpf, linux-modules, linux-trace-kernel, Petr Mladek
In-Reply-To: <20251112142003.182062-1-pmladek@suse.com>
__sprint_symbol() might access an invalid pointer when
kallsyms_lookup_buildid() returns a symbol found by
ftrace_mod_address_lookup().
The ftrace lookup function must set both @modname and @modbuildid
the same way as module_address_lookup().
Fixes: 9294523e3768 ("module: add printk formats to add module build ID to stacktraces")
Reviewed-by: Aaron Tomlin <atomlin@atomlin.com>
Acked-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
include/linux/ftrace.h | 6 ++++--
kernel/kallsyms.c | 4 ++--
kernel/trace/ftrace.c | 5 ++++-
3 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 7ded7df6e9b5..a003cf1b32d0 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -87,11 +87,13 @@ struct ftrace_hash;
defined(CONFIG_DYNAMIC_FTRACE)
int
ftrace_mod_address_lookup(unsigned long addr, unsigned long *size,
- unsigned long *off, char **modname, char *sym);
+ unsigned long *off, char **modname,
+ const unsigned char **modbuildid, char *sym);
#else
static inline int
ftrace_mod_address_lookup(unsigned long addr, unsigned long *size,
- unsigned long *off, char **modname, char *sym)
+ unsigned long *off, char **modname,
+ const unsigned char **modbuildid, char *sym)
{
return 0;
}
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index 97b92fc8871d..5bc1646f8639 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -386,8 +386,8 @@ static int kallsyms_lookup_buildid(unsigned long addr,
ret = bpf_address_lookup(addr, symbolsize, offset, namebuf);
if (!ret)
- ret = ftrace_mod_address_lookup(addr, symbolsize,
- offset, modname, namebuf);
+ ret = ftrace_mod_address_lookup(addr, symbolsize, offset,
+ modname, modbuildid, namebuf);
return ret;
}
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 42bd2ba68a82..11f5096fb60c 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -7678,7 +7678,8 @@ ftrace_func_address_lookup(struct ftrace_mod_map *mod_map,
int
ftrace_mod_address_lookup(unsigned long addr, unsigned long *size,
- unsigned long *off, char **modname, char *sym)
+ unsigned long *off, char **modname,
+ const unsigned char **modbuildid, char *sym)
{
struct ftrace_mod_map *mod_map;
int ret = 0;
@@ -7690,6 +7691,8 @@ ftrace_mod_address_lookup(unsigned long addr, unsigned long *size,
if (ret) {
if (modname)
*modname = mod_map->mod->name;
+ if (modbuildid)
+ *modbuildid = module_buildid(mod_map->mod);
break;
}
}
--
2.51.1
^ permalink raw reply related
* [PATCH v2 7/7] kallsyms: Prevent module removal when printing module name and buildid
From: Petr Mladek @ 2025-11-12 14:20 UTC (permalink / raw)
To: Petr Pavlu, Steven Rostedt, Alexei Starovoitov, Andrew Morton,
Kees Cook
Cc: Aaron Tomlin, Daniel Borkmann, John Fastabend, Masami Hiramatsu,
Mark Rutland, Luis Chamberlain, Daniel Gomez, Sami Tolvanen,
linux-kernel, bpf, linux-modules, linux-trace-kernel, Petr Mladek
In-Reply-To: <20251112142003.182062-1-pmladek@suse.com>
kallsyms_lookup_buildid() copies the symbol name into the given buffer
so that it can be safely read anytime later. But it just copies pointers
to mod->name and mod->build_id which might get reused after the related
struct module gets removed.
The lifetime of struct module is synchronized using RCU. Take the rcu
read lock for the entire __sprint_symbol().
Reviewed-by: Aaron Tomlin <atomlin@atomlin.com>
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
kernel/kallsyms.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index 5bc1646f8639..202d39f5493a 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -471,6 +471,9 @@ static int __sprint_symbol(char *buffer, unsigned long address,
unsigned long offset, size;
int len;
+ /* Prevent module removal until modname and modbuildid are printed */
+ guard(rcu)();
+
address += symbol_offset;
len = kallsyms_lookup_buildid(address, &size, &offset, &modname, &buildid,
buffer);
--
2.51.1
^ permalink raw reply related
* [PATCH 0/5] rtla, osnoise: Fixes to address intermittent test issues
From: Crystal Wood @ 2025-11-12 15:25 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-trace-kernel, John Kacur, Tomas Glozar, Costa Shulyupin,
Wander Lairson Costa, Crystal Wood
With these fixes, I'm able to run the tests in a loop overnight with no
failures.
Crystal Wood (5):
rtla/tests: Don't rely on matching ^1ALL
tracing/osnoise: Dump stack on timerlat uret threshold event
tracing/osnoise: Array printk init and cleanup
rtla/timerlat: Exit top main loop on any non-zero wait_retval
rtla: Set stop threshold after all instances are enabled
kernel/trace/trace_osnoise.c | 46 +++++++++++++++++----------
tools/tracing/rtla/src/common.c | 20 ++++++++++++
tools/tracing/rtla/src/common.h | 4 +++
tools/tracing/rtla/src/osnoise.c | 17 +++-------
tools/tracing/rtla/src/osnoise.h | 5 ---
tools/tracing/rtla/src/timerlat.c | 29 ++++++-----------
tools/tracing/rtla/src/timerlat_top.c | 2 +-
tools/tracing/rtla/tests/timerlat.t | 2 +-
8 files changed, 69 insertions(+), 56 deletions(-)
--
2.48.1
^ permalink raw reply
* [PATCH 1/5] rtla/tests: Don't rely on matching ^1ALL
From: Crystal Wood @ 2025-11-12 15:25 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-trace-kernel, John Kacur, Tomas Glozar, Costa Shulyupin,
Wander Lairson Costa, Crystal Wood
In-Reply-To: <20251112152529.956778-1-crwood@redhat.com>
The timerlat "top stop at failed action" test was relying on "ALL" being
printed immediately after the "1" from the threshold action. Besides being
fragile, this depends on stdbuf behavior, which is easy to miss when
recreating the test outside of the framework for debugging purposes.
Instead, use the expected/unexpected text mechanism from the
corresponding osnoise test.
Signed-off-by: Crystal Wood <crwood@redhat.com>
---
tools/tracing/rtla/tests/timerlat.t | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/tracing/rtla/tests/timerlat.t b/tools/tracing/rtla/tests/timerlat.t
index d1cc7106ae26..fd4935fd7b49 100644
--- a/tools/tracing/rtla/tests/timerlat.t
+++ b/tools/tracing/rtla/tests/timerlat.t
@@ -58,7 +58,7 @@ check "multiple actions" \
check "hist stop at failed action" \
"timerlat hist -T 2 --on-threshold shell,command='echo -n 1; false' --on-threshold shell,command='echo -n 2'" 2 "^1# RTLA timerlat histogram$"
check "top stop at failed action" \
- "timerlat top -T 2 --on-threshold shell,command='echo -n 1; false' --on-threshold shell,command='echo -n 2'" 2 "^1ALL"
+ "timerlat top -T 2 --on-threshold shell,command='echo -n abc; false' --on-threshold shell,command='echo -n defgh'" 2 "^abc" "defgh"
check "hist with continue" \
"timerlat hist -T 2 -d 5s --on-threshold shell,command='echo TestOutput' --on-threshold continue" 0 "^TestOutput$"
check "top with continue" \
--
2.48.1
^ permalink raw reply related
* [PATCH 2/5] tracing/osnoise: Dump stack on timerlat uret threshold event
From: Crystal Wood @ 2025-11-12 15:25 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-trace-kernel, John Kacur, Tomas Glozar, Costa Shulyupin,
Wander Lairson Costa, Crystal Wood
In-Reply-To: <20251112152529.956778-1-crwood@redhat.com>
Dump the saved IRQ stack trace regardless of whether the event was
THREAD_CONTEXT or THREAD_URET.
In the uret case, the latency presumably had not yet crossed the
threshold at IRQ time (or else it would have dumped the stack at thread
wakeup time, unless we're racing with a change to the threshold), but it
may have at least contributed -- and this is possible with THREAD_CONTEXT
as well.
In any case, it helps with writing reliable rtla tests if we always get
a stack trace on a threshold event.
Signed-off-by: Crystal Wood <crwood@redhat.com>
---
kernel/trace/trace_osnoise.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/trace_osnoise.c b/kernel/trace/trace_osnoise.c
index a9962d4497e8..bb4d195ec74f 100644
--- a/kernel/trace/trace_osnoise.c
+++ b/kernel/trace/trace_osnoise.c
@@ -2508,9 +2508,12 @@ timerlat_fd_read(struct file *file, char __user *ubuf, size_t count,
notify_new_max_latency(diff);
tlat->tracing_thread = false;
- if (osnoise_data.stop_tracing_total)
- if (time_to_us(diff) >= osnoise_data.stop_tracing_total)
+ if (osnoise_data.stop_tracing_total) {
+ if (time_to_us(diff) >= osnoise_data.stop_tracing_total) {
+ timerlat_dump_stack(time_to_us(diff));
osnoise_stop_tracing();
+ }
+ }
} else {
tlat->tracing_thread = false;
tlat->kthread = current;
--
2.48.1
^ permalink raw reply related
* [PATCH 3/5] tracing/osnoise: Array printk init and cleanup
From: Crystal Wood @ 2025-11-12 15:25 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-trace-kernel, John Kacur, Tomas Glozar, Costa Shulyupin,
Wander Lairson Costa, Crystal Wood
In-Reply-To: <20251112152529.956778-1-crwood@redhat.com>
None of the calls to trace_array_printk_buf() will do anything
if we don't initialize the buffer on instance creation (unless
some other tracer called it), so do that.
Add an osnoise_print() macro to facilitate adding debug prints (without
tainting).
Use trace_array_printk() instead of trace_array_printk_buf(), as we're
only writing to the main buffer (of a non-main instance) anyway -- and
trace_array_printk_buf() skips the check to make sure we're not printing
to the global instance.
Signed-off-by: Crystal Wood <crwood@redhat.com>
---
Technically this isn't needed to make the tests pass, but it was helpful
in debugging the issues and would be nice to have around for next time.
What's the intended use case for trace_array_printk_buf()? It seems like
it should be an internal function if it's not going to check the
instance. I don't see any non-internal uses other than here and in
trace_hwlat.c, which also only prints to the main buffer.
---
kernel/trace/trace_osnoise.c | 39 ++++++++++++++++++++++--------------
1 file changed, 24 insertions(+), 15 deletions(-)
diff --git a/kernel/trace/trace_osnoise.c b/kernel/trace/trace_osnoise.c
index bb4d195ec74f..b53a698f8e81 100644
--- a/kernel/trace/trace_osnoise.c
+++ b/kernel/trace/trace_osnoise.c
@@ -81,6 +81,22 @@ struct osnoise_instance {
static struct list_head osnoise_instances;
+static void osnoise_print(const char *fmt, ...)
+{
+ struct osnoise_instance *inst;
+ struct trace_array *tr;
+ va_list ap;
+
+ rcu_read_lock();
+ list_for_each_entry_rcu(inst, &osnoise_instances, list) {
+ tr = inst->tr;
+ va_start(ap, fmt);
+ trace_array_vprintk(tr, _RET_IP_, fmt, ap);
+ va_end(ap);
+ }
+ rcu_read_unlock();
+}
+
static bool osnoise_has_registered_instances(void)
{
return !!list_first_or_null_rcu(&osnoise_instances,
@@ -121,6 +137,7 @@ static int osnoise_register_instance(struct trace_array *tr)
* trace_types_lock.
*/
lockdep_assert_held(&trace_types_lock);
+ trace_array_init_printk(tr);
inst = kmalloc(sizeof(*inst), GFP_KERNEL);
if (!inst)
@@ -455,15 +472,7 @@ static void print_osnoise_headers(struct seq_file *s)
* osnoise_taint - report an osnoise error.
*/
#define osnoise_taint(msg) ({ \
- struct osnoise_instance *inst; \
- struct trace_buffer *buffer; \
- \
- rcu_read_lock(); \
- list_for_each_entry_rcu(inst, &osnoise_instances, list) { \
- buffer = inst->tr->array_buffer.buffer; \
- trace_array_printk_buf(buffer, _THIS_IP_, msg); \
- } \
- rcu_read_unlock(); \
+ osnoise_print(msg); \
osnoise_data.tainted = true; \
})
@@ -1173,10 +1182,10 @@ static __always_inline void osnoise_stop_exception(char *msg, int cpu)
rcu_read_lock();
list_for_each_entry_rcu(inst, &osnoise_instances, list) {
tr = inst->tr;
- trace_array_printk_buf(tr->array_buffer.buffer, _THIS_IP_,
- "stop tracing hit on cpu %d due to exception: %s\n",
- smp_processor_id(),
- msg);
+ trace_array_printk(tr, _THIS_IP_,
+ "stop tracing hit on cpu %d due to exception: %s\n",
+ smp_processor_id(),
+ msg);
if (test_bit(OSN_PANIC_ON_STOP, &osnoise_options))
panic("tracer hit on cpu %d due to exception: %s\n",
@@ -1346,8 +1355,8 @@ static __always_inline void osnoise_stop_tracing(void)
rcu_read_lock();
list_for_each_entry_rcu(inst, &osnoise_instances, list) {
tr = inst->tr;
- trace_array_printk_buf(tr->array_buffer.buffer, _THIS_IP_,
- "stop tracing hit on cpu %d\n", smp_processor_id());
+ trace_array_printk(tr, _THIS_IP_,
+ "stop tracing hit on cpu %d\n", smp_processor_id());
if (test_bit(OSN_PANIC_ON_STOP, &osnoise_options))
panic("tracer hit stop condition on CPU %d\n", smp_processor_id());
--
2.48.1
^ permalink raw reply related
* [PATCH 4/5] rtla/timerlat: Exit top main loop on any non-zero wait_retval
From: Crystal Wood @ 2025-11-12 15:25 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-trace-kernel, John Kacur, Tomas Glozar, Costa Shulyupin,
Wander Lairson Costa, Crystal Wood
In-Reply-To: <20251112152529.956778-1-crwood@redhat.com>
Comparing to exactly 1 will fail if more than one ring buffer
event was seen since the last call to timerlat_bpf_wait(), which
can happen in some race scenarios.
Signed-off-by: Crystal Wood <crwood@redhat.com>
---
tools/tracing/rtla/src/timerlat_top.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/tracing/rtla/src/timerlat_top.c b/tools/tracing/rtla/src/timerlat_top.c
index da5d5db1bc17..56cf4ad64847 100644
--- a/tools/tracing/rtla/src/timerlat_top.c
+++ b/tools/tracing/rtla/src/timerlat_top.c
@@ -921,7 +921,7 @@ timerlat_top_bpf_main_loop(struct osnoise_tool *tool)
if (!params->common.quiet)
timerlat_print_stats(tool);
- if (wait_retval == 1) {
+ if (wait_retval != 0) {
/* Stopping requested by tracer */
actions_perform(¶ms->common.threshold_actions);
--
2.48.1
^ permalink raw reply related
* [PATCH 5/5] rtla: Set stop threshold after all instances are enabled
From: Crystal Wood @ 2025-11-12 15:25 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-trace-kernel, John Kacur, Tomas Glozar, Costa Shulyupin,
Wander Lairson Costa, Crystal Wood
In-Reply-To: <20251112152529.956778-1-crwood@redhat.com>
This avoids startup races where one of the instances hit a threshold
before all instances were enabled, and thus tracing stops without
the relevant event. In particular, this is not uncommon with the
tests that set a very tight threshold and then complain if there's
no analysis.
This also ensures that we don't stop tracing during a warmup.
The downside is a small chance of having an event over the threshold
early in the output, without stopping on it, which could cause user
confusion. This should be less likely if the warmup feature is used, but
that doesn't eliminate the race window, just the odds of an unusual spike
right at that moment.
Signed-off-by: Crystal Wood <crwood@redhat.com>
---
tools/tracing/rtla/src/common.c | 20 ++++++++++++++++++++
tools/tracing/rtla/src/common.h | 4 ++++
tools/tracing/rtla/src/osnoise.c | 17 ++++-------------
tools/tracing/rtla/src/osnoise.h | 5 -----
tools/tracing/rtla/src/timerlat.c | 29 ++++++++++-------------------
5 files changed, 38 insertions(+), 37 deletions(-)
diff --git a/tools/tracing/rtla/src/common.c b/tools/tracing/rtla/src/common.c
index b197037fc58b..46e0263d6ae8 100644
--- a/tools/tracing/rtla/src/common.c
+++ b/tools/tracing/rtla/src/common.c
@@ -348,3 +348,23 @@ int hist_main_loop(struct osnoise_tool *tool)
return retval;
}
+
+int osn_set_stop(struct osnoise_tool *tool)
+{
+ struct common_params *params = tool->params;
+ int retval;
+
+ retval = osnoise_set_stop_us(tool->context, params->stop_us);
+ if (retval) {
+ err_msg("Failed to set stop us\n");
+ return retval;
+ }
+
+ retval = osnoise_set_stop_total_us(tool->context, params->stop_total_us);
+ if (retval) {
+ err_msg("Failed to set stop total us\n");
+ return retval;
+ }
+
+ return 0;
+}
diff --git a/tools/tracing/rtla/src/common.h b/tools/tracing/rtla/src/common.h
index 355f113a14a3..64004a030a56 100644
--- a/tools/tracing/rtla/src/common.h
+++ b/tools/tracing/rtla/src/common.h
@@ -148,7 +148,11 @@ void osnoise_destroy_tool(struct osnoise_tool *top);
struct osnoise_tool *osnoise_init_tool(char *tool_name);
struct osnoise_tool *osnoise_init_trace_tool(const char *tracer);
bool osnoise_trace_is_off(struct osnoise_tool *tool, struct osnoise_tool *record);
+int osnoise_set_stop_us(struct osnoise_context *context, long long stop_us);
+int osnoise_set_stop_total_us(struct osnoise_context *context,
+ long long stop_total_us);
int common_apply_config(struct osnoise_tool *tool, struct common_params *params);
int top_main_loop(struct osnoise_tool *tool);
int hist_main_loop(struct osnoise_tool *tool);
+int osn_set_stop(struct osnoise_tool *tool);
diff --git a/tools/tracing/rtla/src/osnoise.c b/tools/tracing/rtla/src/osnoise.c
index 312c511fa004..945eb61efc46 100644
--- a/tools/tracing/rtla/src/osnoise.c
+++ b/tools/tracing/rtla/src/osnoise.c
@@ -1128,18 +1128,6 @@ osnoise_apply_config(struct osnoise_tool *tool, struct osnoise_params *params)
goto out_err;
}
- retval = osnoise_set_stop_us(tool->context, params->common.stop_us);
- if (retval) {
- err_msg("Failed to set stop us\n");
- goto out_err;
- }
-
- retval = osnoise_set_stop_total_us(tool->context, params->common.stop_total_us);
- if (retval) {
- err_msg("Failed to set stop total us\n");
- goto out_err;
- }
-
retval = osnoise_set_tracing_thresh(tool->context, params->threshold);
if (retval) {
err_msg("Failed to set tracing_thresh\n");
@@ -1184,9 +1172,12 @@ int osnoise_enable(struct osnoise_tool *tool)
debug_msg("Error cleaning up the buffer");
return retval;
}
-
}
+ retval = osn_set_stop(tool);
+ if (retval)
+ return retval;
+
return 0;
}
diff --git a/tools/tracing/rtla/src/osnoise.h b/tools/tracing/rtla/src/osnoise.h
index 895687030c0b..3c927c179b35 100644
--- a/tools/tracing/rtla/src/osnoise.h
+++ b/tools/tracing/rtla/src/osnoise.h
@@ -34,12 +34,7 @@ int osnoise_set_runtime_period(struct osnoise_context *context,
unsigned long long period);
void osnoise_restore_runtime_period(struct osnoise_context *context);
-int osnoise_set_stop_us(struct osnoise_context *context,
- long long stop_us);
void osnoise_restore_stop_us(struct osnoise_context *context);
-
-int osnoise_set_stop_total_us(struct osnoise_context *context,
- long long stop_total_us);
void osnoise_restore_stop_total_us(struct osnoise_context *context);
int osnoise_set_timerlat_period_us(struct osnoise_context *context,
diff --git a/tools/tracing/rtla/src/timerlat.c b/tools/tracing/rtla/src/timerlat.c
index 19bab3d189f9..fe93cdc5354f 100644
--- a/tools/tracing/rtla/src/timerlat.c
+++ b/tools/tracing/rtla/src/timerlat.c
@@ -59,25 +59,6 @@ timerlat_apply_config(struct osnoise_tool *tool, struct timerlat_params *params)
goto out_err;
}
- if (params->mode != TRACING_MODE_BPF) {
- /*
- * In tracefs and mixed mode, timerlat tracer handles stopping
- * on threshold
- */
- retval = osnoise_set_stop_us(tool->context, params->common.stop_us);
- if (retval) {
- err_msg("Failed to set stop us\n");
- goto out_err;
- }
-
- retval = osnoise_set_stop_total_us(tool->context, params->common.stop_total_us);
- if (retval) {
- err_msg("Failed to set stop total us\n");
- goto out_err;
- }
- }
-
-
retval = osnoise_set_timerlat_period_us(tool->context,
params->timerlat_period_us ?
params->timerlat_period_us :
@@ -200,6 +181,16 @@ int timerlat_enable(struct osnoise_tool *tool)
}
}
+ /*
+ * In tracefs and mixed mode, timerlat tracer handles stopping
+ * on threshold
+ */
+ if (params->mode != TRACING_MODE_BPF) {
+ retval = osn_set_stop(tool);
+ if (retval)
+ return retval;
+ }
+
return 0;
}
--
2.48.1
^ permalink raw reply related
* Re: [PATCH v3 RESEND] function_graph: Enable funcgraph-args and funcgraph-retaddr to work simultaneously
From: Steven Rostedt @ 2025-11-12 15:30 UTC (permalink / raw)
To: Masami Hiramatsu (Google)
Cc: Donglin Peng, linux-trace-kernel, linux-kernel, Donglin Peng,
Sven Schnelle
In-Reply-To: <20251112141705.a7f2f79f6d7b058201307e89@kernel.org>
On Wed, 12 Nov 2025 14:17:05 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> > #if defined(CONFIG_FUNCTION_GRAPH_RETVAL) || defined(CONFIG_FUNCTION_GRAPH_RETADDR)
> > @@ -869,10 +858,12 @@ static void print_graph_retval(struct trace_seq *s, struct ftrace_graph_ent_entr
> > trace_seq_printf(s, "%ps", func);
> >
> > if (args_size >= FTRACE_REGS_MAX_ARGS * sizeof(long)) {
> > - print_function_args(s, entry->args, (unsigned long)func);
> > + print_function_args(s, entry->args + ARGS_OFFS(args_size),
> > + (unsigned long)func);
> > trace_seq_putc(s, ';');
> > - } else
> > + } else {
>
> nit: you don't need to add braces for a single line block.
>
> > trace_seq_puts(s, "();");
> > + }
> >
Actually you do.
The rule for if/else is if one side requires { }, then both do.
https://docs.kernel.org/process/coding-style.html#placing-braces-and-spaces
Yes, this has been inconsistent and my older code breaks this rule, but I
try to follow it in newer code.
-- Steve
^ permalink raw reply
* Re: [PATCH v3 RESEND] function_graph: Enable funcgraph-args and funcgraph-retaddr to work simultaneously
From: Steven Rostedt @ 2025-11-12 15:38 UTC (permalink / raw)
To: Donglin Peng
Cc: Masami Hiramatsu, linux-trace-kernel, linux-kernel, Donglin Peng,
Sven Schnelle
In-Reply-To: <CAErzpms8oRkqJhxk1R6LMUq1GeZT3TqkPOm2yRAfY1ph_F2YNw@mail.gmail.com>
On Wed, 12 Nov 2025 17:00:10 +0800
Donglin Peng <dolinux.peng@gmail.com> wrote:
> I have a question regarding the behavior of the funcgraph-args option.
>
> Currently, when the funcgraph-args flag is set, the function arguments
> are saved.
> However, when the flag is cleared, the trace output still displays the function
> arguments unconditionally.
Yes. The flag affects the behavior of the recording and not the displaying
of the arguments. There's no reason *not* to show them if they have already
been recorded. Recording the arguments causes a noticeable overhead which
is why it is optional.
>
> I expected that the flag would control both saving and displaying the arguments.
> But it seems that the current design only controls the saving. This
> behavior seems
> inconsistent.
Not at all. The option is only for the recording. Otherwise, one might
expect than they could do the recording and then after the fact enable
"func-args" and expect to see them. That would be more inconsistent.
>
> I think we should add a check for the flag in the display part,
> specifically before
> calling print_function_args, to ensure that the arguments are only
> displayed when
> the flag is set.
>
> What are your thoughts?
Big nope!
The retaddr is different, as the information is somewhat redundant to the
call chain and may cause more noise then needed.
-- Steve
^ permalink raw reply
* Re: [PATCH] trace/pid_list: optimize pid_list->lock contention
From: Steven Rostedt @ 2025-11-12 15:42 UTC (permalink / raw)
To: Yongliang Gao
Cc: Sebastian Andrzej Siewior, mhiramat, mathieu.desnoyers,
linux-kernel, linux-trace-kernel, Yongliang Gao, Huang Cun,
frankjpliu
In-Reply-To: <CAJxhyqDCFRT_fPWHb67x-PUu+Om91UrbrQEifcF7m+dkZ35dqA@mail.gmail.com>
On Wed, 12 Nov 2025 13:27:10 +0800
Yongliang Gao <leonylgao@gmail.com> wrote:
> Thank you for your detailed response and the proposed RCU-like approach.
>
> I've looked into using a regular seqlock instead of the current
> implementation, but as you pointed out, the write side is indeed a
> critical path. More importantly, I found that even with seqlock, the
> write_seqlock() function internally uses spin_lock() which on
> PREEMPT_RT gets converted to an mutex. This would cause the same
> issues we're trying to avoid - potential sleep in atomic contexts.
I believe there is a raw_read_seqcount() functionality that is safe for
PREEMPT_RT. Have you looked into using that?
-- Steve
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox