[lustre-devel] [PATCH 24/24] lustre: discard TCD_MAX_TYPES
James Simmons
jsimmons at infradead.org
Sun Jun 24 13:37:10 PDT 2018
> As well as CFS_TCD_TYPE_CNT we have TCD_MAX_TYPES which has a larger
> value but a similar meaning. Discard it and just use
> CFS_TCD_TYPE_CNT.
>
> Two places relied on the fact that TCD_MAX_TYPES was larger and so
> there would be NULLs at the end of the array. Change
> them to check the array size properly.
>
> Signed-off-by: NeilBrown <neilb at suse.com>
> ---
>
> Thanks for testing James!
> I found two problems.
> 1/ I had
> - for (i = 0; cfs_trace_data[i] && i < CFS_TCD_TYPE_CNT; i++) {
> instead of
> + for (i = 0; i < CFS_TCD_TYPE_CNT && cfs_trace_data[i]; i++) {
>
> So it could dereference beyond the end of an array. I don't this was
> the problem.
>
> 2/ I hadn't changed
> - for (i = 0; cfs_trace_data[i]; i++) \
> to
> + for (i = 0; i < CFS_TCD_TYPE_CNT && cfs_trace_data[i]; i++) \
>
> so if cfs_trace_data[4] was non NULL, bad things could happen.
> I suspect this is what happened to you.
>
> Thanks,
> NeilBrown
Much better :-)
> drivers/staging/lustre/lnet/libcfs/tracefile.c | 8 ++++----
> drivers/staging/lustre/lnet/libcfs/tracefile.h | 5 ++---
> 2 files changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/staging/lustre/lnet/libcfs/tracefile.c b/drivers/staging/lustre/lnet/libcfs/tracefile.c
> index cdef67391a72..cc399d580444 100644
> --- a/drivers/staging/lustre/lnet/libcfs/tracefile.c
> +++ b/drivers/staging/lustre/lnet/libcfs/tracefile.c
> @@ -50,7 +50,7 @@
> #include "tracefile.h"
>
> /* XXX move things up to the top, comment */
> -union cfs_trace_data_union (*cfs_trace_data[TCD_MAX_TYPES])[NR_CPUS] __cacheline_aligned;
> +union cfs_trace_data_union (*cfs_trace_data[CFS_TCD_TYPE_CNT])[NR_CPUS] __cacheline_aligned;
>
> char cfs_tracefile[TRACEFILE_NAME_SIZE];
> long long cfs_tracefile_size = CFS_TRACEFILE_SIZE;
> @@ -145,8 +145,8 @@ void cfs_trace_unlock_tcd(struct cfs_trace_cpu_data *tcd, int walking)
> spin_unlock(&tcd->tcd_lock);
> }
>
> -#define cfs_tcd_for_each_type_lock(tcd, i, cpu) \
> - for (i = 0; cfs_trace_data[i] && \
> +#define cfs_tcd_for_each_type_lock(tcd, i, cpu) \
> + for (i = 0; i < CFS_TCD_TYPE_CNT && cfs_trace_data[i] && \
> (tcd = &(*cfs_trace_data[i])[cpu].tcd) && \
> cfs_trace_lock_tcd(tcd, 1); cfs_trace_unlock_tcd(tcd, 1), i++)
>
> @@ -1381,7 +1381,7 @@ static void cfs_trace_cleanup(void)
> cfs_trace_console_buffers[i][j] = NULL;
> }
>
> - for (i = 0; cfs_trace_data[i]; i++) {
> + for (i = 0; i < CFS_TCD_TYPE_CNT && cfs_trace_data[i]; i++) {
> kfree(cfs_trace_data[i]);
> cfs_trace_data[i] = NULL;
> }
> diff --git a/drivers/staging/lustre/lnet/libcfs/tracefile.h b/drivers/staging/lustre/lnet/libcfs/tracefile.h
> index 23faecf886c1..87b00fc70b70 100644
> --- a/drivers/staging/lustre/lnet/libcfs/tracefile.h
> +++ b/drivers/staging/lustre/lnet/libcfs/tracefile.h
> @@ -184,11 +184,10 @@ union cfs_trace_data_union {
> char __pad[L1_CACHE_ALIGN(sizeof(struct cfs_trace_cpu_data))];
> };
>
> -#define TCD_MAX_TYPES 8
> -extern union cfs_trace_data_union (*cfs_trace_data[TCD_MAX_TYPES])[NR_CPUS];
> +extern union cfs_trace_data_union (*cfs_trace_data[CFS_TCD_TYPE_CNT])[NR_CPUS];
>
> #define cfs_tcd_for_each(tcd, i, j) \
> - for (i = 0; cfs_trace_data[i]; i++) \
> + for (i = 0; i < CFS_TCD_TYPE_CNT && cfs_trace_data[i]; i++) \
> for (j = 0, ((tcd) = &(*cfs_trace_data[i])[j].tcd); \
> j < num_possible_cpus(); \
> j++, (tcd) = &(*cfs_trace_data[i])[j].tcd)
> --
> 2.14.0.rc0.dirty
>
>
More information about the lustre-devel
mailing list