On Mon, 2011-11-28 at 03:07 -0500, Christoph Hellwig wrote:
> > This patch adds TARGET_SCF_ACK_KREF for target_submit_cmd() that passes
> > a 'ack_kref' parameter into target_get_sess_cmd() to determine when
> > the fabric caller is expecting a second kref_put() from fabric
> > packet acknowledgement.
>
> The flag name isn't really very descriptive, I'd rather either chose
> one that describes why we are doing it, or find a way to drop the
> additional reference even for drivers not currently doing it.
>
So fabrics who have an explicit acknowledgment before release in another
context use the flag to grab the extra kref. I'm fine with ideas on a
better name for this..?
As for eventually trying to drop this alltogether, we will need to
re-think ->check_stop_free() usage for active I/O shutdown to determine
when the response codepath may no longer access se_cmd, or may
optionally release se_cmd in response codepath because the explict ack
has already returned from hw.
> > -void target_get_sess_cmd(struct se_session *se_sess, struct se_cmd *se_cmd)
> > +void target_get_sess_cmd(struct se_session *se_sess, struct se_cmd *se_cmd,
> > + bool ack_kref)
> > {
> > unsigned long flags;
> >
> > kref_init(&se_cmd->cmd_kref);
> > - kref_get(&se_cmd->cmd_kref);
> > + /*
> > + * Add a second kref if the fabric caller is expecting to handle
> > + * fabric acknowledgement that requires two target_put_sess_cmd()
> > + * invocations before se_cmd descriptor release.
> > + */
> > + if (ack_kref == true)
> > + kref_get(&se_cmd->cmd_kref);
> >
> > spin_lock_irqsave(&se_sess->sess_cmd_lock, flags);
> > list_add_tail(&se_cmd->se_cmd_list, &se_sess->sess_cmd_list);
>
> Why don't you simply do the kref_get after target_get_sess_cmd has
> returned, avoiding the parameter to target_get_sess_cmd?
>
Removing this left-over conversion cruft now.
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
