Re: [PATCH v6 1/7] scsi: sr: support runtime pm for ODD

2012-09-10 Thread Aaron Lu 
On 09/07/2012 02:50 AM, Alan Stern wrote:
 On Thu, 6 Sep 2012, Oliver Neukum wrote:
 
 But in the long run that wouldn't be a good solution.  What I'd really 
 like is a way to do the status polling without having it reset the 
 idle timer.

 Oliver, what do you think?  Would that be a good solution?

 Well, we could introduce a flag into the requests for the polls.
 But best would be to simply declare a device immediately idle
 as soon as we learn that it has no medium. No special casing
 would be needed.

 We could do that, but what about idle drives that do have media?

 Then we do have a problem. To handle this optimally we'd have to make
 a difference between the first time a new medium is noticed and later
 polls.
 
 That's right.  It shouldn't be too difficult to accomplish.
 
 One case to watch out for is a ZPODD with no media and an open door.  
 We should put the drive into runtime suspend immediately, but continue
 polling and leave the power on.  The runtime suspend after each poll
 will to see if the door is shut; when it is then polling can be turned
 off and power removed.
 
 This leads to a question: How should the sr device tell its parent 
 whether or not to turn off the power?  Aaron's current patches simply 
 rely on the device being runtime suspended, but that won't work with 
 this scheme.  Do we need a ready_to_power_down flag?


Thanks for the suggestions, I've tried to use these ideas and please
take a look to see if below code did the job.

Note that I didn't call disk_block_events in sr_suspend, as there is a
race that I didn't know how to resolve:

sr_suspend  sr_check_events
  disk_block_events   scsi_autopm_get_device
wait for all delayedwait for suspend finish
events checking work
to finish

So it's possible when sr_suspend is executing, sr_check_events is also
executing and disk_block_events will wait for sr_check_events to finish
while sr_check_events waits for this device's suspend routine finish
before the scsi_autopm_get_device can proceed.

I used a flag called powered_off, if it is set, the sr_check_events will
simply return.

diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index 902b5a4..f91c922 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -869,9 +869,14 @@ void ata_acpi_set_state(struct ata_port *ap, pm_message_t 
state)
 
if (state.event != PM_EVENT_ON) {
acpi_state = acpi_pm_device_sleep_state(
-   dev-sdev-sdev_gendev, NULL, ACPI_STATE_D3);
-   if (acpi_state  0)
+   dev-sdev-sdev_gendev, NULL,
+   dev-sdev-ready_to_power_off ?
+   ACPI_STATE_D3 : ACPI_STATE_D0);
+   if (acpi_state  0) {
acpi_bus_set_power(handle, acpi_state);
+   if (acpi_state == ACPI_STATE_D3)
+   dev-sdev-powered_off = 1;
+   }
/* TBD: need to check if it's runtime pm request */
acpi_pm_device_run_wake(
dev-sdev-sdev_gendev, true);
@@ -880,6 +885,7 @@ void ata_acpi_set_state(struct ata_port *ap, pm_message_t 
state)
acpi_pm_device_run_wake(
dev-sdev-sdev_gendev, false);
acpi_bus_set_power(handle, ACPI_STATE_D0);
+   dev-sdev-powered_off = 0;
}
}
 
@@ -985,8 +991,10 @@ static void ata_acpi_wake_dev(acpi_handle handle, u32 
event, void *context)
struct ata_device *ata_dev = context;
 
if (event == ACPI_NOTIFY_DEVICE_WAKE  ata_dev 
-   pm_runtime_suspended(ata_dev-sdev-sdev_gendev))
-   scsi_autopm_get_device(ata_dev-sdev);
+   pm_runtime_suspended(ata_dev-sdev-sdev_gendev)) {
+   ata_dev-sdev-need_eject = 1;
+   pm_runtime_resume(ata_dev-sdev-sdev_gendev);
+   }
 }
 
 static void ata_acpi_add_pm_notifier(struct ata_device *dev)
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index 5fc97d2..890cee2 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -45,6 +45,7 @@
 #include linux/blkdev.h
 #include linux/mutex.h
 #include linux/slab.h
+#include linux/pm_runtime.h
 #include asm/uaccess.h
 
 #include scsi/scsi.h
@@ -79,6 +80,8 @@ static DEFINE_MUTEX(sr_mutex);
 static int sr_probe(struct device *);
 static int sr_remove(struct device *);
 static int sr_done(struct scsi_cmnd *);
+static int sr_suspend(struct device *, pm_message_t msg);
+static int sr_resume(struct device *);
 
 static struct scsi_driver sr_template = {
.owner  = THIS_MODULE,
@@ -86,6 +89,8 @@ static struct scsi_driver sr_template = {

Re: [PATCH v6 1/7] scsi: sr: support runtime pm for ODD

2012-09-10 Thread Oliver Neukum 
On Monday 10 September 2012 17:16:22 Aaron Lu wrote:

 +static int sr_resume(struct device *dev)
 +{
 + struct scsi_cd *cd;
 + struct scsi_sense_hdr sshdr;
 +
 + cd = dev_get_drvdata(dev);
 +
 + if (!cd-device-powered_off)
 + return 0;
 +
 + /* get the disk ready */
 + scsi_test_unit_ready(cd-device, SR_TIMEOUT, MAX_RETRIES, sshdr);
 +
 + /* if user wakes up the ODD, eject the tray */
 + if (cd-device-need_eject) {
 + cd-device-need_eject = 0;
 + if (!(cd-cdi.mask  CDC_CLOSE_TRAY))
 + sr_tray_move(cd-cdi, 1);

1. Even if the door is locked?
2. sr_tray_move allocates memory with GFP_KERNEL. This smells of a deadlock.

Regards
Oliver

--
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: Odd behavior of a SAS-2 backplane with SGPIO commands

2012-09-10 Thread Pasi Kärkkäinen 
On Sun, Aug 19, 2012 at 09:47:17AM -0400, Rich wrote:
 I've got open discussions with both Supermicro and LSI at the moment,
 and have shipped one of them a backplane and HBA to demonstrate this
 since they couldn't replicate it easily in-house.
 

Hello,

Any replies from Supermicro/LSI ? 

-- Pasi


--
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: Odd behavior of a SAS-2 backplane with SGPIO commands

2012-09-10 Thread Emmanuel Florac 
Le Mon, 10 Sep 2012 16:47:11 +0300
Pasi Kärkkäinen pa...@iki.fi écrivait:

 Any replies from Supermicro/LSI ? 
 

Only loosely related, but Supermicro replaced recently my 846E26 (dual
expander backplane) with 846E16 (single expander). Apparently they
gave up getting the E26 to work properly or something: LSI expander
firmware problem.

In another (very large scale, high end) setup, many different 60 slots
5 LSI expanders chassis had a general failure of the 5th drawer.
Another LSI SAS-2 expander firmware problem.

I could start a rant about the evil of proprietary firmware, etc. You
get my meaning :)

-- 

Emmanuel Florac |   Direction technique
|   Intellique
|   eflo...@intellique.com
|   +33 1 78 94 84 02

--
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: Odd behavior of a SAS-2 backplane with SGPIO commands

2012-09-10 Thread Pasi Kärkkäinen 
On Mon, Sep 10, 2012 at 06:01:42PM +0200, Emmanuel Florac wrote:
 Le Mon, 10 Sep 2012 16:47:11 +0300
 Pasi Kärkkäinen pa...@iki.fi écrivait:
 
  Any replies from Supermicro/LSI ? 
  
 
 Only loosely related, but Supermicro replaced recently my 846E26 (dual
 expander backplane) with 846E16 (single expander). Apparently they
 gave up getting the E26 to work properly or something: LSI expander
 firmware problem.
 
 In another (very large scale, high end) setup, many different 60 slots
 5 LSI expanders chassis had a general failure of the 5th drawer.
 Another LSI SAS-2 expander firmware problem.
 
 I could start a rant about the evil of proprietary firmware, etc. You
 get my meaning :)
 

Yeah, this is a good example why we're trying to get the LEDs working with 
direct attach (non-expander) backplanes :)

-- Pasi

--
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

[GIT PULL] SCSI fixes for 3.6-rc5

2012-09-10 Thread James Bottomley 
Just a note to everyone:  I had actually prepared this fix set before I
left for KS + Plumbers, so it's been incubating much longer than it
should have.  I'll be picking up my three week backlog this week, so
more fixes will then be forthcoming

This set consist of three minor and one fairly major (the device not
ready causing offlining problem which is a serious regression introduced
by the media change update) fixes.

The patch is available here:

git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git scsi-fixes

The short changelog is:

James Bottomley (1):
  Fix 'Device not ready' issue on mpt2sas

Kashyap Desai (1):
  megaraid_sas: Move poll_aen_lock initializer

Mike Snitzer (1):
  scsi_lib: fix scsi_io_completion's SG_IO error propagation

sreekanth.re...@lsi.com (1):
  mpt2sas: Fix for Driver oops, when loading driver with max_queue_depth 
command line option to a very small value

And the diffstat:

 drivers/scsi/megaraid/megaraid_sas_base.c |  3 ++-
 drivers/scsi/mpt2sas/mpt2sas_base.c   | 13 -
 drivers/scsi/scsi_error.c | 10 ++
 drivers/scsi/scsi_lib.c   |  5 -
 drivers/scsi/scsi_scan.c  | 10 ++
 5 files changed, 34 insertions(+), 7 deletions(-)

Full diff is below.

James

---

diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c 
b/drivers/scsi/megaraid/megaraid_sas_base.c
index dc27598..ed38454 100644
--- a/drivers/scsi/megaraid/megaraid_sas_base.c
+++ b/drivers/scsi/megaraid/megaraid_sas_base.c
@@ -4066,7 +4066,6 @@ megasas_probe_one(struct pci_dev *pdev, const struct 
pci_device_id *id)
spin_lock_init(instance-cmd_pool_lock);
spin_lock_init(instance-hba_lock);
spin_lock_init(instance-completion_lock);
-   spin_lock_init(poll_aen_lock);
 
mutex_init(instance-aen_mutex);
mutex_init(instance-reset_mutex);
@@ -5392,6 +5391,8 @@ static int __init megasas_init(void)
printk(KERN_INFO megasas: %s %s\n, MEGASAS_VERSION,
   MEGASAS_EXT_VERSION);
 
+   spin_lock_init(poll_aen_lock);
+
support_poll_for_event = 2;
support_device_change = 1;
 
diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c 
b/drivers/scsi/mpt2sas/mpt2sas_base.c
index 9d46fcb..b25757d 100644
--- a/drivers/scsi/mpt2sas/mpt2sas_base.c
+++ b/drivers/scsi/mpt2sas/mpt2sas_base.c
@@ -2424,10 +2424,13 @@ _base_allocate_memory_pools(struct MPT2SAS_ADAPTER 
*ioc,  int sleep_flag)
}
 
/* command line tunables  for max controller queue depth */
-   if (max_queue_depth != -1)
-   max_request_credit = (max_queue_depth  facts-RequestCredit)
-   ? max_queue_depth : facts-RequestCredit;
-   else
+   if (max_queue_depth != -1  max_queue_depth != 0) {
+   max_request_credit = min_t(u16, max_queue_depth +
+   ioc-hi_priority_depth + ioc-internal_depth,
+   facts-RequestCredit);
+   if (max_request_credit  MAX_HBA_QUEUE_DEPTH)
+   max_request_credit =  MAX_HBA_QUEUE_DEPTH;
+   } else
max_request_credit = min_t(u16, facts-RequestCredit,
MAX_HBA_QUEUE_DEPTH);
 
@@ -2502,7 +2505,7 @@ _base_allocate_memory_pools(struct MPT2SAS_ADAPTER *ioc,  
int sleep_flag)
/* set the scsi host can_queue depth
 * with some internal commands that could be outstanding
 */
-   ioc-shost-can_queue = ioc-scsiio_depth - (2);
+   ioc-shost-can_queue = ioc-scsiio_depth;
dinitprintk(ioc, printk(MPT2SAS_INFO_FMT scsi host: 
can_queue depth (%d)\n, ioc-name, ioc-shost-can_queue));
 
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index 4a6381c..de2337f 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -42,6 +42,8 @@
 
 #include trace/events/scsi.h
 
+static void scsi_eh_done(struct scsi_cmnd *scmd);
+
 #define SENSE_TIMEOUT  (10*HZ)
 
 /*
@@ -241,6 +243,14 @@ static int scsi_check_sense(struct scsi_cmnd *scmd)
if (! scsi_command_normalize_sense(scmd, sshdr))
return FAILED;  /* no valid sense data */
 
+   if (scmd-cmnd[0] == TEST_UNIT_READY  scmd-scsi_done != scsi_eh_done)
+   /*
+* nasty: for mid-layer issued TURs, we need to return the
+* actual sense data without any recovery attempt.  For eh
+* issued ones, we need to try to recover and interpret
+*/
+   return SUCCESS;
+
if (scsi_sense_is_deferred(sshdr))
return NEEDS_RETRY;
 
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index ffd7773..faa790f 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -776,7 +776,6 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int 
good_bytes)
}
 
if (req-cmd_type == REQ_TYPE_BLOCK_PC) { /* SG_IO ioctl from block 
level */
-

Re: Odd behavior of a SAS-2 backplane with SGPIO commands

2012-09-10 Thread Pasi Kärkkäinen 
On Mon, Sep 10, 2012 at 12:07:45PM -0400, Rich wrote:
 On Mon, Sep 10, 2012 at 12:04 PM, Pasi Kärkkäinen pa...@iki.fi wrote:
  On Mon, Sep 10, 2012 at 06:01:42PM +0200, Emmanuel Florac wrote:
  Le Mon, 10 Sep 2012 16:47:11 +0300
  Pasi Kärkkäinen pa...@iki.fi écrivait:
 
   Any replies from Supermicro/LSI ?
  
 
  Only loosely related, but Supermicro replaced recently my 846E26 (dual
  expander backplane) with 846E16 (single expander). Apparently they
  gave up getting the E26 to work properly or something: LSI expander
  firmware problem.
 
  In another (very large scale, high end) setup, many different 60 slots
  5 LSI expanders chassis had a general failure of the 5th drawer.
  Another LSI SAS-2 expander firmware problem.
 
  I could start a rant about the evil of proprietary firmware, etc. You
  get my meaning :)
 
 
  Yeah, this is a good example why we're trying to get the LEDs working with
  direct attach (non-expander) backplanes :)
 
  -- Pasi
 
 
 I don't have anything useful for people, other than that they have
 shown me an HBA firmware that fixes the LED problem but has other
 problems they're still debugging.
 
 So there does exist code for this firmware which will fix this problem.
 

That's good to know! Let's hope the fix ends up in an official 
LSI HBA firmware update in not-so-distant future.

 [I don't disagree on the evils of proprietary firmware, but this is
 the hand I've been dealt, so.]
 

Yeah, there's still the proprietary LSI HBA firmware, but at least using
direct-attach backplanes has one less :) 

Thanks!

-- Pasi

--
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: Odd behavior of a SAS-2 backplane with SGPIO commands

2012-09-10 Thread Rich 
On Mon, Sep 10, 2012 at 12:13 PM, Pasi Kärkkäinen pa...@iki.fi wrote:
 On Mon, Sep 10, 2012 at 12:07:45PM -0400, Rich wrote:
 On Mon, Sep 10, 2012 at 12:04 PM, Pasi Kärkkäinen pa...@iki.fi wrote:
  On Mon, Sep 10, 2012 at 06:01:42PM +0200, Emmanuel Florac wrote:
  Le Mon, 10 Sep 2012 16:47:11 +0300
  Pasi Kärkkäinen pa...@iki.fi écrivait:
 
   Any replies from Supermicro/LSI ?
  
 
  Only loosely related, but Supermicro replaced recently my 846E26 (dual
  expander backplane) with 846E16 (single expander). Apparently they
  gave up getting the E26 to work properly or something: LSI expander
  firmware problem.
 
  In another (very large scale, high end) setup, many different 60 slots
  5 LSI expanders chassis had a general failure of the 5th drawer.
  Another LSI SAS-2 expander firmware problem.
 
  I could start a rant about the evil of proprietary firmware, etc. You
  get my meaning :)
 
 
  Yeah, this is a good example why we're trying to get the LEDs working with
  direct attach (non-expander) backplanes :)
 
  -- Pasi
 

 I don't have anything useful for people, other than that they have
 shown me an HBA firmware that fixes the LED problem but has other
 problems they're still debugging.

 So there does exist code for this firmware which will fix this problem.


 That's good to know! Let's hope the fix ends up in an official
 LSI HBA firmware update in not-so-distant future.

 [I don't disagree on the evils of proprietary firmware, but this is
 the hand I've been dealt, so.]


 Yeah, there's still the proprietary LSI HBA firmware, but at least using
 direct-attach backplanes has one less :)

 Thanks!

 -- Pasi

Actually, the direct-attach backplanes have 3 passive AMI management
controller chips...which also, presumably, have embedded mutable
firmware.

- Rich
--
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: Odd behavior of a SAS-2 backplane with SGPIO commands

2012-09-10 Thread Pasi Kärkkäinen 
On Mon, Sep 10, 2012 at 12:14:35PM -0400, Rich wrote:
 On Mon, Sep 10, 2012 at 12:13 PM, Pasi Kärkkäinen pa...@iki.fi wrote:
  On Mon, Sep 10, 2012 at 12:07:45PM -0400, Rich wrote:
  On Mon, Sep 10, 2012 at 12:04 PM, Pasi Kärkkäinen pa...@iki.fi wrote:
   On Mon, Sep 10, 2012 at 06:01:42PM +0200, Emmanuel Florac wrote:
   Le Mon, 10 Sep 2012 16:47:11 +0300
   Pasi Kärkkäinen pa...@iki.fi écrivait:
  
Any replies from Supermicro/LSI ?
   
  
   Only loosely related, but Supermicro replaced recently my 846E26 (dual
   expander backplane) with 846E16 (single expander). Apparently they
   gave up getting the E26 to work properly or something: LSI expander
   firmware problem.
  
   In another (very large scale, high end) setup, many different 60 slots
   5 LSI expanders chassis had a general failure of the 5th drawer.
   Another LSI SAS-2 expander firmware problem.
  
   I could start a rant about the evil of proprietary firmware, etc. You
   get my meaning :)
  
  
   Yeah, this is a good example why we're trying to get the LEDs working 
   with
   direct attach (non-expander) backplanes :)
  
   -- Pasi
  
 
  I don't have anything useful for people, other than that they have
  shown me an HBA firmware that fixes the LED problem but has other
  problems they're still debugging.
 
  So there does exist code for this firmware which will fix this problem.
 
 
  That's good to know! Let's hope the fix ends up in an official
  LSI HBA firmware update in not-so-distant future.
 
  [I don't disagree on the evils of proprietary firmware, but this is
  the hand I've been dealt, so.]
 
 
  Yeah, there's still the proprietary LSI HBA firmware, but at least using
  direct-attach backplanes has one less :)
 
  Thanks!
 
  -- Pasi
 
 Actually, the direct-attach backplanes have 3 passive AMI management
 controller chips...which also, presumably, have embedded mutable
 firmware.
 

Yeah, true, but the passive management chips are not part of the SAS 
datapath? 
They're only communicating to the SAS HBA using the sideband signals/pins,
and controlling the LEDs on the backplane? 

-- Pasi

--
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] [SCSI] lpfc 8.3.32: use list_move_tail instead of list_del/list_add_tail

2012-09-10 Thread James Smart 

Acked-by: James Smart james.sm...@emulex.com

Thanks

-- james  s




On 9/5/2012 2:49 AM, Wei Yongjun wrote:

From: Wei Yongjun yongjun_...@trendmicro.com.cn

Using list_move_tail() instead of list_del() + list_add_tail().

spatch with a semantic match is used to found this problem.
(http://coccinelle.lip6.fr/)

Signed-off-by: Wei Yongjun yongjun_...@trendmicro.com.cn
---
  drivers/scsi/lpfc/lpfc_sli.c | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_sli.c b/drivers/scsi/lpfc/lpfc_sli.c
index 9cbd20b..0ada66b 100644
--- a/drivers/scsi/lpfc/lpfc_sli.c
+++ b/drivers/scsi/lpfc/lpfc_sli.c
@@ -15833,8 +15833,7 @@ lpfc_cleanup_pending_mbox(struct lpfc_vport *vport)
(mb-u.mb.mbxCommand != MBX_REG_VPI))
continue;
  
-		list_del(mb-list);

-   list_add_tail(mb-list, mbox_cmd_list);
+   list_move_tail(mb-list, mbox_cmd_list);
}
/* Clean up active mailbox command with the vport */
mb = phba-sli.mbox_active;





--
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] [SCSI] libfcoe: use list_move instead of list_del/list_add

2012-09-10 Thread Zou, Yi 
 From: Wei Yongjun yongjun_...@trendmicro.com.cn
 
 Using list_move() instead of list_del() + list_add().
 
 spatch with a semantic match is used to found this problem.
 (http://coccinelle.lip6.fr/)
 
 Signed-off-by: Wei Yongjun yongjun_...@trendmicro.com.cn
 ---
  drivers/scsi/fcoe/fcoe_ctlr.c | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)
 
 diff --git a/drivers/scsi/fcoe/fcoe_ctlr.c b/drivers/scsi/fcoe/fcoe_ctlr.c
 index 2ebe03a..44b4d99 100644
 --- a/drivers/scsi/fcoe/fcoe_ctlr.c
 +++ b/drivers/scsi/fcoe/fcoe_ctlr.c
 @@ -817,8 +817,7 @@ static unsigned long fcoe_ctlr_age_fcfs(struct fcoe_ctlr
 *fip)
* fcoe_sysfs_fcf_del (which can sleep)
* after the put_cpu().
*/
 - list_del(fcf-list);
 - list_add(fcf-list, del_list);
 + list_move(fcf-list, del_list);
   stats-VLinkFailureCount++;
   } else {
   if (time_after(next_timer, deadline))
 

Looks good to me, thanks,

Reviewed-by: Yi Zou yi@intel.com



--
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] scsi_netlink: Remove dead and buggy code

2012-09-10 Thread David Miller 
From: ebied...@xmission.com (Eric W. Biederman)
Date: Fri, 07 Sep 2012 15:39:21 -0700

 
 The scsi netlink code confuses the netlink port id with a process id,
 going so far as to read NETLINK_CREDS(skb)-pid instead of the correct
 NETLINK_CB(skb).pid.  Fortunately it does not matter because nothing
 registers to respond to scsi netlink requests.
 
 The only interesting use of the scsi_netlink interface is
 fc_host_post_vendor_event which sends a netlink multicast message.
 
 Since nothing registers to handle scsi netlink messages kill all of the
 registration logic, while retaining the same error handling behavior
 preserving the userspace visible behavior and removing all of the
 confused code that thought a netlink port id was a process id.
 
 This was tested with a kernel allyesconfig build which had no problems.
 
 Cc: James Bottomley james.bottom...@parallels.com
 Cc: James Smart james.sm...@emulex.com
 Signed-off-by: Eric W. Biederman ebied...@xmission.com

James et al., please review and ACK.

 ---
  drivers/scsi/scsi_netlink.c |  555 
 ++-
  include/scsi/scsi_netlink.h |   24 --
  2 files changed, 15 insertions(+), 564 deletions(-)
 
 diff --git a/drivers/scsi/scsi_netlink.c b/drivers/scsi/scsi_netlink.c
 index 8818dd6..abd43a3 100644
 --- a/drivers/scsi/scsi_netlink.c
 +++ b/drivers/scsi/scsi_netlink.c
 @@ -33,40 +33,6 @@
  struct sock *scsi_nl_sock = NULL;
  EXPORT_SYMBOL_GPL(scsi_nl_sock);
  
 -static DEFINE_SPINLOCK(scsi_nl_lock);
 -static struct list_head scsi_nl_drivers;
 -
 -static u32   scsi_nl_state;
 -#define STATE_EHANDLER_BSY   0x0001
 -
 -struct scsi_nl_transport {
 - int (*msg_handler)(struct sk_buff *);
 - void (*event_handler)(struct notifier_block *, unsigned long, void *);
 - unsigned int refcnt;
 - int flags;
 -};
 -
 -/* flags values (bit flags) */
 -#define HANDLER_DELETING 0x1
 -
 -static struct scsi_nl_transport transports[SCSI_NL_MAX_TRANSPORTS] =
 - { {NULL, }, };
 -
 -
 -struct scsi_nl_drvr {
 - struct list_head next;
 - int (*dmsg_handler)(struct Scsi_Host *shost, void *payload,
 -  u32 len, u32 pid);
 - void (*devt_handler)(struct notifier_block *nb,
 -  unsigned long event, void *notify_ptr);
 - struct scsi_host_template *hostt;
 - u64 vendor_id;
 - unsigned int refcnt;
 - int flags;
 -};
 -
 -
 -
  /**
   * scsi_nl_rcv_msg - Receive message handler.
   * @skb: socket receive buffer
 @@ -81,7 +47,6 @@ scsi_nl_rcv_msg(struct sk_buff *skb)
  {
   struct nlmsghdr *nlh;
   struct scsi_nl_hdr *hdr;
 - unsigned long flags;
   u32 rlen;
   int err, tport;
  
 @@ -126,22 +91,24 @@ scsi_nl_rcv_msg(struct sk_buff *skb)
   /*
* Deliver message to the appropriate transport
*/
 - spin_lock_irqsave(scsi_nl_lock, flags);
 -
   tport = hdr-transport;
 - if ((tport  SCSI_NL_MAX_TRANSPORTS) 
 - !(transports[tport].flags  HANDLER_DELETING) 
 - (transports[tport].msg_handler)) {
 - transports[tport].refcnt++;
 - spin_unlock_irqrestore(scsi_nl_lock, flags);
 - err = transports[tport].msg_handler(skb);
 - spin_lock_irqsave(scsi_nl_lock, flags);
 - transports[tport].refcnt--;
 - } else
 + if (tport == SCSI_NL_TRANSPORT) {
 + switch (hdr-msgtype) {
 + case SCSI_NL_SHOST_VENDOR:
 + /* Locate the driver that corresponds to the 
 message */
 + err = -ESRCH;
 + break;
 + default:
 + err = -EBADR;
 + break;
 + }
 + if (err)
 + printk(KERN_WARNING %s: Msgtype %d failed - 
 err %d\n,
 +__func__, hdr-msgtype, err);
 + }
 + else
   err = -ENOENT;
  
 - spin_unlock_irqrestore(scsi_nl_lock, flags);
 -
  next_msg:
   if ((err) || (nlh-nlmsg_flags  NLM_F_ACK))
   netlink_ack(skb, nlh, err);
 @@ -150,333 +117,6 @@ next_msg:
   }
  }
  
 -
 -/**
 - * scsi_nl_rcv_event - Event handler for a netlink socket.
 - * @this:event notifier block
 - * @event:   event type
 - * @ptr: event payload
 - *
 - **/
 -static int
 -scsi_nl_rcv_event(struct notifier_block *this, unsigned long event, void 
 *ptr)
 -{
 - struct netlink_notify *n = ptr;
 - struct scsi_nl_drvr *driver;
 - unsigned long flags;
 - int tport;
 -
 - if (n-protocol != NETLINK_SCSITRANSPORT)
 - return NOTIFY_DONE;
 -
 - spin_lock_irqsave(scsi_nl_lock, flags);
 -

[PATCH] st: remove st_mutex

2012-09-10 Thread Lee Duncan 
This patch is based on the misc branch of the SCSI tree
and the 5-patch series for st sent by Jeff Mahoney 8/18/2012
and Ack-ed by Kai on 8/20.

From: Jeff Mahoney je...@suse.com

The st_mutex was created when the BKL was removed, and
prevents simultaneous st_open calls. It is better to
protect just the necessary data.

Signed-off-by: Jeff Mahoney je...@suse.com
Reviewed-by: Lee Duncan ldun...@suse.com
---
 drivers/scsi/st.c | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
index ceca095..98156a9 100644
--- a/drivers/scsi/st.c
+++ b/drivers/scsi/st.c
@@ -75,7 +75,6 @@ static const char *verstr = 20101219;
 #include st_options.h
 #include st.h

-static DEFINE_MUTEX(st_mutex);
 static int buffer_kbs;
 static int max_sg_segs;
 static int try_direct_io = TRY_DIRECT_IO;
@@ -1185,7 +1184,6 @@ static int st_open(struct inode *inode, struct
file *filp)
int dev = TAPE_NR(inode);
char *name;

-   mutex_lock(st_mutex);
/*
 * We really want to do nonseekable_open(inode, filp); here, but some
 * versions of tar incorrectly call lseek on tapes and bail out if that
@@ -1194,7 +1192,6 @@ static int st_open(struct inode *inode, struct
file *filp)
filp-f_mode = ~(FMODE_PREAD | FMODE_PWRITE);

if (!(STp = scsi_tape_get(dev))) {
-   mutex_unlock(st_mutex);
return -ENXIO;
}

@@ -1205,7 +1202,6 @@ static int st_open(struct inode *inode, struct
file *filp)
if (STp-in_use) {
spin_unlock(st_use_lock);
scsi_tape_put(STp);
-   mutex_unlock(st_mutex);
DEB( printk(ST_DEB_MSG %s: Device already in use.\n, name); )
return (-EBUSY);
}
@@ -1259,16 +1255,16 @@ static int st_open(struct inode *inode, struct
file *filp)
retval = (-EIO);
goto err_out;
}
-   mutex_unlock(st_mutex);
return 0;

  err_out:
normalize_buffer(STp-buffer);
+   spin_lock(st_use_lock);
STp-in_use = 0;
+   spin_unlock(st_use_lock);
scsi_tape_put(STp);
if (resumed)
scsi_autopm_put_device(STp-device);
-   mutex_unlock(st_mutex);
return retval;

 }
-- 
1.7.11.5
--
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] scsi_netlink: Remove dead and buggy code

2012-09-10 Thread James Bottomley 
On Mon, 2012-09-10 at 15:07 -0400, David Miller wrote:
 From: ebied...@xmission.com (Eric W. Biederman)
 Date: Fri, 07 Sep 2012 15:39:21 -0700
 
  
  The scsi netlink code confuses the netlink port id with a process id,
  going so far as to read NETLINK_CREDS(skb)-pid instead of the correct
  NETLINK_CB(skb).pid.  Fortunately it does not matter because nothing
  registers to respond to scsi netlink requests.
  
  The only interesting use of the scsi_netlink interface is
  fc_host_post_vendor_event which sends a netlink multicast message.
  
  Since nothing registers to handle scsi netlink messages kill all of the
  registration logic, while retaining the same error handling behavior
  preserving the userspace visible behavior and removing all of the
  confused code that thought a netlink port id was a process id.
  
  This was tested with a kernel allyesconfig build which had no problems.
  
  Cc: James Bottomley james.bottom...@parallels.com
  Cc: James Smart james.sm...@emulex.com
  Signed-off-by: Eric W. Biederman ebied...@xmission.com
 
 James et al., please review and ACK.

I'll defer to the decision of James Smart and the other FC contributors,
since the FC transport class is really the only one using a netlink
interface.

James


--
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] st: remove st_mutex

2012-09-10 Thread Lee Duncan 
Please disregard this patch -- I have the meta-data wrong.

I'll resubmit.

On 09/10/2012 02:59 PM, Lee Duncan wrote:
 This patch is based on the misc branch of the SCSI tree
 and the 5-patch series for st sent by Jeff Mahoney 8/18/2012
 and Ack-ed by Kai on 8/20.
 
 From: Jeff Mahoney je...@suse.com
 
 The st_mutex was created when the BKL was removed, and
 prevents simultaneous st_open calls. It is better to
 protect just the necessary data.
 
 Signed-off-by: Jeff Mahoney je...@suse.com
 Reviewed-by: Lee Duncan ldun...@suse.com
 ---
  drivers/scsi/st.c | 8 ++--
  1 file changed, 2 insertions(+), 6 deletions(-)
 
 diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
 index ceca095..98156a9 100644
 --- a/drivers/scsi/st.c
 +++ b/drivers/scsi/st.c
 @@ -75,7 +75,6 @@ static const char *verstr = 20101219;
  #include st_options.h
  #include st.h
 
 -static DEFINE_MUTEX(st_mutex);
  static int buffer_kbs;
  static int max_sg_segs;
  static int try_direct_io = TRY_DIRECT_IO;
 @@ -1185,7 +1184,6 @@ static int st_open(struct inode *inode, struct
 file *filp)
   int dev = TAPE_NR(inode);
   char *name;
 
 - mutex_lock(st_mutex);
   /*
* We really want to do nonseekable_open(inode, filp); here, but some
* versions of tar incorrectly call lseek on tapes and bail out if that
 @@ -1194,7 +1192,6 @@ static int st_open(struct inode *inode, struct
 file *filp)
   filp-f_mode = ~(FMODE_PREAD | FMODE_PWRITE);
 
   if (!(STp = scsi_tape_get(dev))) {
 - mutex_unlock(st_mutex);
   return -ENXIO;
   }
 
 @@ -1205,7 +1202,6 @@ static int st_open(struct inode *inode, struct
 file *filp)
   if (STp-in_use) {
   spin_unlock(st_use_lock);
   scsi_tape_put(STp);
 - mutex_unlock(st_mutex);
   DEB( printk(ST_DEB_MSG %s: Device already in use.\n, name); )
   return (-EBUSY);
   }
 @@ -1259,16 +1255,16 @@ static int st_open(struct inode *inode, struct
 file *filp)
   retval = (-EIO);
   goto err_out;
   }
 - mutex_unlock(st_mutex);
   return 0;
 
   err_out:
   normalize_buffer(STp-buffer);
 + spin_lock(st_use_lock);
   STp-in_use = 0;
 + spin_unlock(st_use_lock);
   scsi_tape_put(STp);
   if (resumed)
   scsi_autopm_put_device(STp-device);
 - mutex_unlock(st_mutex);
   return retval;
 
  }
 

-- 
Lee Duncan
SUSE Labs
--
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

[PATCHv2] st: remove st_mutex

2012-09-10 Thread Lee Duncan 
From: Hannes Reinecke h...@suse.com

The st_mutex was created when the BKL was removed, and
prevents simultaneous st_open calls. It is better to
protect just the necessary data.

Signed-off-by: Hannes Reinecke h...@suse.com
Reviewed-by: Lee Duncan ldun...@suse.com

This patch is based on the misc branch of the SCSI tree
and the 5-patch series for st sent by Jeff Mahoney 8/18/2012
and Ack-ed by Kai on 8/20.

Changes in v2: patch meta-data corrected
---
 drivers/scsi/st.c | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
index ceca095..98156a9 100644
--- a/drivers/scsi/st.c
+++ b/drivers/scsi/st.c
@@ -75,7 +75,6 @@ static const char *verstr = 20101219;
 #include st_options.h
 #include st.h
 
-static DEFINE_MUTEX(st_mutex);
 static int buffer_kbs;
 static int max_sg_segs;
 static int try_direct_io = TRY_DIRECT_IO;
@@ -1185,7 +1184,6 @@ static int st_open(struct inode *inode, struct file *filp)
int dev = TAPE_NR(inode);
char *name;
 
-   mutex_lock(st_mutex);
/*
 * We really want to do nonseekable_open(inode, filp); here, but some
 * versions of tar incorrectly call lseek on tapes and bail out if that
@@ -1194,7 +1192,6 @@ static int st_open(struct inode *inode, struct file *filp)
filp-f_mode = ~(FMODE_PREAD | FMODE_PWRITE);
 
if (!(STp = scsi_tape_get(dev))) {
-   mutex_unlock(st_mutex);
return -ENXIO;
}
 
@@ -1205,7 +1202,6 @@ static int st_open(struct inode *inode, struct file *filp)
if (STp-in_use) {
spin_unlock(st_use_lock);
scsi_tape_put(STp);
-   mutex_unlock(st_mutex);
DEB( printk(ST_DEB_MSG %s: Device already in use.\n, name); )
return (-EBUSY);
}
@@ -1259,16 +1255,16 @@ static int st_open(struct inode *inode, struct file 
*filp)
retval = (-EIO);
goto err_out;
}
-   mutex_unlock(st_mutex);
return 0;
 
  err_out:
normalize_buffer(STp-buffer);
+   spin_lock(st_use_lock);
STp-in_use = 0;
+   spin_unlock(st_use_lock);
scsi_tape_put(STp);
if (resumed)
scsi_autopm_put_device(STp-device);
-   mutex_unlock(st_mutex);
return retval;
 
 }
-- 
1.7.11.5

--
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

[RFC PATCH 2/5] libfcoe: Create new libfcoe control interfaces

2012-09-10 Thread Robert Love 
This patch is the first in a series that will remove
libfcoe's create, destroy, enable and disable module
parameters and replace them with interface files in
the new /sys/bus/fcoe subsystem.

Old layout:

/sys/module/libfcoe/parameters/{create,destroy,enable,disable,vn2vn_create}

New layout:

/sys/bus/fcoe/ctlr_{create,destroy}
/sys/bus/fcoe/ctlr_X/{enable,disable,start}

This patch moves fcoe drivers to the following
initialization sequence-

1) create/alloc
2) configure
3) start

A control sysfs interface at /sys/bus/fcoe/ctlr_create
is added. Writing the interface name to this file
will allocate memory and create a sysfs entry for a
new fcoe_ctlr_device. The user may then tune the interface in
any desired way. After configuration the user will
echo any value into the /sys/bus/fcoe/devices/ctlr_X/start
interface to proceed with logging in.

VN2VN logins will still use the module parameters.
A follow up patch to this one will make the 'mode'
attribute of the fcoe_ctlr_device writable. Which will
allow a user to change the ctlr's mode to 'VN2VN'.

Signed-off-by: Robert Love robert.w.l...@intel.com
---
 Documentation/ABI/testing/sysfs-bus-fcoe |   43 
 drivers/scsi/fcoe/fcoe.h |9 +++
 drivers/scsi/fcoe/fcoe_ctlr.c|2 -
 drivers/scsi/fcoe/fcoe_sysfs.c   |   78 ++
 drivers/scsi/fcoe/fcoe_transport.c   |  105 +-
 include/scsi/fcoe_sysfs.h|4 +
 include/scsi/libfcoe.h   |   14 
 7 files changed, 250 insertions(+), 5 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-fcoe 
b/Documentation/ABI/testing/sysfs-bus-fcoe
index 1b06ec1..242d4a1 100644
--- a/Documentation/ABI/testing/sysfs-bus-fcoe
+++ b/Documentation/ABI/testing/sysfs-bus-fcoe
@@ -1,8 +1,37 @@
+What:  /sys/bus/fcoe/
+Date:  August 2012
+KernelVersion: TBD
+Contact:   Robert Love robert.w.l...@intel.com, de...@open-fcoe.org
+Description:   The FCoE bus. Attributes in this directory are control 
interfaces.
+Attributes:
+
+   ctlr_create: 'FCoE Controller' instance creation interface. Writing an
+ifname to this file will allocate and populate sysfs 
with a
+fcoe_ctlr_device (ctlr_X). The user can then configure any
+per-port settings and finally write to the 
fcoe_ctlr_device's
+'start' attribute to begin the kernel's discovery and login
+process.
+
+   ctlr_destroy: 'FCoE Controller' instance removal interface. Writing a
+  fcoe_ctlr_device's sysfs name to this file will log the
+  fcoe_ctlr_device out of the fabric or otherwise connected
+  FCoE devices. It will also free all kernel memory 
allocated
+  for this fcoe_ctlr_device and any structures associated
+  with it, this includes the scsi_host.
+
 What:  /sys/bus/fcoe/ctlr_X
 Date:  March 2012
 KernelVersion: TBD
 Contact:   Robert Love robert.w.l...@intel.com, de...@open-fcoe.org
-Description:   'FCoE Controller' instances on the fcoe bus
+Description:   'FCoE Controller' instances on the fcoe bus.
+
+   The FCoE Controller now has a three stage creation process.
+   1) Write interface name to ctlr_create 2) Configure the FCoE
+   Controller (ctlr_X) 3) Write anything to the FCoE
+   Controller's 'start' file to begin discovery and login. The
+   FCoE Controller is destroyed by writing it's name, i.e. ctlr_X
+   to the ctlr_delete file.
+
 Attributes:
 
fcf_dev_loss_tmo: Device loss timeout peroid (see below). Changing
@@ -17,6 +46,18 @@ Attributes:
  FIP VN2VN discovery and login is performed. A FCoE
  Controller only supports one mode at a time.
 
+   start:Start the FCoE controller.
+
+   disable:  Allow FCoE controller's that support disabling
+ to be disabled when any value is written to this
+ file. This attribute is only displayed if the
+ driver supports it.
+
+   enable:   Allow FCoE controller's that support enabling
+ to be enabled when any value is written to this
+ file. This attribute is only displayed if the
+ driver supports it.
+
lesb_link_fail:   Link Error Status Block (LESB) link failure count.
 
lesb_vlink_fail:  Link Error Status Block (LESB) virtual link
diff --git a/drivers/scsi/fcoe/fcoe.h b/drivers/scsi/fcoe/fcoe.h
index a624add..e984627 100644
--- a/drivers/scsi/fcoe/fcoe.h
+++ b/drivers/scsi/fcoe/fcoe.h
@@ -22,6 +22,7 @@
 
 #include linux/skbuff.h
 #include linux/kthread.h
+#include scsi/libfc.h
 
 #define FCOE_MAX_QUEUE_DEPTH   256
 #define

[RFC PATCH 1/5] libfcoe, fcoe: Allow user to set a ctlr's mode

2012-09-10 Thread Robert Love 
This patch makes the 'mode' attribute of a
fcoe_ctlr_device writale. This allows the user
to store the mode with with the ctlr will be in.

Possible modes would be 'Fabric', or 'VN2VN'.

The default mode for a fcoe_ctlr{,_device} is 'Fabric'.
Drivers must implement the set_fcoe_ctlr_mode routine
to support this feature.

libfcoe offers an exported routine to set a fcoe_ctlr's
mode.

Signed-off-by: Robert Love robert.w.l...@intel.com
---
 Documentation/ABI/testing/sysfs-bus-fcoe |8 
 drivers/scsi/fcoe/fcoe_ctlr.c|   22 +++
 drivers/scsi/fcoe/fcoe_sysfs.c   |   61 +-
 include/scsi/fcoe_sysfs.h|1 
 include/scsi/libfcoe.h   |1 
 5 files changed, 91 insertions(+), 2 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-fcoe 
b/Documentation/ABI/testing/sysfs-bus-fcoe
index 469d09c..1b06ec1 100644
--- a/Documentation/ABI/testing/sysfs-bus-fcoe
+++ b/Documentation/ABI/testing/sysfs-bus-fcoe
@@ -9,6 +9,14 @@ Attributes:
  this value will change the dev_loss_tmo for all
  FCFs discovered by this controller.
 
+   mode: Display or change the FCoE Controller's mode. Possible
+ modes are 'Fabric' and 'VN2VN'. If a FCoE Controller
+ is started in 'Fabric' mode then FIP FCF discovery is
+ initiated and ultimately a fabric login is attempted.
+ If a FCoE Controller is started in 'VN2VN' mode then
+ FIP VN2VN discovery and login is performed. A FCoE
+ Controller only supports one mode at a time.
+
lesb_link_fail:   Link Error Status Block (LESB) link failure count.
 
lesb_vlink_fail:  Link Error Status Block (LESB) virtual link
diff --git a/drivers/scsi/fcoe/fcoe_ctlr.c b/drivers/scsi/fcoe/fcoe_ctlr.c
index d68d572..bd899bf 100644
--- a/drivers/scsi/fcoe/fcoe_ctlr.c
+++ b/drivers/scsi/fcoe/fcoe_ctlr.c
@@ -2884,3 +2884,25 @@ void fcoe_ctlr_get_fip_mode(struct fcoe_ctlr_device 
*ctlr_dev)
mutex_unlock(ctlr-ctlr_mutex);
 }
 EXPORT_SYMBOL(fcoe_ctlr_get_fip_mode);
+
+void fcoe_ctlr_set_fip_mode(struct fcoe_ctlr_device *ctlr_dev)
+{
+   struct fcoe_ctlr *ctlr = fcoe_ctlr_device_priv(ctlr_dev);
+
+   mutex_lock(ctlr-ctlr_mutex);
+   switch (ctlr_dev-mode) {
+   case FIP_CONN_TYPE_VN2VN:
+   ctlr-mode = FIP_MODE_VN2VN;
+   break;
+   case FIP_CONN_TYPE_FABRIC:
+   default:
+   ctlr-mode = FIP_MODE_FABRIC;
+   break;
+   }
+
+   /* TODO: Probably need to restart the state machine */
+
+   mutex_unlock(ctlr-ctlr_mutex);
+
+}
+EXPORT_SYMBOL(fcoe_ctlr_set_fip_mode);
diff --git a/drivers/scsi/fcoe/fcoe_sysfs.c b/drivers/scsi/fcoe/fcoe_sysfs.c
index 2bc1631..3fbc556 100644
--- a/drivers/scsi/fcoe/fcoe_sysfs.c
+++ b/drivers/scsi/fcoe/fcoe_sysfs.c
@@ -21,6 +21,7 @@
 #include linux/types.h
 #include linux/kernel.h
 #include linux/etherdevice.h
+#include linux/ctype.h
 
 #include scsi/fcoe_sysfs.h
 
@@ -40,6 +41,46 @@ MODULE_PARM_DESC(fcf_dev_loss_tmo,
  insulate the loss of a fcf. Once this value is
  exceeded, the fcf is removed.);
 
+#define FCOE_MAX_MODENAME_LEN 20
+struct fcoe_ctlr_mode_table {
+   char *modename;
+   enum fip_conn_type mode;
+};
+
+const struct fcoe_ctlr_mode_table ctlr_mode_tbl[] = {
+   {   fabric,   FIP_CONN_TYPE_FABRIC},
+   {   vn2vn,FIP_CONN_TYPE_VN2VN},
+   {   NULL,   -1},
+};
+
+static enum fip_conn_type fcoe_parse_mode(const char *buf,
+ const struct fcoe_ctlr_mode_table *tbl)
+{
+   int modeint = -1, i, rv;
+   char *p, modestr[FCOE_MAX_MODENAME_LEN + 1] = { 0, };
+
+   for (p = (char *)buf; *p; p++)
+   if (!(isdigit(*p) || isspace(*p)))
+   break;
+
+   if (*p)
+   rv = sscanf(buf, %20s, modestr);
+   else
+   rv = sscanf(buf, %d, modeint);
+
+   if (!rv)
+   return FIP_CONN_TYPE_UNKNOWN;
+
+   for (i = 0; tbl[i].modename; i++) {
+   if (modeint == tbl[i].mode)
+   return tbl[i].mode;
+   if (strcmp(modestr, tbl[i].modename) == 0)
+   return tbl[i].mode;
+   }
+
+   return FIP_CONN_TYPE_UNKNOWN;
+}
+
 /*
  * These are used by the fcoe_*_show_function routines, they
  * are intentionally placed in the .c file as they're not intended
@@ -273,8 +314,24 @@ static ssize_t show_ctlr_mode(struct device *dev,
return snprintf(buf, FCOE_CTLR_MODE_MAX_NAMELEN,
%s\n, name);
 }
-static FCOE_DEVICE_ATTR(ctlr, mode, S_IRUGO,
-   show_ctlr_mode, NULL);
+
+static ssize_t store_ctlr_mode(struct device *dev,
+  struct

[RFC PATCH 4/5] bnx2fc: Use new fcoe_sysfs control interface

2012-09-10 Thread Robert Love 
Convert bnx2fc to use the new fcoe_sysfs create, delete,
enable, disable, start and mode.

bnx2fc doesn't support VN2VN. bnx2fc will not initialize
the set_fcoe_ctlr_mode routine and therefore its instances
will always be in FABRIC mode. There was previously an
explicit check for the ctlr's mode, but this is no longer
needed because not implementing set_fcoe_ctlr_mode implies
that the ctlr cannot change from the FABRIC mode.

Signed-off-by: Robert Love robert.w.l...@intel.com
---
 drivers/scsi/bnx2fc/bnx2fc_fcoe.c |   98 +++--
 1 file changed, 60 insertions(+), 38 deletions(-)

diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c 
b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
index f52f668f..560c8c8 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
@@ -56,11 +56,12 @@ static struct scsi_host_template bnx2fc_shost_template;
 static struct fc_function_template bnx2fc_transport_function;
 static struct fcoe_sysfs_function_template bnx2fc_fcoe_sysfs_templ;
 static struct fc_function_template bnx2fc_vport_xport_function;
-static int bnx2fc_create(struct net_device *netdev, enum fip_state fip_mode);
+
+static int bnx2fc_start(struct fcoe_ctlr_device *);
+static int bnx2fc_enable(struct fcoe_ctlr_device *);
+static int bnx2fc_disable(struct fcoe_ctlr_device *);
+
 static void __bnx2fc_destroy(struct bnx2fc_interface *interface);
-static int bnx2fc_destroy(struct net_device *net_device);
-static int bnx2fc_enable(struct net_device *netdev);
-static int bnx2fc_disable(struct net_device *netdev);
 
 static void bnx2fc_recv_frame(struct sk_buff *skb);
 
@@ -1594,8 +1595,7 @@ static void __bnx2fc_destroy(struct bnx2fc_interface 
*interface)
 /**
  * bnx2fc_destroy - Destroy a bnx2fc FCoE interface
  *
- * @buffer: The name of the Ethernet interface to be destroyed
- * @kp: The associated kernel parameter
+ * @netdev: The name of the Ethernet interface to be destroyed
  *
  * Called from sysfs.
  *
@@ -1954,10 +1954,11 @@ static void bnx2fc_ulp_init(struct cnic_dev *dev)
 }
 
 
-static int bnx2fc_disable(struct net_device *netdev)
+static int bnx2fc_disable(struct fcoe_ctlr_device *cdev)
 {
+   struct fcoe_ctlr *ctlr = fcoe_ctlr_device_priv(cdev);
+   struct net_device *netdev = bnx2fc_netdev(ctlr-lp);
struct bnx2fc_interface *interface;
-   struct fcoe_ctlr *ctlr;
int rc = 0;
 
rtnl_lock();
@@ -1980,10 +1981,11 @@ static int bnx2fc_disable(struct net_device *netdev)
 }
 
 
-static int bnx2fc_enable(struct net_device *netdev)
+static int bnx2fc_enable(struct fcoe_ctlr_device *cdev)
 {
+   struct fcoe_ctlr *ctlr = fcoe_ctlr_device_priv(cdev);
+   struct net_device *netdev = bnx2fc_netdev(ctlr-lp);
struct bnx2fc_interface *interface;
-   struct fcoe_ctlr *ctlr;
int rc = 0;
 
rtnl_lock();
@@ -2005,17 +2007,17 @@ static int bnx2fc_enable(struct net_device *netdev)
 }
 
 /**
- * bnx2fc_create - Create bnx2fc FCoE interface
+ * bnx2fc_alloc - Alocate a bnx2fc FCoE interface
  *
- * @buffer: The name of Ethernet interface to create on
- * @kp: The associated kernel param
+ * @netdev: The name of Ethernet interface to allocate on
  *
  * Called from sysfs.
  *
  * Returns: 0 for success
  */
-static int bnx2fc_create(struct net_device *netdev, enum fip_state fip_mode)
+static int bnx2fc_alloc(struct net_device *netdev)
 {
+   struct fcoe_ctlr_device *cdev;
struct fcoe_ctlr *ctlr;
struct bnx2fc_interface *interface;
struct bnx2fc_hba *hba;
@@ -2025,12 +2027,6 @@ static int bnx2fc_create(struct net_device *netdev, enum 
fip_state fip_mode)
int rc = 0;
int vlan_id;
 
-   BNX2FC_MISC_DBG(Entered bnx2fc_create\n);
-   if (fip_mode != FIP_MODE_FABRIC) {
-   printk(KERN_ERR fip mode not FABRIC\n);
-   return -EIO;
-   }
-
rtnl_lock();
 
mutex_lock(bnx2fc_dev_lock);
@@ -2077,13 +2073,17 @@ static int bnx2fc_create(struct net_device *netdev, 
enum fip_state fip_mode)
goto netdev_err;
}
 
-   interface = bnx2fc_interface_create(hba, netdev, fip_mode);
+   /*
+* Create the interface with the default 'Fabric' mode.
+*/
+   interface = bnx2fc_interface_create(hba, netdev, FIP_MODE_FABRIC);
if (!interface) {
printk(KERN_ERR PFX bnx2fc_interface_create failed\n);
goto ifput_err;
}
 
ctlr = bnx2fc_to_ctlr(interface);
+   cdev = fcoe_ctlr_to_ctlr_dev(ctlr);
interface-vlan_id = vlan_id;
interface-vlan_enabled = 1;
 
@@ -2106,20 +2106,6 @@ static int bnx2fc_create(struct net_device *netdev, enum 
fip_state fip_mode)
/* Add interface to if_list */
list_add_tail(interface-list, if_list);
 
-   lport-boot_time = jiffies;
-
-   /* Make this master N_port */
-   ctlr-lp = lport;
-
-   if (!bnx2fc_link_ok(lport)) {
-   fcoe_ctlr_link_up(ctlr);
-

[RFC PATCH 0/5] Reorganize libfcoe control interfaces

2012-09-10 Thread Robert Love 
The following series implements a move from using module parameters
as control interfaces to /sys/bus/fcoe based interfaces. A sysfs infrastructure
was added to the kernel a few cycles ago, this series builds on that work.

It moves the create, vn2vn_create, destroy, enable and disable interfaces
from /sys/module/libfcoe/parameters/ to various places under /sys/bus/fcoe/.
These interfaces simply are not module configurations- they are control
interfaces.

A second goal of this series is to change the initialization sequence for
a FCoE device. The result of this series is that interfaces created using
libfcoe.ko interfaces (i.e. fcoe.ko or bnx2fc.ko) will have the following
starting steps-

1) Create/alloc the port
   - Allocate kernel memory and create per-instance sysfs devices
   - No discovery or login

2) Configure the port
   - Change mode, set ddp_min, etc...

3) Start the port
   - Begins discovery and/or login (depending on mode)

4) Destroy the port
   - Logout and free all memory

I'm looking for feedback on using sysfs files as control interfaces that
the user (application) would write interface names to. I modeled this
series off of the bonding sysfs interface, but it was suggested to me that
it might not be a good example. I belive bonding uses two values per-file
a '+' or a '- to add or delete and then the ifname apended. I am simply
writing the ifname to the ctlr_create or ctlr_destroy.

Series compiled and tested against v3.5. libfcoe.ko compile warning fixed
upstream after v3.5, anyone who compiles this can ignore section mismatch
warning. Also note that a modified fcoemon is needed to use the fcoe system
service against this kernel modification. I'd be happy to provide that
fcoemon code on request.

---

Robert Love (5):
  libfcoe, fcoe: Allow user to set a ctlr's mode
  libfcoe: Create new libfcoe control interfaces
  fcoe: Use new fcoe_sysfs control interface
  bnx2fc: Use new fcoe_sysfs control interface
  libfcoe, fcoe: Remove libfcoe module parameters


 Documentation/ABI/testing/sysfs-bus-fcoe |   51 +++
 drivers/scsi/bnx2fc/bnx2fc_fcoe.c|   98 -
 drivers/scsi/fcoe/fcoe.c |  229 +++---
 drivers/scsi/fcoe/fcoe.h |9 +
 drivers/scsi/fcoe/fcoe_ctlr.c|   24 +++
 drivers/scsi/fcoe/fcoe_sysfs.c   |  139 ++
 drivers/scsi/fcoe/fcoe_transport.c   |  174 ---
 include/scsi/fcoe_sysfs.h|5 +
 include/scsi/libfcoe.h   |   20 ++-
 9 files changed, 445 insertions(+), 304 deletions(-)

-- 

Thanks, //Rob
--
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

[RFC PATCH 5/5] libfcoe, fcoe: Remove libfcoe module parameters

2012-09-10 Thread Robert Love 
This patch removes the create, create_vn2vn, destroy,
enable and disable module parameters. Previous patches
have added these interfaces to the fcoe_sysfs layout
and these misplaced interfaces are no longer necessary.

Signed-off-by: Robert Love robert.w.l...@intel.com
---
 drivers/scsi/fcoe/fcoe_transport.c |  211 
 include/scsi/libfcoe.h |7 -
 2 files changed, 1 insertion(+), 217 deletions(-)

diff --git a/drivers/scsi/fcoe/fcoe_transport.c 
b/drivers/scsi/fcoe/fcoe_transport.c
index 8e50d9a..f9e6e67 100644
--- a/drivers/scsi/fcoe/fcoe_transport.c
+++ b/drivers/scsi/fcoe/fcoe_transport.c
@@ -32,15 +32,11 @@ MODULE_AUTHOR(Open-FCoE.org);
 MODULE_DESCRIPTION(FIP discovery protocol and FCoE transport for FCoE HBAs);
 MODULE_LICENSE(GPL v2);
 
-static int fcoe_transport_create(const char *, struct kernel_param *);
-static int fcoe_transport_destroy(const char *, struct kernel_param *);
 static int fcoe_transport_show(char *buffer, const struct kernel_param *kp);
 static struct fcoe_transport *fcoe_transport_lookup(struct net_device *device);
 static struct fcoe_transport *fcoe_netdev_map_lookup(struct net_device 
*device);
-static int fcoe_transport_enable(const char *, struct kernel_param *);
-static int fcoe_transport_disable(const char *, struct kernel_param *);
 static int libfcoe_device_notification(struct notifier_block *notifier,
-   ulong event, void *ptr);
+  ulong event, void *ptr);
 
 static LIST_HEAD(fcoe_transports);
 static DEFINE_MUTEX(ft_mutex);
@@ -55,29 +51,6 @@ module_param_call(show, NULL, fcoe_transport_show, NULL, 
S_IRUSR);
 __MODULE_PARM_TYPE(show, string);
 MODULE_PARM_DESC(show,  Show attached FCoE transports);
 
-module_param_call(create, fcoe_transport_create, NULL,
- (void *)FIP_MODE_FABRIC, S_IWUSR);
-__MODULE_PARM_TYPE(create, string);
-MODULE_PARM_DESC(create,  Creates fcoe instance on a ethernet interface);
-
-module_param_call(create_vn2vn, fcoe_transport_create, NULL,
- (void *)FIP_MODE_VN2VN, S_IWUSR);
-__MODULE_PARM_TYPE(create_vn2vn, string);
-MODULE_PARM_DESC(create_vn2vn,  Creates a VN_node to VN_node FCoE instance 
-on an Ethernet interface);
-
-module_param_call(destroy, fcoe_transport_destroy, NULL, NULL, S_IWUSR);
-__MODULE_PARM_TYPE(destroy, string);
-MODULE_PARM_DESC(destroy,  Destroys fcoe instance on a ethernet interface);
-
-module_param_call(enable, fcoe_transport_enable, NULL, NULL, S_IWUSR);
-__MODULE_PARM_TYPE(enable, string);
-MODULE_PARM_DESC(enable,  Enables fcoe on a ethernet interface.);
-
-module_param_call(disable, fcoe_transport_disable, NULL, NULL, S_IWUSR);
-__MODULE_PARM_TYPE(disable, string);
-MODULE_PARM_DESC(disable,  Disables fcoe on a ethernet interface.);
-
 /* notification function for packets from net device */
 static struct notifier_block libfcoe_notifier = {
.notifier_call = libfcoe_device_notification,
@@ -730,188 +703,6 @@ out_nodev:
 EXPORT_SYMBOL(fcoe_ctlr_destroy_store);
 
 /**
- * fcoe_transport_create() - Create a fcoe interface
- * @buffer: The name of the Ethernet interface to create on
- * @kp:The associated kernel param
- *
- * Called from sysfs. This holds the ft_mutex while calling the
- * registered fcoe transport's create function.
- *
- * Returns: 0 for success
- */
-static int fcoe_transport_create(const char *buffer, struct kernel_param *kp)
-{
-   int rc = -ENODEV;
-   struct net_device *netdev = NULL;
-   struct fcoe_transport *ft = NULL;
-   enum fip_state fip_mode = (enum fip_state)(long)kp-arg;
-
-   mutex_lock(ft_mutex);
-
-   netdev = fcoe_if_to_netdev(buffer);
-   if (!netdev) {
-   LIBFCOE_TRANSPORT_DBG(Invalid device %s.\n, buffer);
-   goto out_nodev;
-   }
-
-   ft = fcoe_netdev_map_lookup(netdev);
-   if (ft) {
-   LIBFCOE_TRANSPORT_DBG(transport %s already has existing 
- FCoE instance on %s.\n,
- ft-name, netdev-name);
-   rc = -EEXIST;
-   goto out_putdev;
-   }
-
-   ft = fcoe_transport_lookup(netdev);
-   if (!ft) {
-   LIBFCOE_TRANSPORT_DBG(no FCoE transport found for %s.\n,
- netdev-name);
-   goto out_putdev;
-   }
-
-   rc = fcoe_add_netdev_mapping(netdev, ft);
-   if (rc) {
-   LIBFCOE_TRANSPORT_DBG(failed to add new netdev mapping 
- for FCoE transport %s for %s.\n,
- ft-name, netdev-name);
-   goto out_putdev;
-   }
-
-   /* pass to transport create */
-   rc = ft-create ? ft-create(netdev, fip_mode) : -ENODEV;
-   if (rc)
-   fcoe_del_netdev_mapping(netdev);
-
-   LIBFCOE_TRANSPORT_DBG(transport %s %s to create fcoe on %s.\n,
-

[RFC PATCH 3/5] fcoe: Use new fcoe_sysfs control interface

2012-09-10 Thread Robert Love 
Convert fcoe to use the new fcoe_sysfs create, delete,
enable, disable, start and mode.

Signed-off-by: Robert Love robert.w.l...@intel.com
---
 drivers/scsi/fcoe/fcoe.c |  229 +++---
 1 file changed, 115 insertions(+), 114 deletions(-)

diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index fe30b1b..fe9ce98 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -112,10 +112,10 @@ static int fcoe_dcb_app_notification(struct 
notifier_block *notifier,
 ulong event, void *ptr);
 
 static bool fcoe_match(struct net_device *netdev);
-static int fcoe_create(struct net_device *netdev, enum fip_state fip_mode);
+static int fcoe_start(struct fcoe_ctlr_device *cdev);
 static int fcoe_destroy(struct net_device *netdev);
-static int fcoe_enable(struct net_device *netdev);
-static int fcoe_disable(struct net_device *netdev);
+static int fcoe_enable(struct fcoe_ctlr_device *cdev);
+static int fcoe_disable(struct fcoe_ctlr_device *cdev);
 
 static struct fc_seq *fcoe_elsct_send(struct fc_lport *,
  u32 did, struct fc_frame *,
@@ -155,6 +155,10 @@ static void fcoe_ctlr_get_lesb(struct fcoe_ctlr_device *);
 static void fcoe_fcf_get_vlan_id(struct fcoe_fcf_device *);
 
 static struct fcoe_sysfs_function_template fcoe_sysfs_templ = {
+   .set_fcoe_ctlr_mode = fcoe_ctlr_set_fip_mode,
+   .set_fcoe_ctlr_start = fcoe_start,
+   .set_fcoe_ctlr_enable = fcoe_enable,
+   .set_fcoe_ctlr_disable = fcoe_disable,
.get_fcoe_ctlr_mode = fcoe_ctlr_get_fip_mode,
.get_fcoe_ctlr_link_fail = fcoe_ctlr_get_lesb,
.get_fcoe_ctlr_vlink_fail = fcoe_ctlr_get_lesb,
@@ -2034,105 +2038,6 @@ out:
 }
 
 /**
- * fcoe_disable() - Disables a FCoE interface
- * @netdev  : The net_device object the Ethernet interface to create on
- *
- * Called from fcoe transport.
- *
- * Returns: 0 for success
- */
-static int fcoe_disable(struct net_device *netdev)
-{
-   struct fcoe_ctlr *ctlr;
-   struct fcoe_interface *fcoe;
-   int rc = 0;
-
-   mutex_lock(fcoe_config_mutex);
-
-   rtnl_lock();
-   fcoe = fcoe_hostlist_lookup_port(netdev);
-   rtnl_unlock();
-
-   if (fcoe) {
-   ctlr = fcoe_to_ctlr(fcoe);
-   fcoe_ctlr_link_down(ctlr);
-   fcoe_clean_pending_queue(ctlr-lp);
-   } else
-   rc = -ENODEV;
-
-   mutex_unlock(fcoe_config_mutex);
-   return rc;
-}
-
-/**
- * fcoe_enable() - Enables a FCoE interface
- * @netdev  : The net_device object the Ethernet interface to create on
- *
- * Called from fcoe transport.
- *
- * Returns: 0 for success
- */
-static int fcoe_enable(struct net_device *netdev)
-{
-   struct fcoe_ctlr *ctlr;
-   struct fcoe_interface *fcoe;
-   int rc = 0;
-
-   mutex_lock(fcoe_config_mutex);
-   rtnl_lock();
-   fcoe = fcoe_hostlist_lookup_port(netdev);
-   rtnl_unlock();
-
-   if (!fcoe) {
-   rc = -ENODEV;
-   goto out;
-   }
-
-   ctlr = fcoe_to_ctlr(fcoe);
-
-   if (!fcoe_link_ok(ctlr-lp))
-   fcoe_ctlr_link_up(ctlr);
-
-out:
-   mutex_unlock(fcoe_config_mutex);
-   return rc;
-}
-
-/**
- * fcoe_destroy() - Destroy a FCoE interface
- * @netdev  : The net_device object the Ethernet interface to create on
- *
- * Called from fcoe transport
- *
- * Returns: 0 for success
- */
-static int fcoe_destroy(struct net_device *netdev)
-{
-   struct fcoe_ctlr *ctlr;
-   struct fcoe_interface *fcoe;
-   struct fc_lport *lport;
-   struct fcoe_port *port;
-   int rc = 0;
-
-   mutex_lock(fcoe_config_mutex);
-   rtnl_lock();
-   fcoe = fcoe_hostlist_lookup_port(netdev);
-   if (!fcoe) {
-   rc = -ENODEV;
-   goto out_nodev;
-   }
-   ctlr = fcoe_to_ctlr(fcoe);
-   lport = ctlr-lp;
-   port = lport_priv(lport);
-   list_del(fcoe-list);
-   queue_work(fcoe_wq, port-destroy_work);
-out_nodev:
-   rtnl_unlock();
-   mutex_unlock(fcoe_config_mutex);
-   return rc;
-}
-
-/**
  * fcoe_destroy_work() - Destroy a FCoE port in a deferred work context
  * @work: Handle to the FCoE port to be destroyed
  */
@@ -2207,18 +2112,17 @@ static void fcoe_dcb_create(struct fcoe_interface *fcoe)
 }
 
 /**
- * fcoe_create() - Create a fcoe interface
+ * fcoe_alloc() - Allocate memory for a fcoe interface
  * @netdev  : The net_device object the Ethernet interface to create on
- * @fip_mode: The FIP mode for this creation
  *
  * Called from fcoe transport
  *
  * Returns: 0 for success
  */
-static int fcoe_create(struct net_device *netdev, enum fip_state fip_mode)
+static int fcoe_alloc(struct net_device *netdev)
 {
int rc = 0;
-   struct fcoe_ctlr_device *ctlr_dev;
+   struct fcoe_ctlr_device *ctlr_dev = NULL;
struct fcoe_ctlr *ctlr;
struct fcoe_interface *fcoe;
struct fc_lport

Re: [RFC PATCH 1/5] libfcoe, fcoe: Allow user to set a ctlr's mode

2012-09-10 Thread Greg KH 
On Mon, Sep 10, 2012 at 03:59:14PM -0700, Robert Love wrote:
 This patch makes the 'mode' attribute of a
 fcoe_ctlr_device writale. This allows the user
 to store the mode with with the ctlr will be in.
 
 Possible modes would be 'Fabric', or 'VN2VN'.
 
 The default mode for a fcoe_ctlr{,_device} is 'Fabric'.
 Drivers must implement the set_fcoe_ctlr_mode routine
 to support this feature.
 
 libfcoe offers an exported routine to set a fcoe_ctlr's
 mode.
 
 Signed-off-by: Robert Love robert.w.l...@intel.com

Acked-by: Greg Kroah-Hartman gre...@linuxfoundation.org

--
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] Fix a use-after-free triggered by device removal

2012-09-10 Thread Tejun Heo 
Hello,

On Fri, Sep 07, 2012 at 08:57:10AM +0200, Bart Van Assche wrote:
 I'm not sure it would be a good idea to add a blk_queue_dead() check in
 any of the __blk_run_queue() variants since blk_drain_queue() can invoke
 __blk_run_queue() to drain the queue.

Right, we can't cancel requests from block layer which were already
seen by the driver.

 Also, as far as I can see the functions that can insert a request into
 the queue (blk_insert_cloned_request(), queue_unplugged(),
 blk_execute_rq_nowait()) all check whether the queue is dead before
 inserting a request. That should be sufficient to prevent that new
 requests are queued after QUEUE_FLAG_DEAD has been set.

Yes, but does that guarantee that none would call into -request_fn()?
If so, fine; otherwise, we may need to add another state to prevent
that.

Thanks.

-- 
tejun
--
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: [RFC PATCH 0/5] Reorganize libfcoe control interfaces

2012-09-10 Thread Bhanu Prakash Gollapudi 

On 9/10/2012 3:59 PM, Robert Love wrote:

The following series implements a move from using module parameters
as control interfaces to /sys/bus/fcoe based interfaces. A sysfs infrastructure
was added to the kernel a few cycles ago, this series builds on that work.

It moves the create, vn2vn_create, destroy, enable and disable interfaces
from /sys/module/libfcoe/parameters/ to various places under /sys/bus/fcoe/.
These interfaces simply are not module configurations- they are control
interfaces.

A second goal of this series is to change the initialization sequence for
a FCoE device. The result of this series is that interfaces created using
libfcoe.ko interfaces (i.e. fcoe.ko or bnx2fc.ko) will have the following
starting steps-

1) Create/alloc the port
- Allocate kernel memory and create per-instance sysfs devices
- No discovery or login

2) Configure the port
- Change mode, set ddp_min, etc...

3) Start the port
- Begins discovery and/or login (depending on mode)

4) Destroy the port
- Logout and free all memory


Robert, Can you please let me now what is the motivation for this change 
and what problem are we solving with this approach? Is this primarily to 
allow user to set the mode?


I'm concerned that we will be breaking user space compatibility with 
this change, as there should be a corresponding fcoemon/fipvlan change 
along with this, and existing utilities will not work.  Also the way we 
start fcoe will be completely different and the user may need to do the 
scripting changes, if any.


Thanks,
Bhanu



I'm looking for feedback on using sysfs files as control interfaces that
the user (application) would write interface names to. I modeled this
series off of the bonding sysfs interface, but it was suggested to me that
it might not be a good example. I belive bonding uses two values per-file
a '+' or a '- to add or delete and then the ifname apended. I am simply
writing the ifname to the ctlr_create or ctlr_destroy.

Series compiled and tested against v3.5. libfcoe.ko compile warning fixed
upstream after v3.5, anyone who compiles this can ignore section mismatch
warning. Also note that a modified fcoemon is needed to use the fcoe system
service against this kernel modification. I'd be happy to provide that
fcoemon code on request.

---

Robert Love (5):
   libfcoe, fcoe: Allow user to set a ctlr's mode
   libfcoe: Create new libfcoe control interfaces
   fcoe: Use new fcoe_sysfs control interface
   bnx2fc: Use new fcoe_sysfs control interface
   libfcoe, fcoe: Remove libfcoe module parameters


  Documentation/ABI/testing/sysfs-bus-fcoe |   51 +++
  drivers/scsi/bnx2fc/bnx2fc_fcoe.c|   98 -
  drivers/scsi/fcoe/fcoe.c |  229 +++---
  drivers/scsi/fcoe/fcoe.h |9 +
  drivers/scsi/fcoe/fcoe_ctlr.c|   24 +++
  drivers/scsi/fcoe/fcoe_sysfs.c   |  139 ++
  drivers/scsi/fcoe/fcoe_transport.c   |  174 ---
  include/scsi/fcoe_sysfs.h|5 +
  include/scsi/libfcoe.h   |   20 ++-
  9 files changed, 445 insertions(+), 304 deletions(-)




--
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: [RFC PATCH 0/5] Reorganize libfcoe control interfaces

2012-09-10 Thread Love, Robert W 
On Mon 10 Sep 2012 05:05:20 PM PDT, Bhanu Prakash Gollapudi wrote:
 On 9/10/2012 3:59 PM, Robert Love wrote:
 The following series implements a move from using module parameters
 as control interfaces to /sys/bus/fcoe based interfaces. A sysfs
 infrastructure
 was added to the kernel a few cycles ago, this series builds on that
 work.

 It moves the create, vn2vn_create, destroy, enable and disable
 interfaces
 from /sys/module/libfcoe/parameters/ to various places under
 /sys/bus/fcoe/.
 These interfaces simply are not module configurations- they are control
 interfaces.

 A second goal of this series is to change the initialization sequence
 for
 a FCoE device. The result of this series is that interfaces created
 using
 libfcoe.ko interfaces (i.e. fcoe.ko or bnx2fc.ko) will have the
 following
 starting steps-

 1) Create/alloc the port
 - Allocate kernel memory and create per-instance sysfs devices
 - No discovery or login

 2) Configure the port
 - Change mode, set ddp_min, etc...

 3) Start the port
 - Begins discovery and/or login (depending on mode)

 4) Destroy the port
 - Logout and free all memory

 Robert, Can you please let me now what is the motivation for this
 change and what problem are we solving with this approach? Is this
 primarily to allow user to set the mode?


The main problem is that our control interfaces shouldn't be module 
parameters. I think of module parameters as things that globally alter 
the module.

I also think that moving to a create/configure/start model gives us 
more flexibility going forward. We don't have too many FC/FCoE knobs to 
tune right now, but if we wanted to add more we don't have a good way 
to do it without starting the whole discovery/login process and then 
making changes during the discovery/login.

I think the module parameter problem is the justification, but I'm 
trying to be comprehensive in coming up with a flexible interface that 
will allow us to evolve as well.

 I'm concerned that we will be breaking user space compatibility with
 this change, as there should be a corresponding fcoemon/fipvlan change
 along with this, and existing utilities will not work.  Also the way
 we start fcoe will be completely different and the user may need to do
 the scripting changes, if any.

See the last statement from my initial posting (it's below). I have 
patches to modify fcoemon to use these new interfaces. I'd be happy to 
share them, I just didn't want to spam this broad of a audience.


 Thanks,
 Bhanu


 I'm looking for feedback on using sysfs files as control interfaces that
 the user (application) would write interface names to. I modeled this
 series off of the bonding sysfs interface, but it was suggested to me
 that
 it might not be a good example. I belive bonding uses two values
 per-file
 a '+' or a '- to add or delete and then the ifname apended. I am simply
 writing the ifname to the ctlr_create or ctlr_destroy.

 Series compiled and tested against v3.5. libfcoe.ko compile warning
 fixed
 upstream after v3.5, anyone who compiles this can ignore section
 mismatch
 warning. Also note that a modified fcoemon is needed to use the fcoe
 system
 service against this kernel modification. I'd be happy to provide that
 fcoemon code on request.

Re: [RFC PATCH 0/5] Reorganize libfcoe control interfaces

2012-09-10 Thread Bhanu Prakash Gollapudi 

On 9/10/2012 6:41 PM, Love, Robert W wrote:

On Mon 10 Sep 2012 05:05:20 PM PDT, Bhanu Prakash Gollapudi wrote:

On 9/10/2012 3:59 PM, Robert Love wrote:

The following series implements a move from using module parameters
as control interfaces to /sys/bus/fcoe based interfaces. A sysfs
infrastructure
was added to the kernel a few cycles ago, this series builds on that
work.

It moves the create, vn2vn_create, destroy, enable and disable
interfaces
from /sys/module/libfcoe/parameters/ to various places under
/sys/bus/fcoe/.
These interfaces simply are not module configurations- they are control
interfaces.

A second goal of this series is to change the initialization sequence
for
a FCoE device. The result of this series is that interfaces created
using
libfcoe.ko interfaces (i.e. fcoe.ko or bnx2fc.ko) will have the
following
starting steps-

1) Create/alloc the port
 - Allocate kernel memory and create per-instance sysfs devices
 - No discovery or login

2) Configure the port
 - Change mode, set ddp_min, etc...

3) Start the port
 - Begins discovery and/or login (depending on mode)

4) Destroy the port
 - Logout and free all memory


Robert, Can you please let me now what is the motivation for this
change and what problem are we solving with this approach? Is this
primarily to allow user to set the mode?



The main problem is that our control interfaces shouldn't be module
parameters. I think of module parameters as things that globally alter
the module.

I also think that moving to a create/configure/start model gives us
more flexibility going forward. We don't have too many FC/FCoE knobs to
tune right now, but if we wanted to add more we don't have a good way
to do it without starting the whole discovery/login process and then
making changes during the discovery/login.

I think the module parameter problem is the justification, but I'm
trying to be comprehensive in coming up with a flexible interface that
will allow us to evolve as well.


I'm concerned that we will be breaking user space compatibility with
this change, as there should be a corresponding fcoemon/fipvlan change
along with this, and existing utilities will not work.  Also the way
we start fcoe will be completely different and the user may need to do
the scripting changes, if any.


See the last statement from my initial posting (it's below). I have
patches to modify fcoemon to use these new interfaces. I'd be happy to
share them, I just didn't want to spam this broad of a audience.

Thanks Robert for the explanation. Appreciate if you could share the 
fcoeutils patches also.




--
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: [RFC PATCH 1/5] libfcoe, fcoe: Allow user to set a ctlr's mode

2012-09-10 Thread Bart Van Assche 
On 09/11/12 00:59, Robert Love wrote:
 +static enum fip_conn_type fcoe_parse_mode(const char *buf,
 +   const struct fcoe_ctlr_mode_table *tbl)
 +{
 + int modeint = -1, i, rv;
 + char *p, modestr[FCOE_MAX_MODENAME_LEN + 1] = { 0, };
 +
 + for (p = (char *)buf; *p; p++)
 + if (!(isdigit(*p) || isspace(*p)))
 + break;

If you change the declaration of p from char *p into const char *p
you won't need a cast in the above for loop.

[ ... ]

 -static FCOE_DEVICE_ATTR(ctlr, mode, S_IRUGO,
 - show_ctlr_mode, NULL);
 +
 +static ssize_t store_ctlr_mode(struct device *dev,
 +struct device_attribute *attr,
 +const char *buf, size_t count)
 +{
 + struct fcoe_ctlr_device *ctlr = dev_to_ctlr(dev);
 +
 + if (!ctlr-f-set_fcoe_ctlr_mode)
 + return -EINVAL;
 +
 + ctlr-mode = fcoe_parse_mode(buf, ctlr_mode_tbl);

As far as I know sysfs doesn't terminate buf with a '\0' before calling
a store method. Does that mean that you are passing a string that is not
'\0'-terminated to a function that expects a '\0'-terminated string ?

Bart.
--
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