Re: generating a Linux WWN?
The popular solutions I've seen are: 1) Random bytes, or fixed id + random bytes. Even on a worldwide net, collisions are highly unlikely. The problem is that the random bytes are not necessarily random; especially not at boot: get_random_bytes gets its randomness from the entropy pool. The pool starts with the time of the day and some always the same data and then gets feed by time stamps from a few interrupts. This doesn't include network interrupts (or rather only a small handfull of network drivers set the flag -- but at early boot you don't have any network anyways), but is primarily keyboard/mouse and storage interrupts. On headless systems typically only storage interrupts. Now do you see the problem with using get_random_bytes() to get your storage working? In many cases the time of the day in the pool will prevent the worst, but if you have a cluster where all boxes boot at the same time it's quite likely your early randomness will be all the same.(ok cluster boots are usually staggered, but you still have a large number of systems booting at the same time) The whole thing would probably work if you could mix something machine unique into the entropy pool at boot (e.g. a network mac address); but Linux can't do that do to boot strap ordering issues. I agree with the people who say this is a highly dangerous thing to do. It is much better to fail clearly that to create a hard to debug subtle problem in a storage network. If you have prototype hardware that doesn't supply an unique ID then I would suggest you add some module parameter to allow choosing a unique (or even random) one. But don't make it default. -Andi - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: slave_configure for qlogicpti
David Miller wrote:
From: Jeff Garzik [EMAIL PROTECTED]
Date: Sun, 30 Sep 2007 20:52:45 -0400
David Miller wrote:
It compiles :-) You deleted the only uses of scsi_rbuf_{get,put}()
so you can kill those off too.
Seeing as how they are exact duplicates of libata's
ata_scsi_rbuf_{get,put}, I wonder how they got there in the first place
(rather than becoming common code), and I wonder how many more copies
are floating out there...
I bet: 1) they were common code in scsi_lib.c 2) qlogicpti and libata
became the only remaining users so 3) they got copied to those
two users verbatim anticipating that the qlogicpti usage would
eventually be deleted.
I wrote the libata code from scratch, so I was just sorta curious. No
biggie either (read: I'm too lazy to search the history).
After the qlogicpti use goes away, libata-scsi is the remaining user of
that logic. The code is already local to libata, so... all good.
Jeff
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 13/16] gdth: Make one abuse of scsi_cmnd less obvious
On Mon, Oct 01 2007 at 1:21 +0200, Matthew Wilcox [EMAIL PROTECTED] wrote: On Sun, Sep 30, 2007 at 10:28:13PM +0100, Christoph Hellwig wrote: I think it would be better if your whole patch series goes ontop of willy's -done removal series instead. I really hope we can get that one into scsi-misc ASAP. Actually, I think if Boaz simply flips this patch to be after his next one (or merges them ...), there's no real dependency on the -done removal. But this is exactly it, 14th patch is dependent on this one. Because now I have a central place to deallocate the private per-cmnd-info. I have looked at it and in it's original form it mainly conflicts with Jeff's [PATCH 4/16] gdth: Remove 2.4.x support. (And minor other places) What I could do right away is put it at 7-th place. So the first 6 patches can go in now as you said, followed by Matthew's patchset. But on the other hand Matthew's second patch just dropped once a proper per-cmnd-info was setup, so the all thing is entangled in this way. But I totally understand Christoph's motivation with regard to Matthew's patchset. OK My suggestion is as follows: - Accept the first 6 patches right away, I think we have a consensus on that. Except from Achim, I got an out-of-the-office notice from his mailer, so it might take some time. Can we push them in anyway, as these should be pretty safe. - I will advance this patch to 7th place. - I will revive Matthew's second patch to follow these 7 patches. - Mathew can now submit the rest of his patchset minus the gdth patches, as these are taken care of. Following will be a second part. But these will wait Achim's ACKs: - I will unite my 7th 8th patch to now be 9th. (gdth: Remove virt hosts) - Redo 10th patch as per Christoph comments. - I will rebase the rest of the patches to the new conditions. Mainly redoing 14th patch ([PATCH 14/16] gdth: Setup proper per-command private data This is because of conflicts with Matthew's second part) - Christoph or Jeff will work on the finish up of the BUS hotplug API. I have looked at code examples elsewhere in the kernel, and Jeff's master plan sounds very good. But I would hope not to do it myself as it will take me much longer. Jeff it sounds like you have it clearer in your head? James? We need a decision here, about if we can push the first 6 patches ASAP. and than Matthew's 2 patches that I will send soon, after we decide. That is, if Matthew's patchset is to go in NOW. Other wise, there is no rush and it can stay in the order they are now, which is easiest for me. Thanks everyone, I'm awaiting all your comments to proceed. Boaz - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Add a slave_configure method to qlogicpti
By configuring targets in slave_configure, we can eliminate a shadow
queuecommand, a shadow scsi_done, a write to the host template, abuse of
SCp-Message and SCp-Status, a use of kmap_atomic() and sniffing the
results of INQUIRY.
Signed-off-by: Matthew Wilcox [EMAIL PROTECTED]
Acked-by: David S. Miller [EMAIL PROTECTED]
qlogicpti.c | 171 +---
qlogicpti.h |3 -
2 files changed, 27 insertions(+), 147 deletions(-)
diff --git a/drivers/scsi/qlogicpti.c b/drivers/scsi/qlogicpti.c
index 5948872..e93f803 100644
--- a/drivers/scsi/qlogicpti.c
+++ b/drivers/scsi/qlogicpti.c
@@ -310,8 +310,6 @@ static inline void qlogicpti_set_hostdev_defaults(struct
qlogicpti *qpti)
}
qpti-dev_param[i].device_enable = 1;
}
- /* this is very important to set! */
- qpti-sbits = 1 qpti-scsi_id;
}
static int qlogicpti_reset_hardware(struct Scsi_Host *host)
@@ -951,153 +949,35 @@ static inline void update_can_queue(struct Scsi_Host
*host, u_int in_ptr, u_int
host-sg_tablesize = QLOGICPTI_MAX_SG(num_free);
}
-static unsigned int scsi_rbuf_get(struct scsi_cmnd *cmd, unsigned char
**buf_out)
+static int qlogicpti_slave_configure(struct scsi_device *sdev)
{
- unsigned char *buf;
- unsigned int buflen;
-
- if (cmd-use_sg) {
- struct scatterlist *sg;
+ struct qlogicpti *qpti = shost_priv(sdev-host);
+ int tgt = sdev-id;
+ u_short param[6];
- sg = (struct scatterlist *) cmd-request_buffer;
- buf = kmap_atomic(sg-page, KM_IRQ0) + sg-offset;
- buflen = sg-length;
+ /* tags handled in midlayer */
+ /* enable sync mode? */
+ if (sdev-sdtr) {
+ qpti-dev_param[tgt].device_flags |= 0x10;
} else {
- buf = cmd-request_buffer;
- buflen = cmd-request_bufflen;
+ qpti-dev_param[tgt].synchronous_offset = 0;
+ qpti-dev_param[tgt].synchronous_period = 0;
}
-
- *buf_out = buf;
- return buflen;
-}
-
-static void scsi_rbuf_put(struct scsi_cmnd *cmd, unsigned char *buf)
-{
- if (cmd-use_sg) {
- struct scatterlist *sg;
-
- sg = (struct scatterlist *) cmd-request_buffer;
- kunmap_atomic(buf - sg-offset, KM_IRQ0);
- }
-}
-
-/*
- * Until we scan the entire bus with inquiries, go throught this fella...
- */
-static void ourdone(struct scsi_cmnd *Cmnd)
-{
- struct qlogicpti *qpti = (struct qlogicpti *)
Cmnd-device-host-hostdata;
- int tgt = Cmnd-device-id;
- void (*done) (struct scsi_cmnd *);
-
- /* This grot added by DaveM, blame him for ugliness.
-* The issue is that in the 2.3.x driver we use the
-* host_scribble portion of the scsi command as a
-* completion linked list at interrupt service time,
-* so we have to store the done function pointer elsewhere.
-*/
- done = (void (*)(struct scsi_cmnd *))
- (((unsigned long) Cmnd-SCp.Message)
-#ifdef __sparc_v9__
-| ((unsigned long) Cmnd-SCp.Status 32UL)
-#endif
-);
-
- if ((qpti-sbits (1 tgt)) == 0) {
- int ok = host_byte(Cmnd-result) == DID_OK;
- if (Cmnd-cmnd[0] == 0x12 ok) {
- unsigned char *iqd;
- unsigned int iqd_len;
-
- iqd_len = scsi_rbuf_get(Cmnd, iqd);
-
- /* tags handled in midlayer */
- /* enable sync mode? */
- if (iqd[7] 0x10) {
- qpti-dev_param[tgt].device_flags |= 0x10;
- } else {
- qpti-dev_param[tgt].synchronous_offset = 0;
- qpti-dev_param[tgt].synchronous_period = 0;
- }
- /* are we wide capable? */
- if (iqd[7] 0x20) {
- qpti-dev_param[tgt].device_flags |= 0x20;
- }
-
- scsi_rbuf_put(Cmnd, iqd);
-
- qpti-sbits |= (1 tgt);
- } else if (!ok) {
- qpti-sbits |= (1 tgt);
- }
- }
- done(Cmnd);
-}
-
-static int qlogicpti_queuecommand(struct scsi_cmnd *Cmnd, void (*done)(struct
scsi_cmnd *));
-
-static int qlogicpti_queuecommand_slow(struct scsi_cmnd *Cmnd,
- void (*done)(struct scsi_cmnd *))
-{
- struct qlogicpti *qpti = (struct qlogicpti *)
Cmnd-device-host-hostdata;
-
- /*
-* done checking this host adapter?
-* If not, then rewrite the command
-* to finish through ourdone so we
-* can peek at Inquiry data results.
-*/
- if (qpti-sbits qpti-sbits != 0x) {
- /* See above about in
Re: [PATCH 16/16] gdth: !use_sg cleanup and use of scsi accessors
On Sun, Sep 30 2007 at 22:17 +0200, Boaz Harrosh [EMAIL PROTECTED] wrote: gdth_execute() will issue an internal, none scsi-standard commands onto __gdth_queuecommand(). Since it is not recommended to set struct scsi_cmnd IO members in llds, gdth now uses internal IO members for IO. In the case of gdth_execute() these members will be set properly. In case the command was issued from scsi-ml (by gdth_queuecommand) they will be set from scsi IO accessors. * define gdth IO accessors and use them throughout the driver. * use an sg-of-one in gdth_execute() and fix gdth_special_cmd() accordingly. * Clean the not use_sg code path and company Signed-off-by Boaz Harrosh [EMAIL PROTECTED] OK My usual bug in this one. I woke up in the middle of the night with this in my head. diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c --- a/drivers/scsi/gdth.c +++ b/drivers/scsi/gdth.c @@ -517,4 +517,5 @@ int __gdth_execute(struct scsi_device *sdev, gdth_cmd_str *gdtcmd, char *cmnd, sg_init_one(one_sg, gdtcmd, sizeof(*gdtcmd)); gdth_set_sglist(scp, one_sg); +gdth_set_sg_count(scp, 1); gdth_set_bufflen(scp, sizeof(*gdtcmd)); scp-cmd_len = 12; I will resend the patch as reply to this one Boaz - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 16/16 ver2] gdth: !use_sg cleanup and use of scsi accessors
gdth_execute() will issue an internal, none scsi-standard commands
onto __gdth_queuecommand(). Since it is not recommended to set
struct scsi_cmnd IO members in llds, gdth now uses internal IO
members for IO. In the case of gdth_execute() these members will be
set properly. In case the command was issued from scsi-ml
(by gdth_queuecommand) they will be set from scsi IO accessors.
* define gdth IO accessors and use them throughout the driver.
* use an sg-of-one in gdth_execute() and fix gdth_special_cmd()
accordingly.
* Clean the not use_sg code path and company
Signed-off-by Boaz Harrosh [EMAIL PROTECTED]
---
drivers/scsi/gdth.c | 228 --
drivers/scsi/gdth.h |7 --
2 files changed, 109 insertions(+), 126 deletions(-)
diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c
index c712794..ad3ee90 100644
--- a/drivers/scsi/gdth.c
+++ b/drivers/scsi/gdth.c
@@ -85,11 +85,11 @@
/* The meaning of the Scsi_Pointer members in this driver is as follows:
* ptr: Chaining
- * this_residual: unused
- * buffer: unused
- * dma_handle: will drop in !use_sg patch.
- * buffers_residual:unused
- * Status: DMA mem. mappings (FIXME: drop in !use_sg patch.)
+ * this_residual: gdth_bufflen
+ * buffer: gdth_sglist
+ * dma_handle: unused
+ * buffers_residual:gdth_sg_count
+ * Status: unused
* Message: unused
* have_data_in:unused
* sent_command:unused
@@ -132,6 +132,7 @@
#include asm/uaccess.h
#include linux/spinlock.h
#include linux/blkdev.h
+#include linux/scatterlist.h
#include scsi.h
#include scsi/scsi_host.h
@@ -157,7 +158,7 @@ static void gdth_readapp_event(gdth_ha_str *ha, unchar
application,
static void gdth_clear_events(void);
static void gdth_copy_internal_data(gdth_ha_str *ha, Scsi_Cmnd *scp,
-char *buffer,ushort count);
+char *buffer, ushort count, int to_buffer);
static int gdth_internal_cache_cmd(gdth_ha_str *ha, Scsi_Cmnd *scp);
static int gdth_fill_cache_cmd(gdth_ha_str *ha, Scsi_Cmnd *scp, ushort hdrive);
@@ -379,6 +380,47 @@ static const struct file_operations gdth_fops = {
.release = gdth_close,
};
+/*
+ * gdth scsi_command access wrappers.
+ * below 6 functions are used throughout the driver to access scsi_command's
+ * io parameters. The reason we do not use the regular accessors from
+ * scsi_cmnd.h is because of gdth_execute(). Since it is unrecommended for
+ * llds to directly set scsi_cmnd's IO members. This driver will use SCp
+ * members for IO parameters, and will copy scsi_cmnd's members to Scp
+ * members in queuecommand. For internal commands through gdth_execute()
+ * SCp's members will be set directly.
+ */
+static inline unsigned gdth_bufflen(struct scsi_cmnd *cmd)
+{
+ return (unsigned)cmd-SCp.this_residual;
+}
+
+static inline void gdth_set_bufflen(struct scsi_cmnd *cmd, unsigned bufflen)
+{
+ cmd-SCp.this_residual = bufflen;
+}
+
+static inline unsigned gdth_sg_count(struct scsi_cmnd *cmd)
+{
+ return (unsigned)cmd-SCp.buffers_residual;
+}
+
+static inline void gdth_set_sg_count(struct scsi_cmnd *cmd, unsigned sg_count)
+{
+ cmd-SCp.buffers_residual = sg_count;
+}
+
+static inline struct scatterlist *gdth_sglist(struct scsi_cmnd *cmd)
+{
+ return cmd-SCp.buffer;
+}
+
+static inline void gdth_set_sglist(struct scsi_cmnd *cmd,
+ struct scatterlist *sglist)
+{
+ cmd-SCp.buffer = sglist;
+}
+
#include gdth_proc.h
#include gdth_proc.c
@@ -458,6 +500,7 @@ int __gdth_execute(struct scsi_device *sdev, gdth_cmd_str
*gdtcmd, char *cmnd,
gdth_ha_str *ha = shost_priv(sdev-host);
Scsi_Cmnd *scp;
struct gdth_cmndinfo cmndinfo;
+struct scatterlist one_sg;
DECLARE_COMPLETION_ONSTACK(wait);
int rval;
@@ -471,7 +514,10 @@ int __gdth_execute(struct scsi_device *sdev, gdth_cmd_str
*gdtcmd, char *cmnd,
/* use request field to save the ptr. to completion struct. */
scp-request = (struct request *)wait;
scp-timeout_per_command = timeout*HZ;
-scp-request_buffer = gdtcmd;
+sg_init_one(one_sg, gdtcmd, sizeof(*gdtcmd));
+gdth_set_sglist(scp, one_sg);
+gdth_set_sg_count(scp, 1);
+gdth_set_bufflen(scp, sizeof(*gdtcmd));
scp-cmd_len = 12;
memcpy(scp-cmnd, cmnd, 12);
cmndinfo.priority = IOCTL_PRI;
@@ -2309,24 +2355,28 @@ static void gdth_next(gdth_ha_str *ha)
ha-hanum, cmd_index);
}
}
-
+
+/*
+ * gdth_copy_internal_data() - copy to/from a buffer onto a scsi_cmnd's
+ * buffers, kmap_atomic() as needed.
+ */
static void gdth_copy_internal_data(gdth_ha_str *ha, Scsi_Cmnd *scp,
-char *buffer,ushort count)
+
Re: [PATCH 13/16] gdth: Make one abuse of scsi_cmnd less obvious
Boaz Harrosh wrote:
- Christoph or Jeff will work on the finish up of the BUS hotplug API.
I have looked at code examples elsewhere in the kernel, and Jeff's
master plan sounds very good. But I would hope not to do it myself
as it will take me much longer.
Jeff it sounds like you have it clearer in your head?
TBH I won't have time to look at it, though I will be quite happy to
answer as many questions as you can come up with :)
In the on-going effort to kill deprecated pci_find_device (of which this
gdth effort is part of), I converted several ISDN drivers to use the new
ISA, PNP, and PCI APIs:
Branch 'isdn-pci' of
git://git.kernel.org/pub/scm/linux/kernel/git/jgarzik/misc-2.6.git
You can look at those patches for examples.
Really, you should pat yourself on the back, you have already done 96%
of the gdth work required to get us there. :)
Once you understand the key concepts of the new hotplug-style APIs, the
code changes themselves are easy. They are...
* reference HBA information via per-instance pointers stored in
scsi_host-hostdata. Global variables relating to per-HBA information
are to be avoided.
* in the API's -probe() hook,
* detect a single card/device
* allocate and init a single scsi_host and gdth_ha_str
* destroy a single gdth_ha_str in the API's -remove() hook
* shut down a single card/device
* destroy one scsi_host and gdth_ha_str
In practice, this tends to mean converting code like
while ((pdev = pci_find_device(...)) != NULL) {
alloc, init one PCI device
}
to
static int gdth_init_one (struct pci_dev *pdev,
const struct pci_device_id *ent) {
alloc, init one PCI devoce
}
because, as you can see, the loop has been moved to generic code. Also,
it should be self-evident that this new API allows devices to be
attached (hotplugged) long after the module initialization completes,
and other gdth devices are running.
Jeff
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 0/16] gdth combined patchset call for testers
On Sun, Sep 30 2007 at 23:27 +0200, Jeff Garzik [EMAIL PROTECTED] wrote: BTW, when reposting the patches of others, its nice to add a From: header to the very first line of the email body. Andrew does that a lot, and the git tools are aware of this convention. Thanks, yes I was not aware of that at all. Apparently git-am will make me the author of the patch, but adding a From: like you said will preserve the original author. Awaiting comments, I will resend all of these, fixed. Also, hey, if you like our patches, I think you should add your own Signed-off-by line (assuming/hoping you reviewed it). I wanted to, but I thought only if I change something than I'm suppose to put the Signed-off-by. But yes I have reviewed all these patches and am confident in their content. Will fix in next round. Jeff Boaz - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/8] scsi: megaraid_sas - add module param max_sectors, cmd_per_lun
Yang, Bo wrote:
Brian,
Thanks for your review. What is your objection to the current
implementation by using bin_attribute?
Binary attributes are generally used for data which cannot
be parsed by the user. Since this attribute is simply a
single decimal value, it makes most sense to just use a regular
class_device_attribute. Additionally, if you use the
existing shost_attrs infrastructure in the scsi_host_template,
you can simplify your code, since you don't need to manually
create and destroy the sysfs files as scsi core will do this for
you.
-Brian
Bo Yang
*From:* Brian King [mailto:[EMAIL PROTECTED]
*Sent:* Fri 9/28/2007 3:30 PM
*To:* Yang, Bo
*Cc:* linux-scsi@vger.kernel.org; [EMAIL PROTECTED];
[EMAIL PROTECTED]; [EMAIL PROTECTED]; Patro, Sumant
*Subject:* Re: [PATCH 3/8] scsi: megaraid_sas - add module param
max_sectors, cmd_per_lun
bo yang wrote:
+static ssize_t
+sysfs_max_sectors_read(struct kobject *kobj,
+ struct bin_attribute *bin_attr,
+ char *buf, loff_t off, size_t count)
+{
+ struct Scsi_Host *host = class_to_shost(container_of(kobj,
+ struct class_device, kobj));
+ struct megasas_instance *instance =
+ (struct megasas_instance *)host-hostdata;
+
+ count = sprintf(buf, %u\n, instance-max_sectors_per_req);
+
+ return count+1;
+}
+
+static struct bin_attribute sysfs_max_sectors_attr = {
+ .attr = {
+ .name = max_sectors,
+ .mode = S_IRUSR|S_IRGRP|S_IROTH,
+ .owner = THIS_MODULE,
+ },
+ .size = 7,
+ .read = sysfs_max_sectors_read,
+};
Why is this implemented as a binary sysfs attribute? Also, can you use
the existing shost_attrs infrastructure that's in the scsi_host_template
like megaraid_mbox uses?
+
+ /*
+ * Check if the module parameter value for max_sectors can be used
+ */
+ if (max_sectors max_sectors = instance-max_sectors_per_req)
+ instance-max_sectors_per_req = max_sectors;
+ else {
+ if (max_sectors)
+ printk(KERN_INFO
+ megasas: max_sectors should be 0 and
+ = %d\n,
+ instance-max_sectors_per_req);
+ }
Could be simplified to an else if, which would remove one indent level...
-Brian
--
Brian King
Linux on Power Virtualization
IBM Linux Technology Center
--
Brian King
Linux on Power Virtualization
IBM Linux Technology Center
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/5] qla2xxx: add target mode support
FUJITA Tomonori wrote: On Thu, 27 Sep 2007 07:34:52 -0700 Seokmann Ju [EMAIL PROTECTED] wrote: FUJITA Tomonori wrote: On Fri, 21 Sep 2007 07:34:18 -0700 Seokmann Ju [EMAIL PROTECTED] wrote: Andrew Vasquez wrote: On Sat, 01 Sep 2007, FUJITA Tomonori wrote: This adds target mode support to qla2xxx. With set ql2enable_target_mode module parameter to 1, the driver runs in target mode. By default, ql2enable_target_mode is set to 0, and the driver should work in initiator mode as before. The driver could support dual-mode in the future but it doesn't at the moment (we need to add dual-mode support tgt first). It is based on scst qla2xxx target mode driver. Mike converted the driver to use tgt long ago. I changed it to use the latest (mainline) version of qla2xxx driver and tgt, and also converted it to use fc transport class. Thanks for doing this. Some initial comments before a full review is complete, As was seen from the initiator updates needed for 24xx support, there are comparable changes needed in the area of target-mode support for 4gb and 8gb parts. Also, which ISPs and firmwares were exercised with this code? The patch is still under reviewing and will get done, soon. Great, thinks! One more question on typical testing setup. I wonder how should I setup the testing environment esp., for the target-mode. Sorry, I should have explained it with the patch. Probabaly, you need to compile scsi-misc with the qla2xxx target patch and the user-space target code. 1. scsi-misc + the qla2xxx target patch CONFIG_SCSI_TGT=m CONFIG_SCSI_FC_ATTRS=m CONFIG_SCSI_FC_TGT_ATTRS=y CONFIG_SCSI_QLA_FC=m CONFIG_SCSI_QLA_FC_TGT=y 2. the user-space target code git://git.kernel.org/pub/scm/linux/kernel/git/tomo/tgt.git rouen:~/git/tgt/usr$ make FCP=1 KERNELSRC=/home/fujita/git/scsi-misc-2.6 Starting the fc target mode is not so simple now (Mike and I know that we need to fix it...). 1. load scsi_tgt.ko 2. start the user-space daemon Here's a simple example. ./tgt/usr/tgtd ./tgt/usr/tgtadm --lld fc --mode target --op new --tid 1 --targetname volume1 ./tgt/usr/tgtadm --lld fc --mode logicalunit --op new --tid 1 --lun 1 -b /var/tmp/lun1 Above command execution on the system with the HBA with target mode returns invalid request for some reason. Not sure if there are any steps that has to be in place? The configuration is as follow, - two systems + a switch + a target device (JBOD) are involved. - each of systems has a QLogic HBA in it. The HBA on one system is in initiator mode and the other one is in target mode. - each of the port of the HBAs is connected to the switch and a target device (JBOD) is connected to the switch, too. Thank you, Seokmann --- atl-01:/lib/modules/2.6.23-rc3-smp-tgt/kernel/drivers/scsi/qla2xxx # tgtadm --lld fc --mode target --op show Target 1: volume1 System information: Driver: fc Status: running I_T nexus information: LUN information: LUN: 0 Type: controller SCSI ID: deadbeaf1:0 SCSI SN: beaf10 Size: 0 Online: No Poweron/Reset: Yes Removable media: No Backing store: No backing store ACL information: atl-01:/lib/modules/2.6.23-rc3-smp-tgt/kernel/drivers/scsi/qla2xxx # lsmod Module Size Used by qla2xxx 173672 0 ipv6 254244 14 snd_pcm_oss50560 0 snd_mixer_oss 20224 1 snd_pcm_oss snd_seq55152 0 snd_seq_device 12556 1 snd_seq loop 21252 0 dm_mod 56768 0 snd_hda_intel 280092 0 snd_pcm82436 2 snd_pcm_oss,snd_hda_intel snd_timer 26244 2 snd_seq,snd_pcm snd58628 7 snd_pcm_oss,snd_mixer_oss,snd_seq,snd_seq_device,snd_hda_intel,snd_pcm,snd_timer soundcore 11744 1 snd snd_page_alloc 14088 2 snd_hda_intel,snd_pcm parport_pc 41956 1 lp 15396 0 parport38344 2 parport_pc,lp reiserfs 223744 1 edd13124 0 firmware_class 13696 1 qla2xxx sg 37404 0 scsi_transport_fc 45444 1 qla2xxx sd_mod 31616 3 scsi_tgt 18504 3 qla2xxx,scsi_transport_fc sr_mod 19620 0 cdrom 36896 1 sr_mod ata_piix 20356 2 atl-01:/lib/modules/2.6.23-rc3-smp-tgt/kernel/drivers/scsi/qla2xxx # uname -r 2.6.23-rc3-smp-tgt --- - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2]: PCI Error Recovery: Symbios SCSI First Failure
On Thu, Sep 27, 2007 at 06:34:37PM -0500, Linas Vepstas wrote:
Good catch. But no ... and I had to study this a bit. Bear with me:
I agree with the analysis which I've now snipped.
I think the race you describe above is harmless. The first time
that sym_eh_handler() will run, it will be with SYM_EH_ABORT,
in it doesn't matter if we lose that, since the device is hosed
anyway. At some later time, it will run with SYM_EH_DEVICE_RESET
and then SYM_EH_BUS_RESET and then SYM_EH_HOST_RESET, and we won't
miss those, since, by now, sym2_io_error_detected() will have run.
So, by my reading, I'd say that init_completion() in
sym2_io_error_detected() has to stay (although perhaps
it should be replaced by the INIT_COMPLETION() macro.)
Removing it will prevent correct operation on the second
and subsequent errors.
I think the fundamental problem is that completions aren't really
supposed to be used like this. Here's one attempt at using completions
perhaps a little more the way they're supposed to be used, although now
I've written it, I wonder if we shouldn't just use a waitqueue instead.
diff --git a/drivers/scsi/sym53c8xx_2/sym_glue.c
b/drivers/scsi/sym53c8xx_2/sym_glue.c
index e8a4361..b425b89 100644
--- a/drivers/scsi/sym53c8xx_2/sym_glue.c
+++ b/drivers/scsi/sym53c8xx_2/sym_glue.c
@@ -602,6 +602,7 @@ static int sym_eh_handler(int op, char *opname, struct
scsi_cmnd *cmd)
struct sym_hcb *np = SYM_SOFTC_PTR(cmd);
struct sym_ucmd *ucmd = SYM_UCMD_PTR(cmd);
struct Scsi_Host *host = cmd-device-host;
+ struct pci_dev *pdev = np-s.device;
SYM_QUEHEAD *qp;
int cmd_queued = 0;
int sts = -1;
@@ -616,9 +617,19 @@ static int sym_eh_handler(int op, char *opname, struct
scsi_cmnd *cmd)
* point in hurrying; take a leisurely wait.
*/
#define WAIT_FOR_PCI_RECOVERY 35
- if (pci_channel_offline(np-s.device)) {
- int finished_reset = wait_for_completion_timeout(
- np-s.io_reset_wait, WAIT_FOR_PCI_RECOVERY*HZ);
+ if (pci_channel_offline(pdev)) {
+ struct host_data *hostdata = shost_priv(host);
+ int finished_reset = 0;
+ init_completion(eh_done);
+ spin_lock_irq(host-host_lock);
+ if (!hostdata-io_reset)
+ hostdata-io_reset = eh_done;
+ if (!pci_channel_offline(pdev))
+ finished_reset = 1;
+ spin_unlock_irq(host-host_lock);
+ if (!finished_reset)
+ finished_reset = wait_for_completion_timeout(
+ hostdata-io_reset, WAIT_FOR_PCI_RECOVERY*HZ);
if (!finished_reset)
return SCSI_FAILED;
}
@@ -1396,7 +1407,6 @@ static struct Scsi_Host * __devinit sym_attach(struct
scsi_host_template *tpnt,
np-maxoffs = dev-chip.offset_max;
np-maxburst= dev-chip.burst_max;
np-myaddr = dev-host_id;
- init_completion(np-s.io_reset_wait);
/*
* Edit its name.
@@ -1842,15 +1852,12 @@ static void __devexit sym2_remove(struct pci_dev *pdev)
static pci_ers_result_t sym2_io_error_detected(struct pci_dev *pdev,
enum pci_channel_state state)
{
- struct sym_hcb *np = pci_get_drvdata(pdev);
-
/* If slot is permanently frozen, turn everything off */
if (state == pci_channel_io_perm_failure) {
sym2_remove(pdev);
return PCI_ERS_RESULT_DISCONNECT;
}
- init_completion(np-s.io_reset_wait);
disable_irq(pdev-irq);
pci_disable_device(pdev);
@@ -1912,7 +1919,7 @@ static pci_ers_result_t sym2_io_slot_reset(struct pci_dev
*pdev)
sym_name(np));
if (pci_enable_device(pdev)) {
- printk(KERN_ERR %s: Unable to enable afer PCI reset\n,
+ printk(KERN_ERR %s: Unable to enable after PCI reset\n,
sym_name(np));
return PCI_ERS_RESULT_DISCONNECT;
}
@@ -1953,7 +1960,14 @@ static pci_ers_result_t sym2_io_slot_reset(struct
pci_dev *pdev)
static void sym2_io_resume(struct pci_dev *pdev)
{
struct sym_hcb *np = pci_get_drvdata(pdev);
- complete_all(np-s.io_reset_wait);
+ struct Scsi_Host *shost = np-s.host;
+ struct host_data *hostdata = shost_priv(shost);
+
+ spin_lock_irq(shost-host_lock);
+ if (hostdata-io_reset)
+ complete_all(hostdata-io_reset);
+ hostdata-io_reset = NULL;
+ spin_unlock_irq(shost-host_lock);
}
static void sym2_get_signalling(struct Scsi_Host *shost)
diff --git a/drivers/scsi/sym53c8xx_2/sym_glue.h
b/drivers/scsi/sym53c8xx_2/sym_glue.h
index a172cc5..b961f70 100644
--- a/drivers/scsi/sym53c8xx_2/sym_glue.h
+++ b/drivers/scsi/sym53c8xx_2/sym_glue.h
@@ -180,9 +180,6 @@ struct sym_shcb {
char
Re: [PATCH 2/2]: PCI Error Recovery: Symbios SCSI First Failure
On Mon, Oct 01, 2007 at 02:12:47PM -0600, Matthew Wilcox wrote: I think the fundamental problem is that completions aren't really supposed to be used like this. Here's one attempt at using completions perhaps a little more the way they're supposed to be used, Yes, that looks very good to me. I see it solves a bug that I hadn't been quite aware of. I don't understand why struct host_data is preferable to struct sym_shcb (is it because this is the structure that is naturally protectected by the spinlock?) My gut instinct is to say ack, although prudence dictates that I should test first. Which might take a few days... although now I've written it, I wonder if we shouldn't just use a waitqueue instead. I thought that earlier versions of the driver used waitqueues (I vaguely remember eh_wait in the code), which were later converted to completions (I also vaguely recall thinking that the new code was more elegant/simpler). I converted my patch to use the completions likewise, and, as you've clearly shown, did a rather sloppy job in the conversion. I'm tempted to go with this patch; but if you prod, I could attempt a wait-queue based patch. --linas - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/5] add sg segment limitation info to device structure
On Wed, 2007-09-26 at 09:05 -0700, Greg KH wrote:
On Wed, Sep 26, 2007 at 05:58:01PM +0900, FUJITA Tomonori wrote:
iommu code merges sg segments without considering lld's sg segment
restrictions. iommu code can't access to the limitations because they
are in request_queue. This patch adds max_segment_size to device
structure. seg_boundary_mask will be added too later.
Signed-off-by: FUJITA Tomonori [EMAIL PROTECTED]
---
include/linux/device.h |7 +++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/include/linux/device.h b/include/linux/device.h
index 3a38d1f..8046b60 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -443,6 +443,13 @@ struct device {
struct dma_coherent_mem *dma_mem; /* internal for coherent mem
override */
+
+ /*
+* a low level driver may set these to teach IOMMU code about
+* sg limitations.
+*/
+ unsigned int max_segment_size;
Does this really need to be here? Can't it go into the bus specific
device that needs this?
Unfortunately, no. The IOMMU may not be on the bus for the device (on
HPPA for instance, the IOMMU sits on the runway bus which is plugged
into the parisc bus, which finally plugs into the PCI bus) which means
that the iommu itself can only deal with generic devices (because it
doesn't want to do massive casing on the possible busses).
One possibility we could do is to add a
struct dma_device {
struct device dev;
u64 dma_mask;
u64 coherent_dma_mask;
unsigned int max_segment_size;
/* plus any other DMA parameters */
};
but then every bus that can do DMA would need to include a struct
dma_device instead of the struct device they do now. Then the IOMMU
would know it could cast out from struct device to struct dma_device,
but this would be a lot of work to thread through the current
infrastructure.
James
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2]: PCI Error Recovery: Symbios SCSI First Failure
On Mon, Oct 01, 2007 at 05:41:32PM -0500, Linas Vepstas wrote: On Mon, Oct 01, 2007 at 02:12:47PM -0600, Matthew Wilcox wrote: I think the fundamental problem is that completions aren't really supposed to be used like this. Here's one attempt at using completions perhaps a little more the way they're supposed to be used, Yes, that looks very good to me. I see it solves a bug that I hadn't been quite aware of. I don't understand why struct host_data is preferable to struct sym_shcb (is it because this is the structure that is naturally protectected by the spinlock?) The thing to remember is that sym2 is in transition from being a dual BSD/Linux driver to being a purely Linux driver. I know, it's been more than two years, and I'm still not done. My latest thing I'm looking at fixing is to make Scsi_Host very much the primary entity in the sym2 driver, and leave just the device/scripts-accessible stuff in the hcb. My gut instinct is to say ack, although prudence dictates that I should test first. Which might take a few days... Fine by me. Do you have the ability to produce failures on a whim on your platforms? I've been vaguely musing a PCI device failure patch for x86, just so people can test driver failure paths. I thought that earlier versions of the driver used waitqueues (I vaguely remember eh_wait in the code), which were later converted to completions (I also vaguely recall thinking that the new code was more elegant/simpler). I converted my patch to use the completions likewise, and, as you've clearly shown, did a rather sloppy job in the conversion. eh_wait (when I removed it) contained a completion ... I think it used to be a semaphore, some time before 2.6.12 I'm tempted to go with this patch; but if you prod, I could attempt a wait-queue based patch. Let's leave it with a completion for now. I think it works out more efficiently in the end. -- Intel are signing my paycheques ... these opinions are still mine Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step. - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/5] add sg segment limitation info to device structure
On Mon, Oct 01, 2007 at 07:36:10PM -0400, James Bottomley wrote:
One possibility we could do is to add a
struct dma_device {
struct device dev;
u64 dma_mask;
u64 coherent_dma_mask;
unsigned int max_segment_size;
/* plus any other DMA parameters */
};
but then every bus that can do DMA would need to include a struct
dma_device instead of the struct device they do now. Then the IOMMU
would know it could cast out from struct device to struct dma_device,
but this would be a lot of work to thread through the current
infrastructure.
If we're going to do this, then we should go further and include:
- unsigned int irq
- struct resources (how many?)
- struct list_headdma_pools
dma_device might not be the right name. Really, we're talking about
'internal' device types (EISA, PCI, GSC, Zorro, ...) as opposed to
external like usb.
--
Intel are signing my paycheques ... these opinions are still mine
Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step.
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/5] add sg segment limitation info to device structure
On Mon, Oct 01, 2007 at 07:39:02PM -0600, Matthew Wilcox wrote:
On Mon, Oct 01, 2007 at 07:36:10PM -0400, James Bottomley wrote:
One possibility we could do is to add a
struct dma_device {
struct device dev;
u64 dma_mask;
u64 coherent_dma_mask;
unsigned int max_segment_size;
/* plus any other DMA parameters */
};
but then every bus that can do DMA would need to include a struct
dma_device instead of the struct device they do now. Then the IOMMU
would know it could cast out from struct device to struct dma_device,
but this would be a lot of work to thread through the current
infrastructure.
Why not just hang these fields off of a struct device, that way if the
device doesn't/can't do dma, it only has the loss of a single pointer,
not all of these fields?
thanks,
greg k-h
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
