illumos gate

In scsi_hba_tgtmap_set_add(9f), the tgt_privp argument is described as follows:

The tgt_privp argument is a modifiable private value. Initially, tgt_privp points to the value passed in as tgt_priv to either the scsi_hba_tgtmap_set_add() or scsi_hba_tgtmap_tgt_add() functions. The driver may change the value as needed. It will receive the value stored in tgt_privp during the deactivate callback. [...]
The tgt_priv pointer will be set to the value that was passed when the device was added and will reflect any updates made during an activate callback, if present. [...]
The tgt_priv argument will be passed to the activate and deactivate callbacks, allowing the driver to pass around data corresponding to this address.

That isn't what the code does, though. When scsi_hba_tgtmap_set_add() is called, it calls damap_addrset_add() to do the actual work. tgt_privp is stored in a da_map_t.da_ppriv_rpt for later. And later, in dam_addr_activate(), which calls the activate callback passed to scsi_hba_tgtmap_set_add() earlier, the contents of da_ppriv_rpt is saved to da_priv and da_ppriv_rpt is set to NULL:

/*
 * activate an address that has passed the stabilization interval
 */
static void
dam_addr_activate(dam_t *mapp, id_t addrid)
{
    dam_da_t *passp;
    int config_rv;
    char *addrstr;

    mutex_enter(&mapp->dam_lock);
    bitset_add(&mapp->dam_active_set, addrid);
    passp = ddi_get_soft_state(mapp->dam_da, addrid);
    ASSERT(passp);

    /*
     * copy the reported nvlist and provider private data
     */
    addrstr = ddi_strid_id2str(mapp->dam_addr_hash, addrid);
    DTRACE_PROBE3(damap__addr__activate__start,
        char *, mapp->dam_name, dam_t *, mapp,
        char *, addrstr);
    passp->da_nvl = passp->da_nvl_rpt;
    passp->da_ppriv = passp->da_ppriv_rpt;
    passp->da_ppriv_rpt = NULL;
    passp->da_nvl_rpt = NULL;
    passp->da_last_stable = gethrtime();
    passp->da_stable_cnt++;
    mutex_exit(&mapp->dam_lock);
    if (mapp->dam_activate_cb) {
        (*mapp->dam_activate_cb)(mapp->dam_activate_arg, addrstr,
            addrid, &passp->da_ppriv_rpt);
    }

Consequently, the activate callback is always passed NULL as tgt_priv.

There is another issue: The deactivate callback is passed da_ppriv in one place, and da_ppriv_rpt in another. So depending on the circumstances, the deactivate callback may either receive the tgt_priv passed to scsi_hba_tgtmap_set_add(9f), the tgt_priv that the activate callback may have set, or NULL.

It seems no in-gate HBA drivers use the tgt_priv argument, and it would be unclear to me what use it may have the way it is now.

I propose to get rid of da_ppriv_rpt and just use da_ppriv everywhere, thereby changing the semantics of the code to match the documentation: The tgt_priv passed to scsi_hba_tgtmap_set_add(9f) is passed to the activate callback, which may modify it. And whatever value tgt_priv is after the activate callback is always passed to the deactivate callback.