| Message ID | 156944723982.297677.3080857602921528738.stgit@magnolia (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | libfrog/xfs_scrub: fix error handling | expand |
On 9/25/19 4:33 PM, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > Add missing return value checks for everything that the per-thread > variable code calls. > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > --- > libfrog/ptvar.c | 17 +++++++++++++++-- > 1 file changed, 15 insertions(+), 2 deletions(-) > > > diff --git a/libfrog/ptvar.c b/libfrog/ptvar.c > index 6cb58208..55324b71 100644 > --- a/libfrog/ptvar.c > +++ b/libfrog/ptvar.c > @@ -44,8 +44,12 @@ ptvar_alloc( > int ret; > > #ifdef _SC_LEVEL1_DCACHE_LINESIZE > + long l1_dcache; > + > /* Try to prevent cache pingpong by aligning to cacheline size. */ > - size = max(size, sysconf(_SC_LEVEL1_DCACHE_LINESIZE)); > + l1_dcache = sysconf(_SC_LEVEL1_DCACHE_LINESIZE); > + if (l1_dcache > 0) > + size = roundup(size, l1_dcache); > #endif > > ptv = malloc(PTVAR_SIZE(nr, size)); > @@ -88,17 +92,26 @@ ptvar_get( > int *retp) > { > void *p; > + int ret; > > p = pthread_getspecific(ptv->key); > if (!p) { > pthread_mutex_lock(&ptv->lock); > assert(ptv->nr_used < ptv->nr_counters); > p = &ptv->data[(ptv->nr_used++) * ptv->data_size]; > - pthread_setspecific(ptv->key, p); > + ret = pthread_setspecific(ptv->key, p); > + if (ret) > + goto out_unlock; > pthread_mutex_unlock(&ptv->lock); > } > *retp = 0; > return p; > + > +out_unlock: > + ptv->nr_used--; > + pthread_mutex_unlock(&ptv->lock); > + *retp = ret; > + return NULL; As of this patch I'm a little confused by the "error handling" in ptcounter_add(): p = ptvar_get(ptc->var, &ret); assert(ret == 0); ?
On 9/25/19 4:33 PM, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > Add missing return value checks for everything that the per-thread > variable code calls. > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> my question about handling in the caller got addressed by patch 9, good enough. Reviewed-by: Eric Sandeen <sandeen@redhat.com
diff --git a/libfrog/ptvar.c b/libfrog/ptvar.c index 6cb58208..55324b71 100644 --- a/libfrog/ptvar.c +++ b/libfrog/ptvar.c @@ -44,8 +44,12 @@ ptvar_alloc( int ret; #ifdef _SC_LEVEL1_DCACHE_LINESIZE + long l1_dcache; + /* Try to prevent cache pingpong by aligning to cacheline size. */ - size = max(size, sysconf(_SC_LEVEL1_DCACHE_LINESIZE)); + l1_dcache = sysconf(_SC_LEVEL1_DCACHE_LINESIZE); + if (l1_dcache > 0) + size = roundup(size, l1_dcache); #endif ptv = malloc(PTVAR_SIZE(nr, size)); @@ -88,17 +92,26 @@ ptvar_get( int *retp) { void *p; + int ret; p = pthread_getspecific(ptv->key); if (!p) { pthread_mutex_lock(&ptv->lock); assert(ptv->nr_used < ptv->nr_counters); p = &ptv->data[(ptv->nr_used++) * ptv->data_size]; - pthread_setspecific(ptv->key, p); + ret = pthread_setspecific(ptv->key, p); + if (ret) + goto out_unlock; pthread_mutex_unlock(&ptv->lock); } *retp = 0; return p; + +out_unlock: + ptv->nr_used--; + pthread_mutex_unlock(&ptv->lock); + *retp = ret; + return NULL; } /* Iterate all of the per-thread variables. */