|
Description
|
During the review phase of the Clearview UV project, one reviewer pointed out that:
"it seems confusing to have both an smac_mutex and smac_lock
field in the softmac_t, as those are generally synonyms. An RFE should
probably be filed to give one or the other a more descriptive name."
It is a little risky to make the requested change right now as it is the close to the
integration. This bug is to tract this to be done after UV integrated.
It seems like smac_lock may be able to be removed. In particular,
smac_lock only protects smac_lower and smac_state, and smac_state
appears unnecessary since it's always SMAC_INITIALIZED when
smac_lower is NULL and SMAC_READY when smac_lower is non-NULL
(and nothing really uses the state). Further, the only place where
smac_lock is held for any length of time is in softmac_m_close():
rw_enter(&softmac->smac_lock, RW_WRITER);
slp = softmac->smac_lower;
ASSERT(slp != NULL);
/*
* Note that slp is destroyed when lh is closed.
*/
(void) ldi_close(slp->sl_lh, FREAD|FWRITE, kcred);
softmac->smac_state = SOFTMAC_INITIALIZED;
softmac->smac_lower = NULL;
rw_exit(&softmac->smac_lock);
... but that use of smac_lock seems sketchy to me since we
hold it across ldi_close() and ldi_close() may cause putnext()
to be invoked (e.g., if a STREAMS module was pushed on the
lower stream and it decided to do a putnext() prior to calling
qprocesoff()).
Exactly. smac_lock has been removed in my new workspace. The change has been already
reflected in the new webrev I sent out couple of days ago.
|
