Re: [PATCH V4 3/4] Introduce XEN scsiback module
On Wed, 2014-08-13 at 09:02 +0200, Juergen Gross wrote:
On 08/12/2014 11:13 PM, Nicholas A. Bellinger wrote:
SNIP
+
+static int scsiback_port_link(struct se_portal_group *se_tpg,
+ struct se_lun *lun)
+{
+ struct scsiback_tpg *tpg = container_of(se_tpg,
+ struct scsiback_tpg, se_tpg);
+
+ mutex_lock(scsiback_mutex);
+
+ mutex_lock(tpg-tv_tpg_mutex);
+ tpg-tv_tpg_port_count++;
+ mutex_unlock(tpg-tv_tpg_mutex);
+
+ mutex_unlock(scsiback_mutex);
+
+ return 0;
+}
+
AFAICT, no need to hold scsiback_mutex while incrementing
tpg-tv_tpg_port_count.
So there is a guarantee that port_link and port_unlink are never
called in parallel?
Correct. configfs_symlink() only calls create_link() once
type-ct_item_ops-allow_link() - target_fabric_port_link() -
TFO-fabric_post_link() has successfully completed, effectively
preventing configfs_unlink() + subsequent TFO-fabric_pre_unlink()
execution until after configfs_symlink() completes.
--nab
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V4 3/4] Introduce XEN scsiback module
On 08/12/2014 11:13 PM, Nicholas A. Bellinger wrote:
Hi Juergen Co,
Finally had a chance to review this code. Comments are inline below..
Thank you very much for your review!
On Fri, 2014-08-08 at 09:49 +0200, jgr...@suse.com wrote:
From: Juergen Gross jgr...@suse.com
Introduces the XEN pvSCSI backend. With pvSCSI it is possible for a XEN domU
to issue SCSI commands to a SCSI LUN assigned to that domU. The SCSI commands
are passed to the pvSCSI backend in a driver domain (usually Dom0) which is
owner of the physical device. This allows e.g. to use SCSI tape drives in a
XEN domU.
The code is taken from the pvSCSI implementation in XEN done by Fujitsu based
on Linux kernel 2.6.18.
Changes from the original version are:
- port to upstream kernel
- put all code in just one source file
- adapt to Linux style guide
- use target core infrastructure instead doing pure pass-through
- enable module unloading
- support SG-list in grant page(s)
- support task abort
- remove redundant struct backend
- allocate resources dynamically
- correct minor error in scsiback_fast_flush_area
- free allocated resources in case of error during I/O preparation
- remove CDB emulation, now handled by target core infrastructure
Signed-off-by: Juergen Gross jgr...@suse.com
Xen related parts
Acked-by: David Vrabel david.vra...@citrix.com
---
SNIP
diff --git a/drivers/xen/xen-scsiback.c b/drivers/xen/xen-scsiback.c
new file mode 100644
index 000..4a0d6e3
--- /dev/null
+++ b/drivers/xen/xen-scsiback.c
SNIP
+struct scsiback_nacl {
+ /* Binary World Wide unique Port Name for pvscsi Initiator port */
+ u64 iport_wwpn;
+ /* ASCII formatted WWPN for Sas Initiator port */
+ char iport_name[VSCSI_NAMELEN];
+ /* Returned by scsiback_make_nodeacl() */
+ struct se_node_acl se_node_acl;
+};
+
Given that this code is similar to how loopback + vhost-scsi function,
and uses a (locally) generated nexus for each WWPN endpoint, the
scsiback_nacl and associated code will be unused should be be dropped.
Done.
SNIP
+static void scsiback_do_resp_with_sense(char *sense_buffer, int32_t result,
+ uint32_t resid, struct vscsibk_pend *pending_req)
+{
+ struct vscsiif_response *ring_res;
+ struct vscsibk_info *info = pending_req-info;
+ int notify;
+ struct scsi_sense_hdr sshdr;
+ unsigned long flags;
+ unsigned len;
+
+ spin_lock_irqsave(info-ring_lock, flags);
+
+ ring_res = RING_GET_RESPONSE(info-ring, info-ring.rsp_prod_pvt);
+ info-ring.rsp_prod_pvt++;
+
+ ring_res-rslt = result;
+ ring_res-rqid = pending_req-rqid;
+
+ if (sense_buffer != NULL
+ scsi_normalize_sense(sense_buffer, VSCSIIF_SENSE_BUFFERSIZE,
+sshdr)) {
+ len = min_t(unsigned, 8 + sense_buffer[7],
+ VSCSIIF_SENSE_BUFFERSIZE);
+ memcpy(ring_res-sense_buffer, sense_buffer, len);
+ ring_res-sense_len = len;
+ } else {
+ ring_res-sense_len = 0;
+ }
+
+ ring_res-residual_len = resid;
+
+ RING_PUSH_RESPONSES_AND_CHECK_NOTIFY(info-ring, notify);
+ spin_unlock_irqrestore(info-ring_lock, flags);
+
+ if (notify)
+ notify_remote_via_irq(info-irq);
+
+ if (pending_req-v2p)
+ kref_put(pending_req-v2p-kref,
+scsiback_free_translation_entry);
+
+ kmem_cache_free(scsiback_cachep, pending_req);
+}
+
+static void scsiback_cmd_done(struct vscsibk_pend *pending_req)
+{
+ struct vscsibk_info *info = pending_req-info;
+ unsigned char *sense_buffer;
+ unsigned int resid;
+ int errors;
+
+ sense_buffer = pending_req-sense_buffer;
+ resid= pending_req-se_cmd.residual_count;
+ errors = pending_req-result;
+
+ if (errors log_print_stat)
+ scsiback_print_status(sense_buffer, errors, pending_req);
+
+ scsiback_fast_flush_area(pending_req);
+ scsiback_do_resp_with_sense(sense_buffer, errors, resid, pending_req);
+ scsiback_put(info);
+
+ transport_generic_free_cmd(pending_req-se_cmd, 0);
+}
+
The usage here of scsiback_do_resp_with_sense() - kmem_cache_free() for
*pending_req, and then invoking transport_generic_free_cmd() with
pending_req-se_cmd is an free after use bug..
So the way this should work is similar to how loopback currently does
things:
- Move the kmem_cache_free() for pending_req from
scsiback_do_resp_with_sense() to scsiback_release_cmd()
- Remove the transport_generic_free_cmd() from scsiback_cmd_done()
- Copy what tcm_loop_check_stop_free() does into
scsiback_check_stop_free(), and remove target_put_sess_cmd()
Done.
+static void scsiback_cmd_exec(struct vscsibk_pend *pending_req)
+{
+ struct se_cmd *se_cmd = pending_req-se_cmd;
+ struct se_session *sess =
Re: [PATCH V4 3/4] Introduce XEN scsiback module
Hi Juergen Co,
Finally had a chance to review this code. Comments are inline below..
On Fri, 2014-08-08 at 09:49 +0200, jgr...@suse.com wrote:
From: Juergen Gross jgr...@suse.com
Introduces the XEN pvSCSI backend. With pvSCSI it is possible for a XEN domU
to issue SCSI commands to a SCSI LUN assigned to that domU. The SCSI commands
are passed to the pvSCSI backend in a driver domain (usually Dom0) which is
owner of the physical device. This allows e.g. to use SCSI tape drives in a
XEN domU.
The code is taken from the pvSCSI implementation in XEN done by Fujitsu based
on Linux kernel 2.6.18.
Changes from the original version are:
- port to upstream kernel
- put all code in just one source file
- adapt to Linux style guide
- use target core infrastructure instead doing pure pass-through
- enable module unloading
- support SG-list in grant page(s)
- support task abort
- remove redundant struct backend
- allocate resources dynamically
- correct minor error in scsiback_fast_flush_area
- free allocated resources in case of error during I/O preparation
- remove CDB emulation, now handled by target core infrastructure
Signed-off-by: Juergen Gross jgr...@suse.com
Xen related parts
Acked-by: David Vrabel david.vra...@citrix.com
---
SNIP
diff --git a/drivers/xen/xen-scsiback.c b/drivers/xen/xen-scsiback.c
new file mode 100644
index 000..4a0d6e3
--- /dev/null
+++ b/drivers/xen/xen-scsiback.c
SNIP
+struct scsiback_nacl {
+ /* Binary World Wide unique Port Name for pvscsi Initiator port */
+ u64 iport_wwpn;
+ /* ASCII formatted WWPN for Sas Initiator port */
+ char iport_name[VSCSI_NAMELEN];
+ /* Returned by scsiback_make_nodeacl() */
+ struct se_node_acl se_node_acl;
+};
+
Given that this code is similar to how loopback + vhost-scsi function,
and uses a (locally) generated nexus for each WWPN endpoint, the
scsiback_nacl and associated code will be unused should be be dropped.
SNIP
+static void scsiback_do_resp_with_sense(char *sense_buffer, int32_t result,
+ uint32_t resid, struct vscsibk_pend *pending_req)
+{
+ struct vscsiif_response *ring_res;
+ struct vscsibk_info *info = pending_req-info;
+ int notify;
+ struct scsi_sense_hdr sshdr;
+ unsigned long flags;
+ unsigned len;
+
+ spin_lock_irqsave(info-ring_lock, flags);
+
+ ring_res = RING_GET_RESPONSE(info-ring, info-ring.rsp_prod_pvt);
+ info-ring.rsp_prod_pvt++;
+
+ ring_res-rslt = result;
+ ring_res-rqid = pending_req-rqid;
+
+ if (sense_buffer != NULL
+ scsi_normalize_sense(sense_buffer, VSCSIIF_SENSE_BUFFERSIZE,
+ sshdr)) {
+ len = min_t(unsigned, 8 + sense_buffer[7],
+ VSCSIIF_SENSE_BUFFERSIZE);
+ memcpy(ring_res-sense_buffer, sense_buffer, len);
+ ring_res-sense_len = len;
+ } else {
+ ring_res-sense_len = 0;
+ }
+
+ ring_res-residual_len = resid;
+
+ RING_PUSH_RESPONSES_AND_CHECK_NOTIFY(info-ring, notify);
+ spin_unlock_irqrestore(info-ring_lock, flags);
+
+ if (notify)
+ notify_remote_via_irq(info-irq);
+
+ if (pending_req-v2p)
+ kref_put(pending_req-v2p-kref,
+ scsiback_free_translation_entry);
+
+ kmem_cache_free(scsiback_cachep, pending_req);
+}
+
+static void scsiback_cmd_done(struct vscsibk_pend *pending_req)
+{
+ struct vscsibk_info *info = pending_req-info;
+ unsigned char *sense_buffer;
+ unsigned int resid;
+ int errors;
+
+ sense_buffer = pending_req-sense_buffer;
+ resid= pending_req-se_cmd.residual_count;
+ errors = pending_req-result;
+
+ if (errors log_print_stat)
+ scsiback_print_status(sense_buffer, errors, pending_req);
+
+ scsiback_fast_flush_area(pending_req);
+ scsiback_do_resp_with_sense(sense_buffer, errors, resid, pending_req);
+ scsiback_put(info);
+
+ transport_generic_free_cmd(pending_req-se_cmd, 0);
+}
+
The usage here of scsiback_do_resp_with_sense() - kmem_cache_free() for
*pending_req, and then invoking transport_generic_free_cmd() with
pending_req-se_cmd is an free after use bug..
So the way this should work is similar to how loopback currently does
things:
- Move the kmem_cache_free() for pending_req from
scsiback_do_resp_with_sense() to scsiback_release_cmd()
- Remove the transport_generic_free_cmd() from scsiback_cmd_done()
- Copy what tcm_loop_check_stop_free() does into
scsiback_check_stop_free(), and remove target_put_sess_cmd()
+static void scsiback_cmd_exec(struct vscsibk_pend *pending_req)
+{
+ struct se_cmd *se_cmd = pending_req-se_cmd;
+ struct se_session *sess = pending_req-v2p-tpg-tpg_nexus-tvn_se_sess;
+ int rc;
+
+ memset(pending_req-sense_buffer, 0, VSCSIIF_SENSE_BUFFERSIZE);
+
+
Re: [PATCH V4 3/4] Introduce XEN scsiback module
+#include scsi/scsi_dbg.h
+#include scsi/scsi_eh.h
+#include scsi/scsi_tcq.h
What do you need these for? Normally target drivers shouldn't need
these.
+struct vscsibk_emulate {
+ void (*pre_function)(struct vscsibk_pend *, void *);
+ void (*post_function)(struct vscsibk_pend *, void *);
+};
This doesn't seem to be used.
+#define scsiback_get(_b) (atomic_inc((_b)-nr_unreplied_reqs))
+#define scsiback_put(_b) \
+ do {\
+ if (atomic_dec_and_test((_b)-nr_unreplied_reqs)) \
+ wake_up((_b)-waiting_to_free);\
+ } while (0)
Normal Linux style would be to make these inline functions.
+static void scsiback_notify_work(struct vscsibk_info *info)
+{
+ info-waiting_reqs = 1;
+ wake_up(info-wq);
+}
+
+static irqreturn_t scsiback_intr(int irq, void *dev_id)
+{
+ scsiback_notify_work((struct vscsibk_info *)dev_id);
+ return IRQ_HANDLED;
+}
Seems like this driver should get the same threaded irq treatment as
the initiator side?
+static void scsiback_disconnect(struct vscsibk_info *info)
+{
+ if (info-kthread) {
+ kthread_stop(info-kthread);
+ info-kthread = NULL;
+ wake_up(info-shutdown_wq);
+ }
+
+ wait_event(info-waiting_to_free,
+ atomic_read(info-nr_unreplied_reqs) == 0);
+
+ if (info-irq) {
+ unbind_from_irqhandler(info-irq, info);
+ info-irq = 0;
+ }
+
+ if (info-ring.sring) {
+ xenbus_unmap_ring_vfree(info-dev, info-ring.sring);
+ info-ring.sring = NULL;
+ }
+}
Also the same treatment for goto based init failure unwinding.
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
