headers
Dan Williams | 1 Mar 2012 01:28
Picon
Favicon

Re: [PATCH v8 09/13] libsas: enforce eh strategy handlers only in eh context

On Wed, Feb 29, 2012 at 2:05 PM, James Bottomley
<James.Bottomley <at> hansenpartnership.com> wrote:
> On Fri, 2012-02-10 at 00:45 -0800, Dan Williams wrote:
>> The strategy handlers may be called in places that are problematic for
>> libsas (i.e. sata resets outside of domain revalidation filtering /
>> libata link recovery), or problematic for userspace (non-blocking ioctl
>> to sleeping reset functions).  However, these routines are also called
>> for eh escalations and recovery of scsi_eh_prep_cmnd(), so permit them
>> as long as we are running in the host's error handler.
>>
>> Signed-off-by: Dan Williams <dan.j.williams <at> intel.com>
>> ---
>>  drivers/scsi/libsas/sas_scsi_host.c |   15 +++++++++++----
>>  1 files changed, 11 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c
>> index f0b9b7b..1cabedc 100644
>> --- a/drivers/scsi/libsas/sas_scsi_host.c
>> +++ b/drivers/scsi/libsas/sas_scsi_host.c
>>  <at>  <at>  -463,11 +463,14  <at>  <at>  EXPORT_SYMBOL_GPL(sas_get_local_phy);
>>  /* Attempt to send a LUN reset message to a device */
>>  int sas_eh_device_reset_handler(struct scsi_cmnd *cmd)
>>  {
>> -     struct domain_device *dev = cmd_to_domain_dev(cmd);
>> -     struct sas_internal *i =
>> -             to_sas_internal(dev->port->ha->core.shost->transportt);
>> -     struct scsi_lun lun;
>>       int res;
>> +     struct scsi_lun lun;
>> +     struct Scsi_Host *host = cmd->device->host;
(Continue reading)

Permalink | Reply | 2 comments |
headers
Roland Dreier | 1 Mar 2012 07:41
Gravatar

[PATCH] [SCSI] be2iscsi: Remove unused OFFSET_IN_PAGE() macro

From: Roland Dreier <roland <at> purestorage.com>

Signed-off-by: Roland Dreier <roland <at> purestorage.com>
---
 drivers/scsi/be2iscsi/be.h |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/be2iscsi/be.h b/drivers/scsi/be2iscsi/be.h
index 1d7b976..a50b6a9 100644
--- a/drivers/scsi/be2iscsi/be.h
+++ b/drivers/scsi/be2iscsi/be.h
 <at>  <at>  -132,10 +132,6  <at>  <at>  struct be_ctrl_info {
 		((u32)((((size_t)(_address) & (PAGE_SIZE_4K - 1)) +	\
 			(size) + (PAGE_SIZE_4K - 1)) >> PAGE_SHIFT_4K))

-/* Byte offset into the page corresponding to given address */
-#define OFFSET_IN_PAGE(addr)						\
-		((size_t)(addr) & (PAGE_SIZE_4K-1))
-
 /* Returns bit offset within a DWORD of a bitfield */
 #define AMAP_BIT_OFFSET(_struct, field)					\
 		(((size_t)&(((_struct *)0)->field))%32)
--

-- 
1.7.9

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
(Continue reading)

Permalink | Reply | 3 comments |
headers
Lin Ming | 1 Mar 2012 10:02
Picon
Favicon

[RFC PATCH v2 1/8] ACPI: Introduce ACPI D3_COLD state support

From: Zhang Rui <rui.zhang <at> intel.com>

If a device has _PR3, it means the device supports D3_COLD.
Add the ability to validate and enter D3_COLD state in ACPI.

Signed-off-by: Zhang Rui <rui.zhang <at> intel.com>
Signed-off-by: Lin Ming <ming.m.lin <at> intel.com>
---
 drivers/acpi/power.c |    4 ++--
 drivers/acpi/scan.c  |    7 +++++++
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/power.c b/drivers/acpi/power.c
index 9ac2a9f..0d681fb 100644
--- a/drivers/acpi/power.c
+++ b/drivers/acpi/power.c
 <at>  <at>  -500,14 +500,14  <at>  <at>  int acpi_power_transition(struct acpi_device *device, int state)
 {
 	int result;

-	if (!device || (state < ACPI_STATE_D0) || (state > ACPI_STATE_D3))
+	if (!device || (state < ACPI_STATE_D0) || (state > ACPI_STATE_D3_COLD))
 		return -EINVAL;

 	if (device->power.state == state)
 		return 0;

 	if ((device->power.state < ACPI_STATE_D0)
-	    || (device->power.state > ACPI_STATE_D3))
+	    || (device->power.state > ACPI_STATE_D3_COLD))
(Continue reading)

Permalink | Reply | 2 comments |
headers
James Bottomley | 1 Mar 2012 14:26

Re: scsi_wait_scan and scsi_complete_async_scans

On Thu, 2012-02-16 at 09:29 -0500, Alan Stern wrote:
> James or anyone else:
> 
> The purpose of the scsi_wait_scan module is to provide a method for
> initramfs images to wait until all the SCSI drives available at boot-up
> have been discovered and probed, so that the drive containing the root
> filesystem will be available when the time comes to mount it.
> 
> Now, in include/scsi/scsi_scan.h, the declaration of 
> scsi_complete_async_scans() depends on CONFIG_SCSI.  If the SCSI core 
> is built as a module, scsi_complete_async_scans() is defined as an 
> empty inline routine.
> 
> This doesn't seem to make any sense.  The drive containing the root 
> filesystem needs to be available for mounting regardless of whether 
> scsi_mod is built into the kernel or is loaded as a module within an 
> initramfs image.
> 
> In short, is there any reason not to remove the "#ifdef CONFIG_SCSI"  
> from include/scsi/scsi_scan.h?
> 
> My reason for asking is because I need to fix a bug in scsi_pm.c, and 
> the fix involves calling scsi_complete_async_scans().  This has to 
> happen even when CONFIG_SCSI isn't defined, so I'd like to change the 
> header file.  If necessary, I can put the call in scsi_wait_scan.c 
> inside a "#ifdef CONFIG_SCSI" block.

I'm fine with that.  scsi_wait_scan was supposed to be a hack until all
distros got wait for device in initrd sorted out ... but like all hacks
it's lived on way past its time.
(Continue reading)

Permalink | Reply | 1 comment |
headers
Dan Carpenter | 1 Mar 2012 15:22
Picon
Favicon

re: [SCSI] pm8001: Fix bogus interrupt state flag issue.

Hello Santosh Nayak,

The patch bdaefbf580cd: "[SCSI] pm8001: Fix bogus interrupt state 
flag issue." from Feb 26, 2012, leads to the following warning:
drivers/scsi/pm8001/pm8001_hwi.c:2400 mpi_sata_completion()
	 error: double unlock 'irq:'

        } else if (t->uldd_task) {
-               spin_unlock_irqrestore(&t->task_state_lock, flags);
+               spin_unlock_irq(&t->task_state_lock);
                ^^^^^^^^^^^^^^^
                pm8001_ccb_task_free(pm8001_ha, t, ccb, tag);
                mb();/* ditto */
-               spin_unlock_irqrestore(&pm8001_ha->lock, flags);
+               spin_unlock_irq(&pm8001_ha->lock);
                ^^^^^^^^^^^^^^^
It doesn't make sense to enable IRQs twice.  I'm not sure if it should
be the first or second unlock which enables them.

                t->task_done(t);
-               spin_lock_irqsave(&pm8001_ha->lock, flags);
+               spin_lock_irq(&pm8001_ha->lock);
        } else if (!t->uldd_task) {
-               spin_unlock_irqrestore(&t->task_state_lock, flags);
+               spin_unlock_irq(&t->task_state_lock);
                ^^^^^^^^^^^^^^^
                pm8001_ccb_task_free(pm8001_ha, t, ccb, tag);
                mb();/*ditto*/
-               spin_unlock_irqrestore(&pm8001_ha->lock, flags);
+               spin_unlock_irq(&pm8001_ha->lock);
(Continue reading)

Permalink | Reply | 4 comments |
headers
santosh prasad nayak | 1 Mar 2012 15:24
Picon

Re: [SCSI] pm8001: Fix bogus interrupt state flag issue.

The warnings are  there since from the beginning.
Even before my fix.

There are two locks:
1.  pm8001_ha->lock
2. t->task_state_lock

regards
santosh

On Thu, Mar 1, 2012 at 7:52 PM, Dan Carpenter <dan.carpenter <at> oracle.com> wrote:
> Hello Santosh Nayak,
>
> The patch bdaefbf580cd: "[SCSI] pm8001: Fix bogus interrupt state
> flag issue." from Feb 26, 2012, leads to the following warning:
> drivers/scsi/pm8001/pm8001_hwi.c:2400 mpi_sata_completion()
>         error: double unlock 'irq:'
>
>
>        } else if (t->uldd_task) {
> -               spin_unlock_irqrestore(&t->task_state_lock, flags);
> +               spin_unlock_irq(&t->task_state_lock);
>                ^^^^^^^^^^^^^^^
>                pm8001_ccb_task_free(pm8001_ha, t, ccb, tag);
>                mb();/* ditto */
> -               spin_unlock_irqrestore(&pm8001_ha->lock, flags);
> +               spin_unlock_irq(&pm8001_ha->lock);
>                ^^^^^^^^^^^^^^^
> It doesn't make sense to enable IRQs twice.  I'm not sure if it should
> be the first or second unlock which enables them.
(Continue reading)

Permalink | Reply | 2 comments |
headers
Dan Carpenter | 1 Mar 2012 15:37
Picon
Favicon

Re: [SCSI] pm8001: Fix bogus interrupt state flag issue.

On Thu, Mar 01, 2012 at 07:54:57PM +0530, santosh prasad nayak wrote:
> The warnings are  there since from the beginning.
> Even before my fix.
> 
> There are two locks:
> 1.  pm8001_ha->lock
> 2. t->task_state_lock
> 

Yes.  But IRQs are either enabled or disabled for the CPU.  You
can't enable them twice.

In this case we enable them in the first call to unlock and the
second call doesn't do anything.  So possibly the current behavior
is correct and we should change the second unlock or possibly the
current behavior is wrong and we should change the first unlock.

But actually the email was more of just a form letter.  Perhaps Jack
knows what to do here...

regards,
dan carpenter
Permalink | Reply | 1 comment |
headers
santosh prasad nayak | 1 Mar 2012 15:41
Picon

Re: [SCSI] pm8001: Fix bogus interrupt state flag issue.

t->task_state_lock  can be changed to simple spin lock instead of 'irq
spin lock'

Initially I thought to do so. But could not get the intention of the
developer why he used
an irq lock there. Hence left it unchanged. Just changed from 'irqsave
spin lock' to 'irq spin lock'

regard
santosh

On Thu, Mar 1, 2012 at 8:07 PM, Dan Carpenter <dan.carpenter <at> oracle.com> wrote:
> On Thu, Mar 01, 2012 at 07:54:57PM +0530, santosh prasad nayak wrote:
>> The warnings are  there since from the beginning.
>> Even before my fix.
>>
>> There are two locks:
>> 1.  pm8001_ha->lock
>> 2. t->task_state_lock
>>
>
> Yes.  But IRQs are either enabled or disabled for the CPU.  You
> can't enable them twice.
>
> In this case we enable them in the first call to unlock and the
> second call doesn't do anything.  So possibly the current behavior
> is correct and we should change the second unlock or possibly the
> current behavior is wrong and we should change the first unlock.
>
> But actually the email was more of just a form letter.  Perhaps Jack
(Continue reading)

Permalink | Reply | 0 comments |
headers
Alan Stern | 1 Mar 2012 16:58
Picon
Favicon

Re: scsi_wait_scan and scsi_complete_async_scans

On Thu, 1 Mar 2012, James Bottomley wrote:

> On Thu, 2012-02-16 at 09:29 -0500, Alan Stern wrote:
> > James or anyone else:
> > 
> > The purpose of the scsi_wait_scan module is to provide a method for
> > initramfs images to wait until all the SCSI drives available at boot-up
> > have been discovered and probed, so that the drive containing the root
> > filesystem will be available when the time comes to mount it.
> > 
> > Now, in include/scsi/scsi_scan.h, the declaration of 
> > scsi_complete_async_scans() depends on CONFIG_SCSI.  If the SCSI core 
> > is built as a module, scsi_complete_async_scans() is defined as an 
> > empty inline routine.
> > 
> > This doesn't seem to make any sense.  The drive containing the root 
> > filesystem needs to be available for mounting regardless of whether 
> > scsi_mod is built into the kernel or is loaded as a module within an 
> > initramfs image.
> > 
> > In short, is there any reason not to remove the "#ifdef CONFIG_SCSI"  
> > from include/scsi/scsi_scan.h?
> > 
> > My reason for asking is because I need to fix a bug in scsi_pm.c, and 
> > the fix involves calling scsi_complete_async_scans().  This has to 
> > happen even when CONFIG_SCSI isn't defined, so I'd like to change the 
> > header file.  If necessary, I can put the call in scsi_wait_scan.c 
> > inside a "#ifdef CONFIG_SCSI" block.
> 
> I'm fine with that.  scsi_wait_scan was supposed to be a hack until all
(Continue reading)

Permalink | Reply | 0 comments |
headers
Lee Duncan | 1 Mar 2012 17:44
Gravatar

Re: [PATCH] [SCSI] st: expand ability to write immediate filemarks

Kai:

Your argument makes sense. I will resubmit the patch without the module
variable, and with a little more added to the documentation concerning
the possible dangers of writing immediate EOFs.

On 02/28/2012 11:05 PM, Kai Mäkisara wrote:
> On 02/29/2012 12:03 AM, Lee Duncan wrote:
>> Hi Kai:
>>
>> Thanks for your response ...
>>
>> On 02/28/2012 11:40 AM, Kai Makisara wrote:
>>> On Wed, 15 Feb 2012, Lee Duncan wrote:
>>>
>>>> The st tape driver recently added the MTWEOFI ioctl, which writes
>>>> a tape filemark (EOF), like the MTWEOF ioctl, except that MTWEOFI
>>>> returns immediately. This makes certain applications, like backup
>>>> software, run much more quickly on buffered tape drives.
>>>>
> ...
>>>>
>>>>   static int st_dev_max;
>>>>   static int st_nr_dev;
>>>>  <at>  <at>  -103,6 +104,8  <at>  <at>  module_param_named(max_sg_segs, max_sg_segs,
>>>> int, 0);
>>>>   MODULE_PARM_DESC(max_sg_segs, "Maximum number of scatter/gather
>>>> segments to use (256)");
>>>>   module_param_named(try_direct_io, try_direct_io, int, 0);
>>>>   MODULE_PARM_DESC(try_direct_io, "Try direct I/O between user
(Continue reading)

Permalink | Reply | 0 comments |

Gmane