> On Jun 21, 2024, at 1:21 PM, Anna Schumaker <schumaker.anna@xxxxxxxxx> wrote:
>
> Hi Chuck,
>
> On Fri, Jun 21, 2024 at 12:22 PM <cel@xxxxxxxxxx> wrote:
>>
>> From: Chuck Lever <chuck.lever@xxxxxxxxxx>
>>
>> An administrator cannot take action on these messages, but the
>> reported errors might be helpful for troubleshooting. Transition
>> them to trace points so these events appear in the trace log and
>> can be easily lined up with other traced NFS client operations.
>>
>> Examples:
>>
>> append_writer-6147 [000] 80.247393: bl_pr_key_reg: device=sdb key=0x666dcdabf29514fe
>> append_writer-6147 [000] 80.247842: bl_pr_key_unreg: device=sdb key=0x666dcdabf29514fe
>>
>> umount.nfs4-6172 [002] 84.950409: bl_pr_key_unreg_err: device=sdb key=0x666dcdabf29514fe error=24
>>
>> Christoph points out that:
>>> ... Note that the disk_name isn't really what
>>> we'd want to trace anyway, as it misses the partition information.
>>> The normal way to print the device name is the %pg printk specifier,
>>> but I'm not sure how to correctly use that for tracing which wants
>>> a string in the entry for binary tracing.
>>
>> The trace points copy the pr_info() that they are replacing, and
>> show only the parent device name and not the partition. I'm still
>> looking into how to record both parts of the device name.
>>
>> Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
>> ---
>> fs/nfs/blocklayout/dev.c | 30 +++++++-------
>> fs/nfs/nfs4trace.c | 7 ++++
>> fs/nfs/nfs4trace.h | 88 ++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 111 insertions(+), 14 deletions(-)
>>
>> diff --git a/fs/nfs/blocklayout/dev.c b/fs/nfs/blocklayout/dev.c
>> index 568f685dee4b..6c5d290ca81d 100644
>> --- a/fs/nfs/blocklayout/dev.c
>> +++ b/fs/nfs/blocklayout/dev.c
>> @@ -10,6 +10,7 @@
>> #include <linux/pr.h>
>>
>> #include "blocklayout.h"
>> +#include "../nfs4trace.h"
>>
>> #define NFSDBG_FACILITY NFSDBG_PNFS_LD
>>
>> @@ -26,12 +27,14 @@ bl_free_device(struct pnfs_block_dev *dev)
>> if (test_and_clear_bit(PNFS_BDEV_REGISTERED, &dev->flags)) {
>> struct block_device *bdev = file_bdev(dev->bdev_file);
>> const struct pr_ops *ops = bdev->bd_disk->fops->pr_ops;
>> - int error;
>> + int status;
>>
>> - error = ops->pr_register(file_bdev(dev->bdev_file),
>> - dev->pr_key, 0, false);
>> - if (error)
>> - pr_err("failed to unregister PR key.\n");
>> + status = ops->pr_register(bdev, dev->pr_key, 0, false);
>> + if (status)
>> + trace_bl_pr_key_unreg_err(bdev, dev->pr_key,
>> + status);
>> + else
>> + trace_bl_pr_key_unreg(bdev, dev->pr_key);
>> }
>>
>> if (dev->bdev_file)
>> @@ -243,10 +246,10 @@ static bool bl_pr_register_scsi(struct pnfs_block_dev *dev)
>>
>> status = ops->pr_register(bdev, 0, dev->pr_key, true);
>> if (status) {
>> - pr_err("pNFS: failed to register key for block device %s.",
>> - bdev->bd_disk->disk_name);
>> + trace_bl_pr_key_reg_err(bdev, dev->pr_key, status);
>> return false;
>> }
>> + trace_bl_pr_key_reg(bdev, dev->pr_key);
>> return true;
>> }
>>
>> @@ -351,8 +354,9 @@ bl_parse_scsi(struct nfs_server *server, struct pnfs_block_dev *d,
>> struct pnfs_block_volume *volumes, int idx, gfp_t gfp_mask)
>> {
>> struct pnfs_block_volume *v = &volumes[idx];
>> - struct file *bdev_file;
>> + struct block_device *bdev;
>> const struct pr_ops *ops;
>> + struct file *bdev_file;
>> int error;
>>
>> if (!bl_validate_designator(v))
>> @@ -373,8 +377,9 @@ bl_parse_scsi(struct nfs_server *server, struct pnfs_block_dev *d,
>> return PTR_ERR(bdev_file);
>> }
>> d->bdev_file = bdev_file;
>> + bdev = file_bdev(bdev_file);
>>
>> - d->len = bdev_nr_bytes(file_bdev(d->bdev_file));
>> + d->len = bdev_nr_bytes(bdev);
>> d->map = bl_map_simple;
>> d->pr_key = v->scsi.pr_key;
>>
>> @@ -383,13 +388,10 @@ bl_parse_scsi(struct nfs_server *server, struct pnfs_block_dev *d,
>> goto out_blkdev_put;
>> }
>>
>> - pr_info("pNFS: using block device %s (reservation key 0x%llx)\n",
>> - file_bdev(d->bdev_file)->bd_disk->disk_name, d->pr_key);
>> -
>> - ops = file_bdev(d->bdev_file)->bd_disk->fops->pr_ops;
>> + ops = bdev->bd_disk->fops->pr_ops;
>> if (!ops) {
>> pr_err("pNFS: block device %s does not support reservations.",
>> - file_bdev(d->bdev_file)->bd_disk->disk_name);
>> + bdev->bd_disk->disk_name);
>> error = -EINVAL;
>> goto out_blkdev_put;
>> }
>> diff --git a/fs/nfs/nfs4trace.c b/fs/nfs/nfs4trace.c
>> index d22c6670f770..389941ccc9c9 100644
>> --- a/fs/nfs/nfs4trace.c
>> +++ b/fs/nfs/nfs4trace.c
>> @@ -2,6 +2,8 @@
>> /*
>> * Copyright (c) 2013 Trond Myklebust <Trond.Myklebust@xxxxxxxxxx>
>> */
>> +#include <uapi/linux/pr.h>
>> +#include <linux/blkdev.h>
>> #include <linux/nfs_fs.h>
>> #include "nfs4_fs.h"
>> #include "internal.h"
>> @@ -29,5 +31,10 @@ EXPORT_TRACEPOINT_SYMBOL_GPL(ff_layout_read_error);
>> EXPORT_TRACEPOINT_SYMBOL_GPL(ff_layout_write_error);
>> EXPORT_TRACEPOINT_SYMBOL_GPL(ff_layout_commit_error);
>>
>> +EXPORT_TRACEPOINT_SYMBOL_GPL(bl_pr_key_reg);
>> +EXPORT_TRACEPOINT_SYMBOL_GPL(bl_pr_key_reg_err);
>> +EXPORT_TRACEPOINT_SYMBOL_GPL(bl_pr_key_unreg);
>> +EXPORT_TRACEPOINT_SYMBOL_GPL(bl_pr_key_unreg_err);
>> +
>> EXPORT_TRACEPOINT_SYMBOL_GPL(fl_getdevinfo);
>> #endif
>> diff --git a/fs/nfs/nfs4trace.h b/fs/nfs/nfs4trace.h
>> index 4de8780a7c48..f2090a491fcb 100644
>> --- a/fs/nfs/nfs4trace.h
>> +++ b/fs/nfs/nfs4trace.h
>> @@ -2153,6 +2153,94 @@ TRACE_EVENT(ff_layout_commit_error,
>> )
>> );
>>
>> +DECLARE_EVENT_CLASS(pnfs_bl_pr_key_class,
>> + TP_PROTO(
>> + const struct block_device *bdev,
>> + u64 key
>> + ),
>> + TP_ARGS(bdev, key),
>> + TP_STRUCT__entry(
>> + __field(u64, key)
>> + __field(dev_t, dev)
>> + __string(device, bdev->bd_disk->disk_name)
>> + ),
>> + TP_fast_assign(
>> + __entry->key = key;
>> + __entry->dev = bdev->bd_dev;
> ^^^^^^^
> b4 tells me this patch adds trailing whitespace at the line above:
>
> Patch failed at 0004 nfs/blocklayout: SCSI layout trace points for
> reservation key reg/unreg
> /home/anna/Projects/linux-nfs.git/.git/rebase-apply/patch:135:
> trailing whitespace.
> __entry->dev = bdev->bd_dev;
> /home/anna/Projects/linux-nfs.git/.git/rebase-apply/patch:188:
> trailing whitespace.
> __entry->dev = bdev->bd_dev;
> error: patch failed: fs/nfs/blocklayout/dev.c:383
> error: fs/nfs/blocklayout/dev.c: patch does not apply
Fixed in my tree. Will appear in v3 of this series.
>> + __assign_str(device);
>> + ),
>> + TP_printk("dev=%d,%d (%s) key=0x%016llx",
>> + MAJOR(__entry->dev), MINOR(__entry->dev),
>> + __get_str(device), __entry->key
>> + )
>> +);
>> +
>> +#define DEFINE_NFS4_BLOCK_PRKEY_EVENT(name) \
>> + DEFINE_EVENT(pnfs_bl_pr_key_class, name, \
>> + TP_PROTO( \
>> + const struct block_device *bdev, \
>> + u64 key \
>> + ), \
>> + TP_ARGS(bdev, key))
>> +DEFINE_NFS4_BLOCK_PRKEY_EVENT(bl_pr_key_reg);
>> +DEFINE_NFS4_BLOCK_PRKEY_EVENT(bl_pr_key_unreg);
>> +
>> +/*
>> + * From uapi/linux/pr.h
>> + */
>> +TRACE_DEFINE_ENUM(PR_STS_SUCCESS);
>> +TRACE_DEFINE_ENUM(PR_STS_IOERR);
>> +TRACE_DEFINE_ENUM(PR_STS_RESERVATION_CONFLICT);
>> +TRACE_DEFINE_ENUM(PR_STS_RETRY_PATH_FAILURE);
>> +TRACE_DEFINE_ENUM(PR_STS_PATH_FAST_FAILED);
>> +TRACE_DEFINE_ENUM(PR_STS_PATH_FAILED);
>> +
>> +#define show_pr_status(x) \
>> + __print_symbolic(x, \
>> + { PR_STS_SUCCESS, "SUCCESS" }, \
>> + { PR_STS_IOERR, "IOERR" }, \
>> + { PR_STS_RESERVATION_CONFLICT, "RESERVATION_CONFLICT" }, \
>> + { PR_STS_RETRY_PATH_FAILURE, "RETRY_PATH_FAILURE" }, \
>> + { PR_STS_PATH_FAST_FAILED, "PATH_FAST_FAILED" }, \
>> + { PR_STS_PATH_FAILED, "PATH_FAILED" })
>> +
>> +DECLARE_EVENT_CLASS(pnfs_bl_pr_key_err_class,
>> + TP_PROTO(
>> + const struct block_device *bdev,
>> + u64 key,
>> + int status
>> + ),
>> + TP_ARGS(bdev, key, status),
>> + TP_STRUCT__entry(
>> + __field(u64, key)
>> + __field(dev_t, dev)
>> + __field(unsigned long, status)
>> + __string(device, bdev->bd_disk->disk_name)
>> + ),
>> + TP_fast_assign(
>> + __entry->key = key;
>> + __entry->dev = bdev->bd_dev;
> ^^^^^^
> And for this line, too.
>
> Thanks,
> Anna
>
>> + __entry->status = status;
>> + __assign_str(device);
>> + ),
>> + TP_printk("dev=%d,%d (%s) key=0x%016llx status=%s",
>> + MAJOR(__entry->dev), MINOR(__entry->dev),
>> + __get_str(device), __entry->key,
>> + show_pr_status(__entry->status)
>> + )
>> +);
>> +
>> +#define DEFINE_NFS4_BLOCK_PRKEY_ERR_EVENT(name) \
>> + DEFINE_EVENT(pnfs_bl_pr_key_err_class, name, \
>> + TP_PROTO( \
>> + const struct block_device *bdev, \
>> + u64 key, \
>> + int status \
>> + ), \
>> + TP_ARGS(bdev, key, status))
>> +DEFINE_NFS4_BLOCK_PRKEY_ERR_EVENT(bl_pr_key_reg_err);
>> +DEFINE_NFS4_BLOCK_PRKEY_ERR_EVENT(bl_pr_key_unreg_err);
>> +
>> #ifdef CONFIG_NFS_V4_2
>> TRACE_DEFINE_ENUM(NFS4_CONTENT_DATA);
>> TRACE_DEFINE_ENUM(NFS4_CONTENT_HOLE);
>> --
>> 2.45.1
--
Chuck Lever
