Re: [PATCH v2 0/9] Phase out pci_enable_msi_block()
On Fri, Jan 17, 2014 at 02:00:32PM -0700, Bjorn Helgaas wrote: > > As the release is supposedly this weekend, do you prefer > > the patches to go to your tree or to individual trees after > > the release? > > I'd be happy to merge them, except for the fact that they probably > wouldn't have any time in -next before I ask Linus to pull them. So > how about if we wait until after the release, ask the area maintainers > to take them, and if they don't take them, I'll put them in my tree > for v3.15? Patch 11 depends on patches 1-10, so I am not sure how to better handle it. Whatever works for you ;) I am only concerned with a regression fix "ahci: Fix broken fallback to single MSI mode" which would be nice to have in 3.14. But it seems pretty much too late. > Bjorn Thanks! -- Regards, Alexander Gordeev agord...@redhat.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: Disk spin-up optimization during system resume
On Fri, Jan 17, 2014 at 7:18 PM, Phillip Susi wrote: > -BEGIN PGP SIGNED MESSAGE- > Hash: SHA512 > > On 01/17/2014 05:17 PM, Dan Williams wrote: >> Well, "at all" is overstating it. The system was idle and now >> it's being woken up to do some work. Inactivity timers are >> invalidated and now the decision becomes minimize access latency, >> or lazily wake up. > > Sure, but that work may or may not involve disk access. Think of a > laptop you are using to watch a video on youtube. You need to get up > for a minute so you suspend, come back, and continue watching... no > need for disk IO just because you paused for a few minutes. For a > single disk system you may argue that it is likely the disk will be > needed, but for multiple disk systems, that probability goes down. > >>> It puts needless wear and tear on disks. >> >> I'm not sure spin up cycles are predictive of drive failure? > > Drive makers rate them to handle only so many start/stop cycles before > they fail, which is why SMART keeps track of the count. Any time you > move mechanical parts, they wear out. They also recommend maximum intervals of spin down time. Mind you its on the order of weeks, but spinning up the drive help keeps the bearing lubricant evenly distributed. What I was getting at is do these patches have a measurable impact on drive lifetime? For example how many spin ups is this saving vs deferring in your setup. I buy the power saving impact, but wearing out the drive prematurely is harder to accept without data. >> I'm not arguing about the attach rate of HDDs in shipping systems, >> I'm asking what is the system suspend / resume frequency of such >> systems and is it worth carrying that complexity in the kernel and >> injecting unnecessary resume latency for what amounts to a niche >> use case. > > It doesn't add any latency for an SSD, or even an ATA HDD that does > not have PuiS enabled. SCSI disks are more likely to be in larger > systems where it is more likely that at least some of the drives won't > be accessed soon, and if you really want to avoid the latency, then > you can configure your user space pm scripts to disable runtime pm > prior to suspend to get the old behavior back. > Larger systems are less likely to ever sleep. I think it's a bit of an interface surprise to have pm_runtime disable have side effects only for scsi_disk devices. I think lazy_resume needs to be an explicit attribute of the disk. For ata devices any command wakes up the drive. Perhaps we could simply do the same for scsi devices. In lazy_resume mode flag the device as "needs spinup" at sd_resume time and schedule it when a command arrives. -- 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: Disk spin-up optimization during system resume
-BEGIN PGP SIGNED MESSAGE- Hash: SHA512 On 01/17/2014 08:41 PM, Alan Stern wrote: > The intention is that this will help on systems with more than one > disk drive. The one containing the core OS files and the journal > will certainly spin up right away, but the others may not. I am hoping to prevent that as well with the regular laptop-mode type knobs. -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.14 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCgAGBQJS2fPUAAoJEI5FoCIzSKrwLj4H/3DVvgo/35w+zyBT6ZQCn+0c uV0oD+8hPFIr7yY5ynOP0HGdSOtXgBxo7sOz/1yED/jnOOgyx11oma+Y0lkT1PSw OK3+Du8xLDAtomNwng3/Wul4tGKV/tkAL5giblztk8ZLkC5zC4mOkuD44bqF8shq 4NzDvA/25KGEcVAHoFml7BxDbLirWkhtyVW75DnVlAIel8nIOL7AuYOP8xap++k4 SfYTsTLMpN46j/dorWvFtQ9tfmYeqEltPgtGejT89oDeFwyVegotSiuFsib1vu82 gKVKGkcPdazW18V3LoPnz3QSIb0uU2tgzCFY45ZqS7g9f87V0bFQMvO3iZ9F3qI= =ddyN -END PGP SIGNATURE- -- 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: Disk spin-up optimization during system resume
-BEGIN PGP SIGNED MESSAGE- Hash: SHA512 On 01/17/2014 08:24 PM, Todd E Brandt wrote: > 10 seconds?! The only possible way that could happen is if the test > unit ready SCSI command actually triggered an ata port wakeup; at > which point you've already done the one thing you were trying not > to do. A typical ATA disk is already spinning up on its own as soon as power is applied. They don't respond to any commands until they finish. -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.14 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCgAGBQJS2fL5AAoJEI5FoCIzSKrw0MMIAIy8Ue/Ny+pq095ZHJjOtfq2 gsEQgfEjp1s2Aekv4md/H2tstit8CC1dKkIhppuLJV/0BZgckDEO+ee8SmVKf+8I ctolLIAKy+vP6jAYHaOVUGvwWEyaLsVyAuPulunvDKs9crRijI/7fFK/RtbAVQSS +fVnhriAlm7Vsd/okt3sgXMiVQGzlA5dA+0NrklAf4OZckeC/6ZqSrz3z1j1jc1k dyItvfQxdG9Sys6DrxPJGF+FKHu6btZ/m060jRSth2+i62qtTHkgLJRe0QnjmGKK 0G8AtI5WYy10igtB5xDTr8tSfxnc9bmruZQNOU+jHEerIeBDxUAAomN5t9157Sc= =O0TD -END PGP SIGNATURE- -- 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: Disk spin-up optimization during system resume
-BEGIN PGP SIGNED MESSAGE- Hash: SHA512 On 01/17/2014 05:17 PM, Dan Williams wrote: > Well, "at all" is overstating it. The system was idle and now > it's being woken up to do some work. Inactivity timers are > invalidated and now the decision becomes minimize access latency, > or lazily wake up. Sure, but that work may or may not involve disk access. Think of a laptop you are using to watch a video on youtube. You need to get up for a minute so you suspend, come back, and continue watching... no need for disk IO just because you paused for a few minutes. For a single disk system you may argue that it is likely the disk will be needed, but for multiple disk systems, that probability goes down. >> It puts needless wear and tear on disks. > > I'm not sure spin up cycles are predictive of drive failure? Drive makers rate them to handle only so many start/stop cycles before they fail, which is why SMART keeps track of the count. Any time you move mechanical parts, they wear out. > I'm not arguing about the attach rate of HDDs in shipping systems, > I'm asking what is the system suspend / resume frequency of such > systems and is it worth carrying that complexity in the kernel and > injecting unnecessary resume latency for what amounts to a niche > use case. It doesn't add any latency for an SSD, or even an ATA HDD that does not have PuiS enabled. SCSI disks are more likely to be in larger systems where it is more likely that at least some of the drives won't be accessed soon, and if you really want to avoid the latency, then you can configure your user space pm scripts to disable runtime pm prior to suspend to get the old behavior back. -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.14 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCgAGBQJS2fJ/AAoJEI5FoCIzSKrwA+MH/jRP5zLyhA9RV7PICwP8LO8j x9+p/I/WVdduMJnqxMQOX6TE9nWbsTHrCBxgI/MiUmQeiYnTTU8VTaOjLosJ5K+j tu01bFZ2VNLQmv2ND9U0OAvf4drIqxEtyucZ4MgXwKYxUP3VRs19R26ns9LlOstD YsxloBVF0Fm+q7LWIH3piiLJGpie/e7e7J3c7FGwtPItTFy8cDSOAMdtvbXpfE+N M82TP+egl3DFwQ2dMhXBwwSh0MzXb/6/OCZZcl7rMf/G62sdrIG5onv0CRDHvcoF t0Lz9HjQtHL5d59pTu2t7kkz7VjOfTHsp+hRTNWZNNFxK95ULMW+JEc/q53H9gY= =y+KW -END PGP SIGNATURE- -- 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: Disk spin-up optimization during system resume
On Fri, 17 Jan 2014, Dan Williams wrote: > > The intention is that this will help on systems with more than one > > disk drive. The one containing the core OS files and the journal will > > certainly spin up right away, but the others may not. > > > > To tell the truth, I'm not sure how useful this feature would be to > > most people. But there probably are a few, like Phillip, who will be > > glad to have it. That seems like sufficient justification for adding > > it. > > > > Hmm, given that an ATA "start" command is just a "read verify" I > wonder if this can be done more simply than tying runtime and system > resume together. Could the lazy resume knob simply cause sd_resume to > be skipped? The first real command that comes down will achieve the > same effect. That might work for ATA disks, but it wouldn't work for other sorts. SCSI disks need to be spun up explicitly before they get a READ command. Alan Stern -- 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: Disk spin-up optimization during system resume
On Fri, Jan 17, 2014 at 5:41 PM, Alan Stern wrote: > On Fri, 17 Jan 2014, Tejun Heo wrote: > >> On Fri, Jan 17, 2014 at 03:15:54PM -0500, Alan Stern wrote: >> > You will have to argue this point with Phillip. >> > >> > If necessary, we could add a sysfs attribute to force a spin-up during >> > system resume. Or you could disable runtime PM for the disk, but that >> > has its own disadvantages. >> >> Isn't immediate spin-up trivial to implement from anywhere? I'm not >> sure whether we'll end up defaulting to the lazy behavior or not but >> if we do requiring userland to echo something to sysfs to configure >> immediate spin-up feels a bit silly when userland might as well just >> issue a dummy command to force spinup. > > Then turn it around. Make the new sysfs attribute enable a lazy > resume. > >> And, yes, I agree with Dan that avoiding spinup of harddrives on >> system resume seems a bit niche in its usefulness. suspend/resume >> cycle at the very least generates logs which most likely will be >> committed to the drive sooner or later. >> >> What kind of use cases are we expecting for the lazy behavior? > > The intention is that this will help on systems with more than one > disk drive. The one containing the core OS files and the journal will > certainly spin up right away, but the others may not. > > To tell the truth, I'm not sure how useful this feature would be to > most people. But there probably are a few, like Phillip, who will be > glad to have it. That seems like sufficient justification for adding > it. > Hmm, given that an ATA "start" command is just a "read verify" I wonder if this can be done more simply than tying runtime and system resume together. Could the lazy resume knob simply cause sd_resume to be skipped? The first real command that comes down will achieve the same effect. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/2] asynchronous ata port resume
On Wed, Jan 15, 2014 at 8:38 PM, Todd E Brandt wrote: > On resume, the ATA port driver currently waits until the AHCI controller > finishes executing the port wakeup command. This patch changes the > ata_port_resume callback to issue the wakeup and then return immediately, > thus allowing the next device in the pm queue to resume. Any commands > issued to the AHCI hardware during the wakeup will be queued up and > executed once the port is physically online. Thus no information is lost. > > This patch applies to all ata resume callbacks: resume, restore, and thaw for > code simplicity. The primary benefit is to S3 resume. > > The return value from ata_resume_async is now an indicator of whether > the resume was initiated, rather than if it was completed. I'm letting the ata > driver assume control over its own error reporting in this case (which it does > already). If you look at the ata_port resume code you'll see that the > ata_port_resume callback returns the status of the ahci_port_resume callback, > which is always 0. So I don't see any harm in ignoring it. > > If somebody requests suspend while ATA port resume is still running, the > request is queued until the resume has completed. I've tested > that case very heavily. Basically if you do two suspends in a row (within > seconds of each other) you lose any performance benefit, but that's a use > case that should happen only very rarerly (and wouldn't be expected to > be of any power benefit since too many sequential suspends actually > takes more power than just letting the machine stay in run mode). > > v4: Jan 15, 2014 > [in response to Tejun Huo] > - completely removed the pm_result member of the ata_port struct > - result from ata_port_suspend/resume is now returned from the function >instead of into a pointer location > - ata_port_request_pm now only returns 0/-EAGAIN for pass/fail > > Signed-off-by: Todd Brandt > Signed-off-by: Arjan van de Ven You may want to split out the pure "do resume async" changes from the return code and api cleanup. Doubtful that anything depends on the return code but at least gives a bisection point for each conceptual change. -- 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: Disk spin-up optimization during system resume
On Fri, 17 Jan 2014, Tejun Heo wrote: > On Fri, Jan 17, 2014 at 03:15:54PM -0500, Alan Stern wrote: > > You will have to argue this point with Phillip. > > > > If necessary, we could add a sysfs attribute to force a spin-up during > > system resume. Or you could disable runtime PM for the disk, but that > > has its own disadvantages. > > Isn't immediate spin-up trivial to implement from anywhere? I'm not > sure whether we'll end up defaulting to the lazy behavior or not but > if we do requiring userland to echo something to sysfs to configure > immediate spin-up feels a bit silly when userland might as well just > issue a dummy command to force spinup. Then turn it around. Make the new sysfs attribute enable a lazy resume. > And, yes, I agree with Dan that avoiding spinup of harddrives on > system resume seems a bit niche in its usefulness. suspend/resume > cycle at the very least generates logs which most likely will be > committed to the drive sooner or later. > > What kind of use cases are we expecting for the lazy behavior? The intention is that this will help on systems with more than one disk drive. The one containing the core OS files and the journal will certainly spin up right away, but the others may not. To tell the truth, I'm not sure how useful this feature would be to most people. But there probably are a few, like Phillip, who will be glad to have it. That seems like sufficient justification for adding it. Alan Stern -- 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: Disk spin-up optimization during system resume
On Fri, Jan 17, 2014 at 03:24:33PM -0500, Tejun Heo wrote: > On Fri, Jan 17, 2014 at 03:15:54PM -0500, Alan Stern wrote: > > You will have to argue this point with Phillip. > > > > If necessary, we could add a sysfs attribute to force a spin-up during > > system resume. Or you could disable runtime PM for the disk, but that > > has its own disadvantages. > > Isn't immediate spin-up trivial to implement from anywhere? I'm not > sure whether we'll end up defaulting to the lazy behavior or not but > if we do requiring userland to echo something to sysfs to configure > immediate spin-up feels a bit silly when userland might as well just > issue a dummy command to force spinup. > > And, yes, I agree with Dan that avoiding spinup of harddrives on > system resume seems a bit niche in its usefulness. suspend/resume > cycle at the very least generates logs which most likely will be > committed to the drive sooner or later. > > What kind of use cases are we expecting for the lazy behavior? Phillip and Alan, some test data would also be helpful to back up your point. > > 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: Disk spin-up optimization during system resume
On Fri, Jan 17, 2014 at 09:57:47AM -0500, Alan Stern wrote:
> On Thu, 16 Jan 2014, Todd E Brandt wrote:
>
> > On Thu, Jan 16, 2014 at 03:05:40PM -0500, Alan Stern wrote:
> > > On Thu, 16 Jan 2014, Todd E Brandt wrote:
> > >
> > > > > Does this plan sound reasonable to everyone? Are there important
> > > > > aspects I haven't considered (such as interactions between the SCSI
> > > > > and
> > > > > ATA layers)?
> > > > >
> > > > > Alan Stern
> > > > >
> > > >
> > > > Both approaches employ non-blocking resume of the scsi disks so why
> > > > don't
> > > > we treat these two patch sets as parts one and two. My patch just spins
> > > > everything up but sets everything to non-blocking, so it will take care
> > > > of the most common use cases. Your patch will then fine-tune that
> > > > behavior
> > > > to only spin up those disks that are actually needed. I don't think
> > > > there
> > > > are any serious conflicts between the two patch sets.
> > >
> > > Hmmm. In your 2/2 patch, sd_resume_complete() gets called in atomic
> > > context. I would need a process context in order to carry out a
> > > runtime resume. Any suggestions?
> >
> > The complete call is really just there to printk any errors and free
> > the sense buffer. Why would you want to carry out a runtime resume in
> > it?
>
> From my earlier message:
>
> Otherwise, check to see whether the disk is spinning up all
> by itself. This presumably will involve issuing a TEST UNIT
> READY command and checking the sense data. Maybe something
> different will be needed for ATA disks; I don't know.
>
> If the disk is spinning, or if you can't tell, call
> pm_runtime_resume() to put the disk back into the
> RPM_ACTIVE state (and spin it up in the case where you
> couldn't tell what it was doing).
>
> As you can see, the runtime resume is carried out in the completion
> routine for the TEST UNIT READY command. Since the command is likely
> to take a long time (Phillip said it can take up to 10 seconds for ATA
> disks), we want it to be asynchronous, like the START-STOP command.
10 seconds?! The only possible way that could happen is if the test unit
ready SCSI command actually triggered an ata port wakeup; at which point
you've already done the one thing you were trying not to do.
>
> > > Also, I don't understand the point of your 1/2 patch. If the whole
> > > point of that patch was to carry out the ATA port resume asynchronously
> > > ("thus allowing the next device in the pm queue to resume"), doesn't
> > > device_enable_async_suspend() already do that for you?
> > >
> > > Alan Stern
> > >
> >
> > The device_enable_async_suspend call is used to set a pm flag which lets
> > the power manager know that this device can be added to the async suspend
> > queue. Basically that means that it can be suspended/resumed in parallel
> > with all the other async devices, but resume still waits for all those
> > devices to finish before completing system resume.
>
> Correct.
>
> > My patch makes ata port and scsi disk resume 'non-blocking' (asynchronous
> > with respect to system resume). Which means once they're called the power
> > manager pays no more attention to them and will complete system resume
> > whenever. Both the power manager and the ata subsystems call these
> > features 'asynchronous' so it can be confusing.
>
> I see. That's not explained very well in the patch description -- it
> talks about going on to resume the next device on the list, but not
> about waiting for the overall system resume to finish.
I'll word it differently if I have to resubmit again, but it does assume
a working knowledge of the pm subsystem.
>
> Alan Stern
>
--
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: [GIT PULL] target fixes for v3.13
On Thu, Jan 16, 2014 at 4:09 PM, Nicholas A. Bellinger
wrote:
>
> This change allows the percpu_ida tag allocator to optionally use
> interruptible sleep that iscsi-target expects, while still leaving the
> functionality + interface for existing percpu_ida consumers unchanged.
I'm not pulling this. Passing in TASK_RUNNING to prepare_to_wait() is
insane (because if it ever were to actually wait, that would be a
bug), and afaik this would be the first time anybody ever does that.
Yes, yes, it may "work", but I'm not pulling that kind of hack just
before a release.
Quite frankly, it looks like you want to have a tristate argument ("no
wait", "wait interruptibly", "wait uninterruptibly") to that
percpu_ida_alloc() function. Fine. But dammit, using this kind of
hackery, and then having two *different* calling conventions (one
mis-using the gfp_t for legacy reasons, and one now using the task
state flags in odd ways) is just not acceptable.
Now, neither of those two is perfect, but I can see why you want to
use the task state ones to say which kind of interruptible you want..
But I really don't like suddenly having a
prepare_to_wait(TASK_RUNNING) caller without any discussion, and I
*really* don't like having two completely different models for this
hack.
So quite frankly, I'd much prefer:
- talk to the scheduler people, and make them aware of the fact that
you are going to pass in TASK_RUNNING to prepare_to_wait(). It works
with the code as-is, as long as you don't actually then wait.
- Don't do that wrapper function with a totally different calling
convention logic. Instead, just change all the callers explicitly.
>From a quick look, you really only have a couple of cases:
(a) target/iscsi, which wants the new ternary argument
(b) vhost/scsi.c, which uses GFP_ATOMIC and can be changed to TASK_RUNNABLE
(c) block/blk-mq-tag.c, which already hates the current insane
thing, and uses __GFP_WAIT and (gfp & ~__GFP_WAIT) and other hacks,
and is obviously *very* aware of the internal hackery in the current
percpu_ida_alloc() argument. So I'm getting the feeling that that
whole thing might actually be *happier* with the TASK_xyz flags.
So I really think this needs cleanup, and that hacky "passing in
TASK_RUNNING to prepare_to_wait()" needs to be made official. And yes,
that implies that it's too late to try to push this through for 3.13,
this goes into the next merge window and can be backported.
Added the appropriate people to the Cc..
Linus
--
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: Disk spin-up optimization during system resume
On Fri, Jan 17, 2014 at 12:49 PM, Phillip Susi wrote: > -BEGIN PGP SIGNED MESSAGE- > Hash: SHA1 > > On 1/17/2014 2:31 PM, Dan Williams wrote: >> Once dpm_resume of the disk is asynchronous, is there much >> incremental gain by further deferring spin up? The drawback of >> doing on-demand resume of the disk is that you incur the full >> resume latency right when you need the data. System resume is a >> strong hint to warm the disks back up, and they will go back to >> sleep if unused. > > It isn't a strong hint to warm the disks back up at all. Well, "at all" is overstating it. The system was idle and now it's being woken up to do some work. Inactivity timers are invalidated and now the decision becomes minimize access latency, or lazily wake up. > You *may* > access *a* disk soon after resume, then again, you may not. Either > way, it isn't good to spin up *all* disks after a resume only to have > them stop again after a few minutes when they aren't accessed. Hiding resume latency is a good reason to resume the disk as soon as possible. >It puts needless wear and tear on disks. I'm not sure spin up cycles are predictive of drive failure? >> Of course it reduces the power savings if dpm_suspend/resume is a >> frequent occurrence. However, are systems that make aggressive use >> of system suspend shipping with rotating media, or do they ship >> with disks that are cheap to resume? If suspend is not frequent I >> wonder if it is worth the trouble/latency cost to keep the disk(s) >> asleep. > > The vast majority of systems still use HDD; SSD isn't *that* popular. I'm not arguing about the attach rate of HDDs in shipping systems, I'm asking what is the system suspend / resume frequency of such systems and is it worth carrying that complexity in the kernel and injecting unnecessary resume latency for what amounts to a niche use case. > Also there is the power savings of leaving the disk in standby when > there is no need to "spin" it up, which while fast on an SSD, does > still increase power consumption and so should be avoided. > > In any case, this will have no affect for most people since ATA disks > spin up on their own by default. So the use case is even more niche... -- 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: bio_integrity_verify() bug causing READ verify to be silently skipped
> "nab" == Nicholas A Bellinger writes: >> That breaks partial completion, though. I'll take a look at Kent's >> changes... nab> Ping..? Any updates on a proper bugfix for this..? I did put your patch in my queue and have been working on a fix for the partial completion case. The latter requires a bit of massaging that interferes with other pending changes. Given that your patch does address a valid issue I'm OK with Jens putting it in as is. I'll build upon it for my changes. -- Martin K. Petersen Oracle Linux Engineering -- 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: Why is (2 < 2) true? Is it a gcc bug?
On Fri, Jan 17, 2014 at 1:02 PM, Markus Trippelsdorf
wrote:
> On 2014.01.17 at 11:58 -0800, Alexei Starovoitov wrote:
>> On Fri, Jan 17, 2014 at 9:58 AM, Alexei Starovoitov
>> wrote:
>> > On Fri, Jan 17, 2014 at 5:37 AM, Dorau, Lukasz
>> > wrote:
>> >> Hi
>> >>
>> >> My story is very simply...
>> >> I applied the following patch:
>> >>
>> >> diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c
>> >> --- a/drivers/scsi/isci/init.c
>> >> +++ b/drivers/scsi/isci/init.c
>> >> @@ -698,8 +698,11 @@ static int isci_pci_probe(struct pci_dev *pdev,
>> >> const struct pci_device_id *id)
>> >> if (err)
>> >> goto err_host_alloc;
>> >>
>> >> - for_each_isci_host(i, isci_host, pdev)
>> >> + for_each_isci_host(i, isci_host, pdev) {
>> >> + pr_err("(%d < %d) == %d\n",\
>> >> + i, SCI_MAX_CONTROLLERS, (i < SCI_MAX_CONTROLLERS));
>> >> scsi_scan_host(to_shost(isci_host));
>> >> + }
>> >>
>> >> return 0;
>> >>
>> >> --
>> >> 1.8.3.1
>> >>
>> >> Then I issued the command 'modprobe isci' on platform with two SCU
>> >> controllers (Patsburg D or T chipset)
>> >> and received the following, very strange, output:
>> >>
>> >> (0 < 2) == 1
>> >> (1 < 2) == 1
>> >> (2 < 2) == 1
>> >>
>> >> Can anyone explain why (2 < 2) is true? Is it a gcc bug?
>> >
>> > gcc sees that i < array_size is the same as i < 2 as part of loop
>> > condition, so
>> > it optimizes (i < sci_max_controllers) into constant 1.
>> > and emits printk like:
>> > printk ("\13(%d < %d) == %d\n", i_382, 2, 1);
>> >
>> >> (The kernel was compiled using gcc version 4.8.2.)
>> >
>> > it actually looks to be gcc 4.8 bug.
>> > Can you try gcc 4.7 ?
>> >
>>
>> It is interesting GCC 4.8 bug,
>> since it seems to expose issues in two compiler passes.
>>
>> here is test case:
>>
>> struct isci_host;
>> struct isci_orom;
>>
>> struct isci_pci_info {
>> struct isci_host *hosts[2];
>> struct isci_orom *orom;
>> } v = {{(struct isci_host *)1,(struct isci_host *)1}, 0};
>>
>> int printf(const char *fmt, ...);
>>
>> int isci_pci_probe()
>> {
>> int i;
>> struct isci_host *isci_host;
>>
>> for (i = 0, isci_host = v.hosts[i];
>>i < 2 && isci_host;
>>isci_host = v.hosts[++i]) {
>> printf("(%d < %d) == %d\n", i, 2, (i < 2));
>> }
>>
>> return 0;
>> }
>>
>> int main()
>> {
>> isci_pci_probe();
>> }
>>
>> $ gcc bug.c
>> $./a.out
>> 0 < 2) == 1
>> (1 < 2) == 1
>> $ gcc bug.c -O2
>> $ ./a.out
>> (0 < 2) == 1
>> (1 < 2) == 1
>> Segmentation fault (core dumped)
>
> Your testcase is invalid:
>
> markus@x4 tmp % clang -fsanitize=undefined -Wall -Wextra -O2 bug.c
> markus@x4 tmp % ./a.out
> (0 < 2) == 1
> (1 < 2) == 1
> bug.c:16:20: runtime error: index 2 out of bounds for type 'struct isci_host
> *[2]'
>
> As Jakub Jelinek said on IRC, changing the loop to e.g.:
>
> for (i = 0;
>i < 2 && (isci_host = v.hosts[i]);
>i++) {
>
> fixes the issue.
testcase was reduced from drivers/scsi/isci/host.h written back in
2011 (commit b329aff107)
#define for_each_isci_host(id, ihost, pdev) \
for (id = 0, ihost = to_pci_info(pdev)->hosts[id]; \
id < ARRAY_SIZE(to_pci_info(pdev)->hosts) && ihost; \
ihost = to_pci_info(pdev)->hosts[++id])
yes, it does access 3rd element of 2 element array and will read junk.
C standard says the behavior is undefined and comes handy in compiler defense,
but in this case compiler has obviously all the information to make
right decision
instead of misoptimizing the code.
So yes, the loop is erroneous, non-portable, etc, but gcc can be smarter.
--
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: XHCI issues: WD MyBook 1230 - reset SuperSpeed USB device
Gentlemen, First of all, thank you very much for looking into this issue. Alan, for the sake of keeping the thread tidy in archives, I am not going to change the $SUBJECT of this e-mail but I wholeheartedly agree that this is not an xHCI issue. Mea culpa; I did not know that at the time I first reported it. Douglas, Hannes: I believe we are definitely on to something. I do _not_ run smartd but this may be caused by something in my Gnome3 GUI environment. I have noticed that when I am not logged in to Gnome and work just in the text console, plugging in the USB drive and accessing it works without any issues. Only when I am logged into my Gnome and plug the drive in, Gnome obviously tries to do something with the drive that causes it to stall and recover in 30 seconds. I have not yet been able to identify what it is. I am not subscribed to linux-scsi mailing list so if you believe anything of this is relevant please forward that mail there. Thank you! Best regards, Peter On 17.01.2014 21:25, Douglas Gilbert wrote: On 14-01-17 02:35 AM, Hannes Reinecke wrote: On 01/16/2014 09:48 PM, Alan Stern wrote: It's now clear that this is _not_ an XHCI issue, contrary to what $SUBJECT says. On Thu, 16 Jan 2014, Peter Palúch wrote: Alan, I am attaching the usbmon trace after the drive has been plugged into the USB-2 port. Record of the drive in stall should occur around the file offset 87808 (decimal). The log was done on the 3.12.7 kernel without CONFIG_PM. Should I do a usbmon trace on my regular kernel with CONFIG_PM as well? No need. dmesg transcript: root@bach:/tmp# dmesg usb 4-1.2: new high-speed USB device number 5 using ehci-pci usb 4-1.2: New USB device found, idVendor=1058, idProduct=1230 usb 4-1.2: New USB device strings: Mfr=2, Product=3, SerialNumber=1 usb 4-1.2: Product: My Book 1230 usb 4-1.2: Manufacturer: Western Digital usb 4-1.2: SerialNumber: 574D43344E30323438393836 usb-storage 4-1.2:1.0: USB Mass Storage device detected scsi6 : usb-storage 4-1.2:1.0 usbcore: registered new interface driver usb-storage scsi 6:0:0:0: Direct-Access WD My Book 1230 1050 PQ: 0 ANSI: 6 scsi 6:0:0:1: Enclosure WD SES Device 1050 PQ: 0 ANSI: 6 sd 6:0:0:0: Attached scsi generic sg2 type 0 scsi 6:0:0:1: Attached scsi generic sg3 type 13 sd 6:0:0:0: [sdb] Spinning up disk... .ready sd 6:0:0:0: [sdb] 732558336 4096-byte logical blocks: (3.00 TB/2.72 TiB) sd 6:0:0:0: [sdb] Write Protect is off sd 6:0:0:0: [sdb] Mode Sense: 53 00 10 08 sd 6:0:0:0: [sdb] No Caching mode page found sd 6:0:0:0: [sdb] Assuming drive cache: write through sd 6:0:0:0: [sdb] 732558336 4096-byte logical blocks: (3.00 TB/2.72 TiB) sd 6:0:0:0: [sdb] No Caching mode page found sd 6:0:0:0: [sdb] Assuming drive cache: write through sdb: sdb1 sd 6:0:0:0: [sdb] 732558336 4096-byte logical blocks: (3.00 TB/2.72 TiB) sd 6:0:0:0: [sdb] No Caching mode page found sd 6:0:0:0: [sdb] Assuming drive cache: write through sd 6:0:0:0: [sdb] Attached SCSI disk usb 4-1.2: reset high-speed USB device number 5 using ehci-pci It looks like the reset occurred because the computer sent an ATA-passthru command to the disk, and the disk wasn't prepared to handle it properly. The firmware crashed, requiring a reset. If anyone can explain, the command bytes in question were: 85082e00 ec00 SCSI ATA PASS-THROUGH(16) command issuing the mandatory ATA command 0xec which is IDENTIFY DEVICE. [See SAT-3 drafts for more information on the pass-through command.] and the sense data was: 7201001d 000e 090c 5d00 0100 0050 ATA spec says there should not (normally) be an error issued by that command; but there was: $ sg_decode_sense -n 7201001d 000e 090c 5d00 0100 0050 Descriptor format, current; Sense key: Recovered Error Additional sense: ATA pass through information available Descriptor type: ATA Status Return extend=0 error=0x0 sector_count=0x0 lba=0x00 device=0x0 status=0x50 Looks reasonable at the SCSI level, not sure about the ATA level, perhaps others can comment. I don't know what either of these means, or even what software was responsible for sending this command. It appears to have come from some user program, though, not the kernel. Possibly something run by udev. Probably smartd. The logic there is _less_ than perfect. Guilty as charged. Silly us, fancy smartmontools issuing mandatory ATA commands over a USB transport (MSC ?) and expecting the device to be well-behaved :-) Try to disable smartd and check if the issue remains. Probably will help. Throwing away all your USB storage devices (apart from those that do UAS(P)) would be even more helpful (to us). Perhaps smartmontools could do a better job of identifying USB connected storage devices and avoid them. Doug Gilbert [a smartmontools maintainer] -- Ing. Peter Palúch, Ph.D. CCIE R&S #23527, CCIP, CCAI, Cisco Designated VIP 2011-2014
Re: Why is (2 < 2) true? Is it a gcc bug?
On 2014.01.17 at 11:58 -0800, Alexei Starovoitov wrote:
> On Fri, Jan 17, 2014 at 9:58 AM, Alexei Starovoitov
> wrote:
> > On Fri, Jan 17, 2014 at 5:37 AM, Dorau, Lukasz
> > wrote:
> >> Hi
> >>
> >> My story is very simply...
> >> I applied the following patch:
> >>
> >> diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c
> >> --- a/drivers/scsi/isci/init.c
> >> +++ b/drivers/scsi/isci/init.c
> >> @@ -698,8 +698,11 @@ static int isci_pci_probe(struct pci_dev *pdev, const
> >> struct pci_device_id *id)
> >> if (err)
> >> goto err_host_alloc;
> >>
> >> - for_each_isci_host(i, isci_host, pdev)
> >> + for_each_isci_host(i, isci_host, pdev) {
> >> + pr_err("(%d < %d) == %d\n",\
> >> + i, SCI_MAX_CONTROLLERS, (i < SCI_MAX_CONTROLLERS));
> >> scsi_scan_host(to_shost(isci_host));
> >> + }
> >>
> >> return 0;
> >>
> >> --
> >> 1.8.3.1
> >>
> >> Then I issued the command 'modprobe isci' on platform with two SCU
> >> controllers (Patsburg D or T chipset)
> >> and received the following, very strange, output:
> >>
> >> (0 < 2) == 1
> >> (1 < 2) == 1
> >> (2 < 2) == 1
> >>
> >> Can anyone explain why (2 < 2) is true? Is it a gcc bug?
> >
> > gcc sees that i < array_size is the same as i < 2 as part of loop
> > condition, so
> > it optimizes (i < sci_max_controllers) into constant 1.
> > and emits printk like:
> > printk ("\13(%d < %d) == %d\n", i_382, 2, 1);
> >
> >> (The kernel was compiled using gcc version 4.8.2.)
> >
> > it actually looks to be gcc 4.8 bug.
> > Can you try gcc 4.7 ?
> >
>
> It is interesting GCC 4.8 bug,
> since it seems to expose issues in two compiler passes.
>
> here is test case:
>
> struct isci_host;
> struct isci_orom;
>
> struct isci_pci_info {
> struct isci_host *hosts[2];
> struct isci_orom *orom;
> } v = {{(struct isci_host *)1,(struct isci_host *)1}, 0};
>
> int printf(const char *fmt, ...);
>
> int isci_pci_probe()
> {
> int i;
> struct isci_host *isci_host;
>
> for (i = 0, isci_host = v.hosts[i];
>i < 2 && isci_host;
>isci_host = v.hosts[++i]) {
> printf("(%d < %d) == %d\n", i, 2, (i < 2));
> }
>
> return 0;
> }
>
> int main()
> {
> isci_pci_probe();
> }
>
> $ gcc bug.c
> $./a.out
> 0 < 2) == 1
> (1 < 2) == 1
> $ gcc bug.c -O2
> $ ./a.out
> (0 < 2) == 1
> (1 < 2) == 1
> Segmentation fault (core dumped)
Your testcase is invalid:
markus@x4 tmp % clang -fsanitize=undefined -Wall -Wextra -O2 bug.c
markus@x4 tmp % ./a.out
(0 < 2) == 1
(1 < 2) == 1
bug.c:16:20: runtime error: index 2 out of bounds for type 'struct isci_host
*[2]'
As Jakub Jelinek said on IRC, changing the loop to e.g.:
for (i = 0;
i < 2 && (isci_host = v.hosts[i]);
i++) {
fixes the issue.
--
Markus
--
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 v2 0/9] Phase out pci_enable_msi_block()
On Fri, Jan 17, 2014 at 9:02 AM, Alexander Gordeev wrote: > This series is against "next" branch in Bjorn's repo: > git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git > > Changes from v1 to v2: > - added a regression fix "ahci: Fix broken fallback to single > MSI mode" as patch 1/9; > - the series is reordered to move the regression fix in front; > - at Bjorn's request pci_enable_msi() is un-deprecated; > - as result, pci_enable_msi_range(pdev, 1, 1) styled calls > rolled back to pci_enable_msi(pdev); > - nvme bug fix moved out as a separate patch 5/9 "nvme: Fix > invalid call to irq_set_affinity_hint()" > - patches changelog elaborated a bit; > > Bjorn, > > As the release is supposedly this weekend, do you prefer > the patches to go to your tree or to individual trees after > the release? I'd be happy to merge them, except for the fact that they probably wouldn't have any time in -next before I ask Linus to pull them. So how about if we wait until after the release, ask the area maintainers to take them, and if they don't take them, I'll put them in my tree for v3.15? Bjorn -- 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: Disk spin-up optimization during system resume
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 1/17/2014 3:24 PM, Tejun Heo wrote: > Isn't immediate spin-up trivial to implement from anywhere? I'm > not sure whether we'll end up defaulting to the lazy behavior or > not but if we do requiring userland to echo something to sysfs to > configure immediate spin-up feels a bit silly when userland might > as well just issue a dummy command to force spinup. Or have your user space pm scripts disable runtime pm on the disk before suspend. Or just don't enable PuiS if you have an ATA disk. > And, yes, I agree with Dan that avoiding spinup of harddrives on > system resume seems a bit niche in its usefulness. suspend/resume > cycle at the very least generates logs which most likely will be > committed to the drive sooner or later. Preferably later, after the disk must be woken anyway for some reads :) > What kind of use cases are we expecting for the lazy behavior? Not all systems only have a single drive. There may be a tendency for IO to the drive with the root fs on it after a resume, but multi drive systems often end up not touching the other disks so they go back into suspend shortly after resume, so the start/stop cycle was just needless wear and tear on the drive. -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.17 (MingW32) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBAgAGBQJS2ZjCAAoJEI5FoCIzSKrwavMH+weGHd6Jjj4Z5Q7LMQMDaM6G eYn/Rfbia0Adk9VTdte7/5cOJDB2iSA9tv3G3KBODdMOyIVUViRgUtG3/Zh2t22z 5qq+F4Yj1V5cjEvFnNvYFA/RfW3W8yACYNR+KgJ6d88q1ZSoQ1R0KuhnHa4YA8Cf hN0lIdaYeIDdqXX7IvcMJb/9xlYVUuzgezvf4gdamQFvJDg4AspIuYXZMIhxy3Al TAX/yM0oHLrLA/4dJcTzoKkkUkx456oxa2i8DauM1gNWbuKEWSgG4hBG3ME2e6tU 9QbYLGOfg3naXcXEvNM1CL8xebh7V0mK4u4LXe/yePwyJNN9gIEDNmYX5xixeJQ= =5Qzv -END PGP SIGNATURE- -- 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: Disk spin-up optimization during system resume
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 1/17/2014 2:31 PM, Dan Williams wrote: > Once dpm_resume of the disk is asynchronous, is there much > incremental gain by further deferring spin up? The drawback of > doing on-demand resume of the disk is that you incur the full > resume latency right when you need the data. System resume is a > strong hint to warm the disks back up, and they will go back to > sleep if unused. It isn't a strong hint to warm the disks back up at all. You *may* access *a* disk soon after resume, then again, you may not. Either way, it isn't good to spin up *all* disks after a resume only to have them stop again after a few minutes when they aren't accessed. It puts needless wear and tear on disks. > Of course it reduces the power savings if dpm_suspend/resume is a > frequent occurrence. However, are systems that make aggressive use > of system suspend shipping with rotating media, or do they ship > with disks that are cheap to resume? If suspend is not frequent I > wonder if it is worth the trouble/latency cost to keep the disk(s) > asleep. The vast majority of systems still use HDD; SSD isn't *that* popular. Also there is the power savings of leaving the disk in standby when there is no need to "spin" it up, which while fast on an SSD, does still increase power consumption and so should be avoided. In any case, this will have no affect for most people since ATA disks spin up on their own by default. In my case, I have a backup disk that I rarely access so I enabled power up in standby on the disk and don't want the kernel starting and stopping it every time I resume. -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.17 (MingW32) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBAgAGBQJS2ZdsAAoJEI5FoCIzSKrwsm0H/3JyAGvgRF2GWZquo7o+z9Mr dUpkKfEvWpP5ZbrM+aJ6pJ6CUNABjpq4U+kjrEQ+KdydpStSokOLWPlwxBQbTm6R oz9SAfBofZJHTPa2Jzxadh6atHSSzTg8HOQBmJ5aVU0gGOJR5wo3b8ddSAEBAXLH NftNMwqOxgvrqG3GMmN1aAJV5qAXjJe1itAfnTfvbfBjrBqI15aIDCDXom+a5zj9 wUO7tuaoB+O3S5lI4eaeY5VIQrh5AbaozJST2s4Nzch9T/Ki5LEDDCVooUaQ+aU2 lEfJZbSMttD+J/SiTLnPoIPxEe72c7JcemjE0r7aPqLeE8AZHYrylttvIHIIStA= =It+N -END PGP SIGNATURE- -- 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: Why is (2 < 2) true? Is it a gcc bug?
Alexei Starovoitov writes: > > disable Value Range Propagation pass: > -fdisable-tree-vrp1 -fdisable-tree-vrp2 > > and complete unroll pass: > -fdisable-tree-cunrolli Can you file a gcc bug with test case? -Andi -- a...@linux.intel.com -- Speaking for myself only -- 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: XHCI issues: WD MyBook 1230 - reset SuperSpeed USB device
On 14-01-17 02:35 AM, Hannes Reinecke wrote: On 01/16/2014 09:48 PM, Alan Stern wrote: It's now clear that this is _not_ an XHCI issue, contrary to what $SUBJECT says. On Thu, 16 Jan 2014, Peter Palúch wrote: Alan, I am attaching the usbmon trace after the drive has been plugged into the USB-2 port. Record of the drive in stall should occur around the file offset 87808 (decimal). The log was done on the 3.12.7 kernel without CONFIG_PM. Should I do a usbmon trace on my regular kernel with CONFIG_PM as well? No need. dmesg transcript: root@bach:/tmp# dmesg usb 4-1.2: new high-speed USB device number 5 using ehci-pci usb 4-1.2: New USB device found, idVendor=1058, idProduct=1230 usb 4-1.2: New USB device strings: Mfr=2, Product=3, SerialNumber=1 usb 4-1.2: Product: My Book 1230 usb 4-1.2: Manufacturer: Western Digital usb 4-1.2: SerialNumber: 574D43344E30323438393836 usb-storage 4-1.2:1.0: USB Mass Storage device detected scsi6 : usb-storage 4-1.2:1.0 usbcore: registered new interface driver usb-storage scsi 6:0:0:0: Direct-Access WD My Book 1230 1050 PQ: 0 ANSI: 6 scsi 6:0:0:1: Enclosure WD SES Device 1050 PQ: 0 ANSI: 6 sd 6:0:0:0: Attached scsi generic sg2 type 0 scsi 6:0:0:1: Attached scsi generic sg3 type 13 sd 6:0:0:0: [sdb] Spinning up disk... .ready sd 6:0:0:0: [sdb] 732558336 4096-byte logical blocks: (3.00 TB/2.72 TiB) sd 6:0:0:0: [sdb] Write Protect is off sd 6:0:0:0: [sdb] Mode Sense: 53 00 10 08 sd 6:0:0:0: [sdb] No Caching mode page found sd 6:0:0:0: [sdb] Assuming drive cache: write through sd 6:0:0:0: [sdb] 732558336 4096-byte logical blocks: (3.00 TB/2.72 TiB) sd 6:0:0:0: [sdb] No Caching mode page found sd 6:0:0:0: [sdb] Assuming drive cache: write through sdb: sdb1 sd 6:0:0:0: [sdb] 732558336 4096-byte logical blocks: (3.00 TB/2.72 TiB) sd 6:0:0:0: [sdb] No Caching mode page found sd 6:0:0:0: [sdb] Assuming drive cache: write through sd 6:0:0:0: [sdb] Attached SCSI disk usb 4-1.2: reset high-speed USB device number 5 using ehci-pci It looks like the reset occurred because the computer sent an ATA-passthru command to the disk, and the disk wasn't prepared to handle it properly. The firmware crashed, requiring a reset. If anyone can explain, the command bytes in question were: 85082e00 ec00 SCSI ATA PASS-THROUGH(16) command issuing the mandatory ATA command 0xec which is IDENTIFY DEVICE. [See SAT-3 drafts for more information on the pass-through command.] and the sense data was: 7201001d 000e 090c 5d00 0100 0050 ATA spec says there should not (normally) be an error issued by that command; but there was: $ sg_decode_sense -n 7201001d 000e 090c 5d00 0100 0050 Descriptor format, current; Sense key: Recovered Error Additional sense: ATA pass through information available Descriptor type: ATA Status Return extend=0 error=0x0 sector_count=0x0 lba=0x00 device=0x0 status=0x50 Looks reasonable at the SCSI level, not sure about the ATA level, perhaps others can comment. I don't know what either of these means, or even what software was responsible for sending this command. It appears to have come from some user program, though, not the kernel. Possibly something run by udev. Probably smartd. The logic there is _less_ than perfect. Guilty as charged. Silly us, fancy smartmontools issuing mandatory ATA commands over a USB transport (MSC ?) and expecting the device to be well-behaved :-) Try to disable smartd and check if the issue remains. Probably will help. Throwing away all your USB storage devices (apart from those that do UAS(P)) would be even more helpful (to us). Perhaps smartmontools could do a better job of identifying USB connected storage devices and avoid them. Doug Gilbert [a smartmontools maintainer] -- 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: Disk spin-up optimization during system resume
On Fri, Jan 17, 2014 at 03:15:54PM -0500, Alan Stern wrote: > You will have to argue this point with Phillip. > > If necessary, we could add a sysfs attribute to force a spin-up during > system resume. Or you could disable runtime PM for the disk, but that > has its own disadvantages. Isn't immediate spin-up trivial to implement from anywhere? I'm not sure whether we'll end up defaulting to the lazy behavior or not but if we do requiring userland to echo something to sysfs to configure immediate spin-up feels a bit silly when userland might as well just issue a dummy command to force spinup. And, yes, I agree with Dan that avoiding spinup of harddrives on system resume seems a bit niche in its usefulness. suspend/resume cycle at the very least generates logs which most likely will be committed to the drive sooner or later. What kind of use cases are we expecting for the lazy behavior? 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: Disk spin-up optimization during system resume
On Fri, 17 Jan 2014, Dan Williams wrote: > Once dpm_resume of the disk is asynchronous, is there much incremental > gain by further deferring spin up? The drawback of doing on-demand > resume of the disk is that you incur the full resume latency right > when you need the data. System resume is a strong hint to warm the > disks back up, and they will go back to sleep if unused. > > Of course it reduces the power savings if dpm_suspend/resume is a > frequent occurrence. However, are systems that make aggressive use of > system suspend shipping with rotating media, or do they ship with > disks that are cheap to resume? If suspend is not frequent I wonder > if it is worth the trouble/latency cost to keep the disk(s) asleep. You will have to argue this point with Phillip. If necessary, we could add a sysfs attribute to force a spin-up during system resume. Or you could disable runtime PM for the disk, but that has its own disadvantages. Alan Stern -- 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
[PATCH 1/1] scsi: hpsa resubmit all PCI IDs
From: Mike Miller
This patch has every ID we have in our svn repository. Some controllers were
cancelled, others added, now the cancelled ones are back. This patch made
against linux-3.13.0-rc8. Now the tables are in sync with one another.
Thanks to Tomas Henzl for pointing out the earlier problem.
Signed-off-by: Mike Miller
---
drivers/scsi/hpsa.c | 14 +-
1 files changed, 13 insertions(+), 1 deletions(-)
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 348b207..614b9cb 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -91,7 +91,6 @@ static const struct pci_device_id hpsa_pci_device_id[] = {
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSE, 0x103C, 0x3249},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSE, 0x103C, 0x324A},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSE, 0x103C, 0x324B},
- {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSE, 0x103C, 0x3233},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSF, 0x103C, 0x3350},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSF, 0x103C, 0x3351},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSF, 0x103C, 0x3352},
@@ -100,6 +99,7 @@ static const struct pci_device_id hpsa_pci_device_id[] = {
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSF, 0x103C, 0x3354},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSF, 0x103C, 0x3355},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSF, 0x103C, 0x3356},
+ {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C, 0x1920},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C, 0x1921},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C, 0x1922},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C, 0x1923},
@@ -108,15 +108,20 @@ static const struct pci_device_id hpsa_pci_device_id[] = {
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C, 0x1926},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C, 0x1928},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C, 0x1929},
+ {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C, 0x192A},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C, 0x21BD},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C, 0x21BE},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C, 0x21BF},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C, 0x21C0},
+ {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C, 0x21C1},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C, 0x21C2},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C, 0x21C3},
+ {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C, 0x21C4},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C, 0x21C5},
+ {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C, 0x21C6},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C, 0x21C7},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C, 0x21C8},
+ {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C, 0x21C9},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C, 0x21CA},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C, 0x21CB},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C, 0x21CC},
@@ -149,22 +154,29 @@ static struct board_type products[] = {
{0x3354103C, "Smart Array P420i", &SA5_access},
{0x3355103C, "Smart Array P220i", &SA5_access},
{0x3356103C, "Smart Array P721m", &SA5_access},
+ {0x1920103C, "Smart Array P430i", &SA5_access},
{0x1921103C, "Smart Array P830i", &SA5_access},
{0x1922103C, "Smart Array P430", &SA5_access},
{0x1923103C, "Smart Array P431", &SA5_access},
{0x1924103C, "Smart Array P830", &SA5_access},
+ {0x1925103C, "Smart Array P831", &SA5_access},
{0x1926103C, "Smart Array P731m", &SA5_access},
{0x1928103C, "Smart Array P230i", &SA5_access},
{0x1929103C, "Smart Array P530", &SA5_access},
+ {0x192A103C, "Smart Array P531", &SA5_access},
{0x21BD103C, "Smart Array", &SA5_access},
{0x21BE103C, "Smart Array", &SA5_access},
{0x21BF103C, "Smart Array", &SA5_access},
{0x21C0103C, "Smart Array", &SA5_access},
+ {0x21C1103C, "Smart Array", &SA5_access},
{0x21C2103C, "Smart Array", &SA5_access},
{0x21C3103C, "Smart Array", &SA5_access},
+ {0x21C4103C, "Smart Array", &SA5_access},
{0x21C5103C, "Smart Array", &SA5_access},
+ {0x21C6103C, "Smart Array", &SA5_access},
{0x21C7103C, "Smart Array", &SA5_access},
{0x21C8103C, "Smart Array", &SA5_access},
+ {0x21C9103C, "Smart Array", &SA5_access},
{0x21CA103C, "Smart Array", &SA5_access},
{0x21CB103C, "Smart Array", &SA5_access},
{0x21CC103C, "Smart Array", &SA5_access},
--
To unsubscribe from this
Re: Why is (2 < 2) true? Is it a gcc bug?
On Fri, Jan 17, 2014 at 9:58 AM, Alexei Starovoitov
wrote:
> On Fri, Jan 17, 2014 at 5:37 AM, Dorau, Lukasz wrote:
>> Hi
>>
>> My story is very simply...
>> I applied the following patch:
>>
>> diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c
>> --- a/drivers/scsi/isci/init.c
>> +++ b/drivers/scsi/isci/init.c
>> @@ -698,8 +698,11 @@ static int isci_pci_probe(struct pci_dev *pdev, const
>> struct pci_device_id *id)
>> if (err)
>> goto err_host_alloc;
>>
>> - for_each_isci_host(i, isci_host, pdev)
>> + for_each_isci_host(i, isci_host, pdev) {
>> + pr_err("(%d < %d) == %d\n",\
>> + i, SCI_MAX_CONTROLLERS, (i < SCI_MAX_CONTROLLERS));
>> scsi_scan_host(to_shost(isci_host));
>> + }
>>
>> return 0;
>>
>> --
>> 1.8.3.1
>>
>> Then I issued the command 'modprobe isci' on platform with two SCU
>> controllers (Patsburg D or T chipset)
>> and received the following, very strange, output:
>>
>> (0 < 2) == 1
>> (1 < 2) == 1
>> (2 < 2) == 1
>>
>> Can anyone explain why (2 < 2) is true? Is it a gcc bug?
>
> gcc sees that i < array_size is the same as i < 2 as part of loop condition,
> so
> it optimizes (i < sci_max_controllers) into constant 1.
> and emits printk like:
> printk ("\13(%d < %d) == %d\n", i_382, 2, 1);
>
>> (The kernel was compiled using gcc version 4.8.2.)
>
> it actually looks to be gcc 4.8 bug.
> Can you try gcc 4.7 ?
>
> gcc 4.7 compiles your loop into the following:
> :
> # i_382 = PHI <0(73), i_73(74)>
> # isci_host_148 = PHI
> printk ("\13(%d < %d) == %d\n", i_382, 2, 1);
> D.43295_70 = MEM[(struct isci_host *)isci_host_148 + 18632B];
> # DEBUG D#6 => isci_host_148
> # DEBUG ihost s=> ihost
> scsi_scan_host (D.43295_70);
> # DEBUG pdev => pdev_17(D)
> # DEBUG pdev => pdev_17(D)
> D.43629_353 = dev_get_drvdata (D.42809_20);
> i_73 = i_382 + 1;
> # DEBUG i => i_73
> isci_host_74 = MEM[(struct isci_pci_info *)D.43629_353].hosts[i_73];
> # DEBUG isci_host => isci_host_74
> # DEBUG isci_host => isci_host_74
> # DEBUG i => i_73
> i.9_79 = (unsigned int) i_73;
> D.42849_65 = i.9_79 <= 1;
> D.42850_66 = isci_host_74 != 0B;
> D.42851_67 = D.42850_66 & D.42849_65;
> if (D.42851_67 != 0)
> goto ;
> else
> goto ;
>
> which looks correct to me.
>
> while gcc 4.8.2 into:
> :
> # i_73 = PHI
> # isci_host_274 = PHI
> # DEBUG isci_host => isci_host_274
> # DEBUG i => i_73
> printk ("\13(%d < %d) == %d\n", i_73, 2, 1);
> _79 = MEM[(struct isci_host *)isci_host_274 + 18632B];
> # DEBUG D#6 => isci_host_274
> # DEBUG ihost => D#6
> scsi_scan_host (_79);
> # DEBUG pdev => pdev_26(D)
> # DEBUG pdev => pdev_26(D)
> _97 = dev_get_drvdata (_29);
> i_82 = i_73 + 1;
> # DEBUG i => i_82
> isci_host_83 = MEM[(struct isci_pci_info *)_97].hosts[i_82];
> # DEBUG isci_host => isci_host_83
> # DEBUG isci_host => isci_host_83
> # DEBUG i => i_82
> if (isci_host_83 != 0B)
> goto ;
> else
> goto ;
>
> :
> goto ;
>
> in case of gcc4.8 the i<=1 comparison got optimized out and only
> isci_host !=0 is left,
> which looks incorrect.
It is interesting GCC 4.8 bug,
since it seems to expose issues in two compiler passes.
here is test case:
struct isci_host;
struct isci_orom;
struct isci_pci_info {
struct isci_host *hosts[2];
struct isci_orom *orom;
} v = {{(struct isci_host *)1,(struct isci_host *)1}, 0};
int printf(const char *fmt, ...);
int isci_pci_probe()
{
int i;
struct isci_host *isci_host;
for (i = 0, isci_host = v.hosts[i];
i < 2 && isci_host;
isci_host = v.hosts[++i]) {
printf("(%d < %d) == %d\n", i, 2, (i < 2));
}
return 0;
}
int main()
{
isci_pci_probe();
}
$ gcc bug.c
$./a.out
0 < 2) == 1
(1 < 2) == 1
$ gcc bug.c -O2
$ ./a.out
(0 < 2) == 1
(1 < 2) == 1
Segmentation fault (core dumped)
workaround:
disable Value Range Propagation pass:
-fdisable-tree-vrp1 -fdisable-tree-vrp2
and complete unroll pass:
-fdisable-tree-cunrolli
--
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: Disk spin-up optimization during system resume
On Fri, Jan 17, 2014 at 6:57 AM, Alan Stern wrote: > On Thu, 16 Jan 2014, Todd E Brandt wrote: >> My patch makes ata port and scsi disk resume 'non-blocking' (asynchronous >> with respect to system resume). Which means once they're called the power >> manager pays no more attention to them and will complete system resume >> whenever. Both the power manager and the ata subsystems call these >> features 'asynchronous' so it can be confusing. > > I see. That's not explained very well in the patch description -- it > talks about going on to resume the next device on the list, but not > about waiting for the overall system resume to finish. > Once dpm_resume of the disk is asynchronous, is there much incremental gain by further deferring spin up? The drawback of doing on-demand resume of the disk is that you incur the full resume latency right when you need the data. System resume is a strong hint to warm the disks back up, and they will go back to sleep if unused. Of course it reduces the power savings if dpm_suspend/resume is a frequent occurrence. However, are systems that make aggressive use of system suspend shipping with rotating media, or do they ship with disks that are cheap to resume? If suspend is not frequent I wonder if it is worth the trouble/latency cost to keep the disk(s) asleep. -- Dan -- 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: Disk spin-up optimization during system resume
Hello, James. On Fri, Jan 17, 2014 at 10:39:37AM -0800, James Bottomley wrote: > The specific worry is the writeback cache. If the flush fails and we > power down with dirty blocks in the cache, those blocks are lost but the > filesystem still thinks they're committed. I think as long as you're > using flush barriers on your system, this isn't a problem, but if you're > not using barriers, it can lead to undetected corruption that blows up > later. If it were a choice between suspend my laptop and save my data, > I'd choose the latter. But the question is do we care ... perhaps we > can just say you have to use flush barriers if you want to suspend. Ooh, yeah, flush failure is special. That said, I think the right way to deal with that is marking the device as failed and fail writes / flushes afterwards instead of failing suspend. It's hightly unlikely the device is in any useable state after failing flushes anyway and failing suspend has potential to lead to pretty dramatic failure conditions (device overheating in the bag would be a common one) too. 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: Disk spin-up optimization during system resume
On Fri, 2014-01-17 at 05:21 -0500, Tejun Heo wrote: > Hello, > > On Fri, Jan 17, 2014 at 11:16:49AM +0100, Oliver Neukum wrote: > > The START-STOP may result in an error. What do you do in that case? > > At least for libata, worrying about suspend/resume failures don't make > whole lot of sense. If suspend failed, just proceed with suspend. If > the device can't be woken up afterwards, that's that. The specific worry is the writeback cache. If the flush fails and we power down with dirty blocks in the cache, those blocks are lost but the filesystem still thinks they're committed. I think as long as you're using flush barriers on your system, this isn't a problem, but if you're not using barriers, it can lead to undetected corruption that blows up later. If it were a choice between suspend my laptop and save my data, I'd choose the latter. But the question is do we care ... perhaps we can just say you have to use flush barriers if you want to suspend. 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 0/24] qla4xxx: 5.04.00-k4: Updates for scsi "misc" branch
On 12/16/2013 05:49 AM, vikas.chaudh...@qlogic.com wrote: > From: Vikas Chaudhary > > James, > > Please apply the following patches to the scsi tree at your earliest > convenience. > These patches are on top of other qla4xxx patch posted on list here: > http://marc.info/?l=linux-scsi&m=138511809612830&w=2 > > Nilesh Javali (5): > qla4xxx: ISP8xxx: Correct retry of adapter initialization > qla4xxx: Improve loopback failure messages > qla4xxx: Rename ACB_STATE macros with IP_ADDRSTATE macros > qla4xxx: Clear DDB index map upon connection close failure > qla4xxx: Handle IPv6 AEN notifications > > Tej Parkash (4): > qla4xxx: Fixed AER reset sequence for ISP83xx/ISP84xx > qla4xxx: Fix processing response queue during probe > qla4xxx: Fix pending IO completion in reset path before initiating chip > reset > qla4xxx: Driver not able to collect minidump for ISP84xx > > Vikas Chaudhary (15): > qla4xxx: Print WARN_ONCE() if iSCSI function presence bit removed > qla4xxx: Fix comments in code > qla4xxx: Use IDC_CTRL bit1 directly instead of AF_83XX_NO_FWDUMP flag. > qla4xxx: Correctly handle msleep_interruptible > qla4xxx: Return correct error status from func qla4xxx_request_irqs() > qla4xxx: Fix failure of IDC Time Extend mailbox command > qla4xxx: Reduce rom-lock contention during reset recovery. > qla4xxx: Fix failure of mbox 0x31 > qla4xxx: Remove unused code from qla4xxx_set_ifcb() > qla4xxx: Updated print for device login, logout path > qla4xxx: Update print statements in qla4xxx_mailbox_command() > qla4xxx: Update print statements in func qla4xxx_eh_abort() > qla4xxx: Update print statements in func qla4xxx_do_dpc() > qla4xxx: Fix sparse warnings > qla4xxx: Update driver version to 5.04.00-k4 > > Thanks, > Vikas. > Patchset looks ok with fixed up [PATCH 16/24] patch here "[PATCH V1 16/24] qla4xxx: Fix failure of mbox 0x31" http://marc.info/?l=linux-scsi&m=138995390232212&w=2 Reviewed-by: Mike Christie -- 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 V1 16/24] qla4xxx: Fix failure of mbox 0x31
On 01/17/2014 03:43 AM, vikas.chaudh...@qlogic.com wrote:
> From: Vikas Chaudhary
>
> Issue:
> While unloading driver MBOX 0x31 fail as DDB logout (MBOX 0x56)
> operation is not completed.
>
> Fix:
> Wait for DDB Logout completion before MBOX 0x31
>
> Signed-off-by: Vikas Chaudhary
> ---
> drivers/scsi/qla4xxx/ql4_os.c | 56
> ---
> 1 file changed, 48 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
> index a27da31..7807a13 100644
> --- a/drivers/scsi/qla4xxx/ql4_os.c
> +++ b/drivers/scsi/qla4xxx/ql4_os.c
> @@ -8886,10 +8886,56 @@ static void qla4xxx_prevent_other_port_reinit(struct
> scsi_qla_host *ha)
> }
> }
>
> +static void qla4xxx_destroy_ddb(struct scsi_qla_host *ha,
> + struct ddb_entry *ddb_entry)
> +{
> + struct dev_db_entry *fw_ddb_entry = NULL;
> + dma_addr_t fw_ddb_entry_dma;
> + unsigned long wtime;
> + uint32_t ddb_state;
> + int options;
> + int status;
> +
> + options = LOGOUT_OPTION_CLOSE_SESSION;
> + if (qla4xxx_session_logout_ddb(ha, ddb_entry, options) == QLA_ERROR) {
> + ql4_printk(KERN_ERR, ha, "%s: Logout failed\n", __func__);
> + goto clear_ddb;
> + }
> +
> + fw_ddb_entry = dma_alloc_coherent(&ha->pdev->dev, sizeof(*fw_ddb_entry),
> + &fw_ddb_entry_dma, GFP_KERNEL);
> + if (!fw_ddb_entry) {
> + ql4_printk(KERN_ERR, ha,
> +"%s: Unable to allocate dma buffer\n", __func__);
> + goto clear_ddb;
> + }
> +
> + wtime = jiffies + (HZ * LOGOUT_TOV);
> + do {
> + status = qla4xxx_get_fwddb_entry(ha, ddb_entry->fw_ddb_index,
> + fw_ddb_entry, fw_ddb_entry_dma,
> + NULL, NULL, &ddb_state, NULL,
> + NULL, NULL);
> + if (status == QLA_ERROR)
> + goto free_ddb;
> +
> + if ((ddb_state == DDB_DS_NO_CONNECTION_ACTIVE) ||
> + (ddb_state == DDB_DS_SESSION_FAILED))
> + goto free_ddb;
> +
> + schedule_timeout_uninterruptible(HZ);
> + } while ((time_after(wtime, jiffies)));
> +
> +free_ddb:
> + dma_free_coherent(&ha->pdev->dev, sizeof(*fw_ddb_entry),
> + fw_ddb_entry, fw_ddb_entry_dma);
> +clear_ddb:
> + qla4xxx_clear_ddb_entry(ha, ddb_entry->fw_ddb_index);
> +}
Looks ok now.
Reviewed-by: Mike Christie
--
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: Disk spin-up optimization during system resume
On Fri, 17 Jan 2014, Tejun Heo wrote: > Hello, > > On Fri, Jan 17, 2014 at 11:16:49AM +0100, Oliver Neukum wrote: > > The START-STOP may result in an error. What do you do in that case? > > At least for libata, worrying about suspend/resume failures don't make > whole lot of sense. If suspend failed, just proceed with suspend. If > the device can't be woken up afterwards, that's that. There isn't > anything we could have done differently anyway. The same for resume, > if spinup fails, the device is dud and the following commands will > invoke EH actions and will eventually fail. Again, there really isn't > any *choice* to make. Just making sure the errors are handled > gracefully (ie. don't crash) and the following commands are handled > correctly should be enough. Absolutely. Even with normal synchronous resumes, we don't do anything if an error occurs, other than printing it in the kernel log. Alan Stern -- 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: Why is (2 < 2) true? Is it a gcc bug?
On Fri, Jan 17, 2014 at 5:37 AM, Dorau, Lukasz wrote:
> Hi
>
> My story is very simply...
> I applied the following patch:
>
> diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c
> --- a/drivers/scsi/isci/init.c
> +++ b/drivers/scsi/isci/init.c
> @@ -698,8 +698,11 @@ static int isci_pci_probe(struct pci_dev *pdev, const
> struct pci_device_id *id)
> if (err)
> goto err_host_alloc;
>
> - for_each_isci_host(i, isci_host, pdev)
> + for_each_isci_host(i, isci_host, pdev) {
> + pr_err("(%d < %d) == %d\n",\
> + i, SCI_MAX_CONTROLLERS, (i < SCI_MAX_CONTROLLERS));
> scsi_scan_host(to_shost(isci_host));
> + }
>
> return 0;
>
> --
> 1.8.3.1
>
> Then I issued the command 'modprobe isci' on platform with two SCU
> controllers (Patsburg D or T chipset)
> and received the following, very strange, output:
>
> (0 < 2) == 1
> (1 < 2) == 1
> (2 < 2) == 1
>
> Can anyone explain why (2 < 2) is true? Is it a gcc bug?
gcc sees that i < array_size is the same as i < 2 as part of loop condition, so
it optimizes (i < sci_max_controllers) into constant 1.
and emits printk like:
printk ("\13(%d < %d) == %d\n", i_382, 2, 1);
> (The kernel was compiled using gcc version 4.8.2.)
it actually looks to be gcc 4.8 bug.
Can you try gcc 4.7 ?
gcc 4.7 compiles your loop into the following:
:
# i_382 = PHI <0(73), i_73(74)>
# isci_host_148 = PHI
printk ("\13(%d < %d) == %d\n", i_382, 2, 1);
D.43295_70 = MEM[(struct isci_host *)isci_host_148 + 18632B];
# DEBUG D#6 => isci_host_148
# DEBUG ihost s=> ihost
scsi_scan_host (D.43295_70);
# DEBUG pdev => pdev_17(D)
# DEBUG pdev => pdev_17(D)
D.43629_353 = dev_get_drvdata (D.42809_20);
i_73 = i_382 + 1;
# DEBUG i => i_73
isci_host_74 = MEM[(struct isci_pci_info *)D.43629_353].hosts[i_73];
# DEBUG isci_host => isci_host_74
# DEBUG isci_host => isci_host_74
# DEBUG i => i_73
i.9_79 = (unsigned int) i_73;
D.42849_65 = i.9_79 <= 1;
D.42850_66 = isci_host_74 != 0B;
D.42851_67 = D.42850_66 & D.42849_65;
if (D.42851_67 != 0)
goto ;
else
goto ;
which looks correct to me.
while gcc 4.8.2 into:
:
# i_73 = PHI
# isci_host_274 = PHI
# DEBUG isci_host => isci_host_274
# DEBUG i => i_73
printk ("\13(%d < %d) == %d\n", i_73, 2, 1);
_79 = MEM[(struct isci_host *)isci_host_274 + 18632B];
# DEBUG D#6 => isci_host_274
# DEBUG ihost => D#6
scsi_scan_host (_79);
# DEBUG pdev => pdev_26(D)
# DEBUG pdev => pdev_26(D)
_97 = dev_get_drvdata (_29);
i_82 = i_73 + 1;
# DEBUG i => i_82
isci_host_83 = MEM[(struct isci_pci_info *)_97].hosts[i_82];
# DEBUG isci_host => isci_host_83
# DEBUG isci_host => isci_host_83
# DEBUG i => i_82
if (isci_host_83 != 0B)
goto ;
else
goto ;
:
goto ;
in case of gcc4.8 the i<=1 comparison got optimized out and only
isci_host !=0 is left,
which looks incorrect.
--
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
[LSF/MM TOPIC] [ATTEND] new generation copy offload (ODX)
Hannes Reinecke and I would like to attend LSF and present a talk on the new generation copy offload (a.k.a. ODX). Most discussions about copy offload at the storage level are based on T10's 15 year old xcopy(LID1), specifically its disk->disk copy. This year T10 hopes to standardize a new generation copy offload whose generic name is xcopy(LID4). A subset of xcopy(LID4) that does disk->disk copies and already has vendors with products supporting it, has the market name of ODX. ODX is ROD (Representation Of Data) based (as is xcopy(LID4)) where the copy is split into two steps: disk->held [SCSI POPULATE TOKEN command] held->disk [SCSI WRITE USING TOKEN command] We would like to discuss how the interfaces (both to the block layer and the filesystem) should look like for implementing ROD based copies. Further we would like to discuss if and how the Linux target subsystem can be expanded to support ODX in the future, in addition to the already existing support for xcopy(LID1). An example of one of the new ODX/xcopy(LID4) capabilities can be found in this post: http://www.spinics.net/lists/linux-scsi/msg71411.html We hope to be able to demonstrate some low level Linux utilities doing ODX copies on a remote array. Doug Gilbert -- 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: Why is (2 < 2) true? Is it a gcc bug?
On Friday, January 17, 2014 5:40 PM Sebastian Riemer
wrote:
> On 17.01.2014 14:55, Dorau, Lukasz wrote:
> >
> > Some additional information:
> >
> > The loop 'for' in macro ' for_each_isci_host ' defined as
> (drivers/scsi/isci/host.h:313):
> >
> > #define for_each_isci_host(id, ihost, pdev) \
> > for (id = 0, ihost = to_pci_info(pdev)->hosts[id]; \
> > id < ARRAY_SIZE(to_pci_info(pdev)->hosts) && ihost; \
> > ihost = to_pci_info(pdev)->hosts[++id])
> >
> > should be executed only for i = 0 and 1, because:
> > ARRAY_SIZE(to_pci_info(pdev)->hosts) = SCI_MAX_CONTROLLERS = 2
> >
> > but it was executed also for i=2 regardless the above loop's end condition.
>
> to_pci_info() can return NULL in dev_get_drvdata(). The use of
> ARRAY_SIZE() is inappropriate.
>
> #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) +
> __must_be_array(arr))
>
> #define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
>
> #define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); }))
>
> I would say that this was supposed to trigger a build error but it
> didn't and added 1 to the loop end condition.
>
> Can you test putting SCI_MAX_CONTROLLERS to the loop end condition, please?
>
Replacing 'ARRAY_SIZE(to_pci_info(pdev)->hosts)' with 'SCI_MAX_CONTROLLERS' in
the definition of the ' for_each_isci_host ' macro does not help. I have
checked it.
The following patch helps:
http://marc.info/?l=linux-scsi&m=138987823011697&w=2
Lukasz
--
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: Why is (2 < 2) true? Is it a gcc bug?
On 17.01.2014 14:55, Dorau, Lukasz wrote:
> On Friday, January 17, 2014 2:37 PM Dorau, Lukasz
> wrote:
>>
>> Hi
>>
>> My story is very simply...
>> I applied the following patch:
>>
>> diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c
>> --- a/drivers/scsi/isci/init.c
>> +++ b/drivers/scsi/isci/init.c
>> @@ -698,8 +698,11 @@ static int isci_pci_probe(struct pci_dev *pdev, const
>> struct
>> pci_device_id *id)
>> if (err)
>> goto err_host_alloc;
>>
>> -for_each_isci_host(i, isci_host, pdev)
>> +for_each_isci_host(i, isci_host, pdev) {
>> +pr_err("(%d < %d) == %d\n",\
>> + i, SCI_MAX_CONTROLLERS, (i < SCI_MAX_CONTROLLERS));
>> scsi_scan_host(to_shost(isci_host));
>> +}
>>
>> return 0;
>>
>> --
>> 1.8.3.1
>>
>> Then I issued the command 'modprobe isci' on platform with two SCU
>> controllers
>> (Patsburg D or T chipset)
>> and received the following, very strange, output:
>>
>> (0 < 2) == 1
>> (1 < 2) == 1
>> (2 < 2) == 1
>>
>> Can anyone explain why (2 < 2) is true? Is it a gcc bug?
>>
>> (The kernel was compiled using gcc version 4.8.2.)
>>
>
> Some additional information:
>
> The loop 'for' in macro ' for_each_isci_host ' defined as
> (drivers/scsi/isci/host.h:313):
>
> #define for_each_isci_host(id, ihost, pdev) \
> for (id = 0, ihost = to_pci_info(pdev)->hosts[id]; \
> id < ARRAY_SIZE(to_pci_info(pdev)->hosts) && ihost; \
> ihost = to_pci_info(pdev)->hosts[++id])
>
> should be executed only for i = 0 and 1, because:
> ARRAY_SIZE(to_pci_info(pdev)->hosts) = SCI_MAX_CONTROLLERS = 2
>
> but it was executed also for i=2 regardless the above loop's end condition.
to_pci_info() can return NULL in dev_get_drvdata(). The use of
ARRAY_SIZE() is inappropriate.
#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) +
__must_be_array(arr))
#define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
#define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); }))
I would say that this was supposed to trigger a build error but it
didn't and added 1 to the loop end condition.
Can you test putting SCI_MAX_CONTROLLERS to the loop end condition, please?
Cheers,
Sebastian
--
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
[PATCH v2 0/9] Phase out pci_enable_msi_block()
This series is against "next" branch in Bjorn's repo: git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git Changes from v1 to v2: - added a regression fix "ahci: Fix broken fallback to single MSI mode" as patch 1/9; - the series is reordered to move the regression fix in front; - at Bjorn's request pci_enable_msi() is un-deprecated; - as result, pci_enable_msi_range(pdev, 1, 1) styled calls rolled back to pci_enable_msi(pdev); - nvme bug fix moved out as a separate patch 5/9 "nvme: Fix invalid call to irq_set_affinity_hint()" - patches changelog elaborated a bit; Bjorn, As the release is supposedly this weekend, do you prefer the patches to go to your tree or to individual trees after the release? Thanks! Alexander Gordeev (9): ahci: Fix broken fallback to single MSI mode ahci: Use pci_enable_msi_range() ipr: Get rid of superfluous call to pci_disable_msi/msix() ipr: Use pci_enable_msi_range() and pci_enable_msix_range() nvme: Fix invalid call to irq_set_affinity_hint() nvme: Use pci_enable_msi_range() and pci_enable_msix_range() vfio: Use pci_enable_msi_range() and pci_enable_msix_range() ath10k: Use pci_enable_msi_range() wil6210: Use pci_enable_msi_range() drivers/ata/ahci.c | 18 +- drivers/block/nvme-core.c | 33 - drivers/net/wireless/ath/ath10k/pci.c | 20 +- drivers/net/wireless/ath/wil6210/pcie_bus.c | 36 ++- drivers/scsi/ipr.c | 51 +- drivers/vfio/pci/vfio_pci_intrs.c | 12 -- 6 files changed, 72 insertions(+), 98 deletions(-) -- 1.7.7.6 -- 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 1/1] scsi: hpsa, add all PCI ID's that HP has in svn
On 01/17/2014 04:17 PM, Miller, Mike (OS Dev) wrote:
>
>
>> -Original Message-
>> From: Tomas Henzl [mailto:the...@redhat.com]
>> Sent: Friday, January 17, 2014 9:14 AM
>> To: Miller, Mike (OS Dev); Andrew Morton; James E. J. Bottomley
>> Cc: LKML; LKML-scsi; h...@suse.de; Stephen M. Cameron
>> Subject: Re: [PATCH 1/1] scsi: hpsa, add all PCI ID's that HP has in svn
>>
>> On 01/16/2014 08:51 PM, Mike Miller wrote:
>>> From: Mike Miller
>>>
>>> This patch has every ID we have in our svn repository. Some
>>> controllers were cancelled, others added, now the cancelled ones are
>>> back. Apparently the debate rages on about which controllers are
>>> cancelled, which are not, whatever. Please accept this patch. It is
>>> dependent upon the patches I sent yesterday.
>>> This patch made/tested against kernel-3.13.0-rc8
>>>
>>> Signed-off-by: Mike Miller
>>> ---
>>> drivers/scsi/hpsa.c | 12
>>> 1 files changed, 12 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index
>>> 348b207..3affec5 100644
>>> --- a/drivers/scsi/hpsa.c
>>> +++ b/drivers/scsi/hpsa.c
>>> @@ -100,6 +100,7 @@ static const struct pci_device_id
>> hpsa_pci_device_id[] = {
>>> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSF, 0x103C,
>> 0x3354},
>>> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSF, 0x103C,
>> 0x3355},
>>> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSF, 0x103C,
>> 0x3356},
>>> + {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C,
>> 0x1920},
>>> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C,
>> 0x1921},
>>> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C,
>> 0x1922},
>>> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C,
>> 0x1923},
>>> @@ -108,15 +109,19 @@ static const struct pci_device_id
>> hpsa_pci_device_id[] = {
>>> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C,
>> 0x1926},
>>> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C,
>> 0x1928},
>>> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C,
>> 0x1929},
>>> + {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C,
>> 0x192A},
>>> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
>> 0x21BD},
>>> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
>> 0x21BE},
>>> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
>> 0x21BF},
>>> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
>> 0x21C0},
>>> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
>> 0x21C2},
>>> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
>> 0x21C3},
>>> + {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
>> 0x21C4},
>>> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
>> 0x21C5},
>>> + {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
>> 0x21C6},
>>> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
>> 0x21C7},
>>> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
>> 0x21C8},
>>> + {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
>> 0x21C9},
>>> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
>> 0x21CA},
>>> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
>> 0x21CB},
>>> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
>> 0x21CC},
>>> @@ -149,22 +154,29 @@ static struct board_type products[] = {
>>> {0x3354103C, "Smart Array P420i", &SA5_access},
>>> {0x3355103C, "Smart Array P220i", &SA5_access},
>>> {0x3356103C, "Smart Array P721m", &SA5_access},
>>> + {0x1920103C, "Smart Array P430i", &SA5_access},
>>> {0x1921103C, "Smart Array P830i", &SA5_access},
>>> {0x1922103C, "Smart Array P430", &SA5_access},
>>> {0x1923103C, "Smart Array P431", &SA5_access},
>>> {0x1924103C, "Smart Array P830", &SA5_access},
>>> + {0x1925103C, "Smart Array P831", &SA5_access},
>>> {0x1926103C, "Smart Array P731m", &SA5_access},
>>> {0x1928103C, "Smart Array P230i", &SA5_access},
>>> {0x1929103C, "Smart Array P530", &SA5_access},
>>> + {0x192A103C, "Smart Array P531", &SA5_access},
>>> {0x21BD103C, "Smart Array", &SA5_access},
>>> {0x21BE103C, "Smart Array", &SA5_access},
>>> {0x21BF103C, "Smart Array", &SA5_access},
>>> {0x21C0103C, "Smart Array", &SA5_access},
>>> + {0x21C1103C, "Smart Array", &SA5_access},
>>
>> I think that both tables should be in sync - but there is no 0x21c1 in
>> hpsa_pci_device_id (and the 0x1925), could you clarify that?
>>
>> Thanks, Tomas
>
> Crap. It seems like these menial tasks get the better of me. I'll correct and
> resubmit.
>
Hmm. Seeing that hpsa is now well capable of supporting even older
SmartArray controllers, is there really a point in updating the
PCI-ID list every time?
Wouldn't you be better off by just using the catch-all phrase
and remove all the individual IDs?
That way you would just have to update _one_ list ...
Cheers,
Hannes
--
Dr. Hanne
RE: [PATCH 1/1] scsi: hpsa, add all PCI ID's that HP has in svn
> -Original Message-
> From: Hannes Reinecke [mailto:h...@suse.de]
> Sent: Friday, January 17, 2014 9:31 AM
> To: Miller, Mike (OS Dev)
> Cc: Tomas Henzl; Andrew Morton; James E. J. Bottomley; LKML; LKML-scsi;
> Stephen M. Cameron
> Subject: Re: [PATCH 1/1] scsi: hpsa, add all PCI ID's that HP has in svn
>
> On 01/17/2014 04:17 PM, Miller, Mike (OS Dev) wrote:
> >
> >
> >> -Original Message-
> >> From: Tomas Henzl [mailto:the...@redhat.com]
> >> Sent: Friday, January 17, 2014 9:14 AM
> >> To: Miller, Mike (OS Dev); Andrew Morton; James E. J. Bottomley
> >> Cc: LKML; LKML-scsi; h...@suse.de; Stephen M. Cameron
> >> Subject: Re: [PATCH 1/1] scsi: hpsa, add all PCI ID's that HP has in
> >> svn
> >>
> >> On 01/16/2014 08:51 PM, Mike Miller wrote:
> >>> From: Mike Miller
> >>>
> >>> This patch has every ID we have in our svn repository. Some
> >>> controllers were cancelled, others added, now the cancelled ones are
> >>> back. Apparently the debate rages on about which controllers are
> >>> cancelled, which are not, whatever. Please accept this patch. It is
> >>> dependent upon the patches I sent yesterday.
> >>> This patch made/tested against kernel-3.13.0-rc8
> >>>
> >>> Signed-off-by: Mike Miller
> >>> ---
> >>> drivers/scsi/hpsa.c | 12
> >>> 1 files changed, 12 insertions(+), 0 deletions(-)
> >>>
> >>> diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index
> >>> 348b207..3affec5 100644
> >>> --- a/drivers/scsi/hpsa.c
> >>> +++ b/drivers/scsi/hpsa.c
> >>> @@ -100,6 +100,7 @@ static const struct pci_device_id
> >> hpsa_pci_device_id[] = {
> >>> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSF, 0x103C,
> >> 0x3354},
> >>> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSF, 0x103C,
> >> 0x3355},
> >>> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSF, 0x103C,
> >> 0x3356},
> >>> + {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C,
> >> 0x1920},
> >>> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C,
> >> 0x1921},
> >>> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C,
> >> 0x1922},
> >>> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C,
> >> 0x1923},
> >>> @@ -108,15 +109,19 @@ static const struct pci_device_id
> >> hpsa_pci_device_id[] = {
> >>> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C,
> >> 0x1926},
> >>> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C,
> >> 0x1928},
> >>> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C,
> >> 0x1929},
> >>> + {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C,
> >> 0x192A},
> >>> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
> >> 0x21BD},
> >>> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
> >> 0x21BE},
> >>> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
> >> 0x21BF},
> >>> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
> >> 0x21C0},
> >>> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
> >> 0x21C2},
> >>> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
> >> 0x21C3},
> >>> + {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
> >> 0x21C4},
> >>> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
> >> 0x21C5},
> >>> + {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
> >> 0x21C6},
> >>> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
> >> 0x21C7},
> >>> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
> >> 0x21C8},
> >>> + {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
> >> 0x21C9},
> >>> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
> >> 0x21CA},
> >>> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
> >> 0x21CB},
> >>> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
> >> 0x21CC},
> >>> @@ -149,22 +154,29 @@ static struct board_type products[] = {
> >>> {0x3354103C, "Smart Array P420i", &SA5_access},
> >>> {0x3355103C, "Smart Array P220i", &SA5_access},
> >>> {0x3356103C, "Smart Array P721m", &SA5_access},
> >>> + {0x1920103C, "Smart Array P430i", &SA5_access},
> >>> {0x1921103C, "Smart Array P830i", &SA5_access},
> >>> {0x1922103C, "Smart Array P430", &SA5_access},
> >>> {0x1923103C, "Smart Array P431", &SA5_access},
> >>> {0x1924103C, "Smart Array P830", &SA5_access},
> >>> + {0x1925103C, "Smart Array P831", &SA5_access},
> >>> {0x1926103C, "Smart Array P731m", &SA5_access},
> >>> {0x1928103C, "Smart Array P230i", &SA5_access},
> >>> {0x1929103C, "Smart Array P530", &SA5_access},
> >>> + {0x192A103C, "Smart Array P531", &SA5_access},
> >>> {0x21BD103C, "Smart Array", &SA5_access},
> >>> {0x21BE103C, "Smart Array", &SA5_access},
> >>> {0x21BF103C, "Smart Array", &SA5_access},
> >>> {0x21C0103C, "Smart Array", &SA5_access},
> >>> + {0x21C1103C, "Smart Array", &SA5_access},
> >>
> >> I think that both tables should be in sync - but there is no 0x21c1
> >> in hpsa_pci_device_id (and the 0x1925), could you clarify that?
> >>
> >
[PATCH v2 4/9] ipr: Use pci_enable_msi_range() and pci_enable_msix_range()
As result deprecation of MSI-X/MSI enablement functions
pci_enable_msix() and pci_enable_msi_block() all drivers
using these two interfaces need to be updated to use the
new pci_enable_msi_range() and pci_enable_msix_range()
interfaces.
Signed-off-by: Alexander Gordeev
---
drivers/scsi/ipr.c | 47 ++-
1 files changed, 18 insertions(+), 29 deletions(-)
diff --git a/drivers/scsi/ipr.c b/drivers/scsi/ipr.c
index fb57e21..3841298 100644
--- a/drivers/scsi/ipr.c
+++ b/drivers/scsi/ipr.c
@@ -9245,47 +9245,36 @@ ipr_get_chip_info(const struct pci_device_id *dev_id)
static int ipr_enable_msix(struct ipr_ioa_cfg *ioa_cfg)
{
struct msix_entry entries[IPR_MAX_MSIX_VECTORS];
- int i, err, vectors;
+ int i, vectors;
for (i = 0; i < ARRAY_SIZE(entries); ++i)
entries[i].entry = i;
- vectors = ipr_number_of_msix;
+ vectors = pci_enable_msix_range(ioa_cfg->pdev, entries,
+ 1, ipr_number_of_msix);
+ if (vectors < 0)
+ return vectors;
- while ((err = pci_enable_msix(ioa_cfg->pdev, entries, vectors)) > 0)
- vectors = err;
+ for (i = 0; i < vectors; i++)
+ ioa_cfg->vectors_info[i].vec = entries[i].vector;
+ ioa_cfg->nvectors = vectors;
- if (err < 0)
- return err;
-
- if (!err) {
- for (i = 0; i < vectors; i++)
- ioa_cfg->vectors_info[i].vec = entries[i].vector;
- ioa_cfg->nvectors = vectors;
- }
-
- return err;
+ return 0;
}
static int ipr_enable_msi(struct ipr_ioa_cfg *ioa_cfg)
{
- int i, err, vectors;
+ int i, vectors;
- vectors = ipr_number_of_msix;
+ vectors = pci_enable_msi_range(ioa_cfg->pdev, 1, ipr_number_of_msix);
+ if (vectors < 0)
+ return vectors;
- while ((err = pci_enable_msi_block(ioa_cfg->pdev, vectors)) > 0)
- vectors = err;
+ for (i = 0; i < vectors; i++)
+ ioa_cfg->vectors_info[i].vec = ioa_cfg->pdev->irq + i;
+ ioa_cfg->nvectors = vectors;
- if (err < 0)
- return err;
-
- if (!err) {
- for (i = 0; i < vectors; i++)
- ioa_cfg->vectors_info[i].vec = ioa_cfg->pdev->irq + i;
- ioa_cfg->nvectors = vectors;
- }
-
- return err;
+ return 0;
}
static void name_msi_vectors(struct ipr_ioa_cfg *ioa_cfg)
@@ -9350,7 +9339,7 @@ static irqreturn_t ipr_test_intr(int irq, void *devp)
* ipr_test_msi - Test for Message Signaled Interrupt (MSI) support.
* @pdev: PCI device struct
*
- * Description: The return value from pci_enable_msi() can not always be
+ * Description: The return value from pci_enable_msi_range() can not always be
* trusted. This routine sets up and initiates a test interrupt to determine
* if the interrupt is received via the ipr_test_intr() service routine.
* If the tests fails, the driver will fall back to LSI.
--
1.7.7.6
--
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
[PATCH v2 3/9] ipr: Get rid of superfluous call to pci_disable_msi/msix()
There is no need to call pci_disable_msi() or pci_disable_msix()
in case the call to pci_enable_msi() or pci_enable_msix() failed.
Signed-off-by: Alexander Gordeev
---
drivers/scsi/ipr.c |8 ++--
1 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/scsi/ipr.c b/drivers/scsi/ipr.c
index 36ac1c3..fb57e21 100644
--- a/drivers/scsi/ipr.c
+++ b/drivers/scsi/ipr.c
@@ -9255,10 +9255,8 @@ static int ipr_enable_msix(struct ipr_ioa_cfg *ioa_cfg)
while ((err = pci_enable_msix(ioa_cfg->pdev, entries, vectors)) > 0)
vectors = err;
- if (err < 0) {
- pci_disable_msix(ioa_cfg->pdev);
+ if (err < 0)
return err;
- }
if (!err) {
for (i = 0; i < vectors; i++)
@@ -9278,10 +9276,8 @@ static int ipr_enable_msi(struct ipr_ioa_cfg *ioa_cfg)
while ((err = pci_enable_msi_block(ioa_cfg->pdev, vectors)) > 0)
vectors = err;
- if (err < 0) {
- pci_disable_msi(ioa_cfg->pdev);
+ if (err < 0)
return err;
- }
if (!err) {
for (i = 0; i < vectors; i++)
--
1.7.7.6
--
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 1/1] scsi: hpsa, add all PCI ID's that HP has in svn
On 01/17/2014 04:31 PM, Hannes Reinecke wrote:
> On 01/17/2014 04:17 PM, Miller, Mike (OS Dev) wrote:
>>
>>> -Original Message-
>>> From: Tomas Henzl [mailto:the...@redhat.com]
>>> Sent: Friday, January 17, 2014 9:14 AM
>>> To: Miller, Mike (OS Dev); Andrew Morton; James E. J. Bottomley
>>> Cc: LKML; LKML-scsi; h...@suse.de; Stephen M. Cameron
>>> Subject: Re: [PATCH 1/1] scsi: hpsa, add all PCI ID's that HP has in svn
>>>
>>> On 01/16/2014 08:51 PM, Mike Miller wrote:
From: Mike Miller
This patch has every ID we have in our svn repository. Some
controllers were cancelled, others added, now the cancelled ones are
back. Apparently the debate rages on about which controllers are
cancelled, which are not, whatever. Please accept this patch. It is
dependent upon the patches I sent yesterday.
This patch made/tested against kernel-3.13.0-rc8
Signed-off-by: Mike Miller
---
drivers/scsi/hpsa.c | 12
1 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index
348b207..3affec5 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -100,6 +100,7 @@ static const struct pci_device_id
>>> hpsa_pci_device_id[] = {
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSF, 0x103C,
>>> 0x3354},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSF, 0x103C,
>>> 0x3355},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSF, 0x103C,
>>> 0x3356},
+ {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C,
>>> 0x1920},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C,
>>> 0x1921},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C,
>>> 0x1922},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C,
>>> 0x1923},
@@ -108,15 +109,19 @@ static const struct pci_device_id
>>> hpsa_pci_device_id[] = {
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C,
>>> 0x1926},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C,
>>> 0x1928},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C,
>>> 0x1929},
+ {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C,
>>> 0x192A},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
>>> 0x21BD},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
>>> 0x21BE},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
>>> 0x21BF},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
>>> 0x21C0},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
>>> 0x21C2},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
>>> 0x21C3},
+ {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
>>> 0x21C4},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
>>> 0x21C5},
+ {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
>>> 0x21C6},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
>>> 0x21C7},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
>>> 0x21C8},
+ {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
>>> 0x21C9},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
>>> 0x21CA},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
>>> 0x21CB},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
>>> 0x21CC},
@@ -149,22 +154,29 @@ static struct board_type products[] = {
{0x3354103C, "Smart Array P420i", &SA5_access},
{0x3355103C, "Smart Array P220i", &SA5_access},
{0x3356103C, "Smart Array P721m", &SA5_access},
+ {0x1920103C, "Smart Array P430i", &SA5_access},
{0x1921103C, "Smart Array P830i", &SA5_access},
{0x1922103C, "Smart Array P430", &SA5_access},
{0x1923103C, "Smart Array P431", &SA5_access},
{0x1924103C, "Smart Array P830", &SA5_access},
+ {0x1925103C, "Smart Array P831", &SA5_access},
{0x1926103C, "Smart Array P731m", &SA5_access},
{0x1928103C, "Smart Array P230i", &SA5_access},
{0x1929103C, "Smart Array P530", &SA5_access},
+ {0x192A103C, "Smart Array P531", &SA5_access},
{0x21BD103C, "Smart Array", &SA5_access},
{0x21BE103C, "Smart Array", &SA5_access},
{0x21BF103C, "Smart Array", &SA5_access},
{0x21C0103C, "Smart Array", &SA5_access},
+ {0x21C1103C, "Smart Array", &SA5_access},
>>> I think that both tables should be in sync - but there is no 0x21c1 in
>>> hpsa_pci_device_id (and the 0x1925), could you clarify that?
>>>
>>> Thanks, Tomas
>> Crap. It seems like these menial tasks get the better of me. I'll correct
>> and resubmit.
>>
> Hmm. Seeing that hpsa is now well capable of supporting even older
> SmartArray controllers, is there really a point in updating the
> PCI-ID list every time?
It depends on whether HP actually wishes to support even the oldest ccis
Re: Why is (2 < 2) true? Is it a gcc bug?
On 01/17/2014 08:55 AM, Dorau, Lukasz wrote:
On Friday, January 17, 2014 2:58 PM Richard Weinberger
wrote:
Can you reproduce this using a standalone test?
I.e:
#include
int main()
{
assert(2 < 2 != 1);
return 0;
}
No, I can't of course.
The test might need to be more complex in order to prevent the compiler from
optimizing away the issue. Or, you could try -O0.
Steven J. Magnani "I claim this network for MARS!
www.digidescorp.com Earthling, return my space modulator!"
#include
--
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 1/1] scsi: hpsa, add all PCI ID's that HP has in svn
> -Original Message-
> From: Tomas Henzl [mailto:the...@redhat.com]
> Sent: Friday, January 17, 2014 9:14 AM
> To: Miller, Mike (OS Dev); Andrew Morton; James E. J. Bottomley
> Cc: LKML; LKML-scsi; h...@suse.de; Stephen M. Cameron
> Subject: Re: [PATCH 1/1] scsi: hpsa, add all PCI ID's that HP has in svn
>
> On 01/16/2014 08:51 PM, Mike Miller wrote:
> > From: Mike Miller
> >
> > This patch has every ID we have in our svn repository. Some
> > controllers were cancelled, others added, now the cancelled ones are
> > back. Apparently the debate rages on about which controllers are
> > cancelled, which are not, whatever. Please accept this patch. It is
> > dependent upon the patches I sent yesterday.
> > This patch made/tested against kernel-3.13.0-rc8
> >
> > Signed-off-by: Mike Miller
> > ---
> > drivers/scsi/hpsa.c | 12
> > 1 files changed, 12 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index
> > 348b207..3affec5 100644
> > --- a/drivers/scsi/hpsa.c
> > +++ b/drivers/scsi/hpsa.c
> > @@ -100,6 +100,7 @@ static const struct pci_device_id
> hpsa_pci_device_id[] = {
> > {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSF, 0x103C,
> 0x3354},
> > {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSF, 0x103C,
> 0x3355},
> > {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSF, 0x103C,
> 0x3356},
> > + {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C,
> 0x1920},
> > {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C,
> 0x1921},
> > {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C,
> 0x1922},
> > {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C,
> 0x1923},
> > @@ -108,15 +109,19 @@ static const struct pci_device_id
> hpsa_pci_device_id[] = {
> > {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C,
> 0x1926},
> > {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C,
> 0x1928},
> > {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C,
> 0x1929},
> > + {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C,
> 0x192A},
> > {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
> 0x21BD},
> > {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
> 0x21BE},
> > {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
> 0x21BF},
> > {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
> 0x21C0},
> > {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
> 0x21C2},
> > {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
> 0x21C3},
> > + {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
> 0x21C4},
> > {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
> 0x21C5},
> > + {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
> 0x21C6},
> > {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
> 0x21C7},
> > {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
> 0x21C8},
> > + {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
> 0x21C9},
> > {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
> 0x21CA},
> > {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
> 0x21CB},
> > {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C,
> 0x21CC},
> > @@ -149,22 +154,29 @@ static struct board_type products[] = {
> > {0x3354103C, "Smart Array P420i", &SA5_access},
> > {0x3355103C, "Smart Array P220i", &SA5_access},
> > {0x3356103C, "Smart Array P721m", &SA5_access},
> > + {0x1920103C, "Smart Array P430i", &SA5_access},
> > {0x1921103C, "Smart Array P830i", &SA5_access},
> > {0x1922103C, "Smart Array P430", &SA5_access},
> > {0x1923103C, "Smart Array P431", &SA5_access},
> > {0x1924103C, "Smart Array P830", &SA5_access},
> > + {0x1925103C, "Smart Array P831", &SA5_access},
> > {0x1926103C, "Smart Array P731m", &SA5_access},
> > {0x1928103C, "Smart Array P230i", &SA5_access},
> > {0x1929103C, "Smart Array P530", &SA5_access},
> > + {0x192A103C, "Smart Array P531", &SA5_access},
> > {0x21BD103C, "Smart Array", &SA5_access},
> > {0x21BE103C, "Smart Array", &SA5_access},
> > {0x21BF103C, "Smart Array", &SA5_access},
> > {0x21C0103C, "Smart Array", &SA5_access},
> > + {0x21C1103C, "Smart Array", &SA5_access},
>
> I think that both tables should be in sync - but there is no 0x21c1 in
> hpsa_pci_device_id (and the 0x1925), could you clarify that?
>
> Thanks, Tomas
Crap. It seems like these menial tasks get the better of me. I'll correct and
resubmit.
-- mikem
>
> > {0x21C2103C, "Smart Array", &SA5_access},
> > {0x21C3103C, "Smart Array", &SA5_access},
> > + {0x21C4103C, "Smart Array", &SA5_access},
> > {0x21C5103C, "Smart Array", &SA5_access},
> > + {0x21C6103C, "Smart Array", &SA5_access},
> > {0x21C7103C, "Smart Array", &SA5_access},
> > {0x21C8103C, "Smart Array", &SA5_access},
> > + {0x21C9103C, "Smart Array", &SA5_access},
> > {0x21CA103C, "Smart
Re: [PATCH 1/1] scsi: hpsa, add all PCI ID's that HP has in svn
On 01/16/2014 08:51 PM, Mike Miller wrote:
> From: Mike Miller
>
> This patch has every ID we have in our svn repository. Some controllers were
> cancelled, others added, now the cancelled ones are back. Apparently the
> debate rages on about which controllers are cancelled, which are not,
> whatever. Please accept this patch. It is dependent upon the patches I sent
> yesterday.
> This patch made/tested against kernel-3.13.0-rc8
>
> Signed-off-by: Mike Miller
> ---
> drivers/scsi/hpsa.c | 12
> 1 files changed, 12 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
> index 348b207..3affec5 100644
> --- a/drivers/scsi/hpsa.c
> +++ b/drivers/scsi/hpsa.c
> @@ -100,6 +100,7 @@ static const struct pci_device_id hpsa_pci_device_id[] = {
> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSF, 0x103C, 0x3354},
> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSF, 0x103C, 0x3355},
> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSF, 0x103C, 0x3356},
> + {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C, 0x1920},
> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C, 0x1921},
> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C, 0x1922},
> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C, 0x1923},
> @@ -108,15 +109,19 @@ static const struct pci_device_id hpsa_pci_device_id[]
> = {
> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C, 0x1926},
> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C, 0x1928},
> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C, 0x1929},
> + {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C, 0x192A},
> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C, 0x21BD},
> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C, 0x21BE},
> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C, 0x21BF},
> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C, 0x21C0},
> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C, 0x21C2},
> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C, 0x21C3},
> + {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C, 0x21C4},
> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C, 0x21C5},
> + {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C, 0x21C6},
> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C, 0x21C7},
> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C, 0x21C8},
> + {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C, 0x21C9},
> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C, 0x21CA},
> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C, 0x21CB},
> {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSI, 0x103C, 0x21CC},
> @@ -149,22 +154,29 @@ static struct board_type products[] = {
> {0x3354103C, "Smart Array P420i", &SA5_access},
> {0x3355103C, "Smart Array P220i", &SA5_access},
> {0x3356103C, "Smart Array P721m", &SA5_access},
> + {0x1920103C, "Smart Array P430i", &SA5_access},
> {0x1921103C, "Smart Array P830i", &SA5_access},
> {0x1922103C, "Smart Array P430", &SA5_access},
> {0x1923103C, "Smart Array P431", &SA5_access},
> {0x1924103C, "Smart Array P830", &SA5_access},
> + {0x1925103C, "Smart Array P831", &SA5_access},
> {0x1926103C, "Smart Array P731m", &SA5_access},
> {0x1928103C, "Smart Array P230i", &SA5_access},
> {0x1929103C, "Smart Array P530", &SA5_access},
> + {0x192A103C, "Smart Array P531", &SA5_access},
> {0x21BD103C, "Smart Array", &SA5_access},
> {0x21BE103C, "Smart Array", &SA5_access},
> {0x21BF103C, "Smart Array", &SA5_access},
> {0x21C0103C, "Smart Array", &SA5_access},
> + {0x21C1103C, "Smart Array", &SA5_access},
I think that both tables should be in sync - but there is no 0x21c1 in
hpsa_pci_device_id
(and the 0x1925), could you clarify that?
Thanks, Tomas
> {0x21C2103C, "Smart Array", &SA5_access},
> {0x21C3103C, "Smart Array", &SA5_access},
> + {0x21C4103C, "Smart Array", &SA5_access},
> {0x21C5103C, "Smart Array", &SA5_access},
> + {0x21C6103C, "Smart Array", &SA5_access},
> {0x21C7103C, "Smart Array", &SA5_access},
> {0x21C8103C, "Smart Array", &SA5_access},
> + {0x21C9103C, "Smart Array", &SA5_access},
> {0x21CA103C, "Smart Array", &SA5_access},
> {0x21CB103C, "Smart Array", &SA5_access},
> {0x21CC103C, "Smart Array", &SA5_access},
> --
> 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
--
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_error: disable eh_deadline if no host_reset_handler is set
On 01/07/2014 02:33 PM, Ewan Milne wrote:
> On Thu, 2013-12-12 at 12:08 +0100, Hannes Reinecke wrote:
>> When the host template doesn't declare an eh_host_reset_handler
>> the eh_deadline mechanism is pointless and will set the
>> device to offline. So disable eh_deadline if no
>> eh_host_reset_handler is present.
>>
>> Cc: Ewan Milne
>> Signed-off-by: Hannes Reinecke
>> ---
>> drivers/scsi/hosts.c | 2 +-
>> drivers/scsi/scsi_sysfs.c | 4 +++-
>> 2 files changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
>> index 6966def..351afa4 100644
>> --- a/drivers/scsi/hosts.c
>> +++ b/drivers/scsi/hosts.c
>> @@ -396,7 +396,7 @@ struct Scsi_Host *scsi_host_alloc(struct
>> scsi_host_template *sht, int privsize)
>> shost->unchecked_isa_dma = sht->unchecked_isa_dma;
>> shost->use_clustering = sht->use_clustering;
>> shost->ordered_tag = sht->ordered_tag;
>> -if (shost_eh_deadline == -1)
>> +if (shost_eh_deadline == -1 || !sht->eh_host_reset_handler)
>> shost->eh_deadline = -1;
>> else if ((ulong) shost_eh_deadline * HZ > INT_MAX) {
>> shost_printk(KERN_WARNING, shost,
>> diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
>> index 9117d0b..7434e26 100644
>> --- a/drivers/scsi/scsi_sysfs.c
>> +++ b/drivers/scsi/scsi_sysfs.c
>> @@ -300,7 +300,9 @@ store_shost_eh_deadline(struct device *dev, struct
>> device_attribute *attr,
>> int ret = -EINVAL;
>> unsigned long deadline, flags;
>>
>> -if (shost->transportt && shost->transportt->eh_strategy_handler)
>> +if (shost->transportt &&
>> +(shost->transportt->eh_strategy_handler ||
>> + !shost->transportt->eh_host_reset_handler))
>
> I think this bit is supposed to be:
>
> if ((shost->transportt &&
> shost->transportt->eh_strategy_handler) ||
> !shost->hostt->eh_host_reset_handler)
> return ret;
>
> because the field is in the scsi_host_template structure, right?
>
Right. Obviously.
I'll be sending an updated patch.
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
h...@suse.de +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
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
[PATCH] scsi_error: disable eh_deadline if no host_reset_handler is set
When the host template doesn't declare an eh_host_reset_handler
the eh_deadline mechanism is pointless and will set the
device to offline. So disable eh_deadline if no
eh_host_reset_handler is present.
Cc: Ewan Milne
Signed-off-by: Hannes Reinecke
---
drivers/scsi/hosts.c | 2 +-
drivers/scsi/scsi_sysfs.c | 4 +++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index f28ea07..3cbb57a 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -398,7 +398,7 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template
*sht, int privsize)
shost->ordered_tag = sht->ordered_tag;
shost->no_write_same = sht->no_write_same;
- if (shost_eh_deadline == -1)
+ if (shost_eh_deadline == -1 || !sht->eh_host_reset_handler)
shost->eh_deadline = -1;
else if ((ulong) shost_eh_deadline * HZ > INT_MAX) {
shost_printk(KERN_WARNING, shost,
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 9117d0b..f2151cd 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -300,7 +300,9 @@ store_shost_eh_deadline(struct device *dev, struct
device_attribute *attr,
int ret = -EINVAL;
unsigned long deadline, flags;
- if (shost->transportt && shost->transportt->eh_strategy_handler)
+ if (shost->transportt &&
+ (shost->transportt->eh_strategy_handler ||
+!shost->hostt->eh_host_reset_handler))
return ret;
if (!strncmp(buf, "off", strlen("off")))
--
1.7.12.4
--
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: Disk spin-up optimization during system resume
On Thu, 16 Jan 2014, Todd E Brandt wrote:
> On Thu, Jan 16, 2014 at 03:05:40PM -0500, Alan Stern wrote:
> > On Thu, 16 Jan 2014, Todd E Brandt wrote:
> >
> > > > Does this plan sound reasonable to everyone? Are there important
> > > > aspects I haven't considered (such as interactions between the SCSI and
> > > > ATA layers)?
> > > >
> > > > Alan Stern
> > > >
> > >
> > > Both approaches employ non-blocking resume of the scsi disks so why don't
> > > we treat these two patch sets as parts one and two. My patch just spins
> > > everything up but sets everything to non-blocking, so it will take care
> > > of the most common use cases. Your patch will then fine-tune that behavior
> > > to only spin up those disks that are actually needed. I don't think there
> > > are any serious conflicts between the two patch sets.
> >
> > Hmmm. In your 2/2 patch, sd_resume_complete() gets called in atomic
> > context. I would need a process context in order to carry out a
> > runtime resume. Any suggestions?
>
> The complete call is really just there to printk any errors and free
> the sense buffer. Why would you want to carry out a runtime resume in
> it?
>From my earlier message:
Otherwise, check to see whether the disk is spinning up all
by itself. This presumably will involve issuing a TEST UNIT
READY command and checking the sense data. Maybe something
different will be needed for ATA disks; I don't know.
If the disk is spinning, or if you can't tell, call
pm_runtime_resume() to put the disk back into the
RPM_ACTIVE state (and spin it up in the case where you
couldn't tell what it was doing).
As you can see, the runtime resume is carried out in the completion
routine for the TEST UNIT READY command. Since the command is likely
to take a long time (Phillip said it can take up to 10 seconds for ATA
disks), we want it to be asynchronous, like the START-STOP command.
> > Also, I don't understand the point of your 1/2 patch. If the whole
> > point of that patch was to carry out the ATA port resume asynchronously
> > ("thus allowing the next device in the pm queue to resume"), doesn't
> > device_enable_async_suspend() already do that for you?
> >
> > Alan Stern
> >
>
> The device_enable_async_suspend call is used to set a pm flag which lets
> the power manager know that this device can be added to the async suspend
> queue. Basically that means that it can be suspended/resumed in parallel
> with all the other async devices, but resume still waits for all those
> devices to finish before completing system resume.
Correct.
> My patch makes ata port and scsi disk resume 'non-blocking' (asynchronous
> with respect to system resume). Which means once they're called the power
> manager pays no more attention to them and will complete system resume
> whenever. Both the power manager and the ata subsystems call these
> features 'asynchronous' so it can be confusing.
I see. That's not explained very well in the patch description -- it
talks about going on to resume the next device on the list, but not
about waiting for the overall system resume to finish.
Alan Stern
--
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: Why is (2 < 2) true? Is it a gcc bug?
On Friday, January 17, 2014 2:58 PM Richard Weinberger
wrote:
>
> Can you reproduce this using a standalone test?
> I.e:
> #include
>
> int main()
> {
> assert(2 < 2 != 1);
>
> return 0;
> }
>
No, I can't of course.
Lukasz
--
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 3.12 033/118] usb: xhci: Link TRB must not occur within a USB payload burst [NEW HARDWARE]
From: walt > Oy, Sarah! ;) I put the ASMedia adapter in my older amd64 machine, and, well, > the stupid thing Just Works(TM) with kernel 3.12.7! (Yes, with the same disk > docking station, too.) > > I can't believe the adapter works perfectly in a different computer. Have you > seen this kind of thing before? Could be a horrid timing race between the cpu and xchi controller. If the cpu manages to write a NOP or LINK TRB for a following transfer before the controller polls the next entry (after raising the IRQ) then the controller might process the LINK and then get confused when it can't process the linked-to TRB. This might not sound likely, but PCIe has significant latency. > At the moment I have two machines using your xhci driver and both work > perfectly, > so I thank you again :) > > I'm not sure where to go with this next. I could put the adapter back in the > other machine again if you have more patches to test. Can you try the patch I posted that stops the ownership on LINK TRBs being changed before that on the linked-to TRB? I got a private mail from someone indicating that my earlier 'minimal' patch helped an ASMedia controller talking to the asx189_178a ethernet hardware. David
Re: Why is (2 < 2) true? Is it a gcc bug?
On Fri, Jan 17, 2014 at 2:37 PM, Dorau, Lukasz wrote:
> Hi
>
> My story is very simply...
> I applied the following patch:
>
> diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c
> --- a/drivers/scsi/isci/init.c
> +++ b/drivers/scsi/isci/init.c
> @@ -698,8 +698,11 @@ static int isci_pci_probe(struct pci_dev *pdev, const
> struct pci_device_id *id)
> if (err)
> goto err_host_alloc;
>
> - for_each_isci_host(i, isci_host, pdev)
> + for_each_isci_host(i, isci_host, pdev) {
> + pr_err("(%d < %d) == %d\n",\
> + i, SCI_MAX_CONTROLLERS, (i < SCI_MAX_CONTROLLERS));
> scsi_scan_host(to_shost(isci_host));
> + }
>
> return 0;
>
> --
> 1.8.3.1
>
> Then I issued the command 'modprobe isci' on platform with two SCU
> controllers (Patsburg D or T chipset)
> and received the following, very strange, output:
>
> (0 < 2) == 1
> (1 < 2) == 1
> (2 < 2) == 1
>
> Can anyone explain why (2 < 2) is true? Is it a gcc bug?
>
> (The kernel was compiled using gcc version 4.8.2.)
>
Can you reproduce this using a standalone test?
I.e:
#include
int main()
{
assert(2 < 2 != 1);
return 0;
}
--
Thanks,
//richard
--
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: Why is (2 < 2) true? Is it a gcc bug?
On Friday, January 17, 2014 2:37 PM Dorau, Lukasz
wrote:
>
> Hi
>
> My story is very simply...
> I applied the following patch:
>
> diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c
> --- a/drivers/scsi/isci/init.c
> +++ b/drivers/scsi/isci/init.c
> @@ -698,8 +698,11 @@ static int isci_pci_probe(struct pci_dev *pdev, const
> struct
> pci_device_id *id)
> if (err)
> goto err_host_alloc;
>
> - for_each_isci_host(i, isci_host, pdev)
> + for_each_isci_host(i, isci_host, pdev) {
> + pr_err("(%d < %d) == %d\n",\
> +i, SCI_MAX_CONTROLLERS, (i < SCI_MAX_CONTROLLERS));
> scsi_scan_host(to_shost(isci_host));
> + }
>
> return 0;
>
> --
> 1.8.3.1
>
> Then I issued the command 'modprobe isci' on platform with two SCU controllers
> (Patsburg D or T chipset)
> and received the following, very strange, output:
>
> (0 < 2) == 1
> (1 < 2) == 1
> (2 < 2) == 1
>
> Can anyone explain why (2 < 2) is true? Is it a gcc bug?
>
> (The kernel was compiled using gcc version 4.8.2.)
>
Some additional information:
The loop 'for' in macro ' for_each_isci_host ' defined as
(drivers/scsi/isci/host.h:313):
#define for_each_isci_host(id, ihost, pdev) \
for (id = 0, ihost = to_pci_info(pdev)->hosts[id]; \
id < ARRAY_SIZE(to_pci_info(pdev)->hosts) && ihost; \
ihost = to_pci_info(pdev)->hosts[++id])
should be executed only for i = 0 and 1, because:
ARRAY_SIZE(to_pci_info(pdev)->hosts) = SCI_MAX_CONTROLLERS = 2
but it was executed also for i=2 regardless the above loop's end condition.
Lukasz
--
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
Why is (2 < 2) true? Is it a gcc bug?
Hi
My story is very simply...
I applied the following patch:
diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c
--- a/drivers/scsi/isci/init.c
+++ b/drivers/scsi/isci/init.c
@@ -698,8 +698,11 @@ static int isci_pci_probe(struct pci_dev *pdev, const
struct pci_device_id *id)
if (err)
goto err_host_alloc;
- for_each_isci_host(i, isci_host, pdev)
+ for_each_isci_host(i, isci_host, pdev) {
+ pr_err("(%d < %d) == %d\n",\
+ i, SCI_MAX_CONTROLLERS, (i < SCI_MAX_CONTROLLERS));
scsi_scan_host(to_shost(isci_host));
+ }
return 0;
--
1.8.3.1
Then I issued the command 'modprobe isci' on platform with two SCU controllers
(Patsburg D or T chipset)
and received the following, very strange, output:
(0 < 2) == 1
(1 < 2) == 1
(2 < 2) == 1
Can anyone explain why (2 < 2) is true? Is it a gcc bug?
(The kernel was compiled using gcc version 4.8.2.)
Lukasz
--
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 16/24] qla4xxx: Fix failure of mbox 0x31
On 17/01/14 2:00 PM, "Mike Christie" wrote:
>
>> +static void qla4xxx_destroy_ddb(struct scsi_qla_host *ha,
>> +struct ddb_entry *ddb_entry)
>> +{
>> +struct dev_db_entry *fw_ddb_entry = NULL;
>> +dma_addr_t fw_ddb_entry_dma;
>> +unsigned long wtime;
>> +uint32_t ddb_state;
>> +int options;
>> +int status;
>> +
>> +options = LOGOUT_OPTION_CLOSE_SESSION;
>> +if (qla4xxx_session_logout_ddb(ha, ddb_entry, options) == QLA_ERROR) {
>> +ql4_printk(KERN_ERR, ha, "%s: Logout failed\n", __func__);
>> +goto clear_ddb;
>> +}
>> +
>> +fw_ddb_entry = dma_alloc_coherent(&ha->pdev->dev,
>>sizeof(*fw_ddb_entry),
>> + &fw_ddb_entry_dma, GFP_KERNEL);
>> +if (!fw_ddb_entry) {
>> +ql4_printk(KERN_ERR, ha,
>> + "%s: Unable to allocate dma buffer\n", __func__);
>> +goto clear_ddb;
>> +}
>> +
>> +wtime = jiffies + (HZ * LOGOUT_TOV);
>> +do {
>> +status = qla4xxx_get_fwddb_entry(ha, ddb_entry->fw_ddb_index,
>> + fw_ddb_entry, fw_ddb_entry_dma,
>> + NULL, NULL, &ddb_state, NULL,
>> + NULL, NULL);
>> +if (status == QLA_ERROR)
>> +goto clear_ddb;
>> +
>> +if ((ddb_state == DDB_DS_NO_CONNECTION_ACTIVE) ||
>> +(ddb_state == DDB_DS_SESSION_FAILED))
>> +goto clear_ddb;
>> +
>> +schedule_timeout_uninterruptible(HZ);
>> +} while ((time_after(wtime, jiffies)));
>
>
>I think there is a missing dma_free_coherent
>
>> +
>> +clear_ddb:
>> +qla4xxx_clear_ddb_entry(ha, ddb_entry->fw_ddb_index);
>> +return;
>> +}
>> +
>
>Don't need return statement there.
Thanks a lot for review.
Updated patch is posted here:-
http://marc.info/?l=linux-scsi&m=138995390232212&w=2
Thanks,
Vikas.
<>
Re: Disk spin-up optimization during system resume
Hello, On Fri, Jan 17, 2014 at 11:16:49AM +0100, Oliver Neukum wrote: > The START-STOP may result in an error. What do you do in that case? At least for libata, worrying about suspend/resume failures don't make whole lot of sense. If suspend failed, just proceed with suspend. If the device can't be woken up afterwards, that's that. There isn't anything we could have done differently anyway. The same for resume, if spinup fails, the device is dud and the following commands will invoke EH actions and will eventually fail. Again, there really isn't any *choice* to make. Just making sure the errors are handled gracefully (ie. don't crash) and the following commands are handled correctly should be enough. 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
[PATCH V1 16/24] qla4xxx: Fix failure of mbox 0x31
From: Vikas Chaudhary
Issue:
While unloading driver MBOX 0x31 fail as DDB logout (MBOX 0x56)
operation is not completed.
Fix:
Wait for DDB Logout completion before MBOX 0x31
Signed-off-by: Vikas Chaudhary
---
drivers/scsi/qla4xxx/ql4_os.c | 56 ---
1 file changed, 48 insertions(+), 8 deletions(-)
diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
index a27da31..7807a13 100644
--- a/drivers/scsi/qla4xxx/ql4_os.c
+++ b/drivers/scsi/qla4xxx/ql4_os.c
@@ -8886,10 +8886,56 @@ static void qla4xxx_prevent_other_port_reinit(struct
scsi_qla_host *ha)
}
}
+static void qla4xxx_destroy_ddb(struct scsi_qla_host *ha,
+ struct ddb_entry *ddb_entry)
+{
+ struct dev_db_entry *fw_ddb_entry = NULL;
+ dma_addr_t fw_ddb_entry_dma;
+ unsigned long wtime;
+ uint32_t ddb_state;
+ int options;
+ int status;
+
+ options = LOGOUT_OPTION_CLOSE_SESSION;
+ if (qla4xxx_session_logout_ddb(ha, ddb_entry, options) == QLA_ERROR) {
+ ql4_printk(KERN_ERR, ha, "%s: Logout failed\n", __func__);
+ goto clear_ddb;
+ }
+
+ fw_ddb_entry = dma_alloc_coherent(&ha->pdev->dev, sizeof(*fw_ddb_entry),
+ &fw_ddb_entry_dma, GFP_KERNEL);
+ if (!fw_ddb_entry) {
+ ql4_printk(KERN_ERR, ha,
+ "%s: Unable to allocate dma buffer\n", __func__);
+ goto clear_ddb;
+ }
+
+ wtime = jiffies + (HZ * LOGOUT_TOV);
+ do {
+ status = qla4xxx_get_fwddb_entry(ha, ddb_entry->fw_ddb_index,
+fw_ddb_entry, fw_ddb_entry_dma,
+NULL, NULL, &ddb_state, NULL,
+NULL, NULL);
+ if (status == QLA_ERROR)
+ goto free_ddb;
+
+ if ((ddb_state == DDB_DS_NO_CONNECTION_ACTIVE) ||
+ (ddb_state == DDB_DS_SESSION_FAILED))
+ goto free_ddb;
+
+ schedule_timeout_uninterruptible(HZ);
+ } while ((time_after(wtime, jiffies)));
+
+free_ddb:
+ dma_free_coherent(&ha->pdev->dev, sizeof(*fw_ddb_entry),
+ fw_ddb_entry, fw_ddb_entry_dma);
+clear_ddb:
+ qla4xxx_clear_ddb_entry(ha, ddb_entry->fw_ddb_index);
+}
+
static void qla4xxx_destroy_fw_ddb_session(struct scsi_qla_host *ha)
{
struct ddb_entry *ddb_entry;
- int options;
int idx;
for (idx = 0; idx < MAX_DDB_ENTRIES; idx++) {
@@ -8898,13 +8944,7 @@ static void qla4xxx_destroy_fw_ddb_session(struct
scsi_qla_host *ha)
if ((ddb_entry != NULL) &&
(ddb_entry->ddb_type == FLASH_DDB)) {
- options = LOGOUT_OPTION_CLOSE_SESSION;
- if (qla4xxx_session_logout_ddb(ha, ddb_entry, options)
- == QLA_ERROR)
- ql4_printk(KERN_ERR, ha, "%s: Logout failed\n",
- __func__);
-
- qla4xxx_clear_ddb_entry(ha, ddb_entry->fw_ddb_index);
+ qla4xxx_destroy_ddb(ha, ddb_entry);
/*
* we have decremented the reference count of the driver
* when we setup the session to have the driver unload
--
1.8.2.GIT
--
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: Disk spin-up optimization during system resume
On Thu, 2014-01-16 at 11:59 -0500, Alan Stern wrote: > Since the START-STOP and TEST UNIT READY (or REQUEST SENSE or > whatever) > commands are likely to take a long time, they should all be carried > out > asynchronously with respect to the resume procedure. I don't know > what > the best way is to implement this. But it is important to guarantee > that in the RPM_ACTIVE case, the START-STOP command gets sent to the > disk before any other commands. (This isn't an issue in the > RPM_SUSPENDED case, as the block layer will prevent requests being > sent out unless they have the REQ_PM flag set.) > > Does this plan sound reasonable to everyone? Are there important > aspects I haven't considered (such as interactions between the SCSI > and > ATA layers)? The START-STOP may result in an error. What do you do in that case? 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: [PATCH 06/16] scsi_dh_alua: use local buffer for VPD inquiry
On 01/17/2014 10:04 AM, Mike Christie wrote:
> On 12/20/2013 06:13 AM, Hannes Reinecke wrote:
>> VPD inquiry need to be done only once, so we can be using
>> a local buffer here.
>>
>> Signed-off-by: Hannes Reinecke
>> ---
>> drivers/scsi/device_handler/scsi_dh_alua.c | 45
>> ++
>> 1 file changed, 27 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c
>> b/drivers/scsi/device_handler/scsi_dh_alua.c
>> index adc77ef..49952f4 100644
>> --- a/drivers/scsi/device_handler/scsi_dh_alua.c
>> +++ b/drivers/scsi/device_handler/scsi_dh_alua.c
>> @@ -322,16 +322,27 @@ static int alua_check_tpgs(struct scsi_device *sdev,
>> struct alua_dh_data *h)
>> */
>> static int alua_vpd_inquiry(struct scsi_device *sdev, struct alua_dh_data
>> *h)
>> {
>> +unsigned char *buff;
>> +unsigned char bufflen = 36;
>> int len, timeout = ALUA_FAILOVER_TIMEOUT;
>> unsigned char sense[SCSI_SENSE_BUFFERSIZE];
>> struct scsi_sense_hdr sense_hdr;
>> unsigned retval;
>> unsigned char *d;
>> unsigned long expiry;
>> +int err;
>>
>> expiry = round_jiffies_up(jiffies + timeout);
>> retry:
>> -retval = submit_vpd_inquiry(sdev, h->buff, h->bufflen, sense);
>> +buff = kmalloc(bufflen, GFP_ATOMIC);
>> +if (!buff) {
>>
>
> Why GFP_ATOMIC? I think it can be less restrictive in this path. If you
> need GFP_ATOMIC here, then there are places in this code path you would
> want to change from GFP_KERNEL to GFP_ATOMIC.
>
Yeah, all right. This is by no means time-critical.
I'll be fixing up the patch.
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
h...@suse.de +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
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 06/16] scsi_dh_alua: use local buffer for VPD inquiry
On 12/20/2013 06:13 AM, Hannes Reinecke wrote:
> VPD inquiry need to be done only once, so we can be using
> a local buffer here.
>
> Signed-off-by: Hannes Reinecke
> ---
> drivers/scsi/device_handler/scsi_dh_alua.c | 45
> ++
> 1 file changed, 27 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c
> b/drivers/scsi/device_handler/scsi_dh_alua.c
> index adc77ef..49952f4 100644
> --- a/drivers/scsi/device_handler/scsi_dh_alua.c
> +++ b/drivers/scsi/device_handler/scsi_dh_alua.c
> @@ -322,16 +322,27 @@ static int alua_check_tpgs(struct scsi_device *sdev,
> struct alua_dh_data *h)
> */
> static int alua_vpd_inquiry(struct scsi_device *sdev, struct alua_dh_data *h)
> {
> + unsigned char *buff;
> + unsigned char bufflen = 36;
> int len, timeout = ALUA_FAILOVER_TIMEOUT;
> unsigned char sense[SCSI_SENSE_BUFFERSIZE];
> struct scsi_sense_hdr sense_hdr;
> unsigned retval;
> unsigned char *d;
> unsigned long expiry;
> + int err;
>
> expiry = round_jiffies_up(jiffies + timeout);
> retry:
> - retval = submit_vpd_inquiry(sdev, h->buff, h->bufflen, sense);
> + buff = kmalloc(bufflen, GFP_ATOMIC);
> + if (!buff) {
>
Why GFP_ATOMIC? I think it can be less restrictive in this path. If you
need GFP_ATOMIC here, then there are places in this code path you would
want to change from GFP_KERNEL to GFP_ATOMIC.
--
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 5/7] be2iscsi: Fix doorbell format for EQ/CQ/RQ s per SLI spec.
On 01/12/2014 06:42 PM, Jayamohan Kallickal wrote:
> diff --git a/drivers/scsi/be2iscsi/be_cmds.c b/drivers/scsi/be2iscsi/be_cmds.c
> index b14949a..4e1074a 100644
> --- a/drivers/scsi/be2iscsi/be_cmds.c
> +++ b/drivers/scsi/be2iscsi/be_cmds.c
> @@ -436,10 +436,20 @@ static void beiscsi_cq_notify(struct beiscsi_hba *phba,
> u16 qid, bool arm,
> u16 num_popped)
> {
> u32 val = 0;
> - val |= qid & DB_CQ_RING_ID_MASK;
> +
> if (arm)
> val |= 1 << DB_CQ_REARM_SHIFT;
> +
> val |= num_popped << DB_CQ_NUM_POPPED_SHIFT;
> +
> + /* Setting lower order CQ_ID Bits */
> + val |= qid & DB_CQ_RING_ID_LOW_MASK;
> +
> + /* Setting Higher order CQ_ID Bits */
> + val |= (((qid >> DB_CQ_HIGH_FEILD_SHIFT) &
> + DB_CQ_RING_ID_HIGH_MASK)
> + << DB_CQ_HIGH_SET_SHIFT);
> +
> iowrite32(val, phba->db_va + DB_CQ_OFFSET);
> @@ -1155,10 +1164,20 @@ static void hwi_ring_cq_db(struct beiscsi_hba *phba,
> unsigned char rearm, unsigned char event)
> {
> u32 val = 0;
> - val |= id & DB_CQ_RING_ID_MASK;
> +
> if (rearm)
> val |= 1 << DB_CQ_REARM_SHIFT;
> +
> val |= num_processed << DB_CQ_NUM_POPPED_SHIFT;
> +
> + /* Setting lower order CQ_ID Bits */
> + val |= (id & DB_CQ_RING_ID_LOW_MASK);
> +
> + /* Setting Higher order CQ_ID Bits */
> + val |= (((id >> DB_CQ_HIGH_FEILD_SHIFT) &
> + DB_CQ_RING_ID_HIGH_MASK)
> + << DB_CQ_HIGH_SET_SHIFT);
> +
> iowrite32(val, phba->db_va + DB_CQ_OFFSET);
> }
>
Are these 2 functions doing the same thing?
I think hwi_ring_cq_db is also doing the same operations, but with
different values. You should make a function.
--
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 4/7] be2iscsi : Fix statistics update in the driver.
On 01/12/2014 06:42 PM, Jayamohan Kallickal wrote:
>
> diff --git a/drivers/scsi/be2iscsi/be_main.h b/drivers/scsi/be2iscsi/be_main.h
> index 0076119..d21ad9e 100644
> --- a/drivers/scsi/be2iscsi/be_main.h
> +++ b/drivers/scsi/be2iscsi/be_main.h
> @@ -449,6 +449,25 @@ struct beiscsi_conn {
> struct sgl_handle *plogin_sgl_handle;
> struct beiscsi_session *beiscsi_sess;
> struct iscsi_task *task;
> +
> + /* CXN statistics */
> + /* Xmit Counters */
> + uint32_t noptx_pdus;
> + uint32_t login_pdus;
> + uint32_t text_pdus;
> + uint32_t logout_pdus;
> + uint32_t snack_pdus;
> +
> + /* Rx Counters */
> + uint32_t noprx_pdus;
> + uint32_t textrsp_pdus;
> + uint32_t logoutrsp_pdus;
> + uint32_t async_pdus;
> + uint32_t rjt_pdus;
> +
> + /* Error Counters */
> + uint32_t digest_err;
> + uint32_t format_err;
> };
>
You should put these on the iscsi_conn struct then have
__iscsi_complete_pdu handle the counters.
libiscsi should then also setup processing of those stats. You should
make a libiscsi helper which the drivers call. See iscsi_tcp
iscsi_sw_tcp_conn_get_stats call to iscsi_tcp_conn_get_stats for an
example for how it is done with libiscsi_tcp and lower level stats it
manages.
--
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 16/24] qla4xxx: Fix failure of mbox 0x31
> +static void qla4xxx_destroy_ddb(struct scsi_qla_host *ha,
> + struct ddb_entry *ddb_entry)
> +{
> + struct dev_db_entry *fw_ddb_entry = NULL;
> + dma_addr_t fw_ddb_entry_dma;
> + unsigned long wtime;
> + uint32_t ddb_state;
> + int options;
> + int status;
> +
> + options = LOGOUT_OPTION_CLOSE_SESSION;
> + if (qla4xxx_session_logout_ddb(ha, ddb_entry, options) == QLA_ERROR) {
> + ql4_printk(KERN_ERR, ha, "%s: Logout failed\n", __func__);
> + goto clear_ddb;
> + }
> +
> + fw_ddb_entry = dma_alloc_coherent(&ha->pdev->dev, sizeof(*fw_ddb_entry),
> + &fw_ddb_entry_dma, GFP_KERNEL);
> + if (!fw_ddb_entry) {
> + ql4_printk(KERN_ERR, ha,
> +"%s: Unable to allocate dma buffer\n", __func__);
> + goto clear_ddb;
> + }
> +
> + wtime = jiffies + (HZ * LOGOUT_TOV);
> + do {
> + status = qla4xxx_get_fwddb_entry(ha, ddb_entry->fw_ddb_index,
> + fw_ddb_entry, fw_ddb_entry_dma,
> + NULL, NULL, &ddb_state, NULL,
> + NULL, NULL);
> + if (status == QLA_ERROR)
> + goto clear_ddb;
> +
> + if ((ddb_state == DDB_DS_NO_CONNECTION_ACTIVE) ||
> + (ddb_state == DDB_DS_SESSION_FAILED))
> + goto clear_ddb;
> +
> + schedule_timeout_uninterruptible(HZ);
> + } while ((time_after(wtime, jiffies)));
I think there is a missing dma_free_coherent
> +
> +clear_ddb:
> + qla4xxx_clear_ddb_entry(ha, ddb_entry->fw_ddb_index);
> + return;
> +}
> +
Don't need return statement there.
--
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 2/6] SCSI/libiscsi: Reduce locking contention in fast path
On 12/20/2013 12:53 PM, micha...@cs.wisc.edu wrote:
> From: Shlomo Pongratz
>
> Replace the session lock with two locks, a forward lock and
> a backwards lock named frwd_lock and back_lock respectively.
.
>
> Signed-off-by: Shlomo Pongratz
> Signed-off-by: Or Gerlitz
> [minor fix up to apply cleanly]
> Signed-off-by: Mike Christie
James, I messed up and sent the wrong patch. I sent the busted patch in
this email. I am attaching the correct patch.
>From d3d0305a320b46d5ecc67b505a978c1fd35f5bfb Mon Sep 17 00:00:00 2001
From: Shlomo Pongratz
Date: Fri, 17 Jan 2014 01:53:54 -0600
Subject: [PATCH 2/2] SCSI/libiscsi: Reduce locking contention in fast path (v2)
Replace the session lock with two locks, a forward lock and
a backwards lock named frwd_lock and back_lock respectively.
The forward lock protects resources that change while sending a
request to the target, such as cmdsn, queued_cmdsn, and allocating
task from the commands' pool with kfifo_out.
The backward lock protects resources that change while processing
a response or in error path, such as cmdsn_exp, cmdsn_max, and
returning tasks to the commands' pool with kfifo_in.
Under a steady state fast-path situation, that is when one
or more processes/threads submit IO to an iscsi device and
a single kernel upcall (e.g softirq) is dealing with processing
of responses without errors, this patch eliminates the contention
between the queuecommand()/request response/scsi_done() flows
associated with iscsi sessions.
Between the forward and the backward locks exists a strict locking
hierarchy. The mutual exclusion zone protected by the forward lock can
enclose the mutual exclusion zone protected by the backward lock but not
vice versa.
For example, in iscsi_conn_teardown or in iscsi_xmit_data when there is
a failure and __iscsi_put_task is called, the backward lock is taken while
the forward lock is still taken. On the other hand, if in the RX path a nop
is to be sent, for example in iscsi_handle_reject or __iscsi_complete_pdu
than the forward lock is released and the backward lock is taken for the
duration of iscsi_send_nopout, later the backward lock is released and the
forward lock is retaken.
libiscsi_tcp uses two kernel fifos the r2t pool and the r2t queue.
The insertion and deletion from these queues didn't corespond to the
assumption taken by the new forward/backwards session locking paradigm.
That is, in iscsi_tcp_clenup_task which belongs to the RX (backwards)
path, r2t is taken out from r2t queue and inserted to the r2t pool.
In iscsi_tcp_get_curr_r2t which belong to the TX (forward) path, r2t
is also inserted to the r2t pool and another r2t is pulled from r2t
queue.
Only in iscsi_tcp_r2t_rsp which is called in the RX path but can requeue
to the TX path, r2t is taken from the r2t pool and inserted to the r2t
queue.
In order to cope with this situation, two spin locks were added,
pool2queue and queue2pool. The former protects extracting from the
r2t pool and inserting to the r2t queue, and the later protects the
extracing from the r2t queue and inserting to the r2t pool.
Signed-off-by: Shlomo Pongratz
Signed-off-by: Or Gerlitz
[minor fix up to apply cleanly and compile fix]
Signed-off-by: Mike Christie
---
---
drivers/scsi/be2iscsi/be_main.c | 26 +++---
drivers/scsi/bnx2i/bnx2i_hwi.c | 46
drivers/scsi/bnx2i/bnx2i_iscsi.c |8 +-
drivers/scsi/iscsi_tcp.c | 22 ++--
drivers/scsi/libiscsi.c | 214 +
drivers/scsi/libiscsi_tcp.c | 28 +++--
drivers/scsi/qla4xxx/ql4_isr.c |4 +-
include/scsi/libiscsi.h | 17 ++-
include/scsi/libiscsi_tcp.h |2 +
9 files changed, 206 insertions(+), 161 deletions(-)
diff --git a/drivers/scsi/be2iscsi/be_main.c b/drivers/scsi/be2iscsi/be_main.c
index 1f37505..8d82f2c 100644
--- a/drivers/scsi/be2iscsi/be_main.c
+++ b/drivers/scsi/be2iscsi/be_main.c
@@ -232,20 +232,20 @@ static int beiscsi_eh_abort(struct scsi_cmnd *sc)
cls_session = starget_to_session(scsi_target(sc->device));
session = cls_session->dd_data;
- spin_lock_bh(&session->lock);
+ spin_lock_bh(&session->frwd_lock);
if (!aborted_task || !aborted_task->sc) {
/* we raced */
- spin_unlock_bh(&session->lock);
+ spin_unlock_bh(&session->frwd_lock);
return SUCCESS;
}
aborted_io_task = aborted_task->dd_data;
if (!aborted_io_task->scsi_cmnd) {
/* raced or invalid command */
- spin_unlock_bh(&session->lock);
+ spin_unlock_bh(&session->frwd_lock);
return SUCCESS;
}
- spin_unlock_bh(&session->lock);
+ spin_unlock_bh(&session->frwd_lock);
/* Invalidate WRB Posted for this Task */
AMAP_SET_BITS(struct amap_iscsi_wrb, invld,
aborted_io_task->pwrb_handle->pwrb,
@@ -307,9 +307,9 @@ static int beiscsi_eh_devi
