Re: [RFC PATCH 1/4] nfs/blocklayout: SCSI layout trace points for reservation key reg/unreg

[Christoph Hellwig <hch@lst.de>]

>  #define NFSDBG_FACILITY		NFSDBG_PNFS_LD
>  
> @@ -24,14 +25,17 @@ bl_free_device(struct pnfs_block_dev *dev)
>  		kfree(dev->children);
>  	} else {
>  		if (dev->pr_registered) {
> -			const struct pr_ops *ops =
> -				file_bdev(dev->bdev_file)->bd_disk->fops->pr_ops;

If you touch this it might be worth returnin early before the else
above to reduce the indentation here a bit.

>  			if (error)
> -				pr_err("failed to unregister PR key.\n");
> +				trace_bl_pr_key_unreg_err(bdev->bd_disk->disk_name,
> +							  dev->pr_key, error);
> +			else
> +				trace_bl_pr_key_unreg(bdev->bd_disk->disk_name,
> +						      dev->pr_key);

I'd just pass the bdev to the tracepoint and derefence it there only
when tracing is enabled.  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.

> +++ b/fs/nfs/nfs4trace.c
> @@ -29,5 +29,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);

This is weird.  The trace points for nfsd really should be in
fs/nfsd/trace.h and not in fs/nfs/ as that would then pull in
the client code into the server.