On Thu, Feb 13, 2020 at 12:03:32PM -0500, Marcelo Diop-Gonzalez wrote:
> On Wed, Feb 12, 2020 at 1:43 PM Marcelo Diop-Gonzalez
> <marcgonzalez@xxxxxxxxxx> wrote:
> >
> > There are a few places where a service's reference count is incremented,
> > something quick is done, and the refcount is dropped. This can be made
> > a little simpler/faster by not grabbing a reference in these cases.
> >
> > Signed-off-by: Marcelo Diop-Gonzalez <marcgonzalez@xxxxxxxxxx>
> > ---
> > .../interface/vchiq_arm/vchiq_arm.c | 16 ++++-----
> > .../interface/vchiq_arm/vchiq_core.c | 36 +++++++++++++------
> > .../interface/vchiq_arm/vchiq_core.h | 8 ++++-
> > 3 files changed, 40 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> > index 3ed0e4ea7f5c..b377f18aed45 100644
> > --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> > +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> > @@ -2497,11 +2497,11 @@ vchiq_instance_get_use_count(struct vchiq_instance *instance)
> > int use_count = 0, i;
> >
> > i = 0;
> > - while ((service = next_service_by_instance(instance->state,
> > - instance, &i))) {
> > + rcu_read_lock();
> > + while ((service = __next_service_by_instance(instance->state,
> > + instance, &i)))
> > use_count += service->service_use_count;
> > - unlock_service(service);
> > - }
> > + rcu_read_unlock();
> > return use_count;
> > }
> >
> > @@ -2524,11 +2524,11 @@ vchiq_instance_set_trace(struct vchiq_instance *instance, int trace)
> > int i;
> >
> > i = 0;
> > - while ((service = next_service_by_instance(instance->state,
> > - instance, &i))) {
> > + rcu_read_lock();
> > + while ((service = __next_service_by_instance(instance->state,
> > + instance, &i)))
> > service->trace = trace;
> > - unlock_service(service);
> > - }
> > + rcu_read_unlock();
> > instance->trace = (trace != 0);
> > }
> >
> > diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
> > index 65270a5b29db..d7d7f4d9d57f 100644
> > --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
> > +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
> > @@ -222,28 +222,42 @@ find_closed_service_for_instance(struct vchiq_instance *instance,
> > }
> >
> > struct vchiq_service *
> > -next_service_by_instance(struct vchiq_state *state, struct vchiq_instance *instance,
> > - int *pidx)
> > +__next_service_by_instance(struct vchiq_state *state,
> > + struct vchiq_instance *instance,
> > + int *pidx)
> > {
> > struct vchiq_service *service = NULL;
> > int idx = *pidx;
> >
> > - rcu_read_lock();
> > while (idx < state->unused_service) {
> > struct vchiq_service *srv;
> >
> > srv = rcu_dereference(state->services[idx++]);
> > if (srv && srv->srvstate != VCHIQ_SRVSTATE_FREE &&
> > - srv->instance == instance &&
> > - kref_get_unless_zero(&srv->ref_count)) {
> > - service = rcu_pointer_handoff(srv);
> > + srv->instance == instance) {
> > + service = srv;
> > break;
> > }
> > }
> > - rcu_read_unlock();
> >
> > *pidx = idx;
> > + return service;
> > +}
> >
> > +struct vchiq_service *
> > +next_service_by_instance(struct vchiq_state *state,
> > + struct vchiq_instance *instance,
> > + int *pidx)
> > +{
> > + struct vchiq_service *service;
> > +
> > + rcu_read_lock();
> > + service = __next_service_by_instance(state, instance, pidx);
> > + if (service && kref_get_unless_zero(&service->ref_count))
> > + service = rcu_pointer_handoff(service);
> > + else
> > + service = NULL;
> > + rcu_read_unlock();
> > return service;
> > }
>
> ahh wait, this one is buggy..... If kref_get_unless_zero fails then we
> want to keep looking
> for the next one. Greg, since you already applied this one, would it
> be best for me to send
> a patch on top of this or send a V2?
On top is easiest for me, I don't like to revert things :)
thanks,
greg k-h
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
