Messages in this thread | ![/](/images/icornerl.gif) | | From | "Fabio M. De Francesco" <> | Subject | Re: [RFC PATCH v3] cleanup: Add cond_guard() to conditional guards | Date | Thu, 01 Feb 2024 02:08:44 +0100 |
| |
I just noticed that this is not the final version. It misses a semicolon. Please discard this v3. I'm sending v4.
Fabio
On Wednesday, 31 January 2024 14:37:35 CET Fabio M. De Francesco wrote: > Add cond_guard() to conditional guards. > > cond_guard() is used for the _interruptible(), _killable(), and _try > versions of locks. > > It stores a return value to a variable whose address is given to its > second argument, that is either '-1' on failure or '0' on success to > acquire a lock. The returned value can be checked to act accordingly > (e.g., to call printk() and return -EINTR in case of failure of an > _interruptible() variant) > > As the other guards, it avoids to open code the release of the lock > after a goto to an 'out' label. > > This remains an RFC because Dan suggested a slightly different syntax. > > The changes to the CXL code are provided only to show the use of this > macro. If consensus is gathered on this macro, the cleanup of > show_targetN() will be submitted later in a separate patch. > > Cc: Peter Zijlstra <peterz@infradead.org> > Cc: Dan Williams <dan.j.williams@intel.com> > Suggested-by: Ira Weiny <ira.weiny@intel.com> > Signed-off-by: Fabio M. De Francesco > <fabio.maria.de.francesco@linux.intel.com> --- > > Changes from v1 and v2: > I've taken into account Dan's comments (thanks). > > drivers/cxl/core/region.c | 15 +++++---------- > include/linux/cleanup.h | 19 +++++++++++++++++++ > 2 files changed, 24 insertions(+), 10 deletions(-) > > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c > index 0f05692bfec3..560f25bdfd11 100644 > --- a/drivers/cxl/core/region.c > +++ b/drivers/cxl/core/region.c > @@ -668,26 +668,21 @@ static size_t show_targetN(struct cxl_region *cxlr, > char *buf, int pos) struct cxl_endpoint_decoder *cxled; > int rc; > > - rc = down_read_interruptible(&cxl_region_rwsem); > + cond_guard(rwsem_read_intr, &rc, &cxl_region_rwsem); > if (rc) > - return rc; > + return -EINTR; > > if (pos >= p->interleave_ways) { > dev_dbg(&cxlr->dev, "position %d out of range %d\n", pos, > p->interleave_ways); > - rc = -ENXIO; > - goto out; > + return -ENXIO; > } > > cxled = p->targets[pos]; > if (!cxled) > - rc = sysfs_emit(buf, "\n"); > + return sysfs_emit(buf, "\n"); > else > - rc = sysfs_emit(buf, "%s\n", dev_name(&cxled->cxld.dev)); > -out: > - up_read(&cxl_region_rwsem); > - > - return rc; > + return sysfs_emit(buf, "%s\n", dev_name(&cxled->cxld.dev)); > } > > static int match_free_decoder(struct device *dev, void *data) > diff --git a/include/linux/cleanup.h b/include/linux/cleanup.h > index c2d09bc4f976..a4b40d511f9e 100644 > --- a/include/linux/cleanup.h > +++ b/include/linux/cleanup.h > @@ -134,6 +134,20 @@ static inline class_##_name##_t > class_##_name##ext##_constructor(_init_args) \ * an anonymous instance of > the (guard) class, not recommended for * conditional locks. > * > + * cond_guard(name, ret, args...): > + * for conditional locks like mutex_trylock() or > down_read_interruptible(). + * 'ret' is a pointer to a variable where this > macro stores 0 on success + * and -1 on failure to acquire a lock. > + * > + * Example: > + * > + * int ret; > + * cond_guard(rwsem_read_try, &ret, &sem); > + * if (ret) { > + * dev_dbg("down_read_trylock() failed to down 'sem')\n"); > + * return 0; // down_read_trylock() returns 0 on contention > + * } > + * > * scoped_guard (name, args...) { }: > * similar to CLASS(name, scope)(args), except the variable (with the > * explicit name 'scope') is declard in a for-loop such that its scope is > @@ -165,6 +179,11 @@ static inline class_##_name##_t > class_##_name##ext##_constructor(_init_args) \ > > #define __guard_ptr(_name) class_##_name##_lock_ptr > > +#define cond_guard(_name, _ret, args...) \ > + CLASS(_name, scope)(args); \ > + if (!__guard_ptr(_name)(&scope)) *_ret = -1 \ > + else *_ret = 0 > + > #define scoped_guard(_name, args...) \ > for (CLASS(_name, scope)(args), \ > *done = NULL; __guard_ptr(_name)(&scope) && !done; done = (void *)1)
| ![\](/images/icornerr.gif) |