* Re: [PATCH 2/2] NFSD: add trace points to track server copy progress
2023-08-30 23:46 ` [PATCH 2/2] NFSD: add trace points to track server copy progress Dai Ngo
@ 2023-08-31 5:18 ` kernel test robot
2023-08-31 14:13 ` Chuck Lever
1 sibling, 0 replies; 4+ messages in thread
From: kernel test robot @ 2023-08-31 5:18 UTC (permalink / raw)
To: Dai Ngo, chuck.lever, jlayton; +Cc: oe-kbuild-all, linux-nfs
Hi Dai,
kernel test robot noticed the following build errors:
[auto build test ERROR on linus/master]
[also build test ERROR on v6.5 next-20230830]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Dai-Ngo/NFSD-add-trace-points-to-track-server-copy-progress/20230831-074806
base: linus/master
patch link: https://lore.kernel.org/r/1693439219-19467-2-git-send-email-dai.ngo%40oracle.com
patch subject: [PATCH 2/2] NFSD: add trace points to track server copy progress
config: s390-defconfig (https://download.01.org/0day-ci/archive/20230831/202308311350.wOmbsChS-lkp@intel.com/config)
compiler: s390-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230831/202308311350.wOmbsChS-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202308311350.wOmbsChS-lkp@intel.com/
All errors (new ones prefixed by >>):
fs/nfsd/nfs4proc.c: In function 'nfsd4_do_async_copy':
>> fs/nfsd/nfs4proc.c:1730:9: error: implicit declaration of function 'trace_nfsd4_copy_do_async'; did you mean 'nfsd4_copy_is_async'? [-Werror=implicit-function-declaration]
1730 | trace_nfsd4_copy_do_async(copy);
| ^~~~~~~~~~~~~~~~~~~~~~~~~
| nfsd4_copy_is_async
fs/nfsd/nfs4proc.c: In function 'nfsd4_copy':
>> fs/nfsd/nfs4proc.c:1771:17: error: implicit declaration of function 'trace_nfsd4_copy_inter'; did you mean 'trace_nfsd_ctl_time'? [-Werror=implicit-function-declaration]
1771 | trace_nfsd4_copy_inter(copy);
| ^~~~~~~~~~~~~~~~~~~~~~
| trace_nfsd_ctl_time
>> fs/nfsd/nfs4proc.c:1778:25: error: implicit declaration of function 'trace_nfsd4_copy_done'; did you mean 'trace_nfsd_read_done'? [-Werror=implicit-function-declaration]
1778 | trace_nfsd4_copy_done(copy, status);
| ^~~~~~~~~~~~~~~~~~~~~
| trace_nfsd_read_done
>> fs/nfsd/nfs4proc.c:1782:17: error: implicit declaration of function 'trace_nfsd4_copy_intra'; did you mean 'trace_nfsd_cb_setup'? [-Werror=implicit-function-declaration]
1782 | trace_nfsd4_copy_intra(copy);
| ^~~~~~~~~~~~~~~~~~~~~~
| trace_nfsd_cb_setup
cc1: some warnings being treated as errors
vim +1730 fs/nfsd/nfs4proc.c
1717
1718 /**
1719 * nfsd4_do_async_copy - kthread function for background server-side COPY
1720 * @data: arguments for COPY operation
1721 *
1722 * Return values:
1723 * %0: Copy operation is done.
1724 */
1725 static int nfsd4_do_async_copy(void *data)
1726 {
1727 struct nfsd4_copy *copy = (struct nfsd4_copy *)data;
1728 __be32 nfserr;
1729
> 1730 trace_nfsd4_copy_do_async(copy);
1731 if (nfsd4_ssc_is_inter(copy)) {
1732 struct file *filp;
1733
1734 filp = nfs42_ssc_open(copy->ss_nsui->nsui_vfsmount,
1735 ©->c_fh, ©->stateid);
1736 if (IS_ERR(filp)) {
1737 switch (PTR_ERR(filp)) {
1738 case -EBADF:
1739 nfserr = nfserr_wrong_type;
1740 break;
1741 default:
1742 nfserr = nfserr_offload_denied;
1743 }
1744 /* ss_mnt will be unmounted by the laundromat */
1745 goto do_callback;
1746 }
1747 nfserr = nfsd4_do_copy(copy, filp, copy->nf_dst->nf_file,
1748 false);
1749 nfsd4_cleanup_inter_ssc(copy->ss_nsui, filp, copy->nf_dst);
1750 } else {
1751 nfserr = nfsd4_do_copy(copy, copy->nf_src->nf_file,
1752 copy->nf_dst->nf_file, false);
1753 }
1754
1755 do_callback:
1756 nfsd4_send_cb_offload(copy, nfserr);
1757 cleanup_async_copy(copy);
1758 return 0;
1759 }
1760
1761 static __be32
1762 nfsd4_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
1763 union nfsd4_op_u *u)
1764 {
1765 struct nfsd4_copy *copy = &u->copy;
1766 __be32 status;
1767 struct nfsd4_copy *async_copy = NULL;
1768
1769 copy->cp_clp = cstate->clp;
1770 if (nfsd4_ssc_is_inter(copy)) {
> 1771 trace_nfsd4_copy_inter(copy);
1772 if (!inter_copy_offload_enable || nfsd4_copy_is_sync(copy)) {
1773 status = nfserr_notsupp;
1774 goto out;
1775 }
1776 status = nfsd4_setup_inter_ssc(rqstp, cstate, copy);
1777 if (status) {
> 1778 trace_nfsd4_copy_done(copy, status);
1779 return nfserr_offload_denied;
1780 }
1781 } else {
> 1782 trace_nfsd4_copy_intra(copy);
1783 status = nfsd4_setup_intra_ssc(rqstp, cstate, copy);
1784 if (status) {
1785 trace_nfsd4_copy_done(copy, status);
1786 return status;
1787 }
1788 }
1789
1790 memcpy(©->fh, &cstate->current_fh.fh_handle,
1791 sizeof(struct knfsd_fh));
1792 if (nfsd4_copy_is_async(copy)) {
1793 struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
1794
1795 status = nfserrno(-ENOMEM);
1796 async_copy = kzalloc(sizeof(struct nfsd4_copy), GFP_KERNEL);
1797 if (!async_copy)
1798 goto out_err;
1799 INIT_LIST_HEAD(&async_copy->copies);
1800 refcount_set(&async_copy->refcount, 1);
1801 async_copy->cp_src = kmalloc(sizeof(*async_copy->cp_src), GFP_KERNEL);
1802 if (!async_copy->cp_src)
1803 goto out_err;
1804 if (!nfs4_init_copy_state(nn, copy))
1805 goto out_err;
1806 memcpy(©->cp_res.cb_stateid, ©->cp_stateid.cs_stid,
1807 sizeof(copy->cp_res.cb_stateid));
1808 dup_copy_fields(copy, async_copy);
1809 async_copy->copy_task = kthread_create(nfsd4_do_async_copy,
1810 async_copy, "%s", "copy thread");
1811 if (IS_ERR(async_copy->copy_task))
1812 goto out_err;
1813 spin_lock(&async_copy->cp_clp->async_lock);
1814 list_add(&async_copy->copies,
1815 &async_copy->cp_clp->async_copies);
1816 spin_unlock(&async_copy->cp_clp->async_lock);
1817 wake_up_process(async_copy->copy_task);
1818 status = nfs_ok;
1819 } else {
1820 status = nfsd4_do_copy(copy, copy->nf_src->nf_file,
1821 copy->nf_dst->nf_file, true);
1822 }
1823 out:
1824 trace_nfsd4_copy_done(copy, status);
1825 release_copy_files(copy);
1826 return status;
1827 out_err:
1828 if (nfsd4_ssc_is_inter(copy)) {
1829 /*
1830 * Source's vfsmount of inter-copy will be unmounted
1831 * by the laundromat. Use copy instead of async_copy
1832 * since async_copy->ss_nsui might not be set yet.
1833 */
1834 refcount_dec(©->ss_nsui->nsui_refcnt);
1835 }
1836 if (async_copy)
1837 cleanup_async_copy(async_copy);
1838 status = nfserrno(-ENOMEM);
1839 goto out;
1840 }
1841
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH 2/2] NFSD: add trace points to track server copy progress
2023-08-30 23:46 ` [PATCH 2/2] NFSD: add trace points to track server copy progress Dai Ngo
2023-08-31 5:18 ` kernel test robot
@ 2023-08-31 14:13 ` Chuck Lever
1 sibling, 0 replies; 4+ messages in thread
From: Chuck Lever @ 2023-08-31 14:13 UTC (permalink / raw)
To: Dai Ngo; +Cc: jlayton, linux-nfs
On Wed, Aug 30, 2023 at 04:46:59PM -0700, Dai Ngo wrote:
> Add trace points on destination server to track inter and intra
> server copy operations.
>
> Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
> ---
> fs/nfsd/nfs4proc.c | 12 +++++--
> fs/nfsd/trace.h | 89 ++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 99 insertions(+), 2 deletions(-)
>
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index 62f6aba6140b..9a780559073f 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -1760,6 +1760,7 @@ static int nfsd4_do_async_copy(void *data)
> struct nfsd4_copy *copy = (struct nfsd4_copy *)data;
> __be32 nfserr;
>
> + trace_nfsd4_copy_do_async(copy);
Nice addition to our observability of COPY operations!
Only comment is we have no other tracepoints named "nfsd4_yada" so
I feel it would be better if these were all renamed "nfsd_yada".
> if (nfsd4_ssc_is_inter(copy)) {
> struct file *filp;
>
> @@ -1800,17 +1801,23 @@ nfsd4_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>
> copy->cp_clp = cstate->clp;
> if (nfsd4_ssc_is_inter(copy)) {
> + trace_nfsd4_copy_inter(copy);
> if (!inter_copy_offload_enable || nfsd4_copy_is_sync(copy)) {
> status = nfserr_notsupp;
> goto out;
> }
> status = nfsd4_setup_inter_ssc(rqstp, cstate, copy);
> - if (status)
> + if (status) {
> + trace_nfsd4_copy_done(copy, status);
> return nfserr_offload_denied;
> + }
> } else {
> + trace_nfsd4_copy_intra(copy);
> status = nfsd4_setup_intra_ssc(rqstp, cstate, copy);
> - if (status)
> + if (status) {
> + trace_nfsd4_copy_done(copy, status);
> return status;
> + }
> }
>
> memcpy(©->fh, &cstate->current_fh.fh_handle,
> @@ -1847,6 +1854,7 @@ nfsd4_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> copy->nf_dst->nf_file, true);
> }
> out:
> + trace_nfsd4_copy_done(copy, status);
> release_copy_files(copy);
> return status;
> out_err:
> diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
> index 803904348871..6256a77c95c9 100644
> --- a/fs/nfsd/trace.h
> +++ b/fs/nfsd/trace.h
> @@ -1863,6 +1863,95 @@ TRACE_EVENT(nfsd_end_grace,
> )
> );
>
> +#ifdef CONFIG_NFS_V4_2
> +DECLARE_EVENT_CLASS(nfsd4_copy_class,
> + TP_PROTO(
> + const struct nfsd4_copy *copy
> + ),
> + TP_ARGS(copy),
> + TP_STRUCT__entry(
> + __field(bool, intra)
> + __field(bool, async)
> + __field(u32, src_cl_boot)
> + __field(u32, src_cl_id)
> + __field(u32, src_so_id)
> + __field(u32, src_si_generation)
> + __field(u32, dst_cl_boot)
> + __field(u32, dst_cl_id)
> + __field(u32, dst_so_id)
> + __field(u32, dst_si_generation)
> + __field(u64, src_cp_pos)
> + __field(u64, dst_cp_pos)
> + __field(u64, cp_count)
> + __sockaddr(addr, sizeof(struct sockaddr_in6))
> + ),
> + TP_fast_assign(
> + const stateid_t *src_stp = ©->cp_src_stateid;
> + const stateid_t *dst_stp = ©->cp_dst_stateid;
> +
> + __entry->intra = test_bit(NFSD4_COPY_F_INTRA, ©->cp_flags);
> + __entry->async = !test_bit(NFSD4_COPY_F_SYNCHRONOUS, ©->cp_flags);
> + __entry->src_cl_boot = src_stp->si_opaque.so_clid.cl_boot;
> + __entry->src_cl_id = src_stp->si_opaque.so_clid.cl_id;
> + __entry->src_so_id = src_stp->si_opaque.so_id;
> + __entry->src_si_generation = src_stp->si_generation;
> + __entry->dst_cl_boot = dst_stp->si_opaque.so_clid.cl_boot;
> + __entry->dst_cl_id = dst_stp->si_opaque.so_clid.cl_id;
> + __entry->dst_so_id = dst_stp->si_opaque.so_id;
> + __entry->dst_si_generation = dst_stp->si_generation;
> + __entry->src_cp_pos = copy->cp_src_pos;
> + __entry->dst_cp_pos = copy->cp_dst_pos;
> + __entry->cp_count = copy->cp_count;
> + __assign_sockaddr(addr, ©->cp_clp->cl_addr,
> + sizeof(struct sockaddr_in6));
> + ),
> + TP_printk("client=%pISpc intra=%d async=%d "
> + "src_stateid[si_generation:0x%x cl_boot:0x%x cl_id:0x%x so_id:0x%x] "
> + "dst_stateid[si_generation:0x%x cl_boot:0x%x cl_id:0x%x so_id:0x%x] "
> + "cp_src_pos=%llu cp_dst_pos=%llu cp_count=%llu",
> + __get_sockaddr(addr), __entry->intra, __entry->async,
> + __entry->src_si_generation, __entry->src_cl_boot,
> + __entry->src_cl_id, __entry->src_so_id,
> + __entry->dst_si_generation, __entry->dst_cl_boot,
> + __entry->dst_cl_id, __entry->dst_so_id,
> + __entry->src_cp_pos, __entry->dst_cp_pos, __entry->cp_count
> + )
> +);
> +
> +#define DEFINE_COPY_EVENT(name) \
> +DEFINE_EVENT(nfsd4_copy_class, nfsd4_copy_##name, \
> + TP_PROTO(const struct nfsd4_copy *copy), \
> + TP_ARGS(copy))
> +
> +DEFINE_COPY_EVENT(inter);
> +DEFINE_COPY_EVENT(intra);
> +DEFINE_COPY_EVENT(do_async);
> +
> +TRACE_EVENT(nfsd4_copy_done,
> + TP_PROTO(
> + const struct nfsd4_copy *copy,
> + __be32 status
> + ),
> + TP_ARGS(copy, status),
> + TP_STRUCT__entry(
> + __field(int, status)
> + __field(bool, intra)
> + __field(bool, async)
> + __sockaddr(addr, sizeof(struct sockaddr_in6))
> + ),
> + TP_fast_assign(
> + __entry->status = be32_to_cpu(status);
> + __entry->intra = test_bit(NFSD4_COPY_F_INTRA, ©->cp_flags);
> + __entry->async = !test_bit(NFSD4_COPY_F_SYNCHRONOUS, ©->cp_flags);
> + __assign_sockaddr(addr, ©->cp_clp->cl_addr,
> + sizeof(struct sockaddr_in6));
> + ),
> + TP_printk("addr=%pISpc status=%d intra=%d async=%d ",
> + __get_sockaddr(addr), __entry->status, __entry->intra, __entry->async
> + )
> +);
> +#endif /* CONFIG_NFS_V4_2 */
> +
> #endif /* _NFSD_TRACE_H */
>
> #undef TRACE_INCLUDE_PATH
> --
> 2.39.3
>
--
Chuck Lever
^ permalink raw reply [flat|nested] 4+ messages in thread