Messages in this thread |  | | | Date | Mon, 8 Jun 2009 16:05:25 -0700 | | From | Andrew Morton <> | | Subject | Re: [Patch 02/12] GRU - add user request to explicitly unload a gru context |
| |
On Mon, 08 Jun 2009 12:16:50 -0500 steiner@sgi.com wrote:
> /* > + * Free all kernel contexts that are not currently in use. > + * Returns 0 if all freed, else number of inuse context. > + */ > +static int gru_free_kernel_contexts(void) > +{ > + struct gru_blade_state *bs; > + struct gru_thread_state *kgts; > + int bid, ret = 0; > + > + for (bid = 0; bid < GRU_MAX_BLADES; bid++) { > + bs = gru_base[bid]; > + if (!bs) > + continue; > + if (down_write_trylock(&bs->bs_kgts_sema)) {
trylocks are always lame - they add a rarely-executed code path where bugs can lurk. They're often an admission that the locking is screwed up.
I don't know if the latter is true here, but it would be helpful to add a comment explaining what's going on, and why this unusual and troublesome locking primitive is being used.
> + kgts = bs->bs_kgts; > + if (kgts && kgts->ts_gru) > + gru_unload_context(kgts, 0); > + kfree(kgts); > + bs->bs_kgts = NULL; > + up_write(&bs->bs_kgts_sema);
nit: the kfree() can be moved outside the locked region.
> + } else { > + ret++; > + } > + } > + return ret; > +}
|  |