On Wed, 2011-10-26 at 08:09 +0200, Bart Van Assche wrote:
> On Mon, Oct 24, 2011 at 7:57 AM, Nicholas A. Bellinger
> <nab@xxxxxxxxxxxxxxx> wrote:
> > +static int srpt_write_pending(struct se_cmd *se_cmd)
> > +{
> > + struct srpt_rdma_ch *ch;
> > + struct srpt_send_ioctx *ioctx;
> > + enum srpt_command_state new_state;
> > + enum rdma_ch_state ch_state;
> > + int ret;
> > +
> > + ioctx = container_of(se_cmd, struct srpt_send_ioctx, cmd);
> > +
> > + new_state = srpt_set_cmd_state(ioctx, SRPT_STATE_NEED_DATA);
> > + WARN_ON(new_state == SRPT_STATE_DONE);
> > +
> > + ch = ioctx->ch;
> > + BUG_ON(!ch);
> > +
> > + ch_state = srpt_get_ch_state(ch);
> > + switch (ch_state) {
> > + case CH_CONNECTING:
> > + /* This code should never be reached. */
> > + __WARN();
> > + ret = -EINVAL;
> > + goto out;
> > + case CH_LIVE:
> > + break;
> > + case CH_DISCONNECTING:
> > + case CH_DRAINING:
> > + case CH_RELEASING:
> > + pr_debug("cmd with tag %lld: channel disconnecting\n",
> > + ioctx->tag);
> > + srpt_set_cmd_state(ioctx, SRPT_STATE_DATA_IN);
> > + ret = -EINVAL;
> > + goto out;
> > + }
> > + ret = srpt_xfer_data(ch, ioctx);
> > +
> > +out:
> > + return ret;
> > +}
>
> If not enough memory is available to perform a SCSI write, this
> function returns -ENOMEM and the LIO core will report that error to
> the initiator. At least when mounting a filesystem on top of ib_srp
> and when performing asynchronous I/O that will result in data loss.
> Shouldn't the LIO core retry writes as long as not enough memory is
> available ?
>
> A similar argument holds for reads.
So target_core_fabric_ops->write_pending(), ->queue_data_in() and
->queue_status() currently check for a -EAGAIN return in order to signal
that QUEUE_FULL logic should kick in for the associated se_cmd
descriptor.
I think it's reasonable to also allow fabric modules to signal
QUEUE_FULL using -ENOMEM as well, so I'll go ahead and change this for
v3.2 w/ ib_srpt and will send out a patch shortly.
Also, I did notice one extra case in srpt_map_sg_to_ib_sge() ->
srpt_xfer_data() that returns -EBUSY, so i'll change this to -EAGAIN to
signal QUEUE_FULL as well.
Thanks for the feedback Bart!
--nab
--
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
