[PATCHv2 2/2] scsi: make 'state' device attribute pollable
While the 'state' attribute can (and will) change occasionally,
calling 'poll()' or 'select()' on it fails as sysfs is never
notified that the state has changed.
With this patch calling 'poll()' or 'select()' will work
properly.
Signed-off-by: Hannes Reinecke
Reviewed-by: Christoph Hellwig
Reviewed-by: Johannes Thumshirn
---
drivers/scsi/scsi_lib.c | 3 +++
drivers/scsi/scsi_transport_srp.c | 5 -
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 9169396..eda41b0 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2655,6 +2655,7 @@ void scsi_exit_queue(void)
}
sdev->sdev_state = state;
+ sysfs_notify(&sdev->sdev_gendev.kobj, NULL, "state");
return 0;
illegal:
@@ -3078,6 +3079,7 @@ int scsi_internal_device_unblock_nowait(struct
scsi_device *sdev,
case SDEV_BLOCK:
case SDEV_TRANSPORT_OFFLINE:
sdev->sdev_state = new_state;
+ sysfs_notify(&sdev->sdev_gendev.kobj, NULL, "state");
break;
case SDEV_CREATED_BLOCK:
if (new_state == SDEV_TRANSPORT_OFFLINE ||
@@ -3085,6 +3087,7 @@ int scsi_internal_device_unblock_nowait(struct
scsi_device *sdev,
sdev->sdev_state = new_state;
else
sdev->sdev_state = SDEV_CREATED;
+ sysfs_notify(&sdev->sdev_gendev.kobj, NULL, "state");
break;
case SDEV_CANCEL:
case SDEV_OFFLINE:
diff --git a/drivers/scsi/scsi_transport_srp.c
b/drivers/scsi/scsi_transport_srp.c
index f617021..698cc46 100644
--- a/drivers/scsi/scsi_transport_srp.c
+++ b/drivers/scsi/scsi_transport_srp.c
@@ -556,8 +556,11 @@ int srp_reconnect_rport(struct srp_rport *rport)
*/
shost_for_each_device(sdev, shost) {
mutex_lock(&sdev->state_mutex);
- if (sdev->sdev_state == SDEV_OFFLINE)
+ if (sdev->sdev_state == SDEV_OFFLINE) {
sdev->sdev_state = SDEV_RUNNING;
+ sysfs_notify(&sdev->sdev_gendev.kobj,
+NULL, "state");
+ }
mutex_unlock(&sdev->state_mutex);
}
} else if (rport->state == SRP_RPORT_RUNNING) {
--
1.8.5.6
[PATCHv2 1/2] scsi_lib: rework scsi_internal_device_unblock_nowait()
Rework scsi_internal_device_unblock_nowait() into using a
switch statement.
No functional changes.
Signed-off-by: Hannes Reinecke
Reviewed-by: Christoph Hellwig
Reviewed-by: Johannes Thumshirn
---
drivers/scsi/scsi_lib.c | 17 +++--
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 1ae531b..9169396 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -3074,19 +3074,24 @@ int scsi_internal_device_unblock_nowait(struct
scsi_device *sdev,
* Try to transition the scsi device to SDEV_RUNNING or one of the
* offlined states and goose the device queue if successful.
*/
- if ((sdev->sdev_state == SDEV_BLOCK) ||
- (sdev->sdev_state == SDEV_TRANSPORT_OFFLINE))
+ switch (sdev->sdev_state) {
+ case SDEV_BLOCK:
+ case SDEV_TRANSPORT_OFFLINE:
sdev->sdev_state = new_state;
- else if (sdev->sdev_state == SDEV_CREATED_BLOCK) {
+ break;
+ case SDEV_CREATED_BLOCK:
if (new_state == SDEV_TRANSPORT_OFFLINE ||
new_state == SDEV_OFFLINE)
sdev->sdev_state = new_state;
else
sdev->sdev_state = SDEV_CREATED;
- } else if (sdev->sdev_state != SDEV_CANCEL &&
-sdev->sdev_state != SDEV_OFFLINE)
+ break;
+ case SDEV_CANCEL:
+ case SDEV_OFFLINE:
+ break;
+ default:
return -EINVAL;
-
+ }
scsi_start_queue(sdev);
return 0;
--
1.8.5.6
[PATCHv2 0/2] scsi: pollable 'state' attribute
Hi all, here's a small patchset to make the 'state' device attribute pollable. It was supposed to be a small patch, but then hch suggested a rework. So there you go. Changes to v2: - Removed duplicate case value found by kbuild robot - Dropped state update patch Hannes Reinecke (2): scsi_lib: rework scsi_internal_device_unblock_nowait() scsi: make 'state' device attribute pollable drivers/scsi/scsi_lib.c | 20 ++-- drivers/scsi/scsi_transport_srp.c | 5 - 2 files changed, 18 insertions(+), 7 deletions(-) -- 1.8.5.6
Re: [PATCHv2 2/4] scsi: Export blacklist flags to sysfs
On 08/11/2017 02:43 AM, Martin K. Petersen wrote: > > Hannes, > >> Each scsi device is scanned according to the found blacklist flags, >> but this information is never presented to sysfs. This makes it quite >> hard to figure out if blacklisting worked as expected. With this >> patch we're exporting an additional attribute 'blacklist' containing >> the blacklist flags for this device. > > There have been changes to the blacklist values over the years so I'm > not so keen on exporting them as a numbers. > Ah, good to know. I was thinking of exporting them as text; 'tis better for a quick check anyway. Would that be acceptable? Cheers, Hannes -- Dr. Hannes ReineckezSeries & Storage h...@suse.com +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg)
Re: [PATCH RESEND 0/6] hpsa: support legacy boards
On 08/10/2017 09:15 PM, Don Brace wrote: >> -Original Message- >> From: Hannes Reinecke [mailto:h...@suse.de] >> Sent: Thursday, August 10, 2017 9:11 AM >> To: James Bottomley ; >> Christoph Hellwig >> Cc: Don Brace ; Martin K. Petersen >> ; Meelis Roos ; linux- >> s...@vger.kernel.org >> Subject: Re: [PATCH RESEND 0/6] hpsa: support legacy boards >> >> EXTERNAL EMAIL >> >> >> On 08/10/2017 04:06 PM, James Bottomley wrote: >>> On Thu, 2017-08-10 at 09:09 +0200, Christoph Hellwig wrote: No device support in Linux is unsupported, sorry. I think we're getting into the corporate bullshit game a little too much here. >>> >>> I think there are two different definitions of supported here. To us, >>> any device to which the driver attaches is "supported". However, if >>> it's never been tested before it may not work very well. In the Linux >>> way, we'll try to fix the bugs when they're reported and in that sense >>> we support the device until nothing in the kernel attaches to its ids >>> anymore. >>> >>> In the corporate world "supported" means we'll sell you a contract >>> giving you certain rights to report bugs and have us fix them. There >>> are definite reasons why corporations only support a small range of new >>> devices, even though devices not on this list may still be attached to >>> by the driver and thus we (Linux Community) would try to fix the bug >>> reports for. >>> >>> I think what you're basically asking for is a different name for the >>> flag, which is fine? how about 'legacy' instead? >>> >> Sure, no problem with that. >> >> Don? >> >> Cheers, >> >> Hannes >> -- >> Dr. Hannes ReineckeTeamlead Storage & Networking >> h...@suse.de +49 911 74053 688 >> SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg >> GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton >> HRB 21284 (AG Nürnberg) > > Ok, but to clarify... > * Will the cciss driver be removed when these patches are applied? Otherwise > what will prevent the cciss driver from loading over these devices? > i.e. how often will users need to be reminded to add > cciss.cciss_allow_hpsa flag? > > The idea is to remove the cciss driver completely once these changes are in. Preferably with the same patchset to avoid any timing issues. Cheers, Hannes -- Dr. Hannes ReineckeTeamlead Storage & Networking h...@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg)
Re: [PATCH 2/3] scsi_lib: rework scsi_internal_device_unblock_nowait()
Hi Hannes,
[auto build test ERROR on mkp-scsi/for-next]
[also build test ERROR on v4.13-rc4 next-20170810]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Hannes-Reinecke/scsi-pollable-state-attribute/20170811-071630
base: https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
config: xtensa-allmodconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 4.9.0
reproduce:
wget
https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=xtensa
All errors (new ones prefixed by >>):
drivers/scsi/scsi_lib.c: In function 'scsi_internal_device_unblock_nowait':
>> drivers/scsi/scsi_lib.c:3089:2: error: duplicate case value
case SDEV_TRANSPORT_OFFLINE:
^
>> drivers/scsi/scsi_lib.c:3079:2: error: previously used here
case SDEV_TRANSPORT_OFFLINE:
^
vim +3089 drivers/scsi/scsi_lib.c
3054
3055 /**
3056 * scsi_internal_device_unblock_nowait - resume a device after a block
request
3057 * @sdev: device to resume
3058 * @new_state: state to set the device to after unblocking
3059 *
3060 * Restart the device queue for a previously suspended SCSI device.
Does not
3061 * sleep.
3062 *
3063 * Returns zero if successful or a negative error code upon failure.
3064 *
3065 * Notes:
3066 * This routine transitions the device to the SDEV_RUNNING state or to
one of
3067 * the offline states (which must be a legal transition) allowing the
midlayer
3068 * to goose the queue for this device.
3069 */
3070 int scsi_internal_device_unblock_nowait(struct scsi_device *sdev,
3071 enum scsi_device_state
new_state)
3072 {
3073 /*
3074 * Try to transition the scsi device to SDEV_RUNNING or one of
the
3075 * offlined states and goose the device queue if successful.
3076 */
3077 switch (sdev->sdev_state) {
3078 case SDEV_BLOCK:
> 3079 case SDEV_TRANSPORT_OFFLINE:
3080 sdev->sdev_state = new_state;
3081 break;
3082 case SDEV_CREATED_BLOCK:
3083 if (new_state == SDEV_TRANSPORT_OFFLINE ||
3084 new_state == SDEV_OFFLINE)
3085 sdev->sdev_state = new_state;
3086 else
3087 sdev->sdev_state = SDEV_CREATED;
3088 break;
> 3089 case SDEV_TRANSPORT_OFFLINE:
3090 case SDEV_CANCEL:
3091 case SDEV_OFFLINE:
3092 break;
3093 default:
3094 return -EINVAL;
3095 }
3096 scsi_start_queue(sdev);
3097
3098 return 0;
3099 }
3100 EXPORT_SYMBOL_GPL(scsi_internal_device_unblock_nowait);
3101
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
Re: [PATCH] scsi-mq: Always unprepare before requeuing a request
Bart Van Assche writes: > On Thu, 2017-08-10 at 20:32 +1000, Michael Ellerman wrote: >> "Martin K. Petersen" writes: >> > > One of the two scsi-mq functions that requeue a request unprepares a >> > > request before requeueing (scsi_io_completion()) but the other >> > > function not (__scsi_queue_insert()). Make sure that a request is >> > > unprepared before requeuing it. >> > >> > Applied to 4.13/scsi-fixes. Thanks much! >> >> This seems to be preventing my Power8 box, which uses IPR, from booting. ... > > Hello Michael, > > Thanks for having reported this early. No worries. > Is there any chance that you can > reproduce this state, press SysRq-w on the console and collect the task > overview that is reported on the console (see also Documentation/admin-guide/ > sysrq.rst)? Here it is below. Doesn't seem to change over time. I can do the scsi_logging_level thing as well as soon as I've got some coffee :) cheers sysrq: SysRq : Show Blocked State taskPC stack pid father swapper/0 D10080 1 0 0x0800 Call Trace: [c003f7583890] [c003f75838e0] 0xc003f75838e0 (unreliable) [c003f7583a60] [c001b3b8] __switch_to+0x2a8/0x460 [c003f7583ac0] [c09abc60] __schedule+0x320/0xaa0 [c003f7583b90] [c09ac420] schedule+0x40/0xb0 [c003f7583bc0] [c0110fc4] async_synchronize_cookie_domain+0xd4/0x150 [c003f7583c30] [c0619f94] wait_for_device_probe+0x44/0xe0 [c003f7583c90] [c0c64ce4] prepare_namespace+0x58/0x248 [c003f7583d00] [c0c64478] kernel_init_freeable+0x310/0x348 [c003f7583dc0] [c000d6e4] kernel_init+0x24/0x150 [c003f7583e30] [c000ba1c] ret_from_kernel_thread+0x5c/0xc0 kworker/u193:0 D12736 6 2 0x0800 Workqueue: events_unbound async_run_entry_fn Call Trace: [c003f7597410] [c0150d00] console_unlock+0x330/0x770 (unreliable) [c003f75975e0] [c001b3b8] __switch_to+0x2a8/0x460 [c003f7597640] [c09abc60] __schedule+0x320/0xaa0 [c003f7597710] [c09ac420] schedule+0x40/0xb0 [c003f7597740] [c09b09d4] schedule_timeout+0x254/0x440 [c003f7597820] [c09aca80] io_schedule_timeout+0x30/0x60 [c003f7597850] [c09ad75c] wait_for_common_io.constprop.2+0xbc/0x1e0 [c003f75978d0] [c0509e6c] blk_execute_rq+0x4c/0x70 [c003f7597920] [c0654abc] scsi_execute+0xfc/0x260 [c003f7597990] [c0654f98] scsi_mode_sense+0x218/0x410 [c003f7597aa0] [c068ee68] sd_revalidate_disk+0x908/0x1cd0 [c003f7597be0] [c0690434] sd_probe_async+0xb4/0x220 [c003f7597c60] [c0110a20] async_run_entry_fn+0x70/0x170 [c003f7597ca0] [c0103904] process_one_work+0x2b4/0x560 [c003f7597d30] [c0103c38] worker_thread+0x88/0x5a0 [c003f7597dc0] [c010bfcc] kthread+0x15c/0x1a0 [c003f7597e30] [c000ba1c] ret_from_kernel_thread+0x5c/0xc0 kworker/u193:1 D12480 412 2 0x0800 Workqueue: events_unbound async_run_entry_fn Call Trace: [c003f5907410] [c0150d00] console_unlock+0x330/0x770 (unreliable) [c003f59075e0] [c001b3b8] __switch_to+0x2a8/0x460 [c003f5907640] [c09abc60] __schedule+0x320/0xaa0 [c003f5907710] [c09ac420] schedule+0x40/0xb0 [c003f5907740] [c09b09d4] schedule_timeout+0x254/0x440 [c003f5907820] [c09aca80] io_schedule_timeout+0x30/0x60 [c003f5907850] [c09ad75c] wait_for_common_io.constprop.2+0xbc/0x1e0 [c003f59078d0] [c0509e6c] blk_execute_rq+0x4c/0x70 [c003f5907920] [c0654abc] scsi_execute+0xfc/0x260 [c003f5907990] [c0654f98] scsi_mode_sense+0x218/0x410 [c003f5907aa0] [c068ee68] sd_revalidate_disk+0x908/0x1cd0 [c003f5907be0] [c0690434] sd_probe_async+0xb4/0x220 [c003f5907c60] [c0110a20] async_run_entry_fn+0x70/0x170 [c003f5907ca0] [c0103904] process_one_work+0x2b4/0x560 [c003f5907d30] [c0103c38] worker_thread+0x88/0x5a0 [c003f5907dc0] [c010bfcc] kthread+0x15c/0x1a0 [c003f5907e30] [c000ba1c] ret_from_kernel_thread+0x5c/0xc0 kworker/u193:2 D12832 421 2 0x0800 Workqueue: events_unbound async_run_entry_fn Call Trace: [c003f4103410] [c003f41035f0] 0xc003f41035f0 (unreliable) [c003f41035e0] [c001b3b8] __switch_to+0x2a8/0x460 [c003f4103640] [c09abc60] __schedule+0x320/0xaa0 [c003f4103710] [c09ac420] schedule+0x40/0xb0 [c003f4103740] [c09b09d4] schedule_timeout+0x254/0x440 [c003f4103820] [c09aca80] io_schedule_timeout+0x30/0x60 [c003f4103850] [c09ad75c] wait_for_common_io.constprop.2+0xbc/0x1e0 [c003f41038d0] [c0509e6c] blk_execute_rq+0x4c/0x70 [c003f4103920] [c0654abc] scsi_execute+0xfc/0x260 [c003f4103990] [c0654f98] scsi_mode_sense+0x218/0x410 [c003f4103aa0] [c000
Re: [PATCHv2 2/4] scsi: Export blacklist flags to sysfs
Hannes, > Each scsi device is scanned according to the found blacklist flags, > but this information is never presented to sysfs. This makes it quite > hard to figure out if blacklisting worked as expected. With this > patch we're exporting an additional attribute 'blacklist' containing > the blacklist flags for this device. There have been changes to the blacklist values over the years so I'm not so keen on exporting them as a numbers. -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCHv2 1/4] scsi_debug: allow to specify inquiry vendor and model
Hannes, > For testing purposes we need to be able to pass in the inquiry > vendor and model. This looks fine to me. -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH V2] sd_zbc: Write unlock zone from sd_uninit_cmnd()
Damien, > Releasing a zone write lock only when the write commnand that acquired > the lock completes can cause deadlocks due to potential command > reordering if the lock owning request is requeued and not > executed. This problem exists only with the scsi-mq path as, unlike > the legacy path, requests are moved out of the dispatch queue before > being prepared and so before locking a zone for a write command. > > Since sd_uninit_cmnd() is now always called when a request is > requeued, call sd_zbc_write_unlock_zone() from that function for write > requests that acquired a zone lock instead of from > sd_done(). Acquisition of a zone lock by a write command is indicated > using the new command flag SCMD_ZONE_WRITE_LOCK. Applied to 4.13/scsi-fixes. Thanks! -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH v2 0/2] virtio_scsi: Set can_queue based on size of virtqueue.
Richard, > v1 was here: > > https://lkml.org/lkml/2017/8/10/689 > > v1 -> v2: > > Remove .can_queue field from the templates. Applied to 4.14/scsi-queue. Thank you! -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH] qedi: Limit number for CQ queues.
Manish, > [qed_sp_iscsi_func_start:189(host_7-0)]Cannot satisfy CQ amount. Queues > requested 8, CQs available 4. Aborting function start > > Above condition will resolve as management firmware is capable of telling > us the number of CQs available for a given PF, qed will communicate the > same number to qedi, So that qedi will know how much CQs are allowed. Applied to 4.14/scsi-queue. Thank you! -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH 00/19] hisi_sas: misc fixes, improvements, and new features
John, > This patchset introduces an array of misc changes, most significantly > including: There were a couple of patches that did multiple things. In the future, please make sure you only make one logical change per patch. Applied to 4.14/scsi-queue. Thanks! -- Martin K. Petersen Oracle Linux Engineering
Re: scsi: pm8001: fix double free in pm8001_pci_probe
Pan, > In function pm8001_pci_probe(), on errors that the control flow jumps to > label err_out_ha_free, function pm8001_free() is called. In pm8001_free(), > scsi_host_put() is called to release shost, which keeps the return value > of scsi_host_alloc(). After pm8001_free() returns, kfree() is called to > free shost again, resulting in a double free bug. This patch removes > scsi_host_put() from pm8001_free() and explicitly calls scsi_host_put() > to release Scsi_Host in need. Applied to 4.14/scsi-queue. -- Martin K. Petersen Oracle Linux Engineering
Re: scsi: qla2xxx: use dma_mapping_error to check map errors
Pan, > The return value of dma_map_single() should be checked by > dma_mapping_error(). However, in function qla26xx_dport_diagnostics(), > its return value is checked against NULL, which could result in > failures. Applied to 4.14/scsi-queue. -- Martin K. Petersen Oracle Linux Engineering
Re: scsi: mvsas: replace kfree with scsi_host_put
Pan, > The return value of scsi_host_alloc() should be released by > scsi_host_put(). However, in function mvs_pci_init(), kfree() > is used. This patch replaces kfree() with scsi_host_put() to avoid > possible memory leaks. Applied to 4.14/scsi-queue. -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH v2] scsi: megaraid_sas: fix allocate instance->pd_info twice
weiping, > fix allocate instance->pd_info twice which was introduced by > 96188a89cc6d. Applied to 4.14/scsi-queue, thank you! -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH 0/5] qcom-ufs: phy/hcd: Refactor phy initialization code
Vivek, > Can you kindly review this patch series (for UFS controller changes) > and consider giving your Ack so that Kishon can pull in the series > through phy tree. SCSI piece looks OK. Would still like Subhash to review the rest. -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH 0/7] smartpqi updates
Don, > These patches are based on Linus's tree > > The changes are: > > - smartpqi-add-pqi-reset-quiesce-support >- allow driver to confirm completion of a reset. > - smartpqi-enhance-bmic-cache-flush >- can now distinguish between shutdown and power > management operation. > - smartpqi-update-pqi-passthru-ioctl >- update DMA direction > - smartpqi-cleanup-doorbell-register-usage >- change how sis mode is re-enabled > - smartpqi-update-kexec-power-down-support >- reset controller on shutdown > - smartpqi-add-in-new-controller-ids >- update for latest hw > - smartpqi-change-driver-version-to-1.1.2-125 Applied to 4.14/scsi-queue. Thanks! -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH 0/5] esp_scsi, mac_esp: Various fixes and cleanups
Finn, > This series has been tested on m68k Macs (ESP236 equivalent). > > Some more testing with different targets and devices (FAS236 etc) > might be nice. Being that the esp_scsi fixes are on error paths, more > review may actually be more valuable than more testing... Applied to 4.14/scsi-queue. Thanks! -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH] arcmsr: add const to bin_attribute structures
Christoph, > Looks good, > > Reviewed-by: Christoph Hellwig Applied to 4.14/scsi-queue. Thanks! -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH v2 3/5] aic7xxx: remove rules for shipped files
Michał, > Please drop this patch (only this one). I didn't notice, that the file > is needed by modpost: OK, done. -- Martin K. Petersen Oracle Linux Engineering
Re: [RFC PATCH 1/6] bsg: fix kernel panic resulting from missing allocation of a reply-buffer
On Fri, Aug 11, 2017 at 12:10:38AM +0200, Benjamin Block wrote: > On Thu, Aug 10, 2017 at 11:32:17AM +0200, Christoph Hellwig wrote: > > We can't use an on-stack buffer for the sense data, as drivers will > > dma to it. So we should reuse the SCSI init_rq_fn() for the BSG > > queues and/or implement the same scheme. > > > ... > > struct sg_io_v4 > +--+ > | | > | request>++ > | + _len | || > | (A)| | BSG Request| > | | | e.g. struct fc_bsg_request | Depends on BSG > implementation > | | || FC vs. iSCSI vs. ... > | | ++ > | | > | response--->++ Used as _Output_ > | + max_len | || User doesn't initialize > | (B)| | BSG Reply | User provides (optional) > | | | e.g. struct fc_bsg_reply | memory; May be NULL. > | | || > | | ++ > | | > | dout_xferp->+---+ Stuff send on the wire by > | + _len | | | the LLD > | (C)| | Transport Protocol IU | Aligned on PAGE_SIZE > | | | e.g. FC-GS-7 CT_IU| > | | | | > | | +---+ > | | > | din_xferp-->+---+ Buffer for response data by > | + _len | | | the LLD > | (D)| | Transport Protocol IU | Aligned on PAGE_SIZE > | | | e.g. FC-GS-7 CT_IU| > | | | | > | | +---+ > +--+ > ... > > struct request (E) > +--+ > | | struct scsi_request > | scsi_request--->+-+ > | | | | > | | | cmd-> Copy of (A) > | | | + _len | Space in struct or kzalloc > | | | (G)| > | | | | > | | | sense---> Space for BSG Reply > | | | + _len | Same Data-Structure as (B) > | | | (H)| NOT actually pointer (B) > | | | | 'reply_buffer' in my patch > | | +-+ > | | > | bio> Mapped via blk_rq_map_user() to (C) dout_xferp > | | > | next_rq-+ > | | | > +--+ | >| > struct request (F)|(if used) > +--+<-+ > | | > | scsi_request---> Unused here > | | > | bio> Mapped via blk_rq_map_user() to (D) din_xferp > | | > +--+ > ... > > struct bsg_job > +-+ > | | > | request---> (G) scsi_request->cmd -> Copy of (A) > | + _len| e.g. struct fc_bsg_request > | | > | reply-> (H) scsi_request->sense -> 'reply_buffer' > | + _len| e.g. struct fc_bsg_reply > | | > | request_payload---> struct scatterlist ... map (E)->bio > | + _len| > | (I) | > | | > | reply_payload-> struct scatterlist ... map (F)->bio > | + _len| > | (J) | > | | > +-+ > > > This worked till it broke. Right now every driver that tries to access > (H) will panic the system, or cause very undefined behavior. I suspect > no driver right now tries to do any DMA into (H); before the regression, > this has been also an on-stack variable (I suspect since BSG was > introduced, haven't checked though). > > The asymmetries between the first struct request (E) and the following > (F) also makes it hard to use the same scheme as in other drivers, where > init_rq_fn() gets to initialize each request in the same'ish way. Or? > Just looking at it right now, this would require some bigger rework that > is not appropriate for a stable bug-fix. > Just some more brain-dump here. One more problem for direct DMA into (H) in the current BSG setup is probably, that the transport classes have each their own private format for the BSG reply (struct fc_bsg_reply and struct iscsi_bsg_reply right now I think). The current stack doesn't take any precaution to properly align this in accords to what the LLDs specifies for the blk-layer... so lets assume struct fc_bsg_reply. This has fields for actual protocol IUs (in contrast to iSCSI, where it only has some vendor-reply buffer [an array with 0 length...]), but they start after so
Re: [RFC PATCH 5/6] bsg: reduce unnecessary arguments for bsg_map_hdr()
On Thu, Aug 10, 2017 at 11:35:31AM +0200, Christoph Hellwig wrote: > On Wed, Aug 09, 2017 at 04:11:19PM +0200, Benjamin Block wrote: > > Since struct bsg_command is now used in every calling case, we don't > > need separation of arguments anymore that are contained in the same > > bsg_command. > > > > Signed-off-by: Benjamin Block > > --- > > block/bsg.c | 13 ++--- > > 1 file changed, 6 insertions(+), 7 deletions(-) > > > > diff --git a/block/bsg.c b/block/bsg.c > > index 8517361a9b3f..6ee2ffca808a 100644 > > --- a/block/bsg.c > > +++ b/block/bsg.c > > @@ -212,11 +212,12 @@ bsg_validate_sgv4_hdr(struct sg_io_v4 *hdr, int *op) > > * map sg_io_v4 to a request. > > */ > > static struct request * > > -bsg_map_hdr(struct bsg_device *bd, struct sg_io_v4 *hdr, fmode_t > > has_write_perm, > > - u8 *reply_buffer) > > +bsg_map_hdr(struct bsg_device *bd, struct bsg_command *bc, > > + fmode_t has_write_perm) > > I wish we could just rename the argument to mode and pass on the > whole file->f_mode while you are cleaning up this code. That should > be a separate patch, though. > Hmm, I did a quick pass through the code and the only place this seems to be used, is to pass it to blk_verify_command() if the subcommand used in the BSG request is a SCSI Command. And this has the same semantics. So I guess this would require adjustments to the whole stack, as this is also used from the 'normal' SG side of the world. Beste Grüße / Best regards, - Benjamin Block -- Linux on z Systems Development / IBM Systems & Technology Group IBM Deutschland Research & Development GmbH Vorsitz. AufsR.: Martina Koederitz /Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen / Registergericht: AmtsG Stuttgart, HRB 243294
Re: [RFC PATCH 4/6] bsg: refactor ioctl to use regular BSG-command infrastructure for SG_IO
On Thu, Aug 10, 2017 at 10:24:56AM +0200, Johannes Thumshirn wrote: > On Wed, Aug 09, 2017 at 04:11:18PM +0200, Benjamin Block wrote: > > + return 0 == (bc->hdr.flags & BSG_FLAG_Q_AT_TAIL); > > return !(bc->hdr.flags & BSG_FLAG_Q_AT_TAIL); and make the function return > bool? I have to admit, this is the 1st time I have seen the above construct. > Hmm yeah, odd. To be honest, I just copied the expression so that it is obvious that no behavior changed, but just the place. I'll replace it with what you suggest. Beste Grüße / Best regards, - Benjamin Block -- Linux on z Systems Development / IBM Systems & Technology Group IBM Deutschland Research & Development GmbH Vorsitz. AufsR.: Martina Koederitz /Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen / Registergericht: AmtsG Stuttgart, HRB 243294
Re: [RFC PATCH 1/6] bsg: fix kernel panic resulting from missing allocation of a reply-buffer
On Thu, Aug 10, 2017 at 11:32:17AM +0200, Christoph Hellwig wrote: > We can't use an on-stack buffer for the sense data, as drivers will > dma to it. So we should reuse the SCSI init_rq_fn() for the BSG > queues and/or implement the same scheme. > BSG is odd in this regard. Here is the data model as far as I understood it from reading the source. The user of the interface provides his input via a sg_io_v4 object. struct sg_io_v4 +--+ | | | request>++ | + _len | || | (A)| | BSG Request| | | | e.g. struct fc_bsg_request | Depends on BSG implementation | | || FC vs. iSCSI vs. ... | | ++ | | | response--->++ Used as _Output_ | + max_len | || User doesn't initialize | (B)| | BSG Reply | User provides (optional) | | | e.g. struct fc_bsg_reply | memory; May be NULL. | | || | | ++ | | | dout_xferp->+---+ Stuff send on the wire by | + _len | | | the LLD | (C)| | Transport Protocol IU | Aligned on PAGE_SIZE | | | e.g. FC-GS-7 CT_IU| | | | | | | +---+ | | | din_xferp-->+---+ Buffer for response data by | + _len | | | the LLD | (D)| | Transport Protocol IU | Aligned on PAGE_SIZE | | | e.g. FC-GS-7 CT_IU| | | | | | | +---+ +--+ This is just a part of it, but the most important for this issue. The BSG driver then encapsulates this into two linked block-requests he passes down to the LLDs. The first block-request (E) is for the Request Data, and the second request (F) for the Response Data. (F) is optional, depending on the whether the user passes both dout_xferp and din_xferp. struct request (E) +--+ | | struct scsi_request | scsi_request--->+-+ | | | | | | | cmd-> Copy of (A) | | | + _len | Space in struct or kzalloc | | | (G)| | | | | | | | sense---> Space for BSG Reply | | | + _len | Same Data-Structure as (B) | | | (H)| NOT actually pointer (B) | | | | 'reply_buffer' in my patch | | +-+ | | | bio> Mapped via blk_rq_map_user() to (C) dout_xferp | | | next_rq-+ | | | +--+ | | struct request (F)|(if used) +--+<-+ | | | scsi_request---> Unused here | | | bio> Mapped via blk_rq_map_user() to (D) din_xferp | | +--+ This is enqueued in the (legacy) blk-queue. In bsg-lib.c this is processed and encapsulated into an other data-structure struct bsg_job struct bsg_job +-+ | | | request---> (G) scsi_request->cmd -> Copy of (A) | + _len| e.g. struct fc_bsg_request | | | reply-> (H) scsi_request->sense -> 'reply_buffer' | + _len| e.g. struct fc_bsg_reply | | | request_payload---> struct scatterlist ... map (E)->bio | + _len| | (I) | | | | reply_payload-> struct scatterlist ... map (F)->bio | + _len| | (J) | | | +-+ This then is finally what the LLD gets to work with, with the callback from the request-queue. Depending on contents of (G) the LLD gets to decide whatever the user-space wants him to do. This depends highly on the transport. In case of zFCP we map (I) and (J) directly into the ring that passes the data to our hardware and the one that the hardware uses to pass back the responses. (This is actually pretty cool if you think about it. Apart from the copy we make of (A) into (G), this whole send was completely 'zero-copy'. Depending on the hardware it can directly DMA onto the wire... I guess most modern cards can do such a thing.) When the answer is coming back, the payload data is expected to be put into (J). If your HW can DMA into this, no more effort. Again, depending on (H), the LLD fills in some information for accounting and such. In case o
Re: SCSI Oops: Unable to handle kernel NULL ptr dereference
[cc's snipped to linux-scsi ] On Thu, 2017-08-10 at 17:05 +, man...@openmail.cc wrote: > Hello, > > I'd like to report this rare panic I experienced today. I've been on > 4.12.3 since it was released and got this panic totally unexpected, > probably when terminating my WL compositor. I attach to this message a > capture of the screen. The problem occurred never before and never > after since, suggesting I will not be able to reproduce it easily. > Perhaps it means something to someone. > > Linux air 4.12.3 #4 SMP Fri Jul 28 12:07:06 CEST 2017 x86_64 Intel(R) > Core(TM)i5-6267U CPU @ 2.90GHz GenuineIntel GNU/Linux > > I don't know what additional information might be useful so if there > is anything else I should provide please tell me. > > Best regards, > Cedric Your stack trace indicates some kind of corruption in a kmem cache used by the SCSI code, uncovered when the block queue was being freed. Can you describe what SCSI hardware is connected to your machine? A snippet of your boot messages showing the hardware probe would help. -Ewan
Re: [PATCH 1/2] virtio: Reduce BUG if total_sg > virtqueue size to WARN.
On Fri, Aug 11, 2017 at 12:41:47AM +0300, Michael S. Tsirkin wrote: > On Thu, Aug 10, 2017 at 10:35:11PM +0100, Richard W.M. Jones wrote: > > On Fri, Aug 11, 2017 at 12:31:44AM +0300, Michael S. Tsirkin wrote: > > > Then we probably should fail probe if vq size is too small. > > > > What does this mean? > > > > Rich. > > We must prevent driver from submitting s/g lists > vq size to device. Fair point. I would have thought (not knowing anything about how this works in the kernel) that the driver should be able to split up scatter-gather lists if they are larger than what the driver supports? Anyway the commit message is derived from what Paolo told me on IRC, so let's see what he says. Rich. > Either tell linux to avoid s/g lists that are too long, or > simply fail request if this happens, or refuse to attach driver to device. > > Later option would look something like this within probe: > > for (i = VIRTIO_SCSI_VQ_BASE; i < num_vqs; i++) > if (vqs[i]->num < MAX_SG_USED_BY_LINUX) > goto err; > > > I don't know what's MAX_SG_USED_BY_LINUX though. > > > -- > > Richard Jones, Virtualization Group, Red Hat > > http://people.redhat.com/~rjones > > Read my programming and virtualization blog: http://rwmj.wordpress.com > > virt-builder quickly builds VMs from scratch > > http://libguestfs.org/virt-builder.1.html -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html
Re: [PATCH 1/2] virtio: Reduce BUG if total_sg > virtqueue size to WARN.
On Thu, Aug 10, 2017 at 10:35:11PM +0100, Richard W.M. Jones wrote: > On Fri, Aug 11, 2017 at 12:31:44AM +0300, Michael S. Tsirkin wrote: > > Then we probably should fail probe if vq size is too small. > > What does this mean? > > Rich. We must prevent driver from submitting s/g lists > vq size to device. Either tell linux to avoid s/g lists that are too long, or simply fail request if this happens, or refuse to attach driver to device. Later option would look something like this within probe: for (i = VIRTIO_SCSI_VQ_BASE; i < num_vqs; i++) if (vqs[i]->num < MAX_SG_USED_BY_LINUX) goto err; I don't know what's MAX_SG_USED_BY_LINUX though. > -- > Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones > Read my programming and virtualization blog: http://rwmj.wordpress.com > virt-builder quickly builds VMs from scratch > http://libguestfs.org/virt-builder.1.html
Re: [PATCH 1/2] virtio: Reduce BUG if total_sg > virtqueue size to WARN.
On Fri, Aug 11, 2017 at 12:31:44AM +0300, Michael S. Tsirkin wrote: > Then we probably should fail probe if vq size is too small. What does this mean? Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html
Re: [PATCH 1/2] virtio: Reduce BUG if total_sg > virtqueue size to WARN.
On Thu, Aug 10, 2017 at 10:30:38PM +0100, Richard W.M. Jones wrote: > On Fri, Aug 11, 2017 at 12:21:16AM +0300, Michael S. Tsirkin wrote: > > On Thu, Aug 10, 2017 at 05:40:34PM +0100, Richard W.M. Jones wrote: > > > If using indirect descriptors, you can make the total_sg as large as > > > you want. > > > > That would be a spec violation though, even if it happens to > > work on current QEMU. > > > > The spec says: > > A driver MUST NOT create a descriptor chain longer than the Queue Size > > of the device. > > > > What prompted this patch? > > Do we ever encounter this situation? > > This patch is needed because the following (2/2) patch will trigger > that BUG_ON if I set virtqueue_size=64 or any smaller value. > > The precise backtrace is below. > > Rich. > > [4.029510] [ cut here ] > [4.030127] kernel BUG at drivers/virtio/virtio_ring.c:299! > [4.030834] invalid opcode: [#1] SMP > [4.031340] Modules linked in: libcrc32c crc8 crc7 crc4 crc_itu_t > virtio_pci virtio_mmio virtio_input virtio_balloon virtio_scsi nd_pmem nd_btt > virtio_net virtio_crypto crypto_engine virtio_console virtio_rng virtio_blk > virtio_ring virtio nfit crc32_generic crct10dif_pclmul crc32c_intel > crc32_pclmul > [4.034606] CPU: 0 PID: 1 Comm: init Not tainted 4.13.0-rc4+ #100 > [4.035354] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS > rel-1.10.2-0-g5f4c7b1-prebuilt.qemu-project.org 04/01/2014 > [4.036770] task: 9a859e243300 task.stack: a731c00cc000 > [4.037506] RIP: 0010:virtqueue_add_sgs+0x23d/0x460 [virtio_ring] > [4.038250] RSP: :a731c00cf6e0 EFLAGS: 00010097 > [4.038898] RAX: RBX: 0011 RCX: > dd0680646c40 > [4.039762] RDX: RSI: a731c00cf7b8 RDI: > 9a85945c4d68 > [4.040634] RBP: a731c00cf748 R08: 9a85945c4a78 R09: > 01080020 > [4.041508] R10: a731c00cf788 R11: 9a859b3d3120 R12: > a731c00cf7d0 > [4.042382] R13: a731c00cf7d0 R14: 0001 R15: > 9a859b3f8200 > [4.043248] FS: () GS:9a859e60() > knlGS: > [4.044232] CS: 0010 DS: ES: CR0: 80050033 > [4.044942] CR2: 7fcff02e931c CR3: 1d23b000 CR4: > 003406f0 > [4.045815] DR0: DR1: DR2: > > [4.046684] DR3: DR6: fffe0ff0 DR7: > 0400 > [4.047559] Call Trace: > [4.047876] virtscsi_add_cmd+0x1c9/0x280 [virtio_scsi] > [4.048528] virtscsi_kick_cmd+0x38/0x90 [virtio_scsi] > [4.049161] virtscsi_queuecommand+0x104/0x280 [virtio_scsi] > [4.049875] virtscsi_queuecommand_single+0x38/0x40 [virtio_scsi] > [4.050628] scsi_dispatch_cmd+0xf9/0x390 > [4.051128] scsi_queue_rq+0x5e5/0x6f0 > [4.051602] blk_mq_dispatch_rq_list+0x1ff/0x3c0 > [4.052175] blk_mq_sched_dispatch_requests+0x199/0x1f0 > [4.052824] __blk_mq_run_hw_queue+0x11d/0x1b0 > [4.053382] __blk_mq_delay_run_hw_queue+0x8d/0xa0 > [4.053972] blk_mq_run_hw_queue+0x14/0x20 > [4.054485] blk_mq_sched_insert_requests+0x96/0x120 > [4.055095] blk_mq_flush_plug_list+0x19d/0x410 > [4.055661] blk_flush_plug_list+0xf9/0x2b0 > [4.056182] blk_finish_plug+0x2c/0x40 > [4.056655] __do_page_cache_readahead+0x32e/0x400 > [4.057261] filemap_fault+0x2fb/0x890 > [4.057731] ? filemap_fault+0x2fb/0x890 > [4.058220] ? find_held_lock+0x3c/0xb0 > [4.058714] ext4_filemap_fault+0x34/0x50 > [4.059212] __do_fault+0x1e/0x110 > [4.059644] __handle_mm_fault+0x6b2/0x1080 > [4.060167] handle_mm_fault+0x178/0x350 > [4.060662] __do_page_fault+0x26e/0x510 > [4.061152] trace_do_page_fault+0x9d/0x290 > [4.061677] do_async_page_fault+0x51/0xa0 > [4.062189] async_page_fault+0x28/0x30 > [4.062667] RIP: 0033:0x7fcff030a24f > [4.063113] RSP: 002b:7ffefc2ad078 EFLAGS: 00010206 > [4.063760] RAX: 7fcff02e931c RBX: 7fcff050f660 RCX: > 7fcff02e935c > [4.064648] RDX: 0664 RSI: RDI: > 7fcff02e931c > [4.065519] RBP: 7ffefc2ad320 R08: 7fcff02e931c R09: > 00027000 > [4.066392] R10: 7fcff02e9980 R11: 0206 R12: > 7ffefc2ad0b0 > [4.067263] R13: 7ffefc2ad408 R14: 0002 R15: > 87f0 > [4.068135] Code: af 01 c7 45 c8 01 00 00 00 45 31 ed e9 9b fe ff ff 31 db > 48 83 7d d0 00 0f 85 3f fe ff ff 0f 0b 48 8b 7d b8 e8 e5 fd 22 de eb 8b <0f> > 0b 0f 0b 44 89 6d a8 45 89 f5 48 8b 45 a0 44 8d 70 01 48 83 > [4.070506] RIP: virtqueue_add_sgs+0x23d/0x460 [virtio_ring] RSP: > a731c00cf6e0 > [4.071434] ---[ end trace 02532659840e2a64 ]--- Then we probably should fail probe if vq size is too small. > > -- > Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones >
Re: [PATCH 1/2] virtio: Reduce BUG if total_sg > virtqueue size to WARN.
On Fri, Aug 11, 2017 at 12:21:16AM +0300, Michael S. Tsirkin wrote: > On Thu, Aug 10, 2017 at 05:40:34PM +0100, Richard W.M. Jones wrote: > > If using indirect descriptors, you can make the total_sg as large as > > you want. > > That would be a spec violation though, even if it happens to > work on current QEMU. > > The spec says: > A driver MUST NOT create a descriptor chain longer than the Queue Size > of the device. > > What prompted this patch? > Do we ever encounter this situation? This patch is needed because the following (2/2) patch will trigger that BUG_ON if I set virtqueue_size=64 or any smaller value. The precise backtrace is below. Rich. [4.029510] [ cut here ] [4.030127] kernel BUG at drivers/virtio/virtio_ring.c:299! [4.030834] invalid opcode: [#1] SMP [4.031340] Modules linked in: libcrc32c crc8 crc7 crc4 crc_itu_t virtio_pci virtio_mmio virtio_input virtio_balloon virtio_scsi nd_pmem nd_btt virtio_net virtio_crypto crypto_engine virtio_console virtio_rng virtio_blk virtio_ring virtio nfit crc32_generic crct10dif_pclmul crc32c_intel crc32_pclmul [4.034606] CPU: 0 PID: 1 Comm: init Not tainted 4.13.0-rc4+ #100 [4.035354] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.10.2-0-g5f4c7b1-prebuilt.qemu-project.org 04/01/2014 [4.036770] task: 9a859e243300 task.stack: a731c00cc000 [4.037506] RIP: 0010:virtqueue_add_sgs+0x23d/0x460 [virtio_ring] [4.038250] RSP: :a731c00cf6e0 EFLAGS: 00010097 [4.038898] RAX: RBX: 0011 RCX: dd0680646c40 [4.039762] RDX: RSI: a731c00cf7b8 RDI: 9a85945c4d68 [4.040634] RBP: a731c00cf748 R08: 9a85945c4a78 R09: 01080020 [4.041508] R10: a731c00cf788 R11: 9a859b3d3120 R12: a731c00cf7d0 [4.042382] R13: a731c00cf7d0 R14: 0001 R15: 9a859b3f8200 [4.043248] FS: () GS:9a859e60() knlGS: [4.044232] CS: 0010 DS: ES: CR0: 80050033 [4.044942] CR2: 7fcff02e931c CR3: 1d23b000 CR4: 003406f0 [4.045815] DR0: DR1: DR2: [4.046684] DR3: DR6: fffe0ff0 DR7: 0400 [4.047559] Call Trace: [4.047876] virtscsi_add_cmd+0x1c9/0x280 [virtio_scsi] [4.048528] virtscsi_kick_cmd+0x38/0x90 [virtio_scsi] [4.049161] virtscsi_queuecommand+0x104/0x280 [virtio_scsi] [4.049875] virtscsi_queuecommand_single+0x38/0x40 [virtio_scsi] [4.050628] scsi_dispatch_cmd+0xf9/0x390 [4.051128] scsi_queue_rq+0x5e5/0x6f0 [4.051602] blk_mq_dispatch_rq_list+0x1ff/0x3c0 [4.052175] blk_mq_sched_dispatch_requests+0x199/0x1f0 [4.052824] __blk_mq_run_hw_queue+0x11d/0x1b0 [4.053382] __blk_mq_delay_run_hw_queue+0x8d/0xa0 [4.053972] blk_mq_run_hw_queue+0x14/0x20 [4.054485] blk_mq_sched_insert_requests+0x96/0x120 [4.055095] blk_mq_flush_plug_list+0x19d/0x410 [4.055661] blk_flush_plug_list+0xf9/0x2b0 [4.056182] blk_finish_plug+0x2c/0x40 [4.056655] __do_page_cache_readahead+0x32e/0x400 [4.057261] filemap_fault+0x2fb/0x890 [4.057731] ? filemap_fault+0x2fb/0x890 [4.058220] ? find_held_lock+0x3c/0xb0 [4.058714] ext4_filemap_fault+0x34/0x50 [4.059212] __do_fault+0x1e/0x110 [4.059644] __handle_mm_fault+0x6b2/0x1080 [4.060167] handle_mm_fault+0x178/0x350 [4.060662] __do_page_fault+0x26e/0x510 [4.061152] trace_do_page_fault+0x9d/0x290 [4.061677] do_async_page_fault+0x51/0xa0 [4.062189] async_page_fault+0x28/0x30 [4.062667] RIP: 0033:0x7fcff030a24f [4.063113] RSP: 002b:7ffefc2ad078 EFLAGS: 00010206 [4.063760] RAX: 7fcff02e931c RBX: 7fcff050f660 RCX: 7fcff02e935c [4.064648] RDX: 0664 RSI: RDI: 7fcff02e931c [4.065519] RBP: 7ffefc2ad320 R08: 7fcff02e931c R09: 00027000 [4.066392] R10: 7fcff02e9980 R11: 0206 R12: 7ffefc2ad0b0 [4.067263] R13: 7ffefc2ad408 R14: 0002 R15: 87f0 [4.068135] Code: af 01 c7 45 c8 01 00 00 00 45 31 ed e9 9b fe ff ff 31 db 48 83 7d d0 00 0f 85 3f fe ff ff 0f 0b 48 8b 7d b8 e8 e5 fd 22 de eb 8b <0f> 0b 0f 0b 44 89 6d a8 45 89 f5 48 8b 45 a0 44 8d 70 01 48 83 [4.070506] RIP: virtqueue_add_sgs+0x23d/0x460 [virtio_ring] RSP: a731c00cf6e0 [4.071434] ---[ end trace 02532659840e2a64 ]--- -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org
Re: [PATCH 1/2] virtio: Reduce BUG if total_sg > virtqueue size to WARN.
On Thu, Aug 10, 2017 at 05:40:34PM +0100, Richard W.M. Jones wrote:
> If using indirect descriptors, you can make the total_sg as large as
> you want.
That would be a spec violation though, even if it happens to
work on current QEMU.
The spec says:
A driver MUST NOT create a descriptor chain longer than the Queue Size
of the device.
What prompted this patch? Do we ever encounter this situation?
> If not, BUG is too serious because the function later
> returns -ENOSPC.
>
> Thanks Paolo Bonzini, Christoph Hellwig.
>
> Signed-off-by: Richard W.M. Jones
> ---
> drivers/virtio/virtio_ring.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index 5e1b548828e6..27cbc1eab868 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -296,7 +296,6 @@ static inline int virtqueue_add(struct virtqueue *_vq,
> }
> #endif
>
> - BUG_ON(total_sg > vq->vring.num);
> BUG_ON(total_sg == 0);
>
> head = vq->free_head;
> @@ -305,8 +304,10 @@ static inline int virtqueue_add(struct virtqueue *_vq,
>* buffers, then go indirect. FIXME: tune this threshold */
> if (vq->indirect && total_sg > 1 && vq->vq.num_free)
> desc = alloc_indirect(_vq, total_sg, gfp);
> - else
> + else {
> desc = NULL;
> + WARN_ON_ONCE(total_sg > vq->vring.num && !vq->indirect);
> + }
>
> if (desc) {
> /* Use a single buffer which doesn't continue */
> --
> 2.13.1
RE: [PATCH RESEND 0/6] hpsa: support legacy boards
> -Original Message- > From: Hannes Reinecke [mailto:h...@suse.de] > Sent: Thursday, August 10, 2017 9:11 AM > To: James Bottomley ; > Christoph Hellwig > Cc: Don Brace ; Martin K. Petersen > ; Meelis Roos ; linux- > s...@vger.kernel.org > Subject: Re: [PATCH RESEND 0/6] hpsa: support legacy boards > > EXTERNAL EMAIL > > > On 08/10/2017 04:06 PM, James Bottomley wrote: > > On Thu, 2017-08-10 at 09:09 +0200, Christoph Hellwig wrote: > >> No device support in Linux is unsupported, sorry. I think we're > >> getting into the corporate bullshit game a little too much here. > > > > I think there are two different definitions of supported here. To us, > > any device to which the driver attaches is "supported". However, if > > it's never been tested before it may not work very well. In the Linux > > way, we'll try to fix the bugs when they're reported and in that sense > > we support the device until nothing in the kernel attaches to its ids > > anymore. > > > > In the corporate world "supported" means we'll sell you a contract > > giving you certain rights to report bugs and have us fix them. There > > are definite reasons why corporations only support a small range of new > > devices, even though devices not on this list may still be attached to > > by the driver and thus we (Linux Community) would try to fix the bug > > reports for. > > > > I think what you're basically asking for is a different name for the > > flag, which is fine? how about 'legacy' instead? > > > Sure, no problem with that. > > Don? > > Cheers, > > Hannes > -- > Dr. Hannes ReineckeTeamlead Storage & Networking > h...@suse.de +49 911 74053 688 > SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg > GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton > HRB 21284 (AG Nürnberg) Ok, but to clarify... * Will the cciss driver be removed when these patches are applied? Otherwise what will prevent the cciss driver from loading over these devices? i.e. how often will users need to be reminded to add cciss.cciss_allow_hpsa flag?
Re: [PATCH 16/29] scsi: esas2r: constify pci_device_id.
Looks good.
Acked-by: Bradley Grove
On 07/30/2017 04:40 AM, Arvind Yadav wrote:
pci_device_id are not supposed to change at runtime. All functions
working with pci_device_id provided by work with
const pci_device_id. So mark the non-const structs as const.
Signed-off-by: Arvind Yadav
---
drivers/scsi/esas2r/esas2r_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/esas2r/esas2r_main.c
b/drivers/scsi/esas2r/esas2r_main.c
index f2e9d8a..81f226b 100644
--- a/drivers/scsi/esas2r/esas2r_main.c
+++ b/drivers/scsi/esas2r/esas2r_main.c
@@ -309,7 +309,7 @@ MODULE_PARM_DESC(interrupt_mode,
"Defines the interrupt mode to use. 0 for legacy"
", 1 for MSI. Default is MSI (1).");
-static struct pci_device_id
+static const struct pci_device_id
esas2r_pci_table[] = {
{ ATTO_VENDOR_ID, 0x0049, ATTO_VENDOR_ID, 0x0049,
0,
This electronic transmission and any attachments hereto are intended only for the use of the individual or entity to which it is addressed and may contain confidential information belonging to ATTO Technology, Inc. If you have reason to believe that you are not the intended recipient, you are hereby notified that any disclosure, copying, distribution or the taking of any action in reliance on the contents of this electronic transmission is strictly prohibited. If you have reason to believe that you have received this transmission in error, please notify ATTO immediately by return e-mail and delete and destroy this communication.
[PATCH] sym53c8xx: Avoid undefined behaviour in drivers/scsi/sym53c8xx_2/sym_hipd.c:762
On parisc I see this UBSAN warning with a sym53c896: UBSAN: Undefined behaviour in ./drivers/scsi/sym53c8xx_2/sym_hipd.c:762:24 index -1903078336 is out of range for type 'u32 [7]' Avoid this warning by switching to dev64_ul(). Signed-off-by: Helge Deller diff --git a/drivers/scsi/sym53c8xx_2/sym_hipd.c b/drivers/scsi/sym53c8xx_2/sym_hipd.c index 6b349e3..15ff285a9 100644 --- a/drivers/scsi/sym53c8xx_2/sym_hipd.c +++ b/drivers/scsi/sym53c8xx_2/sym_hipd.c @@ -759,7 +759,7 @@ static int sym_prepare_setting(struct Scsi_Host *shost, struct sym_hcb *np, stru /* * Maximum synchronous period factor supported by the chip. */ - period = (11 * div_10M[np->clock_divn - 1]) / (4 * np->clock_khz); + period = div64_ul(11 * div_10M[np->clock_divn - 1], 4 * np->clock_khz); np->maxsync = period > 2540 ? 254 : period / 10; /*
[PATCH 7/7] smartpqi: change driver version to 1.1.2-125
From: Kevin Barnett Reviewed-by: Scott Benesh Signed-off-by: Kevin Barnett Signed-off-by: Don Brace --- drivers/scsi/smartpqi/smartpqi_init.c |8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c index 59b6301..83bdbd8 100644 --- a/drivers/scsi/smartpqi/smartpqi_init.c +++ b/drivers/scsi/smartpqi/smartpqi_init.c @@ -40,11 +40,11 @@ #define BUILD_TIMESTAMP #endif -#define DRIVER_VERSION "1.0.4-100" +#define DRIVER_VERSION "1.1.2-125" #define DRIVER_MAJOR 1 -#define DRIVER_MINOR 0 -#define DRIVER_RELEASE 4 -#define DRIVER_REVISION100 +#define DRIVER_MINOR 1 +#define DRIVER_RELEASE 2 +#define DRIVER_REVISION125 #define DRIVER_NAME"Microsemi PQI Driver (v" \ DRIVER_VERSION BUILD_TIMESTAMP ")"
[PATCH 6/7] smartpqi: add in new controller ids
From: Kevin Barnett
Update the driver’s PCI IDs to match the latest
Microsemi controllers
Reviewed-by: Scott Benesh
Signed-off-by: Kevin Barnett
Signed-off-by: Don Brace
---
drivers/scsi/smartpqi/smartpqi_init.c | 36 +
1 file changed, 14 insertions(+), 22 deletions(-)
diff --git a/drivers/scsi/smartpqi/smartpqi_init.c
b/drivers/scsi/smartpqi/smartpqi_init.c
index afd3eed..59b6301 100644
--- a/drivers/scsi/smartpqi/smartpqi_init.c
+++ b/drivers/scsi/smartpqi/smartpqi_init.c
@@ -6822,7 +6822,7 @@ static const struct pci_device_id pqi_pci_id_table[] = {
},
{
PCI_DEVICE_SUB(PCI_VENDOR_ID_ADAPTEC2, 0x028f,
- PCI_VENDOR_ID_ADAPTEC2, 0x0605)
+ PCI_VENDOR_ID_ADAPTEC2, 0x0608)
},
{
PCI_DEVICE_SUB(PCI_VENDOR_ID_ADAPTEC2, 0x028f,
@@ -6854,6 +6854,10 @@ static const struct pci_device_id pqi_pci_id_table[] = {
},
{
PCI_DEVICE_SUB(PCI_VENDOR_ID_ADAPTEC2, 0x028f,
+ PCI_VENDOR_ID_ADAPTEC2, 0x0807)
+ },
+ {
+ PCI_DEVICE_SUB(PCI_VENDOR_ID_ADAPTEC2, 0x028f,
PCI_VENDOR_ID_ADAPTEC2, 0x0900)
},
{
@@ -6890,6 +6894,10 @@ static const struct pci_device_id pqi_pci_id_table[] = {
},
{
PCI_DEVICE_SUB(PCI_VENDOR_ID_ADAPTEC2, 0x028f,
+ PCI_VENDOR_ID_ADAPTEC2, 0x090a)
+ },
+ {
+ PCI_DEVICE_SUB(PCI_VENDOR_ID_ADAPTEC2, 0x028f,
PCI_VENDOR_ID_ADAPTEC2, 0x1200)
},
{
@@ -6922,6 +6930,10 @@ static const struct pci_device_id pqi_pci_id_table[] = {
},
{
PCI_DEVICE_SUB(PCI_VENDOR_ID_ADAPTEC2, 0x028f,
+ PCI_VENDOR_ID_DELL, 0x1fe0)
+ },
+ {
+ PCI_DEVICE_SUB(PCI_VENDOR_ID_ADAPTEC2, 0x028f,
PCI_VENDOR_ID_HP, 0x0600)
},
{
@@ -6938,11 +6950,7 @@ static const struct pci_device_id pqi_pci_id_table[] = {
},
{
PCI_DEVICE_SUB(PCI_VENDOR_ID_ADAPTEC2, 0x028f,
- PCI_VENDOR_ID_HP, 0x0604)
- },
- {
- PCI_DEVICE_SUB(PCI_VENDOR_ID_ADAPTEC2, 0x028f,
- PCI_VENDOR_ID_HP, 0x0606)
+ PCI_VENDOR_ID_HP, 0x0609)
},
{
PCI_DEVICE_SUB(PCI_VENDOR_ID_ADAPTEC2, 0x028f,
@@ -6970,14 +6978,6 @@ static const struct pci_device_id pqi_pci_id_table[] = {
},
{
PCI_DEVICE_SUB(PCI_VENDOR_ID_ADAPTEC2, 0x028f,
- PCI_VENDOR_ID_HP, 0x0656)
- },
- {
- PCI_DEVICE_SUB(PCI_VENDOR_ID_ADAPTEC2, 0x028f,
- PCI_VENDOR_ID_HP, 0x0657)
- },
- {
- PCI_DEVICE_SUB(PCI_VENDOR_ID_ADAPTEC2, 0x028f,
PCI_VENDOR_ID_HP, 0x0700)
},
{
@@ -6998,14 +6998,6 @@ static const struct pci_device_id pqi_pci_id_table[] = {
},
{
PCI_DEVICE_SUB(PCI_VENDOR_ID_ADAPTEC2, 0x028f,
- PCI_VENDOR_ID_HP, 0x1102)
- },
- {
- PCI_DEVICE_SUB(PCI_VENDOR_ID_ADAPTEC2, 0x028f,
- PCI_VENDOR_ID_HP, 0x1150)
- },
- {
- PCI_DEVICE_SUB(PCI_VENDOR_ID_ADAPTEC2, 0x028f,
PCI_ANY_ID, PCI_ANY_ID)
},
{ 0 }
[PATCH 1/7] smartpqi: add pqi reset quiesce support
From: Kevin Barnett
Reviewed-by: Scott Benesh
Signed-off-by: Kevin Barnett
Signed-off-by: Don Brace
---
drivers/scsi/smartpqi/smartpqi.h | 23 +
drivers/scsi/smartpqi/smartpqi_init.c | 58 ++---
drivers/scsi/smartpqi/smartpqi_sis.c | 30 -
drivers/scsi/smartpqi/smartpqi_sis.h |1 +
4 files changed, 98 insertions(+), 14 deletions(-)
diff --git a/drivers/scsi/smartpqi/smartpqi.h b/drivers/scsi/smartpqi/smartpqi.h
index e164ffa..6dd0449 100644
--- a/drivers/scsi/smartpqi/smartpqi.h
+++ b/drivers/scsi/smartpqi/smartpqi.h
@@ -688,6 +688,28 @@ struct pqi_config_table_heartbeat {
__le32 heartbeat_counter;
};
+union pqi_reset_register {
+ struct {
+ u32 reset_type : 3;
+ u32 reserved : 2;
+ u32 reset_action : 3;
+ u32 hold_in_pd1 : 1;
+ u32 reserved2 : 23;
+ } bits;
+ u32 all_bits;
+};
+
+#define PQI_RESET_ACTION_RESET 0x1
+
+#define PQI_RESET_TYPE_NO_RESET0x0
+#define PQI_RESET_TYPE_SOFT_RESET 0x1
+#define PQI_RESET_TYPE_FIRM_RESET 0x2
+#define PQI_RESET_TYPE_HARD_RESET 0x3
+
+#define PQI_RESET_ACTION_COMPLETED 0x2
+
+#define PQI_RESET_POLL_INTERVAL_MSECS 100
+
#define PQI_MAX_OUTSTANDING_REQUESTS ((u32)~0)
#define PQI_MAX_OUTSTANDING_REQUESTS_KDUMP 32
#define PQI_MAX_TRANSFER_SIZE (1024U * 1024U)
@@ -995,6 +1017,7 @@ struct pqi_ctrl_info {
u8 inbound_spanning_supported : 1;
u8 outbound_spanning_supported : 1;
u8 pqi_mode_enabled : 1;
+ u8 pqi_reset_quiesce_supported : 1;
struct list_head scsi_device_list;
spinlock_t scsi_device_list_lock;
diff --git a/drivers/scsi/smartpqi/smartpqi_init.c
b/drivers/scsi/smartpqi/smartpqi_init.c
index cb8f886..ffdc32b 100644
--- a/drivers/scsi/smartpqi/smartpqi_init.c
+++ b/drivers/scsi/smartpqi/smartpqi_init.c
@@ -5889,28 +5889,62 @@ static void pqi_unregister_scsi(struct pqi_ctrl_info
*ctrl_info)
scsi_host_put(shost);
}
-#define PQI_RESET_ACTION_RESET 0x1
+static int pqi_wait_for_pqi_reset_completion(struct pqi_ctrl_info *ctrl_info)
+{
+ int rc = 0;
+ struct pqi_device_registers __iomem *pqi_registers;
+ unsigned long timeout;
+ unsigned int timeout_msecs;
+ union pqi_reset_register reset_reg;
+
+ pqi_registers = ctrl_info->pqi_registers;
+ timeout_msecs = readw(&pqi_registers->max_reset_timeout) * 100;
+ timeout = msecs_to_jiffies(timeout_msecs) + jiffies;
+
+ while (1) {
+ msleep(PQI_RESET_POLL_INTERVAL_MSECS);
+ reset_reg.all_bits = readl(&pqi_registers->device_reset);
+ if (reset_reg.bits.reset_action == PQI_RESET_ACTION_COMPLETED)
+ break;
+ pqi_check_ctrl_health(ctrl_info);
+ if (pqi_ctrl_offline(ctrl_info)) {
+ rc = -ENXIO;
+ break;
+ }
+ if (time_after(jiffies, timeout)) {
+ rc = -ETIMEDOUT;
+ break;
+ }
+ }
-#define PQI_RESET_TYPE_NO_RESET0x0
-#define PQI_RESET_TYPE_SOFT_RESET 0x1
-#define PQI_RESET_TYPE_FIRM_RESET 0x2
-#define PQI_RESET_TYPE_HARD_RESET 0x3
+ return rc;
+}
static int pqi_reset(struct pqi_ctrl_info *ctrl_info)
{
int rc;
- u32 reset_params;
+ union pqi_reset_register reset_reg;
- reset_params = (PQI_RESET_ACTION_RESET << 5) |
- PQI_RESET_TYPE_HARD_RESET;
+ if (ctrl_info->pqi_reset_quiesce_supported) {
+ rc = sis_pqi_reset_quiesce(ctrl_info);
+ if (rc) {
+ dev_err(&ctrl_info->pci_dev->dev,
+ "PQI reset failed during quiesce with error
%d\n",
+ rc);
+ return rc;
+ }
+ }
- writel(reset_params,
- &ctrl_info->pqi_registers->device_reset);
+ reset_reg.all_bits = 0;
+ reset_reg.bits.reset_type = PQI_RESET_TYPE_HARD_RESET;
+ reset_reg.bits.reset_action = PQI_RESET_ACTION_RESET;
- rc = pqi_wait_for_pqi_mode_ready(ctrl_info);
+ writel(reset_reg.all_bits, &ctrl_info->pqi_registers->device_reset);
+
+ rc = pqi_wait_for_pqi_reset_completion(ctrl_info);
if (rc)
dev_err(&ctrl_info->pci_dev->dev,
- "PQI reset failed\n");
+ "PQI reset failed with error %d\n", rc);
return rc;
}
diff --git a/drivers/scsi/smartpqi/smartpqi_sis.c
b/drivers/scsi/smartpqi/smartpqi_sis.c
index e55dfcf..9abbace 100644
--- a/drivers/scsi/smartpqi/smartpqi_sis.c
+++ b/drivers/scsi/smartpqi/smartpqi_sis.c
@@ -36,6 +36,7 @@
#define SIS_ENABLE_IN
[PATCH 5/7] smartpqi: update kexec and power down support
From: Kevin Barnett add PQI reset to driver shutdown callback to work around controller bug. During an 1.) OS shutdown or 2.) kexec outside of a kdump, the Linux kernel will clear BME on our controller. If BME is cleared during a controller/host PCIe transfer, the controller will lock up. So we perform a PQI reset in the driver's shutdown callback function to eliminate the possibility of a controller/host PCIe transfer being active when the kernel clears BME immediately after calling the driver's shutdown callback. Reviewed-by: Scott Benesh Signed-off-by: Kevin Barnett Signed-off-by: Don Brace --- drivers/scsi/smartpqi/smartpqi_init.c |1 + 1 file changed, 1 insertion(+) diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c index 70b1f97..afd3eed 100644 --- a/drivers/scsi/smartpqi/smartpqi_init.c +++ b/drivers/scsi/smartpqi/smartpqi_init.c @@ -6700,6 +6700,7 @@ static void pqi_shutdown(struct pci_dev *pci_dev) * storage. */ rc = pqi_flush_cache(ctrl_info, SHUTDOWN); + pqi_reset(ctrl_info); if (rc == 0) return;
[PATCH 4/7] smartpqi: cleanup doorbell register usage.
From: Kevin Barnett
Reviewed-by: Scott Benesh
Signed-off-by: Kevin Barnett
Signed-off-by: Don Brace
---
drivers/scsi/smartpqi/smartpqi_init.c | 11 ++-
drivers/scsi/smartpqi/smartpqi_sis.c | 105 +++--
drivers/scsi/smartpqi/smartpqi_sis.h |3 -
3 files changed, 17 insertions(+), 102 deletions(-)
diff --git a/drivers/scsi/smartpqi/smartpqi_init.c
b/drivers/scsi/smartpqi/smartpqi_init.c
index 3b05f28..70b1f97 100644
--- a/drivers/scsi/smartpqi/smartpqi_init.c
+++ b/drivers/scsi/smartpqi/smartpqi_init.c
@@ -3008,11 +3008,9 @@ static void pqi_change_irq_mode(struct pqi_ctrl_info
*ctrl_info,
break;
case IRQ_MODE_INTX:
pqi_configure_legacy_intx(ctrl_info, true);
- sis_disable_msix(ctrl_info);
sis_enable_intx(ctrl_info);
break;
case IRQ_MODE_NONE:
- sis_disable_msix(ctrl_info);
break;
}
break;
@@ -3020,14 +3018,12 @@ static void pqi_change_irq_mode(struct pqi_ctrl_info
*ctrl_info,
switch (new_mode) {
case IRQ_MODE_MSIX:
pqi_configure_legacy_intx(ctrl_info, false);
- sis_disable_intx(ctrl_info);
sis_enable_msix(ctrl_info);
break;
case IRQ_MODE_INTX:
break;
case IRQ_MODE_NONE:
pqi_configure_legacy_intx(ctrl_info, false);
- sis_disable_intx(ctrl_info);
break;
}
break;
@@ -6046,7 +6042,12 @@ static int pqi_revert_to_sis_mode(struct pqi_ctrl_info
*ctrl_info)
rc = pqi_reset(ctrl_info);
if (rc)
return rc;
- sis_reenable_sis_mode(ctrl_info);
+ rc = sis_reenable_sis_mode(ctrl_info);
+ if (rc) {
+ dev_err(&ctrl_info->pci_dev->dev,
+ "re-enabling SIS mode failed with error %d\n", rc);
+ return rc;
+ }
pqi_save_ctrl_mode(ctrl_info, SIS_MODE);
return 0;
diff --git a/drivers/scsi/smartpqi/smartpqi_sis.c
b/drivers/scsi/smartpqi/smartpqi_sis.c
index 9abbace..5141bd4 100644
--- a/drivers/scsi/smartpqi/smartpqi_sis.c
+++ b/drivers/scsi/smartpqi/smartpqi_sis.c
@@ -34,12 +34,13 @@
#define SIS_REENABLE_SIS_MODE 0x1
#define SIS_ENABLE_MSIX0x40
#define SIS_ENABLE_INTX0x80
-#define SIS_SOFT_RESET 0x100
+#define SIS_CMD_READY 0x200
#define SIS_TRIGGER_SHUTDOWN 0x80
#define SIS_PQI_RESET_QUIESCE 0x100
-#define SIS_CMD_READY 0x200
+
#define SIS_CMD_COMPLETE 0x1000
#define SIS_CLEAR_CTRL_TO_HOST_DOORBELL0x1000
+
#define SIS_CMD_STATUS_SUCCESS 0x1
#define SIS_CMD_COMPLETE_TIMEOUT_SECS 30
#define SIS_CMD_COMPLETE_POLL_INTERVAL_MSECS 10
@@ -373,66 +374,21 @@ static int sis_wait_for_doorbell_bit_to_clear(
return rc;
}
-/* Enable MSI-X interrupts on the controller. */
-
-void sis_enable_msix(struct pqi_ctrl_info *ctrl_info)
+static inline int sis_set_doorbell_bit(struct pqi_ctrl_info *ctrl_info, u32
bit)
{
- u32 doorbell_register;
-
- doorbell_register =
- readl(&ctrl_info->registers->sis_host_to_ctrl_doorbell);
- doorbell_register |= SIS_ENABLE_MSIX;
+ writel(bit, &ctrl_info->registers->sis_host_to_ctrl_doorbell);
- writel(doorbell_register,
- &ctrl_info->registers->sis_host_to_ctrl_doorbell);
-
- sis_wait_for_doorbell_bit_to_clear(ctrl_info, SIS_ENABLE_MSIX);
+ return sis_wait_for_doorbell_bit_to_clear(ctrl_info, bit);
}
-/* Disable MSI-X interrupts on the controller. */
-
-void sis_disable_msix(struct pqi_ctrl_info *ctrl_info)
+void sis_enable_msix(struct pqi_ctrl_info *ctrl_info)
{
- u32 doorbell_register;
-
- doorbell_register =
- readl(&ctrl_info->registers->sis_host_to_ctrl_doorbell);
- doorbell_register &= ~SIS_ENABLE_MSIX;
-
- writel(doorbell_register,
- &ctrl_info->registers->sis_host_to_ctrl_doorbell);
+ sis_set_doorbell_bit(ctrl_info, SIS_ENABLE_MSIX);
}
void sis_enable_intx(struct pqi_ctrl_info *ctrl_info)
{
- u32 doorbell_register;
-
- doorbell_register =
- readl(&ctrl_info->registers->sis_host_to_ctrl_doorbell);
- doorbell_register |= SIS_ENABLE_INTX;
-
- writel(doorbell_register,
- &ctrl_info->registers->sis_host_to_ctrl_doorbell);
-
- sis_wait_for_doorbell_bit_to_clear(ctrl_info, SIS_ENABLE_INTX);
-}
-
-void sis_disable_intx(struct pqi_ctrl_info *ctrl_info)
-{
- u32 doorbell_re
[PATCH 3/7] smartpqi: update pqi passthru ioctl
From: Kevin Barnett - make pass-thru requests bi-directional Reviewed-by: Scott Benesh Signed-off-by: Kevin Barnett Signed-off-by: Don Brace --- drivers/scsi/smartpqi/smartpqi_init.c |4 1 file changed, 4 insertions(+) diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c index b36d338..3b05f28 100644 --- a/drivers/scsi/smartpqi/smartpqi_init.c +++ b/drivers/scsi/smartpqi/smartpqi_init.c @@ -5499,6 +5499,7 @@ static int pqi_passthru_ioctl(struct pqi_ctrl_info *ctrl_info, void __user *arg) case XFER_NONE: case XFER_WRITE: case XFER_READ: + case XFER_READ | XFER_WRITE: break; default: return -EINVAL; @@ -5539,6 +5540,9 @@ static int pqi_passthru_ioctl(struct pqi_ctrl_info *ctrl_info, void __user *arg) case XFER_READ: request.data_direction = SOP_READ_FLAG; break; + case XFER_READ | XFER_WRITE: + request.data_direction = SOP_BIDIRECTIONAL; + break; } request.task_attribute = SOP_TASK_ATTRIBUTE_SIMPLE;
[PATCH 2/7] smartpqi: enhance BMIC cache flush
From: Kevin Barnett
- distinguish between shutdown and non-shutdown.
Reviewed-by: Scott Benesh
Signed-off-by: Kevin Barnett
Signed-off-by: Don Brace
---
drivers/scsi/smartpqi/smartpqi.h | 21 +++--
drivers/scsi/smartpqi/smartpqi_init.c | 27 ++-
2 files changed, 33 insertions(+), 15 deletions(-)
diff --git a/drivers/scsi/smartpqi/smartpqi.h b/drivers/scsi/smartpqi/smartpqi.h
index 6dd0449..dc3a054 100644
--- a/drivers/scsi/smartpqi/smartpqi.h
+++ b/drivers/scsi/smartpqi/smartpqi.h
@@ -1079,9 +1079,9 @@ enum pqi_ctrl_mode {
#define BMIC_SENSE_CONTROLLER_PARAMETERS 0x64
#define BMIC_SENSE_SUBSYSTEM_INFORMATION 0x66
#define BMIC_WRITE_HOST_WELLNESS 0xa5
-#define BMIC_CACHE_FLUSH 0xc2
+#define BMIC_FLUSH_CACHE 0xc2
-#define SA_CACHE_FLUSH 0x1
+#define SA_FLUSH_CACHE 0x1
#define MASKED_DEVICE(lunid) ((lunid)[3] & 0xc0)
#define CISS_GET_LEVEL_2_BUS(lunid)((lunid)[7] & 0x3f)
@@ -1187,6 +1187,23 @@ struct bmic_identify_physical_device {
u8 padding_to_multiple_of_512[9];
};
+struct bmic_flush_cache {
+ u8 disable_flag;
+ u8 system_power_action;
+ u8 ndu_flush;
+ u8 shutdown_event;
+ u8 reserved[28];
+};
+
+/* for shutdown_event member of struct bmic_flush_cache */
+enum bmic_flush_cache_shutdown_event {
+ NONE_CACHE_FLUSH_ONLY = 0,
+ SHUTDOWN = 1,
+ HIBERNATE = 2,
+ SUSPEND = 3,
+ RESTART = 4
+};
+
#pragma pack()
int pqi_add_sas_host(struct Scsi_Host *shost, struct pqi_ctrl_info *ctrl_info);
diff --git a/drivers/scsi/smartpqi/smartpqi_init.c
b/drivers/scsi/smartpqi/smartpqi_init.c
index ffdc32b..b36d338 100644
--- a/drivers/scsi/smartpqi/smartpqi_init.c
+++ b/drivers/scsi/smartpqi/smartpqi_init.c
@@ -431,10 +431,10 @@ static int pqi_build_raid_path_request(struct
pqi_ctrl_info *ctrl_info,
cdb[1] = CISS_GET_RAID_MAP;
put_unaligned_be32(buffer_length, &cdb[6]);
break;
- case SA_CACHE_FLUSH:
+ case SA_FLUSH_CACHE:
request->data_direction = SOP_WRITE_FLAG;
cdb[0] = BMIC_WRITE;
- cdb[6] = BMIC_CACHE_FLUSH;
+ cdb[6] = BMIC_FLUSH_CACHE;
put_unaligned_be16(buffer_length, &cdb[7]);
break;
case BMIC_IDENTIFY_CONTROLLER:
@@ -585,14 +585,13 @@ static int pqi_identify_physical_device(struct
pqi_ctrl_info *ctrl_info,
return rc;
}
-#define SA_CACHE_FLUSH_BUFFER_LENGTH 4
-
-static int pqi_flush_cache(struct pqi_ctrl_info *ctrl_info)
+static int pqi_flush_cache(struct pqi_ctrl_info *ctrl_info,
+ enum bmic_flush_cache_shutdown_event shutdown_event)
{
int rc;
struct pqi_raid_path_request request;
int pci_direction;
- u8 *buffer;
+ struct bmic_flush_cache *flush_cache;
/*
* Don't bother trying to flush the cache if the controller is
@@ -601,13 +600,15 @@ static int pqi_flush_cache(struct pqi_ctrl_info
*ctrl_info)
if (pqi_ctrl_offline(ctrl_info))
return -ENXIO;
- buffer = kzalloc(SA_CACHE_FLUSH_BUFFER_LENGTH, GFP_KERNEL);
- if (!buffer)
+ flush_cache = kzalloc(sizeof(*flush_cache), GFP_KERNEL);
+ if (!flush_cache)
return -ENOMEM;
+ flush_cache->shutdown_event = shutdown_event;
+
rc = pqi_build_raid_path_request(ctrl_info, &request,
- SA_CACHE_FLUSH, RAID_CTLR_LUNID, buffer,
- SA_CACHE_FLUSH_BUFFER_LENGTH, 0, &pci_direction);
+ SA_FLUSH_CACHE, RAID_CTLR_LUNID, flush_cache,
+ sizeof(*flush_cache), 0, &pci_direction);
if (rc)
goto out;
@@ -618,7 +619,7 @@ static int pqi_flush_cache(struct pqi_ctrl_info *ctrl_info)
pci_direction);
out:
- kfree(buffer);
+ kfree(flush_cache);
return rc;
}
@@ -6693,7 +6694,7 @@ static void pqi_shutdown(struct pci_dev *pci_dev)
* Write all data in the controller's battery-backed cache to
* storage.
*/
- rc = pqi_flush_cache(ctrl_info);
+ rc = pqi_flush_cache(ctrl_info, SHUTDOWN);
if (rc == 0)
return;
@@ -6737,7 +6738,7 @@ static __maybe_unused int pqi_suspend(struct pci_dev
*pci_dev, pm_message_t stat
pqi_cancel_rescan_worker(ctrl_info);
pqi_wait_until_scan_finished(ctrl_info);
pqi_wait_until_lun_reset_finished(ctrl_info);
- pqi_flush_cache(ctrl_info);
+ pqi_flush_cache(ctrl_info, SUSPEND);
pqi_ctrl_block_requests(ctrl_info);
pqi_ctrl_wait_until_quiesced(ctrl_info);
pqi_wait_until_inbound_queues_empty(ctrl_info);
[PATCH 0/7] smartpqi updates
These patches are based on Linus's tree The changes are: - smartpqi-add-pqi-reset-quiesce-support - allow driver to confirm completion of a reset. - smartpqi-enhance-bmic-cache-flush - can now distinguish between shutdown and power management operation. - smartpqi-update-pqi-passthru-ioctl - update DMA direction - smartpqi-cleanup-doorbell-register-usage - change how sis mode is re-enabled - smartpqi-update-kexec-power-down-support - reset controller on shutdown - smartpqi-add-in-new-controller-ids - update for latest hw - smartpqi-change-driver-version-to-1.1.2-125 --- Kevin Barnett (7): smartpqi: add pqi reset quiesce support smartpqi: enhance BMIC cache flush smartpqi: update pqi passthru ioctl smartpqi: cleanup doorbell register usage. smartpqi: update kexec and power down support smartpqi: add in new controller ids smartpqi: change driver version to 1.1.2-125 drivers/scsi/smartpqi/smartpqi.h | 44 ++ drivers/scsi/smartpqi/smartpqi_init.c | 145 - drivers/scsi/smartpqi/smartpqi_sis.c | 111 ++--- drivers/scsi/smartpqi/smartpqi_sis.h |4 - 4 files changed, 159 insertions(+), 145 deletions(-) -- Signature
[PATCH] scsi-sysfs: correct errno for host_reset
if scsi_host_template->host_reset is NULL, when user "echo adapter > /sys/class/scsi_host/hostx/host_reset", -EINVAL will return even string compare successfully. It make user confuse. change to: -EINVAL if string not match "adapter" / "firmware"; -EOPNOTSUPP if string match but not implement ->host_reset. Signed-off-by: weiping zhang --- drivers/scsi/scsi_sysfs.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c index dff8faf..3e664f4 100644 --- a/drivers/scsi/scsi_sysfs.c +++ b/drivers/scsi/scsi_sysfs.c @@ -272,6 +272,8 @@ store_host_reset(struct device *dev, struct device_attribute *attr, if (sht->host_reset) ret = sht->host_reset(shost, type); + else + ret = -EOPNOTSUPP; exit_store_host_reset: if (ret == 0) -- 2.9.4
Re: SCSI Oops: Unable to handle kernel NULL ptr dereference
On 08/10/17 10:05, man...@openmail.cc wrote: > I'd like to report this rare panic I experienced today. I've been on > 4.12.3 since it was released and got this panic totally unexpected, > probably when terminating my WL compositor. I attach to this message a > capture of the screen. The problem occurred never before and never after > since, suggesting I will not be able to reproduce it easily. Perhaps it > means something to someone. > > Linux air 4.12.3 #4 SMP Fri Jul 28 12:07:06 CEST 2017 x86_64 Intel(R) > Core(TM)i5-6267U CPU @ 2.90GHz GenuineIntel GNU/Linux Hello Cedric, A fix for this bug is available in kernel v4.12.4. Please upgrade your kernel. See also https://www.mail-archive.com/linux-scsi@vger.kernel.org/msg64543.html. Bart.
Re: [PATCH v2 2/2] virtio: virtio_scsi: Set can_queue to the length of the virtqueue.
On 10/08/2017 18:56, Richard W.M. Jones wrote:
> Since switching to blk-mq as the default in commit 5c279bd9e406
> ("scsi: default to scsi-mq"), virtio-scsi LUNs consume about 10x as
> much kernel memory.
>
> qemu currently allocates a fixed 128 entry virtqueue. can_queue
> currently is set to 1024. But with indirect descriptors, each command
> in the queue takes 1 virtqueue entry, so the number of commands which
> can be queued is equal to the length of the virtqueue.
>
> Note I intend to send a patch to qemu to allow the virtqueue size to
> be configured from the qemu command line.
>
> Thanks Paolo Bonzini, Christoph Hellwig.
>
> Signed-off-by: Richard W.M. Jones
> ---
> drivers/scsi/virtio_scsi.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
> index 9be211d68b15..7c28e8d4955a 100644
> --- a/drivers/scsi/virtio_scsi.c
> +++ b/drivers/scsi/virtio_scsi.c
> @@ -818,7 +818,6 @@ static struct scsi_host_template
> virtscsi_host_template_single = {
> .eh_timed_out = virtscsi_eh_timed_out,
> .slave_alloc = virtscsi_device_alloc,
>
> - .can_queue = 1024,
> .dma_boundary = UINT_MAX,
> .use_clustering = ENABLE_CLUSTERING,
> .target_alloc = virtscsi_target_alloc,
> @@ -839,7 +838,6 @@ static struct scsi_host_template
> virtscsi_host_template_multi = {
> .eh_timed_out = virtscsi_eh_timed_out,
> .slave_alloc = virtscsi_device_alloc,
>
> - .can_queue = 1024,
> .dma_boundary = UINT_MAX,
> .use_clustering = ENABLE_CLUSTERING,
> .target_alloc = virtscsi_target_alloc,
> @@ -972,6 +970,8 @@ static int virtscsi_probe(struct virtio_device *vdev)
> if (err)
> goto virtscsi_init_failed;
>
> + shost->can_queue = virtqueue_get_vring_size(vscsi->req_vqs[0].vq);
> +
> cmd_per_lun = virtscsi_config_get(vdev, cmd_per_lun) ?: 1;
> shost->cmd_per_lun = min_t(u32, cmd_per_lun, shost->can_queue);
> shost->max_sectors = virtscsi_config_get(vdev, max_sectors) ?: 0x;
>
Acked-by: Paolo Bonzini
[PATCH v2 1/2] virtio: Reduce BUG if total_sg > virtqueue size to WARN.
If using indirect descriptors, you can make the total_sg as large as
you want. If not, BUG is too serious because the function later
returns -ENOSPC.
Thanks Paolo Bonzini, Christoph Hellwig.
Signed-off-by: Richard W.M. Jones
Reviewed-by: Paolo Bonzini
---
drivers/virtio/virtio_ring.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 5e1b548828e6..27cbc1eab868 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -296,7 +296,6 @@ static inline int virtqueue_add(struct virtqueue *_vq,
}
#endif
- BUG_ON(total_sg > vq->vring.num);
BUG_ON(total_sg == 0);
head = vq->free_head;
@@ -305,8 +304,10 @@ static inline int virtqueue_add(struct virtqueue *_vq,
* buffers, then go indirect. FIXME: tune this threshold */
if (vq->indirect && total_sg > 1 && vq->vq.num_free)
desc = alloc_indirect(_vq, total_sg, gfp);
- else
+ else {
desc = NULL;
+ WARN_ON_ONCE(total_sg > vq->vring.num && !vq->indirect);
+ }
if (desc) {
/* Use a single buffer which doesn't continue */
--
2.13.1
[PATCH v2 2/2] virtio: virtio_scsi: Set can_queue to the length of the virtqueue.
Since switching to blk-mq as the default in commit 5c279bd9e406
("scsi: default to scsi-mq"), virtio-scsi LUNs consume about 10x as
much kernel memory.
qemu currently allocates a fixed 128 entry virtqueue. can_queue
currently is set to 1024. But with indirect descriptors, each command
in the queue takes 1 virtqueue entry, so the number of commands which
can be queued is equal to the length of the virtqueue.
Note I intend to send a patch to qemu to allow the virtqueue size to
be configured from the qemu command line.
Thanks Paolo Bonzini, Christoph Hellwig.
Signed-off-by: Richard W.M. Jones
---
drivers/scsi/virtio_scsi.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index 9be211d68b15..7c28e8d4955a 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -818,7 +818,6 @@ static struct scsi_host_template
virtscsi_host_template_single = {
.eh_timed_out = virtscsi_eh_timed_out,
.slave_alloc = virtscsi_device_alloc,
- .can_queue = 1024,
.dma_boundary = UINT_MAX,
.use_clustering = ENABLE_CLUSTERING,
.target_alloc = virtscsi_target_alloc,
@@ -839,7 +838,6 @@ static struct scsi_host_template
virtscsi_host_template_multi = {
.eh_timed_out = virtscsi_eh_timed_out,
.slave_alloc = virtscsi_device_alloc,
- .can_queue = 1024,
.dma_boundary = UINT_MAX,
.use_clustering = ENABLE_CLUSTERING,
.target_alloc = virtscsi_target_alloc,
@@ -972,6 +970,8 @@ static int virtscsi_probe(struct virtio_device *vdev)
if (err)
goto virtscsi_init_failed;
+ shost->can_queue = virtqueue_get_vring_size(vscsi->req_vqs[0].vq);
+
cmd_per_lun = virtscsi_config_get(vdev, cmd_per_lun) ?: 1;
shost->cmd_per_lun = min_t(u32, cmd_per_lun, shost->can_queue);
shost->max_sectors = virtscsi_config_get(vdev, max_sectors) ?: 0x;
--
2.13.1
[PATCH v2 0/2] virtio_scsi: Set can_queue based on size of virtqueue.
v1 was here: https://lkml.org/lkml/2017/8/10/689 v1 -> v2: Remove .can_queue field from the templates. Rich.
Re: [ezIX] #753: LSHW Causing Rewinds On LTO7 Drives
#753: LSHW Causing Rewinds On LTO7 Drives
--+-
Reporter: a.richman@… | Owner: lyonel
Type: defect | Status: new
Priority: major| Milestone:
Component: lshw |Version:
Resolution: | Keywords: lto lto7 rewind
--+-
Description changed by lyonel:
Old description:
> Hi,
>
> We're running into an issue with LTO7 drives on Linux, where LSHW will
> cause the drive to rewind when it queries it. We haven't seen this at
> all on LTO6 drives so it could be an issue with the LTO7 drivers but I
> figured I'd try here first.
>
> I tracked this down by adding debug logging to the scsi/st drivers on
> rewind, the logs below show it was the LSHW process that caused the
> rewind:
> [ 629.456892] st.c break 1 REZERO_UNIT
> [ 629.456897] CPU: 6 PID: 17091 Comm: lshw Not tainted
> 4.4.78-gentoo-r2-rev1.15 #1
> [ 629.456899] Hardware name: Supermicro X9SCL/X9SCM/X9SCL/X9SCM, BIOS
> 2.0b 09/17/2012
> [ 629.456901] 88034f243e30 8136536e
> 880409ff4800
> [ 629.456904] 0006 88034f243e98 a0427411
> 88034f243e48
> [ 629.456907] 88041bf6b808 00155cc02f7ce620
>
> [ 629.456909] Call Trace:
> [ 629.456913] [] dump_stack+0x63/0x85
> [ 629.456918] [] st_int_ioctl+0x401/0x1060 [st]
> [ 629.456922] [] st_flush+0x26f/0x500 [st]
> [ 629.456925] [] ? do_vfs_ioctl+0x283/0x460
> [ 629.456927] [] filp_close+0x2a/0x70
> [ 629.456930] [] __close_fd+0x8c/0xb0
> [ 629.456932] [] SyS_close+0x1e/0x50
> [ 629.456936] [] entry_SYSCALL_64_fastpath+0x16/0x71
>
> Haven't looked too closely at the LSHW source, but I assume it opens LTO
> devices to throw iocotls at them to get statistics etc? And this is
> somehow causing a rewind on LTO7 but not LTO6 or earlier.
>
> Any ideas what could be causing this?
>
> CC'd in linux-scsi in case it's a LTO7 driver specific problem.
>
> Thanks,
> - Alex.
New description:
Hi,
We're running into an issue with LTO7 drives on Linux, where LSHW will
cause the drive to rewind when it queries it. We haven't seen this at all
on LTO6 drives so it could be an issue with the LTO7 drivers but I figured
I'd try here first.
I tracked this down by adding debug logging to the scsi/st drivers on
rewind, the logs below show it was the LSHW process that caused the
rewind:
{{{
[ 629.456892] st.c break 1 REZERO_UNIT
[ 629.456897] CPU: 6 PID: 17091 Comm: lshw Not tainted
4.4.78-gentoo-r2-rev1.15 #1
[ 629.456899] Hardware name: Supermicro X9SCL/X9SCM/X9SCL/X9SCM, BIOS
2.0b 09/17/2012
[ 629.456901] 88034f243e30 8136536e
880409ff4800
[ 629.456904] 0006 88034f243e98 a0427411
88034f243e48
[ 629.456907] 88041bf6b808 00155cc02f7ce620
[ 629.456909] Call Trace:
[ 629.456913] [] dump_stack+0x63/0x85
[ 629.456918] [] st_int_ioctl+0x401/0x1060 [st]
[ 629.456922] [] st_flush+0x26f/0x500 [st]
[ 629.456925] [] ? do_vfs_ioctl+0x283/0x460
[ 629.456927] [] filp_close+0x2a/0x70
[ 629.456930] [] __close_fd+0x8c/0xb0
[ 629.456932] [] SyS_close+0x1e/0x50
[ 629.456936] [] entry_SYSCALL_64_fastpath+0x16/0x71
}}}
Haven't looked too closely at the LSHW source, but I assume it opens LTO
devices to throw iocotls at them to get statistics etc? And this is
somehow causing a rewind on LTO7 but not LTO6 or earlier.
Any ideas what could be causing this?
CC'd in linux-scsi in case it's a LTO7 driver specific problem.
Thanks,
- Alex.
--
--
Ticket URL:
ezIX
ezIX O/S
[ezIX] #753: LSHW Causing Rewinds On LTO7 Drives
#753: LSHW Causing Rewinds On LTO7 Drives -+ Reporter: a.richman@… | Owner: lyonel Type: defect | Status: new Priority: major| Milestone: Component: lshw |Version: Keywords: lto lto7 rewind | -+ Hi, We're running into an issue with LTO7 drives on Linux, where LSHW will cause the drive to rewind when it queries it. We haven't seen this at all on LTO6 drives so it could be an issue with the LTO7 drivers but I figured I'd try here first. I tracked this down by adding debug logging to the scsi/st drivers on rewind, the logs below show it was the LSHW process that caused the rewind: [ 629.456892] st.c break 1 REZERO_UNIT [ 629.456897] CPU: 6 PID: 17091 Comm: lshw Not tainted 4.4.78-gentoo-r2-rev1.15 #1 [ 629.456899] Hardware name: Supermicro X9SCL/X9SCM/X9SCL/X9SCM, BIOS 2.0b 09/17/2012 [ 629.456901] 88034f243e30 8136536e 880409ff4800 [ 629.456904] 0006 88034f243e98 a0427411 88034f243e48 [ 629.456907] 88041bf6b808 00155cc02f7ce620 [ 629.456909] Call Trace: [ 629.456913] [] dump_stack+0x63/0x85 [ 629.456918] [] st_int_ioctl+0x401/0x1060 [st] [ 629.456922] [] st_flush+0x26f/0x500 [st] [ 629.456925] [] ? do_vfs_ioctl+0x283/0x460 [ 629.456927] [] filp_close+0x2a/0x70 [ 629.456930] [] __close_fd+0x8c/0xb0 [ 629.456932] [] SyS_close+0x1e/0x50 [ 629.456936] [] entry_SYSCALL_64_fastpath+0x16/0x71 Haven't looked too closely at the LSHW source, but I assume it opens LTO devices to throw iocotls at them to get statistics etc? And this is somehow causing a rewind on LTO7 but not LTO6 or earlier. Any ideas what could be causing this? CC'd in linux-scsi in case it's a LTO7 driver specific problem. Thanks, - Alex. -- Ticket URL: ezIX ezIX O/S
Re: [PATCH 2/2] virtio: virtio_scsi: Set can_queue to the length of the virtqueue.
On 10/08/2017 18:40, Richard W.M. Jones wrote:
> Since switching to blk-mq as the default in commit 5c279bd9e406
> ("scsi: default to scsi-mq"), virtio-scsi LUNs consume about 10x as
> much kernel memory.
>
> qemu currently allocates a fixed 128 entry virtqueue. can_queue
> currently is set to 1024. But with indirect descriptors, each command
> in the queue takes 1 virtqueue entry, so the number of commands which
> can be queued is equal to the length of the virtqueue.
>
> Note I intend to send a patch to qemu to allow the virtqueue size to
> be configured from the qemu command line.
You can clear .can_queue from the templates. Otherwise looks good.
Paolo
> Thanks Paolo Bonzini, Christoph Hellwig.
>
> Signed-off-by: Richard W.M. Jones
> ---
> drivers/scsi/virtio_scsi.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
> index 9be211d68b15..d6b4ff634c0d 100644
> --- a/drivers/scsi/virtio_scsi.c
> +++ b/drivers/scsi/virtio_scsi.c
> @@ -972,6 +972,8 @@ static int virtscsi_probe(struct virtio_device *vdev)
> if (err)
> goto virtscsi_init_failed;
>
> + shost->can_queue = virtqueue_get_vring_size(vscsi->req_vqs[0].vq);
> +
> cmd_per_lun = virtscsi_config_get(vdev, cmd_per_lun) ?: 1;
> shost->cmd_per_lun = min_t(u32, cmd_per_lun, shost->can_queue);
> shost->max_sectors = virtscsi_config_get(vdev, max_sectors) ?: 0x;
>
Re: [PATCH 1/2] virtio: Reduce BUG if total_sg > virtqueue size to WARN.
On 10/08/2017 18:40, Richard W.M. Jones wrote:
> If using indirect descriptors, you can make the total_sg as large as
> you want. If not, BUG is too serious because the function later
> returns -ENOSPC.
>
> Thanks Paolo Bonzini, Christoph Hellwig.
>
> Signed-off-by: Richard W.M. Jones
> ---
> drivers/virtio/virtio_ring.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index 5e1b548828e6..27cbc1eab868 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -296,7 +296,6 @@ static inline int virtqueue_add(struct virtqueue *_vq,
> }
> #endif
>
> - BUG_ON(total_sg > vq->vring.num);
> BUG_ON(total_sg == 0);
>
> head = vq->free_head;
> @@ -305,8 +304,10 @@ static inline int virtqueue_add(struct virtqueue *_vq,
>* buffers, then go indirect. FIXME: tune this threshold */
> if (vq->indirect && total_sg > 1 && vq->vq.num_free)
> desc = alloc_indirect(_vq, total_sg, gfp);
> - else
> + else {
> desc = NULL;
> + WARN_ON_ONCE(total_sg > vq->vring.num && !vq->indirect);
So we get here only if vq->vq.num_free is zero. In that case,
vq->indirect makes no difference for the appropriateness of the warning
(that is, it should never be issued for indirect descriptors).
> + }
>
> if (desc) {
> /* Use a single buffer which doesn't continue */
>
Reviewed-by: Paolo Bonzini
Paolo
[PATCH 0/2] virtio_scsi: Set can_queue based on size of virtqueue.
Earlier discussion: https://lkml.org/lkml/2017/8/4/601 "Increased memory usage with scsi-mq" Downstream bug report: https://bugzilla.redhat.com/show_bug.cgi?id=1478201
[PATCH 1/2] virtio: Reduce BUG if total_sg > virtqueue size to WARN.
If using indirect descriptors, you can make the total_sg as large as
you want. If not, BUG is too serious because the function later
returns -ENOSPC.
Thanks Paolo Bonzini, Christoph Hellwig.
Signed-off-by: Richard W.M. Jones
---
drivers/virtio/virtio_ring.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 5e1b548828e6..27cbc1eab868 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -296,7 +296,6 @@ static inline int virtqueue_add(struct virtqueue *_vq,
}
#endif
- BUG_ON(total_sg > vq->vring.num);
BUG_ON(total_sg == 0);
head = vq->free_head;
@@ -305,8 +304,10 @@ static inline int virtqueue_add(struct virtqueue *_vq,
* buffers, then go indirect. FIXME: tune this threshold */
if (vq->indirect && total_sg > 1 && vq->vq.num_free)
desc = alloc_indirect(_vq, total_sg, gfp);
- else
+ else {
desc = NULL;
+ WARN_ON_ONCE(total_sg > vq->vring.num && !vq->indirect);
+ }
if (desc) {
/* Use a single buffer which doesn't continue */
--
2.13.1
[PATCH 2/2] virtio: virtio_scsi: Set can_queue to the length of the virtqueue.
Since switching to blk-mq as the default in commit 5c279bd9e406
("scsi: default to scsi-mq"), virtio-scsi LUNs consume about 10x as
much kernel memory.
qemu currently allocates a fixed 128 entry virtqueue. can_queue
currently is set to 1024. But with indirect descriptors, each command
in the queue takes 1 virtqueue entry, so the number of commands which
can be queued is equal to the length of the virtqueue.
Note I intend to send a patch to qemu to allow the virtqueue size to
be configured from the qemu command line.
Thanks Paolo Bonzini, Christoph Hellwig.
Signed-off-by: Richard W.M. Jones
---
drivers/scsi/virtio_scsi.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index 9be211d68b15..d6b4ff634c0d 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -972,6 +972,8 @@ static int virtscsi_probe(struct virtio_device *vdev)
if (err)
goto virtscsi_init_failed;
+ shost->can_queue = virtqueue_get_vring_size(vscsi->req_vqs[0].vq);
+
cmd_per_lun = virtscsi_config_get(vdev, cmd_per_lun) ?: 1;
shost->cmd_per_lun = min_t(u32, cmd_per_lun, shost->can_queue);
shost->max_sectors = virtscsi_config_get(vdev, max_sectors) ?: 0x;
--
2.13.1
Re: Increased memory usage with scsi-mq
On 10/08/2017 17:40, Richard W.M. Jones wrote:
> OK this is looking a bit better now.
>
> With scsi-mq enabled: 175 disks
> virtqueue_size=64: 318 disks *
> virtqueue_size=16: 775 disks *
> With scsi-mq disabled: 1755 disks
> * = new results
>
> I also ran the whole libguestfs test suite with virtqueue_size=16
> (with no failures shown). As this tests many different disk I/O
> operations, it gives me some confidence that things generally work.
Yes, it looks good. I'll grab you on IRC to discuss the commit message.
Paolo
> Do you have any other comments about the patches? I'm not sure I know
> enough to write an intelligent commit message for the kernel patch.
>
> Rich.
>
> --- kernel patch ---
>
> diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
> index 9be211d68b15..d6b4ff634c0d 100644
> --- a/drivers/scsi/virtio_scsi.c
> +++ b/drivers/scsi/virtio_scsi.c
> @@ -972,6 +972,8 @@ static int virtscsi_probe(struct virtio_device *vdev)
> if (err)
> goto virtscsi_init_failed;
>
> + shost->can_queue = virtqueue_get_vring_size(vscsi->req_vqs[0].vq);
> +
> cmd_per_lun = virtscsi_config_get(vdev, cmd_per_lun) ?: 1;
> shost->cmd_per_lun = min_t(u32, cmd_per_lun, shost->can_queue);
> shost->max_sectors = virtscsi_config_get(vdev, max_sectors) ?: 0x;
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index 5e1b548828e6..2d7509da9f39 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -296,7 +296,6 @@ static inline int virtqueue_add(struct virtqueue *_vq,
> }
> #endif
>
> - BUG_ON(total_sg > vq->vring.num);
> BUG_ON(total_sg == 0);
>
> head = vq->free_head;
> @@ -305,8 +304,10 @@ static inline int virtqueue_add(struct virtqueue *_vq,
>* buffers, then go indirect. FIXME: tune this threshold */
> if (vq->indirect && total_sg > 1 && vq->vq.num_free)
> desc = alloc_indirect(_vq, total_sg, gfp);
> - else
> + else {
> desc = NULL;
> + WARN_ON_ONCE(total_sg > vq->vring.num && !vq->indirect);
> +}
>
> if (desc) {
> /* Use a single buffer which doesn't continue */
>
> --- qemu patch ---
>
> diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
> index eb639442d1..aadd99aad1 100644
> --- a/hw/scsi/virtio-scsi.c
> +++ b/hw/scsi/virtio-scsi.c
> @@ -867,10 +867,10 @@ void virtio_scsi_common_realize(DeviceState *dev,
> s->sense_size = VIRTIO_SCSI_SENSE_DEFAULT_SIZE;
> s->cdb_size = VIRTIO_SCSI_CDB_DEFAULT_SIZE;
>
> -s->ctrl_vq = virtio_add_queue(vdev, VIRTIO_SCSI_VQ_SIZE, ctrl);
> -s->event_vq = virtio_add_queue(vdev, VIRTIO_SCSI_VQ_SIZE, evt);
> +s->ctrl_vq = virtio_add_queue(vdev, s->conf.virtqueue_size, ctrl);
> +s->event_vq = virtio_add_queue(vdev, s->conf.virtqueue_size, evt);
> for (i = 0; i < s->conf.num_queues; i++) {
> -s->cmd_vqs[i] = virtio_add_queue(vdev, VIRTIO_SCSI_VQ_SIZE, cmd);
> +s->cmd_vqs[i] = virtio_add_queue(vdev, s->conf.virtqueue_size, cmd);
> }
> }
>
> @@ -917,6 +917,7 @@ static void virtio_scsi_device_unrealize(DeviceState
> *dev, Error **errp)
>
> static Property virtio_scsi_properties[] = {
> DEFINE_PROP_UINT32("num_queues", VirtIOSCSI, parent_obj.conf.num_queues,
> 1),
> +DEFINE_PROP_UINT32("virtqueue_size", VirtIOSCSI,
> parent_obj.conf.virtqueue_size, 128),
> DEFINE_PROP_UINT32("max_sectors", VirtIOSCSI,
> parent_obj.conf.max_sectors,
>0x),
> DEFINE_PROP_UINT32("cmd_per_lun", VirtIOSCSI,
> parent_obj.conf.cmd_per_lun,
> diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h
> index de6ae5a9f6..e30a92d3e7 100644
> --- a/include/hw/virtio/virtio-scsi.h
> +++ b/include/hw/virtio/virtio-scsi.h
> @@ -48,6 +48,7 @@ typedef struct virtio_scsi_config VirtIOSCSIConfig;
>
> struct VirtIOSCSIConf {
> uint32_t num_queues;
> +uint32_t virtqueue_size;
> uint32_t max_sectors;
> uint32_t cmd_per_lun;
> #ifdef CONFIG_VHOST_SCSI
>
>
>
Re: [PATCH] scsi-mq: Always unprepare before requeuing a request
On 08/10/2017 10:26 AM, Bart Van Assche wrote: > On Thu, 2017-08-10 at 20:32 +1000, Michael Ellerman wrote: >> "Martin K. Petersen" writes: One of the two scsi-mq functions that requeue a request unprepares a request before requeueing (scsi_io_completion()) but the other function not (__scsi_queue_insert()). Make sure that a request is unprepared before requeuing it. >>> >>> Applied to 4.13/scsi-fixes. Thanks much! >> >> This seems to be preventing my Power8 box, which uses IPR, from booting. >> >> Bisect said so: >> >> # first bad commit: [270065e92c317845d69095ec8e3d18616b5b39d5] scsi: >> scsi-mq: Always unprepare before requeuing a request >> >> And if I revert that on top of next-20170809 my system boots again. >> >> The symptom is that it just gets "stuck" during boot and never gets to >> mounting root, full log below, the end is: >> >> . >> ready >> ready >> sd 0:2:4:0: [sde] 554287104 512-byte logical blocks: (284 GB/264 GiB) >> sd 0:2:4:0: [sde] 4096-byte physical blocks >> sd 0:2:5:0: [sdf] 272646144 512-byte logical blocks: (140 GB/130 GiB) >> sd 0:2:5:0: [sdf] 4096-byte physical blocks >> sd 0:2:4:0: [sde] Write Protect is off >> sd 0:2:4:0: [sde] Mode Sense: 0b 00 00 08 >> sd 0:2:5:0: [sdf] Write Protect is off >> sd 0:2:5:0: [sdf] Mode Sense: 0b 00 00 08 >> >> >> And it just sits there for at least hours. >> >> I compared a good and bad boot log and there appears to be essentially >> no difference. Certainly nothing that looks suspicous. > > Hello Michael, > > Thanks for having reported this early. Is there any chance that you can > reproduce this state, press SysRq-w on the console and collect the task > overview that is reported on the console (see also Documentation/admin-guide/ > sysrq.rst)? If this is not possible or if that task overview does not report > any blocked tasks, can you add scsi_mod.scsi_logging_level=-1 to the kernel > command line (either through /etc/default/grub or in /boot/grub2/grub.cfg > when using GRUB), reboot the system, capture the console output and report > that output as a reply to this e-mail? I'm building a kernel to try to reproduce this on a machine with ipr. -Brian -- Brian King Power Linux I/O IBM Linux Technology Center
[PATCH 06/19] scsi: hisi_sas: remove repeated device config in v2 hw
From: Xiang Chen This patch removes some repeated configurations: (1) The device id of the device is already set in the alloc function, so we don't need to modify in free device function. (2) Field dev_type and dev_status are configured in hisi_sas_dev_gone(), so there is no need for repeated config in free_device_v3_hw. Signed-off-by: Xiang Chen Signed-off-by: John Garry --- drivers/scsi/hisi_sas/hisi_sas_main.c | 3 --- drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 2 -- 2 files changed, 5 deletions(-) diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c index aaa7296..81ad6cd 100644 --- a/drivers/scsi/hisi_sas/hisi_sas_main.c +++ b/drivers/scsi/hisi_sas/hisi_sas_main.c @@ -716,7 +716,6 @@ static void hisi_sas_dev_gone(struct domain_device *device) struct hisi_sas_device *sas_dev = device->lldd_dev; struct hisi_hba *hisi_hba = dev_to_hisi_hba(device); struct device *dev = hisi_hba->dev; - int dev_id = sas_dev->device_id; dev_info(dev, "found dev[%d:%x] is gone\n", sas_dev->device_id, sas_dev->dev_type); @@ -729,9 +728,7 @@ static void hisi_sas_dev_gone(struct domain_device *device) hisi_hba->hw->free_device(hisi_hba, sas_dev); device->lldd_dev = NULL; memset(sas_dev, 0, sizeof(*sas_dev)); - sas_dev->device_id = dev_id; sas_dev->dev_type = SAS_PHY_UNUSED; - sas_dev->dev_status = HISI_SAS_DEV_NORMAL; } static int hisi_sas_queue_command(struct sas_task *task, gfp_t gfp_flags) diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c index 83d2dca..dc5c551 100644 --- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c +++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c @@ -578,8 +578,6 @@ static void free_device_v3_hw(struct hisi_hba *hisi_hba, memset(itct, 0, sizeof(struct hisi_sas_itct)); hisi_sas_write32(hisi_hba, ENT_INT_SRC3, ENT_INT_SRC3_ITC_INT_MSK); - hisi_hba->devices[dev_id].dev_type = SAS_PHY_UNUSED; - hisi_hba->devices[dev_id].dev_status = HISI_SAS_DEV_NORMAL; /* clear the itct */ hisi_sas_write32(hisi_hba, ITCT_CLR, 0); -- 1.9.1
[PATCH 05/19] scsi: hisi_sas: use array for v2 hw ECC errors
The code to print ECC errors in v2 hw driver is very
repetitive.
This patch condensed the code by looping an array of
errors.
Signed-off-by: John Garry
Signed-off-by: Shiju Jose
---
drivers/scsi/hisi_sas/hisi_sas.h | 8 +
drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 368 +
2 files changed, 197 insertions(+), 179 deletions(-)
diff --git a/drivers/scsi/hisi_sas/hisi_sas.h b/drivers/scsi/hisi_sas/hisi_sas.h
index ef2238c..ad6b2d1 100644
--- a/drivers/scsi/hisi_sas/hisi_sas.h
+++ b/drivers/scsi/hisi_sas/hisi_sas.h
@@ -91,6 +91,14 @@ enum hisi_sas_dev_type {
HISI_SAS_DEV_TYPE_SATA,
};
+struct hisi_sas_hw_error {
+ u32 irq_msk;
+ u32 msk;
+ int shift;
+ const char *msg;
+ int reg;
+};
+
struct hisi_sas_phy {
struct hisi_hba *hisi_hba;
struct hisi_sas_port*port;
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
index 41e8033..bcbc16e 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
@@ -401,6 +401,172 @@ struct hisi_sas_err_record_v2 {
__le32 dma_rx_err_type;
};
+static const struct hisi_sas_hw_error one_bit_ecc_errors[] = {
+ {
+ .irq_msk = BIT(SAS_ECC_INTR_DQE_ECC_1B_OFF),
+ .msk = HGC_DQE_ECC_1B_ADDR_MSK,
+ .shift = HGC_DQE_ECC_1B_ADDR_OFF,
+ .msg = "hgc_dqe_acc1b_intr found: \
+ Ram address is 0x%08X\n",
+ .reg = HGC_DQE_ECC_ADDR,
+ },
+ {
+ .irq_msk = BIT(SAS_ECC_INTR_IOST_ECC_1B_OFF),
+ .msk = HGC_IOST_ECC_1B_ADDR_MSK,
+ .shift = HGC_IOST_ECC_1B_ADDR_OFF,
+ .msg = "hgc_iost_acc1b_intr found: \
+ Ram address is 0x%08X\n",
+ .reg = HGC_IOST_ECC_ADDR,
+ },
+ {
+ .irq_msk = BIT(SAS_ECC_INTR_ITCT_ECC_1B_OFF),
+ .msk = HGC_ITCT_ECC_1B_ADDR_MSK,
+ .shift = HGC_ITCT_ECC_1B_ADDR_OFF,
+ .msg = "hgc_itct_acc1b_intr found: \
+ Ram address is 0x%08X\n",
+ .reg = HGC_ITCT_ECC_ADDR,
+ },
+ {
+ .irq_msk = BIT(SAS_ECC_INTR_IOSTLIST_ECC_1B_OFF),
+ .msk = HGC_LM_DFX_STATUS2_IOSTLIST_MSK,
+ .shift = HGC_LM_DFX_STATUS2_IOSTLIST_OFF,
+ .msg = "hgc_iostl_acc1b_intr found: \
+ memory address is 0x%08X\n",
+ .reg = HGC_LM_DFX_STATUS2,
+ },
+ {
+ .irq_msk = BIT(SAS_ECC_INTR_ITCTLIST_ECC_1B_OFF),
+ .msk = HGC_LM_DFX_STATUS2_ITCTLIST_MSK,
+ .shift = HGC_LM_DFX_STATUS2_ITCTLIST_OFF,
+ .msg = "hgc_itctl_acc1b_intr found: \
+ memory address is 0x%08X\n",
+ .reg = HGC_LM_DFX_STATUS2,
+ },
+ {
+ .irq_msk = BIT(SAS_ECC_INTR_CQE_ECC_1B_OFF),
+ .msk = HGC_CQE_ECC_1B_ADDR_MSK,
+ .shift = HGC_CQE_ECC_1B_ADDR_OFF,
+ .msg = "hgc_cqe_acc1b_intr found: \
+ Ram address is 0x%08X\n",
+ .reg = HGC_CQE_ECC_ADDR,
+ },
+ {
+ .irq_msk = BIT(SAS_ECC_INTR_NCQ_MEM0_ECC_1B_OFF),
+ .msk = HGC_RXM_DFX_STATUS14_MEM0_MSK,
+ .shift = HGC_RXM_DFX_STATUS14_MEM0_OFF,
+ .msg = "rxm_mem0_acc1b_intr found: \
+ memory address is 0x%08X\n",
+ .reg = HGC_RXM_DFX_STATUS14,
+ },
+ {
+ .irq_msk = BIT(SAS_ECC_INTR_NCQ_MEM1_ECC_1B_OFF),
+ .msk = HGC_RXM_DFX_STATUS14_MEM1_MSK,
+ .shift = HGC_RXM_DFX_STATUS14_MEM1_OFF,
+ .msg = "rxm_mem1_acc1b_intr found: \
+ memory address is 0x%08X\n",
+ .reg = HGC_RXM_DFX_STATUS14,
+ },
+ {
+ .irq_msk = BIT(SAS_ECC_INTR_NCQ_MEM2_ECC_1B_OFF),
+ .msk = HGC_RXM_DFX_STATUS14_MEM2_MSK,
+ .shift = HGC_RXM_DFX_STATUS14_MEM2_OFF,
+ .msg = "rxm_mem2_acc1b_intr found: \
+ memory address is 0x%08X\n",
+ .reg = HGC_RXM_DFX_STATUS14,
+ },
+ {
+ .irq_msk = BIT(SAS_ECC_INTR_NCQ_MEM3_ECC_1B_OFF),
+ .msk = HGC_RXM_DFX_STATUS15_MEM3_MSK,
+ .shift = HGC_RXM_DFX_STATUS15_MEM3_OFF,
+ .msg = "rxm_mem3_acc1b_intr found: \
+ memory address is 0x%08X\n",
+ .reg = HGC_RXM_DFX_STATUS15,
+ },
+};
+
+static const struct hisi_sas_hw_error multi_bit_ecc_errors[] = {
+ {
+ .irq_msk = BIT(SAS_ECC_INTR_DQE_ECC_MB_OFF),
+ .msk = HGC_DQE_ECC_MB_ADDR_MSK,
+ .shift = HGC_DQE_ECC_MB_ADDR_OFF,
+ .msg = "hgc_dqe_accbad_int
[PATCH 08/19] scsi: hisi_sas: service interrupt ITCT_CLR interrupt in v2 hw
From: Xiang Chen
This patch is a fix related to free'ing a device in
v2 hw driver.
Before, we polled to ITCT CLR interrupt to check if
a device is free.
This was error prone, as if the interrupt doesn't occur
in 10us, we miss processing it.
To avoid this situation, service this interrupt and
sync the event with a completion.
Signed-off-by: Xiang Chen
Signed-off-by: John Garry
---
drivers/scsi/hisi_sas/hisi_sas.h | 1 +
drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 40 --
2 files changed, 20 insertions(+), 21 deletions(-)
diff --git a/drivers/scsi/hisi_sas/hisi_sas.h b/drivers/scsi/hisi_sas/hisi_sas.h
index ad6b2d1..23a22dc 100644
--- a/drivers/scsi/hisi_sas/hisi_sas.h
+++ b/drivers/scsi/hisi_sas/hisi_sas.h
@@ -141,6 +141,7 @@ struct hisi_sas_dq {
struct hisi_sas_device {
struct hisi_hba *hisi_hba;
struct domain_device*sas_device;
+ struct completion *completion;
struct hisi_sas_dq *dq;
struct list_headlist;
u64 attached_phy;
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
index 9eea0b4..0e3634e 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
@@ -974,12 +974,14 @@ static void setup_itct_v2_hw(struct hisi_hba *hisi_hba,
static void free_device_v2_hw(struct hisi_hba *hisi_hba,
struct hisi_sas_device *sas_dev)
{
+ DECLARE_COMPLETION_ONSTACK(completion);
u64 dev_id = sas_dev->device_id;
- struct device *dev = hisi_hba->dev;
struct hisi_sas_itct *itct = &hisi_hba->itct[dev_id];
u32 reg_val = hisi_sas_read32(hisi_hba, ENT_INT_SRC3);
int i;
+ sas_dev->completion = &completion;
+
/* SoC bug workaround */
if (dev_is_sata(sas_dev->sas_device))
clear_bit(sas_dev->sata_idx, hisi_hba->sata_dev_bitmap);
@@ -989,28 +991,12 @@ static void free_device_v2_hw(struct hisi_hba *hisi_hba,
hisi_sas_write32(hisi_hba, ENT_INT_SRC3,
ENT_INT_SRC3_ITC_INT_MSK);
- /* clear the itct int*/
for (i = 0; i < 2; i++) {
- /* clear the itct table*/
- reg_val = hisi_sas_read32(hisi_hba, ITCT_CLR);
- reg_val |= ITCT_CLR_EN_MSK | (dev_id & ITCT_DEV_MSK);
+ reg_val = ITCT_CLR_EN_MSK | (dev_id & ITCT_DEV_MSK);
hisi_sas_write32(hisi_hba, ITCT_CLR, reg_val);
+ wait_for_completion(sas_dev->completion);
- udelay(10);
- reg_val = hisi_sas_read32(hisi_hba, ENT_INT_SRC3);
- if (ENT_INT_SRC3_ITC_INT_MSK & reg_val) {
- dev_dbg(dev, "got clear ITCT done interrupt\n");
-
- /* invalid the itct state*/
- memset(itct, 0, sizeof(struct hisi_sas_itct));
- hisi_sas_write32(hisi_hba, ENT_INT_SRC3,
-ENT_INT_SRC3_ITC_INT_MSK);
-
- /* clear the itct */
- hisi_sas_write32(hisi_hba, ITCT_CLR, 0);
- dev_dbg(dev, "clear ITCT ok\n");
- break;
- }
+ memset(itct, 0, sizeof(struct hisi_sas_itct));
}
}
@@ -1191,7 +1177,7 @@ static void init_reg_v2_hw(struct hisi_hba *hisi_hba)
hisi_sas_write32(hisi_hba, ENT_INT_SRC3, 0x);
hisi_sas_write32(hisi_hba, ENT_INT_SRC_MSK1, 0x7efefefe);
hisi_sas_write32(hisi_hba, ENT_INT_SRC_MSK2, 0x7efefefe);
- hisi_sas_write32(hisi_hba, ENT_INT_SRC_MSK3, 0x7ffe);
+ hisi_sas_write32(hisi_hba, ENT_INT_SRC_MSK3, 0x7ffe20fe);
hisi_sas_write32(hisi_hba, SAS_ECC_INTR_MSK, 0xfff00c30);
for (i = 0; i < hisi_hba->queue_count; i++)
hisi_sas_write32(hisi_hba, OQ0_INT_SRC_MSK+0x4*i, 0);
@@ -3092,8 +3078,20 @@ static irqreturn_t fatal_axi_int_v2_hw(int irq_no, void
*p)
irq_value);
queue_work(hisi_hba->wq, &hisi_hba->rst_work);
}
+
+ if (irq_value & BIT(ENT_INT_SRC3_ITC_INT_OFF)) {
+ u32 reg_val = hisi_sas_read32(hisi_hba, ITCT_CLR);
+ u32 dev_id = reg_val & ITCT_DEV_MSK;
+ struct hisi_sas_device *sas_dev =
+ &hisi_hba->devices[dev_id];
+
+ hisi_sas_write32(hisi_hba, ITCT_CLR, 0);
+ dev_dbg(dev, "clear ITCT ok\n");
+ complete(sas_dev->completion);
+ }
}
+ hisi_sas_write32(hisi_hba, ENT_INT_SRC3, irq_value);
hisi_sas_write32(hisi_hba, ENT_INT_SRC_MSK3, irq_msk);
return IRQ_HANDLED;
--
1.9.1
[PATCH 01/19] scsi: hisi_sas: fix reset and port ID refresh issues
From: Xiaofei Tan
This patch provides fixes for the following issues:
1. Fix issue of controller reset required to send
commands. For reset process, it may be required to send
commands to the controller, but not during soft reset.
So add HISI_SAS_NOT_ACCEPT_CMD_BIT to prevent executing a
task during this period.
2. Send a broadcast event in rescan topology to detect any
topology changes during reset.
3. Previously it was not ensured that libsas has processed
the PHY up and down events after reset. Potentially this
could cause an issue that we still process the PHY event
after reset. So resolve this by flushing shot workqueue in
LLDD reset.
4. Port ID requires refresh after reset. The port ID
generated after reset is not guaranteed to be the same as
before reset, so it needs to be refreshed for each device's
ITCT.
Signed-off-by: Xiaofei Tan
Signed-off-by: John Garry
---
drivers/scsi/hisi_sas/hisi_sas.h | 4 +-
drivers/scsi/hisi_sas/hisi_sas_main.c | 152 ++---
drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 36
3 files changed, 118 insertions(+), 74 deletions(-)
diff --git a/drivers/scsi/hisi_sas/hisi_sas.h b/drivers/scsi/hisi_sas/hisi_sas.h
index a722f2b..3c4defa 100644
--- a/drivers/scsi/hisi_sas/hisi_sas.h
+++ b/drivers/scsi/hisi_sas/hisi_sas.h
@@ -33,6 +33,7 @@
#define HISI_SAS_MAX_ITCT_ENTRIES 2048
#define HISI_SAS_MAX_DEVICES HISI_SAS_MAX_ITCT_ENTRIES
#define HISI_SAS_RESET_BIT 0
+#define HISI_SAS_REJECT_CMD_BIT1
#define HISI_SAS_STATUS_BUF_SZ (sizeof(struct hisi_sas_status_buffer))
#define HISI_SAS_COMMAND_TABLE_SZ (sizeof(union hisi_sas_command_table))
@@ -201,6 +202,7 @@ struct hisi_sas_hw {
void (*dereg_device)(struct hisi_hba *hisi_hba,
struct domain_device *device);
int (*soft_reset)(struct hisi_hba *hisi_hba);
+ u32 (*get_phys_state)(struct hisi_hba *hisi_hba);
int max_command_entries;
int complete_hdr_size;
};
@@ -408,6 +410,4 @@ extern void hisi_sas_slot_task_free(struct hisi_hba
*hisi_hba,
struct sas_task *task,
struct hisi_sas_slot *slot);
extern void hisi_sas_init_mem(struct hisi_hba *hisi_hba);
-extern void hisi_sas_rescan_topology(struct hisi_hba *hisi_hba, u32 old_state,
-u32 state);
#endif
diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c
b/drivers/scsi/hisi_sas/hisi_sas_main.c
index 4022c3f..bd1d619 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -433,7 +433,7 @@ static int hisi_sas_task_exec(struct sas_task *task, gfp_t
gfp_flags,
struct hisi_sas_device *sas_dev = device->lldd_dev;
struct hisi_sas_dq *dq = sas_dev->dq;
- if (unlikely(test_bit(HISI_SAS_RESET_BIT, &hisi_hba->flags)))
+ if (unlikely(test_bit(HISI_SAS_REJECT_CMD_BIT, &hisi_hba->flags)))
return -EINVAL;
/* protect task_prep and start_delivery sequence */
@@ -967,37 +967,117 @@ static int hisi_sas_debug_issue_ssp_tmf(struct
domain_device *device,
sizeof(ssp_task), tmf);
}
+static void hisi_sas_refresh_port_id(struct hisi_hba *hisi_hba,
+ struct asd_sas_port *sas_port, enum sas_linkrate linkrate)
+{
+ struct hisi_sas_device *sas_dev;
+ struct domain_device *device;
+ int i;
+
+ for (i = 0; i < HISI_SAS_MAX_DEVICES; i++) {
+ sas_dev = &hisi_hba->devices[i];
+ device = sas_dev->sas_device;
+ if ((sas_dev->dev_type == SAS_PHY_UNUSED)
+ || !device || (device->port != sas_port))
+ continue;
+
+ hisi_hba->hw->free_device(hisi_hba, sas_dev);
+
+ /* Update linkrate of directly attached device. */
+ if (!device->parent)
+ device->linkrate = linkrate;
+
+ hisi_hba->hw->setup_itct(hisi_hba, sas_dev);
+ }
+}
+
+static void hisi_sas_rescan_topology(struct hisi_hba *hisi_hba, u32 old_state,
+ u32 state)
+{
+ struct sas_ha_struct *sas_ha = &hisi_hba->sha;
+ struct asd_sas_port *_sas_port = NULL;
+ int phy_no;
+
+ for (phy_no = 0; phy_no < hisi_hba->n_phy; phy_no++) {
+ struct hisi_sas_phy *phy = &hisi_hba->phy[phy_no];
+ struct asd_sas_phy *sas_phy = &phy->sas_phy;
+ struct asd_sas_port *sas_port = sas_phy->port;
+ struct hisi_sas_port *port = to_hisi_sas_port(sas_port);
+ bool do_port_check = !!(_sas_port != sas_port);
+
+ if (!sas_phy->phy->enabled)
+ continue;
+
+ /* Report PHY state change to libsas */
+ if (state & (1 << phy_no)) {
+ if (do_port_check && sas_port) {
+ struct domain_device *dev = sas
[PATCH 03/19] scsi: hisi_sas: fix v2 hw underflow residual value
From: Xiang Chen
The value dw0 is the residual bytes when UNDERFLOW error
happens, but we filled the residual with the value of dw3
before. So change the residual from dw3 to dw0.
Signed-off-by: Xiang Chen
Signed-off-by: John Garry
---
drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
index 8c504b4..a762b25 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
@@ -1972,7 +1972,7 @@ static void slot_err_v2_hw(struct hisi_hba *hisi_hba,
}
case DMA_RX_DATA_LEN_UNDERFLOW:
{
- ts->residual = dma_rx_err_type;
+ ts->residual = trans_tx_fail_type;
ts->stat = SAS_DATA_UNDERRUN;
break;
}
@@ -2098,7 +2098,7 @@ static void slot_err_v2_hw(struct hisi_hba *hisi_hba,
}
case DMA_RX_DATA_LEN_UNDERFLOW:
{
- ts->residual = dma_rx_err_type;
+ ts->residual = trans_tx_fail_type;
ts->stat = SAS_DATA_UNDERRUN;
break;
}
--
1.9.1
[PATCH 11/19] scsi: hisi_sas: Modify v3 hw STP_LINK_TIMER setting
From: Xiang Chen
Modify STP link timer from 10ms to 500ms. Also add
the register address.
Signed-off-by: Xiang Chen
Signed-off-by: John Garry
---
drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
index dc5c551..bb79b776 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
@@ -137,6 +137,7 @@
#define TX_HARDRST_MSK (0x1 << TX_HARDRST_OFF)
#define RX_IDAF_DWORD0 (PORT_BASE + 0xc4)
#define RXOP_CHECK_CFG_H (PORT_BASE + 0xfc)
+#define STP_LINK_TIMER (PORT_BASE + 0x120)
#define SAS_SSP_CON_TIMER_CFG (PORT_BASE + 0x134)
#define SAS_SMP_CON_TIMER_CFG (PORT_BASE + 0x138)
#define SAS_STP_CON_TIMER_CFG (PORT_BASE + 0x13c)
@@ -401,6 +402,8 @@ static void init_reg_v3_hw(struct hisi_hba *hisi_hba)
0xa0064);
hisi_sas_phy_write32(hisi_hba, i, SAS_STP_CON_TIMER_CFG,
0xa0064);
+ hisi_sas_phy_write32(hisi_hba, i, STP_LINK_TIMER,
+0x7f7a120);
}
for (i = 0; i < hisi_hba->queue_count; i++) {
/* Delivery queue */
--
1.9.1
[PATCH 14/19] scsi: hisi_sas: update some v3 register init settings
From: Xiang Chen
This patch updates some register setting according
to recommendation from HW designer and experiment.
Signed-off-by: Xiang Chen
Signed-off-by: John Garry
---
drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 15 ---
1 file changed, 4 insertions(+), 11 deletions(-)
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
index 992ccc2..efc2e82 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
@@ -23,14 +23,11 @@
#define PHY_STATE 0x24
#define PHY_PORT_NUM_MA0x28
#define PHY_CONN_RATE 0x30
-#define AXI_AHB_CLK_CFG0x3c
#define ITCT_CLR 0x44
#define ITCT_CLR_EN_OFF16
#define ITCT_CLR_EN_MSK(0x1 << ITCT_CLR_EN_OFF)
#define ITCT_DEV_OFF 0
#define ITCT_DEV_MSK (0x7ff << ITCT_DEV_OFF)
-#define AXI_USER1 0x48
-#define AXI_USER2 0x4c
#define IO_SATA_BROKEN_MSG_ADDR_LO 0x58
#define IO_SATA_BROKEN_MSG_ADDR_HI 0x5c
#define SATA_INITI_D2H_STORE_ADDR_LO 0x60
@@ -355,8 +352,6 @@ static void init_reg_v3_hw(struct hisi_hba *hisi_hba)
/* Global registers init */
hisi_sas_write32(hisi_hba, DLVRY_QUEUE_ENABLE,
(u32)((1ULL << hisi_hba->queue_count) - 1));
- hisi_sas_write32(hisi_hba, AXI_USER1, 0x0);
- hisi_sas_write32(hisi_hba, AXI_USER2, 0x4060);
hisi_sas_write32(hisi_hba, HGC_SAS_TXFAIL_RETRY_CTRL, 0x108);
hisi_sas_write32(hisi_hba, CFG_1US_TIMER_TRSH, 0xd);
hisi_sas_write32(hisi_hba, INT_COAL_EN, 0x1);
@@ -372,15 +367,14 @@ static void init_reg_v3_hw(struct hisi_hba *hisi_hba)
hisi_sas_write32(hisi_hba, CHNL_PHYUPDOWN_INT_MSK, 0x0);
hisi_sas_write32(hisi_hba, CHNL_ENT_INT_MSK, 0x0);
hisi_sas_write32(hisi_hba, HGC_COM_INT_MSK, 0x0);
- hisi_sas_write32(hisi_hba, SAS_ECC_INTR_MSK, 0xfff00c30);
+ hisi_sas_write32(hisi_hba, SAS_ECC_INTR_MSK, 0x0);
hisi_sas_write32(hisi_hba, AWQOS_AWCACHE_CFG, 0xf0f0);
hisi_sas_write32(hisi_hba, ARQOS_ARCACHE_CFG, 0xf0f0);
for (i = 0; i < hisi_hba->queue_count; i++)
hisi_sas_write32(hisi_hba, OQ0_INT_SRC_MSK+0x4*i, 0);
- hisi_sas_write32(hisi_hba, AXI_AHB_CLK_CFG, 1);
hisi_sas_write32(hisi_hba, HYPER_STREAM_ID_EN_CFG, 1);
- hisi_sas_write32(hisi_hba, CFG_MAX_TAG, 0xfff07fff);
+ hisi_sas_write32(hisi_hba, AXI_MASTER_CFG_BASE, 0x3);
for (i = 0; i < hisi_hba->n_phy; i++) {
hisi_sas_phy_write32(hisi_hba, i, PROG_PHY_LINK_RATE, 0x801);
@@ -390,7 +384,6 @@ static void init_reg_v3_hw(struct hisi_hba *hisi_hba)
hisi_sas_phy_write32(hisi_hba, i, RXOP_CHECK_CFG_H, 0x1000);
hisi_sas_phy_write32(hisi_hba, i, CHL_INT1_MSK, 0x);
hisi_sas_phy_write32(hisi_hba, i, CHL_INT2_MSK, 0x8bff);
- hisi_sas_phy_write32(hisi_hba, i, SL_CFG, 0x83f801fc);
hisi_sas_phy_write32(hisi_hba, i, PHY_CTRL_RDY_MSK, 0x0);
hisi_sas_phy_write32(hisi_hba, i, PHYCTRL_NOT_RDY_MSK, 0x0);
hisi_sas_phy_write32(hisi_hba, i, PHYCTRL_DWS_RESET_MSK, 0x0);
@@ -399,9 +392,9 @@ static void init_reg_v3_hw(struct hisi_hba *hisi_hba)
hisi_sas_phy_write32(hisi_hba, i, PHYCTRL_OOB_RESTART_MSK, 0x0);
hisi_sas_phy_write32(hisi_hba, i, PHY_CTRL, 0x199b4fa);
hisi_sas_phy_write32(hisi_hba, i, SAS_SSP_CON_TIMER_CFG,
-0xa0064);
+0xa03e8);
hisi_sas_phy_write32(hisi_hba, i, SAS_STP_CON_TIMER_CFG,
-0xa0064);
+0xa03e8);
hisi_sas_phy_write32(hisi_hba, i, STP_LINK_TIMER,
0x7f7a120);
}
--
1.9.1
[PATCH 10/19] scsi: hisi_sas: add status and command buffer for internal abort
From: Xiang Chen
For v3 hw, internal abort function required status and command buffer
to be set, so add necessary code for this.
Signed-off-by: Xiang Chen
Signed-off-by: John Garry
---
drivers/scsi/hisi_sas/hisi_sas_main.c | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c
b/drivers/scsi/hisi_sas/hisi_sas_main.c
index 86868ec..7e642c8 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -1363,12 +1363,21 @@ static int hisi_sas_query_task(struct sas_task *task)
slot->port = port;
task->lldd_task = slot;
+ slot->buf = dma_pool_alloc(hisi_hba->buffer_pool,
+ GFP_ATOMIC, &slot->buf_dma);
+ if (!slot->buf) {
+ rc = -ENOMEM;
+ goto err_out_tag;
+ }
+
memset(slot->cmd_hdr, 0, sizeof(struct hisi_sas_cmd_hdr));
+ memset(hisi_sas_cmd_hdr_addr_mem(slot), 0, HISI_SAS_COMMAND_TABLE_SZ);
+ memset(hisi_sas_status_buf_addr_mem(slot), 0, HISI_SAS_STATUS_BUF_SZ);
rc = hisi_sas_task_prep_abort(hisi_hba, slot, device_id,
abort_flag, task_tag);
if (rc)
- goto err_out_tag;
+ goto err_out_buf;
list_add_tail(&slot->entry, &sas_dev->list);
@@ -1386,6 +1395,9 @@ static int hisi_sas_query_task(struct sas_task *task)
return 0;
+err_out_buf:
+ dma_pool_free(hisi_hba->buffer_pool, slot->buf,
+ slot->buf_dma);
err_out_tag:
spin_lock_irqsave(&hisi_hba->lock, flags);
hisi_sas_slot_index_free(hisi_hba, slot_idx);
--
1.9.1
[PATCH 09/19] scsi: hisi_sas: support zone management commands
From: Xiaofei Tan Add two ATA commands, ATA_CMD_ZAC_MGMT_IN and ATA_CMD_ZAC_MGMT_OUT in hisi_sas_get_ata_protocol(), to support SATA SMR disk. Signed-off-by: Xiaofei Tan Signed-off-by: John Garry --- drivers/scsi/hisi_sas/hisi_sas_main.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c index 81ad6cd..86868ec 100644 --- a/drivers/scsi/hisi_sas/hisi_sas_main.c +++ b/drivers/scsi/hisi_sas/hisi_sas_main.c @@ -61,6 +61,7 @@ u8 hisi_sas_get_ata_protocol(u8 cmd, int direction) case ATA_CMD_WRITE_QUEUED: case ATA_CMD_WRITE_LOG_DMA_EXT: case ATA_CMD_WRITE_STREAM_DMA_EXT: + case ATA_CMD_ZAC_MGMT_IN: return HISI_SAS_SATA_PROTOCOL_DMA; case ATA_CMD_CHK_POWER: @@ -73,6 +74,7 @@ u8 hisi_sas_get_ata_protocol(u8 cmd, int direction) case ATA_CMD_SET_FEATURES: case ATA_CMD_STANDBY: case ATA_CMD_STANDBYNOW1: + case ATA_CMD_ZAC_MGMT_OUT: return HISI_SAS_SATA_PROTOCOL_NONDATA; default: if (direction == DMA_NONE) -- 1.9.1
[PATCH 02/19] scsi: hisi_sas: avoid potential v2 hw interrupt issue
From: Xiang Chen
When some interrupts happen together, we need to process
every interrupt one-by-one, and should not return
immediately when one interrupt process is finished being
processed.
Signed-off-by: Xiang Chen
Signed-off-by: John Garry
---
drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 19 ++-
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
index a6be33c..8c504b4 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
@@ -2606,6 +2606,7 @@ static irqreturn_t int_phy_updown_v2_hw(int irq_no, void
*p)
struct hisi_hba *hisi_hba = p;
u32 irq_msk;
int phy_no = 0;
+ irqreturn_t res = IRQ_NONE;
irq_msk = (hisi_sas_read32(hisi_hba, HGC_INVLD_DQE_INFO)
>> HGC_INVLD_DQE_INFO_FB_CH0_OFF) & 0x1ff;
@@ -2620,15 +2621,15 @@ static irqreturn_t int_phy_updown_v2_hw(int irq_no,
void *p)
case CHL_INT0_SL_PHY_ENABLE_MSK:
/* phy up */
if (phy_up_v2_hw(phy_no, hisi_hba) ==
- IRQ_NONE)
- return IRQ_NONE;
+ IRQ_HANDLED)
+ res = IRQ_HANDLED;
break;
case CHL_INT0_NOT_RDY_MSK:
/* phy down */
if (phy_down_v2_hw(phy_no, hisi_hba) ==
- IRQ_NONE)
- return IRQ_NONE;
+ IRQ_HANDLED)
+ res = IRQ_HANDLED;
break;
case (CHL_INT0_NOT_RDY_MSK |
@@ -2638,13 +2639,13 @@ static irqreturn_t int_phy_updown_v2_hw(int irq_no,
void *p)
if (reg_value & BIT(phy_no)) {
/* phy up */
if (phy_up_v2_hw(phy_no, hisi_hba) ==
- IRQ_NONE)
- return IRQ_NONE;
+ IRQ_HANDLED)
+ res = IRQ_HANDLED;
} else {
/* phy down */
if (phy_down_v2_hw(phy_no, hisi_hba) ==
- IRQ_NONE)
- return IRQ_NONE;
+ IRQ_HANDLED)
+ res = IRQ_HANDLED;
}
break;
@@ -2657,7 +2658,7 @@ static irqreturn_t int_phy_updown_v2_hw(int irq_no, void
*p)
phy_no++;
}
- return IRQ_HANDLED;
+ return res;
}
static void phy_bcast_v2_hw(int phy_no, struct hisi_hba *hisi_hba)
--
1.9.1
[PATCH 07/19] scsi: hisi_sas: add irq and tasklet cleanup in v2 hw
From: Xiang Chen
This patch adds support to clean-up allocated IRQs and
kill tasklets when probe fails and for driver removal.
Signed-off-by: Xiang Chen
Signed-off-by: John Garry
---
drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 96 +-
1 file changed, 49 insertions(+), 47 deletions(-)
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
index bcbc16e..9eea0b4 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
@@ -3290,97 +3290,92 @@ static int interrupt_init_v2_hw(struct hisi_hba
*hisi_hba)
{
struct platform_device *pdev = hisi_hba->platform_dev;
struct device *dev = &pdev->dev;
- int i, irq, rc, irq_map[128];
-
+ int irq, rc, irq_map[128];
+ int i, phy_no, fatal_no, queue_no, k;
for (i = 0; i < 128; i++)
irq_map[i] = platform_get_irq(pdev, i);
for (i = 0; i < HISI_SAS_PHY_INT_NR; i++) {
- int idx = i;
-
- irq = irq_map[idx + 1]; /* Phy up/down is irq1 */
- if (!irq) {
- dev_err(dev, "irq init: fail map phy interrupt %d\n",
- idx);
- return -ENOENT;
- }
-
+ irq = irq_map[i + 1]; /* Phy up/down is irq1 */
rc = devm_request_irq(dev, irq, phy_interrupts[i], 0,
DRV_NAME " phy", hisi_hba);
if (rc) {
dev_err(dev, "irq init: could not request "
"phy interrupt %d, rc=%d\n",
irq, rc);
- return -ENOENT;
+ rc = -ENOENT;
+ goto free_phy_int_irqs;
}
}
- for (i = 0; i < hisi_hba->n_phy; i++) {
- struct hisi_sas_phy *phy = &hisi_hba->phy[i];
- int idx = i + 72; /* First SATA interrupt is irq72 */
-
- irq = irq_map[idx];
- if (!irq) {
- dev_err(dev, "irq init: fail map phy interrupt %d\n",
- idx);
- return -ENOENT;
- }
+ for (phy_no = 0; phy_no < hisi_hba->n_phy; phy_no++) {
+ struct hisi_sas_phy *phy = &hisi_hba->phy[phy_no];
+ irq = irq_map[phy_no + 72];
rc = devm_request_irq(dev, irq, sata_int_v2_hw, 0,
DRV_NAME " sata", phy);
if (rc) {
dev_err(dev, "irq init: could not request "
"sata interrupt %d, rc=%d\n",
irq, rc);
- return -ENOENT;
+ rc = -ENOENT;
+ goto free_sata_int_irqs;
}
}
- for (i = 0; i < HISI_SAS_FATAL_INT_NR; i++) {
- int idx = i;
-
- irq = irq_map[idx + 81];
- if (!irq) {
- dev_err(dev, "irq init: fail map fatal interrupt %d\n",
- idx);
- return -ENOENT;
- }
-
- rc = devm_request_irq(dev, irq, fatal_interrupts[i], 0,
+ for (fatal_no = 0; fatal_no < HISI_SAS_FATAL_INT_NR; fatal_no++) {
+ irq = irq_map[fatal_no + 81];
+ rc = devm_request_irq(dev, irq, fatal_interrupts[fatal_no], 0,
DRV_NAME " fatal", hisi_hba);
if (rc) {
dev_err(dev,
"irq init: could not request fatal interrupt
%d, rc=%d\n",
irq, rc);
- return -ENOENT;
+ rc = -ENOENT;
+ goto free_fatal_int_irqs;
}
}
- for (i = 0; i < hisi_hba->queue_count; i++) {
- int idx = i + 96; /* First cq interrupt is irq96 */
- struct hisi_sas_cq *cq = &hisi_hba->cq[i];
+ for (queue_no = 0; queue_no < hisi_hba->queue_count; queue_no++) {
+ struct hisi_sas_cq *cq = &hisi_hba->cq[queue_no];
struct tasklet_struct *t = &cq->tasklet;
- irq = irq_map[idx];
- if (!irq) {
- dev_err(dev,
- "irq init: could not map cq interrupt %d\n",
- idx);
- return -ENOENT;
- }
+ irq = irq_map[queue_no + 96];
rc = devm_request_irq(dev, irq, cq_interrupt_v2_hw, 0,
- DRV_NAME " cq", &hisi_hba->cq[i]);
+ DRV_NAME " cq", cq);
if (rc) {
dev_err(dev,
"irq init: could not request cq interrupt %d,
[PATCH 12/19] scsi: hisi_sas: fix v3 hw channel interrupt processing
From: Xiang Chen
The channel interrupt is to process all the interrupts
except PHY UP/DOWN and broadcast interrupt. So we need to
clear all the interrupts except those 3 interrupts after
processing channel interrupts.
Signed-off-by: Xiang Chen
Signed-off-by: John Garry
---
drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
index bb79b776..a8bd557 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
@@ -1260,7 +1260,7 @@ static irqreturn_t int_chnl_int_v3_hw(int irq_no, void *p)
if (irq_msk & (2 << (phy_no * 4)) && irq_value0) {
hisi_sas_phy_write32(hisi_hba, phy_no,
CHL_INT0, irq_value0
- & (~CHL_INT0_HOTPLUG_TOUT_MSK)
+ & (~CHL_INT0_SL_RX_BCST_ACK_MSK)
& (~CHL_INT0_SL_PHY_ENABLE_MSK)
& (~CHL_INT0_NOT_RDY_MSK));
}
--
1.9.1
[PATCH 04/19] scsi: hisi_sas: add v2 hw DFX feature
From: Xiaofei Tan
Add DFX feature for v2 hw. We are adding support for
the following errors:
- loss_of_dword_sync_count
- invalid_dword_count
- phy_reset_problem_count
- running_disparity_error_count
Signed-off-by: Xiaofei Tan
Signed-off-by: John Garry
---
drivers/scsi/hisi_sas/hisi_sas.h | 1 +
drivers/scsi/hisi_sas/hisi_sas_main.c | 7 ++-
drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 22 ++
3 files changed, 29 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/hisi_sas/hisi_sas.h b/drivers/scsi/hisi_sas/hisi_sas.h
index 3c4defa..ef2238c 100644
--- a/drivers/scsi/hisi_sas/hisi_sas.h
+++ b/drivers/scsi/hisi_sas/hisi_sas.h
@@ -193,6 +193,7 @@ struct hisi_sas_hw {
void (*phy_enable)(struct hisi_hba *hisi_hba, int phy_no);
void (*phy_disable)(struct hisi_hba *hisi_hba, int phy_no);
void (*phy_hard_reset)(struct hisi_hba *hisi_hba, int phy_no);
+ void (*get_events)(struct hisi_hba *hisi_hba, int phy_no);
void (*phy_set_linkrate)(struct hisi_hba *hisi_hba, int phy_no,
struct sas_phy_linkrates *linkrates);
enum sas_linkrate (*phy_get_max_linkrate)(void);
diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c
b/drivers/scsi/hisi_sas/hisi_sas_main.c
index bd1d619..aaa7296 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -764,7 +764,12 @@ static int hisi_sas_control_phy(struct asd_sas_phy
*sas_phy, enum phy_func func,
case PHY_FUNC_SET_LINK_RATE:
hisi_hba->hw->phy_set_linkrate(hisi_hba, phy_no, funcdata);
break;
-
+ case PHY_FUNC_GET_EVENTS:
+ if (hisi_hba->hw->get_events) {
+ hisi_hba->hw->get_events(hisi_hba, phy_no);
+ break;
+ }
+ /* fallthru */
case PHY_FUNC_RELEASE_SPINUP_HOLD:
default:
return -EOPNOTSUPP;
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
index a762b25..41e8033 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
@@ -256,6 +256,8 @@
#define LINK_DFX2_RCVR_HOLD_STS_MSK(0x1 << LINK_DFX2_RCVR_HOLD_STS_OFF)
#define LINK_DFX2_SEND_HOLD_STS_OFF10
#define LINK_DFX2_SEND_HOLD_STS_MSK(0x1 << LINK_DFX2_SEND_HOLD_STS_OFF)
+#define SAS_ERR_CNT4_REG (PORT_BASE + 0x290)
+#define SAS_ERR_CNT6_REG (PORT_BASE + 0x298)
#define PHY_CTRL_RDY_MSK (PORT_BASE + 0x2b0)
#define PHYCTRL_NOT_RDY_MSK(PORT_BASE + 0x2b4)
#define PHYCTRL_DWS_RESET_MSK (PORT_BASE + 0x2b8)
@@ -1360,6 +1362,25 @@ static void phy_hard_reset_v2_hw(struct hisi_hba
*hisi_hba, int phy_no)
start_phy_v2_hw(hisi_hba, phy_no);
}
+static void phy_get_events_v2_hw(struct hisi_hba *hisi_hba, int phy_no)
+{
+ struct hisi_sas_phy *phy = &hisi_hba->phy[phy_no];
+ struct asd_sas_phy *sas_phy = &phy->sas_phy;
+ struct sas_phy *sphy = sas_phy->phy;
+ u32 err4_reg_val, err6_reg_val;
+
+ /* loss dword syn, phy reset problem */
+ err4_reg_val = hisi_sas_phy_read32(hisi_hba, phy_no, SAS_ERR_CNT4_REG);
+
+ /* disparity err, invalid dword */
+ err6_reg_val = hisi_sas_phy_read32(hisi_hba, phy_no, SAS_ERR_CNT6_REG);
+
+ sphy->loss_of_dword_sync_count += (err4_reg_val >> 16) & 0x;
+ sphy->phy_reset_problem_count += err4_reg_val & 0x;
+ sphy->invalid_dword_count += (err6_reg_val & 0xFF) >> 16;
+ sphy->running_disparity_error_count += err6_reg_val & 0xFF;
+}
+
static void start_phys_v2_hw(struct hisi_hba *hisi_hba)
{
int i;
@@ -3457,6 +3478,7 @@ static int soft_reset_v2_hw(struct hisi_hba *hisi_hba)
.phy_enable = enable_phy_v2_hw,
.phy_disable = disable_phy_v2_hw,
.phy_hard_reset = phy_hard_reset_v2_hw,
+ .get_events = phy_get_events_v2_hw,
.phy_set_linkrate = phy_set_linkrate_v2_hw,
.phy_get_max_linkrate = phy_get_max_linkrate_v2_hw,
.max_command_entries = HISI_SAS_COMMAND_ENTRIES_V2_HW,
--
1.9.1
[PATCH 17/19] scsi: hisi_sas: remove phy_down_v3_hw() res variable
Variable res only holds value 0, so remove it.
This cleans up a coccicheck warning.
Signed-off-by: John Garry
---
drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
index 8e4bfad..3620a3e 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
@@ -1200,7 +1200,6 @@ static int phy_up_v3_hw(int phy_no, struct hisi_hba
*hisi_hba)
static int phy_down_v3_hw(int phy_no, struct hisi_hba *hisi_hba)
{
- int res = 0;
u32 phy_state, sl_ctrl, txid_auto;
struct device *dev = hisi_hba->dev;
@@ -1221,7 +1220,7 @@ static int phy_down_v3_hw(int phy_no, struct hisi_hba
*hisi_hba)
hisi_sas_phy_write32(hisi_hba, phy_no, CHL_INT0, CHL_INT0_NOT_RDY_MSK);
hisi_sas_phy_write32(hisi_hba, phy_no, PHYCTRL_NOT_RDY_MSK, 0);
- return res;
+ return 0;
}
static void phy_bcast_v3_hw(int phy_no, struct hisi_hba *hisi_hba)
--
1.9.1
[PATCH 13/19] scsi: hisi_sas: kill tasklet when destroying irq in v3 hw
From: Xiang Chen This patch adds calls to kill CQ takslets v3 hw during probe failure. Signed-off-by: Xiang Chen Signed-off-by: John Garry --- drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c index a8bd557..992ccc2 100644 --- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c +++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c @@ -1802,6 +1802,7 @@ static int hisi_sas_v3_init(struct hisi_hba *hisi_hba) struct hisi_sas_cq *cq = &hisi_hba->cq[i]; free_irq(pci_irq_vector(pdev, i+16), cq); + tasklet_kill(&cq->tasklet); } pci_free_irq_vectors(pdev); } -- 1.9.1
[PATCH 19/19] scsi: hisi_sas: remove driver versioning
The driver version is not updated with changes to the driver,
so it has no value, so just get rid of it.
Signed-off-by: John Garry
---
drivers/scsi/hisi_sas/hisi_sas.h | 2 --
drivers/scsi/hisi_sas/hisi_sas_main.c | 3 ---
drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 1 -
3 files changed, 6 deletions(-)
diff --git a/drivers/scsi/hisi_sas/hisi_sas.h b/drivers/scsi/hisi_sas/hisi_sas.h
index 4c393cd..07f4a4c 100644
--- a/drivers/scsi/hisi_sas/hisi_sas.h
+++ b/drivers/scsi/hisi_sas/hisi_sas.h
@@ -26,8 +26,6 @@
#include
#include
-#define DRV_VERSION "v1.6"
-
#define HISI_SAS_MAX_PHYS 9
#define HISI_SAS_MAX_QUEUES32
#define HISI_SAS_QUEUE_SLOTS 512
diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c
b/drivers/scsi/hisi_sas/hisi_sas_main.c
index 9427835..bdef111 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -2013,8 +2013,6 @@ int hisi_sas_remove(struct platform_device *pdev)
static __init int hisi_sas_init(void)
{
- pr_info("hisi_sas: driver version %s\n", DRV_VERSION);
-
hisi_sas_stt = sas_domain_attach_transport(&hisi_sas_transport_ops);
if (!hisi_sas_stt)
return -ENOMEM;
@@ -2030,7 +2028,6 @@ static __exit void hisi_sas_exit(void)
module_init(hisi_sas_init);
module_exit(hisi_sas_exit);
-MODULE_VERSION(DRV_VERSION);
MODULE_LICENSE("GPL");
MODULE_AUTHOR("John Garry ");
MODULE_DESCRIPTION("HISILICON SAS controller driver");
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
index a20e354..2e5fa97 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
@@ -2005,7 +2005,6 @@ enum {
module_pci_driver(sas_v3_pci_driver);
-MODULE_VERSION(DRV_VERSION);
MODULE_LICENSE("GPL");
MODULE_AUTHOR("John Garry ");
MODULE_DESCRIPTION("HISILICON SAS controller v3 hw driver based on pci
device");
--
1.9.1
[PATCH 16/19] scsi: hisi_sas: add phy_set_linkrate_v3_hw()
From: Xiang Chen
Add function to set linkrate for v3 hw.
Signed-off-by: Xiang Chen
Signed-off-by: John Garry
---
drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 39 ++
1 file changed, 39 insertions(+)
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
index 111e45c..8e4bfad 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
@@ -1680,6 +1680,44 @@ static int hisi_sas_v3_init(struct hisi_hba *hisi_hba)
return 0;
}
+static void phy_set_linkrate_v3_hw(struct hisi_hba *hisi_hba, int phy_no,
+ struct sas_phy_linkrates *r)
+{
+ u32 prog_phy_link_rate =
+ hisi_sas_phy_read32(hisi_hba, phy_no, PROG_PHY_LINK_RATE);
+ struct hisi_sas_phy *phy = &hisi_hba->phy[phy_no];
+ struct asd_sas_phy *sas_phy = &phy->sas_phy;
+ int i;
+ enum sas_linkrate min, max;
+ u32 rate_mask = 0;
+
+ if (r->maximum_linkrate == SAS_LINK_RATE_UNKNOWN) {
+ max = sas_phy->phy->maximum_linkrate;
+ min = r->minimum_linkrate;
+ } else if (r->minimum_linkrate == SAS_LINK_RATE_UNKNOWN) {
+ max = r->maximum_linkrate;
+ min = sas_phy->phy->minimum_linkrate;
+ } else
+ return;
+
+ sas_phy->phy->maximum_linkrate = max;
+ sas_phy->phy->minimum_linkrate = min;
+
+ min -= SAS_LINK_RATE_1_5_GBPS;
+ max -= SAS_LINK_RATE_1_5_GBPS;
+
+ for (i = 0; i <= max; i++)
+ rate_mask |= 1 << (i * 2);
+
+ prog_phy_link_rate &= ~0xff;
+ prog_phy_link_rate |= rate_mask;
+
+ hisi_sas_phy_write32(hisi_hba, phy_no, PROG_PHY_LINK_RATE,
+ prog_phy_link_rate);
+
+ phy_hard_reset_v3_hw(hisi_hba, phy_no);
+}
+
static void interrupt_disable_v3_hw(struct hisi_hba *hisi_hba)
{
struct pci_dev *pdev = hisi_hba->pci_dev;
@@ -1760,6 +1798,7 @@ static int soft_reset_v3_hw(struct hisi_hba *hisi_hba)
.phy_disable = disable_phy_v3_hw,
.phy_hard_reset = phy_hard_reset_v3_hw,
.phy_get_max_linkrate = phy_get_max_linkrate_v3_hw,
+ .phy_set_linkrate = phy_set_linkrate_v3_hw,
.dereg_device = dereg_device_v3_hw,
.soft_reset = soft_reset_v3_hw,
.get_phys_state = get_phys_state_v3_hw,
--
1.9.1
[PATCH 15/19] scsi: hisi_sas: add reset handler for v3 hw
From: Xiang Chen
Use ACPI "_RST" method to reset the controller, since
FLR is not supported.
Function hisi_sas_stop_phys() is introduced to remove
some code duplication.
Signed-off-by: Xiang Chen
Signed-off-by: John Garry
---
drivers/scsi/hisi_sas/hisi_sas.h | 2 +
drivers/scsi/hisi_sas/hisi_sas_main.c | 9 ++
drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 24 +
drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 158 +
4 files changed, 157 insertions(+), 36 deletions(-)
diff --git a/drivers/scsi/hisi_sas/hisi_sas.h b/drivers/scsi/hisi_sas/hisi_sas.h
index 23a22dc..4c393cd 100644
--- a/drivers/scsi/hisi_sas/hisi_sas.h
+++ b/drivers/scsi/hisi_sas/hisi_sas.h
@@ -15,6 +15,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -402,6 +403,7 @@ struct hisi_sas_slot_buf_table {
extern struct scsi_transport_template *hisi_sas_stt;
extern struct scsi_host_template *hisi_sas_sht;
+extern void hisi_sas_stop_phys(struct hisi_hba *hisi_hba);
extern void hisi_sas_init_add(struct hisi_hba *hisi_hba);
extern int hisi_sas_alloc(struct hisi_hba *hisi_hba, struct Scsi_Host *shost);
extern void hisi_sas_free(struct hisi_hba *hisi_hba);
diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c
b/drivers/scsi/hisi_sas/hisi_sas_main.c
index 7e642c8..4112afd 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -127,6 +127,15 @@ struct hisi_sas_port *to_hisi_sas_port(struct asd_sas_port
*sas_port)
}
EXPORT_SYMBOL_GPL(to_hisi_sas_port);
+void hisi_sas_stop_phys(struct hisi_hba *hisi_hba)
+{
+ int phy_no;
+
+ for (phy_no = 0; phy_no < hisi_hba->n_phy; phy_no++)
+ hisi_hba->hw->phy_disable(hisi_hba, phy_no);
+}
+EXPORT_SYMBOL_GPL(hisi_sas_stop_phys);
+
static void hisi_sas_slot_index_clear(struct hisi_hba *hisi_hba, int slot_idx)
{
void *bitmap = hisi_hba->slot_index_tags;
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
index 0e3634e..779af97 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
@@ -1486,25 +1486,12 @@ static void start_phy_v2_hw(struct hisi_hba *hisi_hba,
int phy_no)
enable_phy_v2_hw(hisi_hba, phy_no);
}
-static void stop_phy_v2_hw(struct hisi_hba *hisi_hba, int phy_no)
-{
- disable_phy_v2_hw(hisi_hba, phy_no);
-}
-
-static void stop_phys_v2_hw(struct hisi_hba *hisi_hba)
-{
- int i;
-
- for (i = 0; i < hisi_hba->n_phy; i++)
- stop_phy_v2_hw(hisi_hba, i);
-}
-
static void phy_hard_reset_v2_hw(struct hisi_hba *hisi_hba, int phy_no)
{
struct hisi_sas_phy *phy = &hisi_hba->phy[phy_no];
u32 txid_auto;
- stop_phy_v2_hw(hisi_hba, phy_no);
+ disable_phy_v2_hw(hisi_hba, phy_no);
if (phy->identify.device_type == SAS_END_DEVICE) {
txid_auto = hisi_sas_phy_read32(hisi_hba, phy_no, TXID_AUTO);
hisi_sas_phy_write32(hisi_hba, phy_no, TXID_AUTO,
@@ -1533,7 +1520,7 @@ static void phy_get_events_v2_hw(struct hisi_hba
*hisi_hba, int phy_no)
sphy->running_disparity_error_count += err6_reg_val & 0xFF;
}
-static void start_phys_v2_hw(struct hisi_hba *hisi_hba)
+static void phys_init_v2_hw(struct hisi_hba *hisi_hba)
{
int i;
@@ -1548,11 +1535,6 @@ static void start_phys_v2_hw(struct hisi_hba *hisi_hba)
}
}
-static void phys_init_v2_hw(struct hisi_hba *hisi_hba)
-{
- start_phys_v2_hw(hisi_hba);
-}
-
static void sl_notify_v2_hw(struct hisi_hba *hisi_hba, int phy_no)
{
u32 sl_control;
@@ -3429,7 +3411,7 @@ static int soft_reset_v2_hw(struct hisi_hba *hisi_hba)
interrupt_disable_v2_hw(hisi_hba);
hisi_sas_write32(hisi_hba, DLVRY_QUEUE_ENABLE, 0x0);
- stop_phys_v2_hw(hisi_hba);
+ hisi_sas_stop_phys(hisi_hba);
mdelay(10);
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
index efc2e82..111e45c 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
@@ -165,6 +165,31 @@
#define PHYCTRL_PHY_ENA_MSK(PORT_BASE + 0x2bc)
#define SL_RX_BCAST_CHK_MSK(PORT_BASE + 0x2c0)
#define PHYCTRL_OOB_RESTART_MSK(PORT_BASE + 0x2c4)
+#define DMA_TX_STATUS (PORT_BASE + 0x2d0)
+#define DMA_TX_STATUS_BUSY_OFF 0
+#define DMA_TX_STATUS_BUSY_MSK (0x1 << DMA_TX_STATUS_BUSY_OFF)
+#define DMA_RX_STATUS (PORT_BASE + 0x2e8)
+#define DMA_RX_STATUS_BUSY_OFF 0
+#define DMA_RX_STATUS_BUSY_MSK (0x1 << DMA_RX_STATUS_BUSY_OFF)
+
+#define MAX_ITCT_HW4096 /* max the hw can support */
+#define DEFAULT_ITCT_HW2048 /* reset value, not reprogrammed */
+#if (HISI_SAS_MAX_DEVICES > DEFAULT_ITCT_HW)
+#error Max ITCT exceeded
+#endif
+
+#define AXI_MASTER_CFG_BASE(0x5000)
+#define AM_CTRL_GLOBAL
[PATCH 18/19] scsi: hisi_sas: replace kfree with scsi_host_put
Instances of kfree(shost) should be replaced with
scsi_host_put().
In addition, a missing scsi_host_put() is added for
error path in hisi_sas_shost_alloc_pci() and v3 driver
removal.
Signed-off-by: Pan Bian # For main.c changes
Signed-off-by: John Garry
---
drivers/scsi/hisi_sas/hisi_sas_main.c | 6 +++---
drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 13 +
2 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c
b/drivers/scsi/hisi_sas/hisi_sas_main.c
index 4112afd..9427835 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -1900,7 +1900,7 @@ static struct Scsi_Host *hisi_sas_shost_alloc(struct
platform_device *pdev,
return shost;
err_out:
- kfree(shost);
+ scsi_host_put(shost);
dev_err(dev, "shost alloc failed\n");
return NULL;
}
@@ -1991,7 +1991,7 @@ int hisi_sas_probe(struct platform_device *pdev,
scsi_remove_host(shost);
err_out_ha:
hisi_sas_free(hisi_hba);
- kfree(shost);
+ scsi_host_put(shost);
return rc;
}
EXPORT_SYMBOL_GPL(hisi_sas_probe);
@@ -2006,7 +2006,7 @@ int hisi_sas_remove(struct platform_device *pdev)
sas_remove_host(sha->core.shost);
hisi_sas_free(hisi_hba);
- kfree(shost);
+ scsi_host_put(shost);
return 0;
}
EXPORT_SYMBOL_GPL(hisi_sas_remove);
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
index 3620a3e..a20e354 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
@@ -1811,8 +1811,10 @@ static int soft_reset_v3_hw(struct hisi_hba *hisi_hba)
struct device *dev = &pdev->dev;
shost = scsi_host_alloc(hisi_sas_sht, sizeof(*hisi_hba));
- if (!shost)
- goto err_out;
+ if (!shost) {
+ dev_err(dev, "shost alloc failed\n");
+ return NULL;
+ }
hisi_hba = shost_priv(shost);
hisi_hba->hw = &hisi_sas_v3_hw;
@@ -1833,6 +1835,7 @@ static int soft_reset_v3_hw(struct hisi_hba *hisi_hba)
return shost;
err_out:
+ scsi_host_put(shost);
dev_err(dev, "shost alloc failed\n");
return NULL;
}
@@ -1941,7 +1944,7 @@ static int soft_reset_v3_hw(struct hisi_hba *hisi_hba)
err_out_register_ha:
scsi_remove_host(shost);
err_out_ha:
- kfree(shost);
+ scsi_host_put(shost);
err_out_regions:
pci_release_regions(pdev);
err_out_disable_device:
@@ -1971,14 +1974,16 @@ static void hisi_sas_v3_remove(struct pci_dev *pdev)
struct device *dev = &pdev->dev;
struct sas_ha_struct *sha = dev_get_drvdata(dev);
struct hisi_hba *hisi_hba = sha->lldd_ha;
+ struct Scsi_Host *shost = sha->core.shost;
sas_unregister_ha(sha);
sas_remove_host(sha->core.shost);
- hisi_sas_free(hisi_hba);
hisi_sas_v3_destroy_irqs(pdev, hisi_hba);
pci_release_regions(pdev);
pci_disable_device(pdev);
+ hisi_sas_free(hisi_hba);
+ scsi_host_put(shost);
}
enum {
--
1.9.1
[PATCH 00/19] hisi_sas: misc fixes, improvements, and new features
This patchset introduces an array of misc changes, most significantly including: - v2 hw reset function - core driver reset handler fixes - DFX feature - some interrupt/tasklet/probe+removal error path cleanup John Garry (4): scsi: hisi_sas: use array for v2 hw ECC errors scsi: hisi_sas: remove phy_down_v3_hw() res variable scsi: hisi_sas: replace kfree with scsi_host_put scsi: hisi_sas: remove driver versioning Xiang Chen (12): scsi: hisi_sas: avoid potential v2 hw interrupt issue scsi: hisi_sas: fix v2 hw underflow residual value scsi: hisi_sas: remove repeated device config in v2 hw scsi: hisi_sas: add irq and tasklet cleanup in v2 hw scsi: hisi_sas: service interrupt ITCT_CLR interrupt in v2 hw scsi: hisi_sas: add status and command buffer for internal abort scsi: hisi_sas: Modify v3 hw STP_LINK_TIMER setting scsi: hisi_sas: fix v3 hw channel interrupt processing scsi: hisi_sas: kill tasklet when destroying irq in v3 hw scsi: hisi_sas: update some v3 register init settings scsi: hisi_sas: add reset handler for v3 hw scsi: hisi_sas: add phy_set_linkrate_v3_hw() Xiaofei Tan (3): scsi: hisi_sas: fix reset and port ID refresh issues scsi: hisi_sas: add v2 hw DFX feature scsi: hisi_sas: support zone management commands drivers/scsi/hisi_sas/hisi_sas.h | 18 +- drivers/scsi/hisi_sas/hisi_sas_main.c | 196 +++ drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 605 + drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 237 +++-- 4 files changed, 656 insertions(+), 400 deletions(-) -- 1.9.1
Re: Increased memory usage with scsi-mq
OK this is looking a bit better now.
With scsi-mq enabled: 175 disks
virtqueue_size=64: 318 disks *
virtqueue_size=16: 775 disks *
With scsi-mq disabled: 1755 disks
* = new results
I also ran the whole libguestfs test suite with virtqueue_size=16
(with no failures shown). As this tests many different disk I/O
operations, it gives me some confidence that things generally work.
Do you have any other comments about the patches? I'm not sure I know
enough to write an intelligent commit message for the kernel patch.
Rich.
--- kernel patch ---
diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index 9be211d68b15..d6b4ff634c0d 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -972,6 +972,8 @@ static int virtscsi_probe(struct virtio_device *vdev)
if (err)
goto virtscsi_init_failed;
+ shost->can_queue = virtqueue_get_vring_size(vscsi->req_vqs[0].vq);
+
cmd_per_lun = virtscsi_config_get(vdev, cmd_per_lun) ?: 1;
shost->cmd_per_lun = min_t(u32, cmd_per_lun, shost->can_queue);
shost->max_sectors = virtscsi_config_get(vdev, max_sectors) ?: 0x;
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 5e1b548828e6..2d7509da9f39 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -296,7 +296,6 @@ static inline int virtqueue_add(struct virtqueue *_vq,
}
#endif
- BUG_ON(total_sg > vq->vring.num);
BUG_ON(total_sg == 0);
head = vq->free_head;
@@ -305,8 +304,10 @@ static inline int virtqueue_add(struct virtqueue *_vq,
* buffers, then go indirect. FIXME: tune this threshold */
if (vq->indirect && total_sg > 1 && vq->vq.num_free)
desc = alloc_indirect(_vq, total_sg, gfp);
- else
+ else {
desc = NULL;
+ WARN_ON_ONCE(total_sg > vq->vring.num && !vq->indirect);
+}
if (desc) {
/* Use a single buffer which doesn't continue */
--- qemu patch ---
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index eb639442d1..aadd99aad1 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -867,10 +867,10 @@ void virtio_scsi_common_realize(DeviceState *dev,
s->sense_size = VIRTIO_SCSI_SENSE_DEFAULT_SIZE;
s->cdb_size = VIRTIO_SCSI_CDB_DEFAULT_SIZE;
-s->ctrl_vq = virtio_add_queue(vdev, VIRTIO_SCSI_VQ_SIZE, ctrl);
-s->event_vq = virtio_add_queue(vdev, VIRTIO_SCSI_VQ_SIZE, evt);
+s->ctrl_vq = virtio_add_queue(vdev, s->conf.virtqueue_size, ctrl);
+s->event_vq = virtio_add_queue(vdev, s->conf.virtqueue_size, evt);
for (i = 0; i < s->conf.num_queues; i++) {
-s->cmd_vqs[i] = virtio_add_queue(vdev, VIRTIO_SCSI_VQ_SIZE, cmd);
+s->cmd_vqs[i] = virtio_add_queue(vdev, s->conf.virtqueue_size, cmd);
}
}
@@ -917,6 +917,7 @@ static void virtio_scsi_device_unrealize(DeviceState *dev,
Error **errp)
static Property virtio_scsi_properties[] = {
DEFINE_PROP_UINT32("num_queues", VirtIOSCSI, parent_obj.conf.num_queues,
1),
+DEFINE_PROP_UINT32("virtqueue_size", VirtIOSCSI,
parent_obj.conf.virtqueue_size, 128),
DEFINE_PROP_UINT32("max_sectors", VirtIOSCSI, parent_obj.conf.max_sectors,
0x),
DEFINE_PROP_UINT32("cmd_per_lun", VirtIOSCSI, parent_obj.conf.cmd_per_lun,
diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h
index de6ae5a9f6..e30a92d3e7 100644
--- a/include/hw/virtio/virtio-scsi.h
+++ b/include/hw/virtio/virtio-scsi.h
@@ -48,6 +48,7 @@ typedef struct virtio_scsi_config VirtIOSCSIConfig;
struct VirtIOSCSIConf {
uint32_t num_queues;
+uint32_t virtqueue_size;
uint32_t max_sectors;
uint32_t cmd_per_lun;
#ifdef CONFIG_VHOST_SCSI
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines. Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top
Re: [PATCH] sd_zbc: Write unlock zone from sd_uninit_cmnd()
On Thu, 2017-08-10 at 11:14 +0900, Damien Le Moal wrote: > I am currently trying different approaches for this. In the mean time, I > would like to see the unlock change patch be applied to fix the deadlock > problem. Hello Damien, That approach sounds fine to me. Bart.
Re: [PATCH] scsi-mq: Always unprepare before requeuing a request
On Thu, 2017-08-10 at 20:32 +1000, Michael Ellerman wrote: > "Martin K. Petersen" writes: > > > One of the two scsi-mq functions that requeue a request unprepares a > > > request before requeueing (scsi_io_completion()) but the other > > > function not (__scsi_queue_insert()). Make sure that a request is > > > unprepared before requeuing it. > > > > Applied to 4.13/scsi-fixes. Thanks much! > > This seems to be preventing my Power8 box, which uses IPR, from booting. > > Bisect said so: > > # first bad commit: [270065e92c317845d69095ec8e3d18616b5b39d5] scsi: scsi-mq: > Always unprepare before requeuing a request > > And if I revert that on top of next-20170809 my system boots again. > > The symptom is that it just gets "stuck" during boot and never gets to > mounting root, full log below, the end is: > > . > ready > ready > sd 0:2:4:0: [sde] 554287104 512-byte logical blocks: (284 GB/264 GiB) > sd 0:2:4:0: [sde] 4096-byte physical blocks > sd 0:2:5:0: [sdf] 272646144 512-byte logical blocks: (140 GB/130 GiB) > sd 0:2:5:0: [sdf] 4096-byte physical blocks > sd 0:2:4:0: [sde] Write Protect is off > sd 0:2:4:0: [sde] Mode Sense: 0b 00 00 08 > sd 0:2:5:0: [sdf] Write Protect is off > sd 0:2:5:0: [sdf] Mode Sense: 0b 00 00 08 > > > And it just sits there for at least hours. > > I compared a good and bad boot log and there appears to be essentially > no difference. Certainly nothing that looks suspicous. Hello Michael, Thanks for having reported this early. Is there any chance that you can reproduce this state, press SysRq-w on the console and collect the task overview that is reported on the console (see also Documentation/admin-guide/ sysrq.rst)? If this is not possible or if that task overview does not report any blocked tasks, can you add scsi_mod.scsi_logging_level=-1 to the kernel command line (either through /etc/default/grub or in /boot/grub2/grub.cfg when using GRUB), reboot the system, capture the console output and report that output as a reply to this e-mail? Thanks, Bart.
Spurious DISK_EVENT_MEDIA_CHANGE on USB DVD hotplug?
Hi Tejun, Kay,
I'm investigating a customer report which manifests itself all the way
up in gnome-session when a BMC hotplug-adds a simulated DVD device. The
user logs into their server's BMC and enables "media redirection", an
emulated DVD device + .iso is dynamically added to the bus... in the
past this has worked well, however, they are now noticing a timing
condition on RHEL7 that prevents gnome from successfully auto-mounting
the DVD media.
With Harald's help, I've done some debugging and we've found out that on
hotplug-add, the kernel sends two uevents (ADD, CHANGE) in short
succession.
(Example with an ordinary, physical USB DVD device on my laptop, is
very similar):
% udevadm monitor -k -e
KERNEL[2409061.130338] add
/devices/pci:00/:00:14.0/usb3/3-9/3-9.3/3-9.3:1.0/host20/target20:0:0/20:0:0:0/block/sr1
(block)
ACTION=add
DEVNAME=/dev/sr1
DEVPATH=/devices/pci:00/:00:14.0/usb3/3-9/3-9.3/3-9.3:1.0/host20/target20:0:0/20:0:0:0/block/sr1
DEVTYPE=disk
MAJOR=11
MINOR=1
SEQNUM=5885
SUBSYSTEM=block
...
KERNEL[2409061.134076] change
/devices/pci:00/:00:14.0/usb3/3-9/3-9.3/3-9.3:1.0/host20/target20:0:0/20:0:0:0/block/sr1
(block)
ACTION=change
DEVNAME=/dev/sr1
DEVPATH=/devices/pci:00/:00:14.0/usb3/3-9/3-9.3/3-9.3:1.0/host20/target20:0:0/20:0:0:0/block/sr1
DEVTYPE=disk
> DISK_MEDIA_CHANGE=1
MAJOR=11
MINOR=1
SEQNUM=5889
SUBSYSTEM=block
(Both of these events trigger a call out to the 'cdrom_id' userspace
program, the latter of which interferes with the gnome-session
auto-mounting feature.)
With a systemtap probe, I can also see that there are four userspace
openers of the cdrom when it is added:
(parent)-> (child) : system-tap probe-point
---
> systemd-udevd(849) -> systemd-udevd(6783) :
> module("cdrom").function("cdrom_open@drivers/cdrom/cdrom.c:980")
systemd-udevd(6783) -> cdrom_id(6791) :
module("cdrom").function("cdrom_open@drivers/cdrom/cdrom.c:980")
systemd-udevd(849) -> systemd-udevd(6783) :
module("cdrom").function("cdrom_open@drivers/cdrom/cdrom.c:980")
systemd-udevd(6783) -> cdrom_id(6794) :
module("cdrom").function("cdrom_open@drivers/cdrom/cdrom.c:980")
where on the first opener, the kernel eventually invokes
sr_mod::sr_check_events() and gets a DISK_EVENT_MEDIA_CHANGE return
code.
In the case of my USB DVD -> laptop example, there is no media in my
device, however I still see the DISK_EVENT_MEDIA_CHANGE event. This is
a bit confusing, and I was wondering if there was an explanation for
the following:
drivers/scsi/sr.c :: sr_probe()
disk->events = DISK_EVENT_MEDIA_CHANGE | DISK_EVENT_EJECT_REQUEST;
...
cd->media_present = 1;
DISK_EVENT_MEDIA_CHANGE events will pass through to userspace and
for some reason cd->media_present defaults to 1? More on that below...
drivers/scsi/sr.c :: sr_check_events()
...
do_tur:
/* let's see whether the media is there with TUR */
last_present = cd->media_present;
ret = scsi_test_unit_ready(cd->device, SR_TIMEOUT, MAX_RETRIES, &sshdr);
/*
* Media is considered to be present if TUR succeeds or fails with
* sense data indicating something other than media-not-present
* (ASC 0x3a).
*/
cd->media_present = scsi_status_is_good(ret) ||
(scsi_sense_valid(&sshdr) && sshdr.asc != 0x3a);
if (last_present != cd->media_present)
cd->device->changed = 1;
if (cd->device->changed) {
events |= DISK_EVENT_MEDIA_CHANGE;
cd->device->changed = 0;
cd->tur_changed = true;
}
...
sr_check_events() compares the previous (in this case, default)
media_present value with what the TUR returns. If it has changed, then
turn on the DISK_EVENT_MEDIA_CHANGE event bit.
In my laptop USB DVD case, !scsi_status_is_good and sshdr.asc == 0x3a,
so last_present (1) and cd->media_present (0) mis-compare and the change
event is set. That does not seem intuitive to me, is this a bug?
Bringing this back to the reported BMC case, which presumably does have
"media" present in the virtual device... is it reasonable to expect a
DISK_EVENT_MEDIA_CHANGE even for a new device that contains media? (I
haven't verified, but in this case GET_EVENT_STATUS_NOTIFICATION might
be enough to set media present.)
If there is documentation that explains DISK_EVENT_MEDIA_CHANGE conditions
somewhere, feel free to point me there.
Thanks,
-- Joe
Re: Increased memory usage with scsi-mq
On 10/08/2017 16:16, Richard W.M. Jones wrote:
> On Thu, Aug 10, 2017 at 02:53:58PM +0200, Paolo Bonzini wrote:
>> can_queue and cmd_per_lun are different. can_queue should be set to the
>> value of vq->vring.num where vq is the command virtqueue (the first one
>> is okay if there's >1).
>>
>> If you want to change it, you'll have to do so in QEMU.
>
> Here are a couple more patches I came up with, the first to Linux,
> the second to qemu.
>
> There are a few problems ...
>
> (1) Setting virtqueue_size to < 128 causes a BUG_ON failure in
> virtio_ring.c:virtqueue_add at:
>
> BUG_ON(total_sg > vq->vring.num);
That bug is bogus. You can change it to
WARN_ON_ONCE(total_sg > vq->vring.num && !vq->indirect);
and move it in the "else" here:
if (vq->indirect && total_sg > 1 && vq->vq.num_free)
desc = alloc_indirect(_vq, total_sg, gfp);
else
desc = NULL;
You just get a -ENOSPC after the WARN, so no need to crash the kernel!
> (2) Can/should the ctrl, event and cmd queue sizes be set to the same
> values? Or should ctrl & event be left at 128?
It's okay if they're all the same.
> (3) It seems like it might be a problem that virtqueue_size is not
> passed through the virtio_scsi_conf struct (which is ABI between the
> kernel and qemu AFAICT and so not easily modifiable). However I think
> this is not a problem because virtqueue size is stored in the
> Virtqueue Descriptor table, which is how the kernel gets the value.
> Is that right?
Yes.
Paolo
> Rich.
>
>
> --- kernel patch ---
>
> diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
> index 9be211d68b15..d6b4ff634c0d 100644
> --- a/drivers/scsi/virtio_scsi.c
> +++ b/drivers/scsi/virtio_scsi.c
> @@ -972,6 +972,8 @@ static int virtscsi_probe(struct virtio_device *vdev)
> if (err)
> goto virtscsi_init_failed;
>
> + shost->can_queue = virtqueue_get_vring_size(vscsi->req_vqs[0].vq);
> +
> cmd_per_lun = virtscsi_config_get(vdev, cmd_per_lun) ?: 1;
> shost->cmd_per_lun = min_t(u32, cmd_per_lun, shost->can_queue);
> shost->max_sectors = virtscsi_config_get(vdev, max_sectors) ?: 0x;
>
>
> --- qemu patch ---
>
> diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
> index eb639442d1..aadd99aad1 100644
> --- a/hw/scsi/virtio-scsi.c
> +++ b/hw/scsi/virtio-scsi.c
> @@ -867,10 +867,10 @@ void virtio_scsi_common_realize(DeviceState *dev,
> s->sense_size = VIRTIO_SCSI_SENSE_DEFAULT_SIZE;
> s->cdb_size = VIRTIO_SCSI_CDB_DEFAULT_SIZE;
>
> -s->ctrl_vq = virtio_add_queue(vdev, VIRTIO_SCSI_VQ_SIZE, ctrl);
> -s->event_vq = virtio_add_queue(vdev, VIRTIO_SCSI_VQ_SIZE, evt);
> +s->ctrl_vq = virtio_add_queue(vdev, s->conf.virtqueue_size, ctrl);
> +s->event_vq = virtio_add_queue(vdev, s->conf.virtqueue_size, evt);
> for (i = 0; i < s->conf.num_queues; i++) {
> -s->cmd_vqs[i] = virtio_add_queue(vdev, VIRTIO_SCSI_VQ_SIZE, cmd);
> +s->cmd_vqs[i] = virtio_add_queue(vdev, s->conf.virtqueue_size, cmd);
> }
> }
>
> @@ -917,6 +917,7 @@ static void virtio_scsi_device_unrealize(DeviceState
> *dev, Error **errp)
>
> static Property virtio_scsi_properties[] = {
> DEFINE_PROP_UINT32("num_queues", VirtIOSCSI, parent_obj.conf.num_queues,
> 1),
> +DEFINE_PROP_UINT32("virtqueue_size", VirtIOSCSI,
> parent_obj.conf.virtqueue_size, 128),
> DEFINE_PROP_UINT32("max_sectors", VirtIOSCSI,
> parent_obj.conf.max_sectors,
>0x),
> DEFINE_PROP_UINT32("cmd_per_lun", VirtIOSCSI,
> parent_obj.conf.cmd_per_lun,
> diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h
> index de6ae5a9f6..e30a92d3e7 100644
> --- a/include/hw/virtio/virtio-scsi.h
> +++ b/include/hw/virtio/virtio-scsi.h
> @@ -48,6 +48,7 @@ typedef struct virtio_scsi_config VirtIOSCSIConfig;
>
> struct VirtIOSCSIConf {
> uint32_t num_queues;
> +uint32_t virtqueue_size;
> uint32_t max_sectors;
> uint32_t cmd_per_lun;
> #ifdef CONFIG_VHOST_SCSI
>
>
Re: [PATCH 1/3] scsi: allow state transition CREATED_BLOCK -> TRANSPORT_OFFLINE
On Thu, 2017-08-10 at 09:05 +0200, Hannes Reinecke wrote: > scsi_internal_device_unblock_nowait() allows a state transition > SDEV_CREATED_BLOCK -> SDEV_TRANSPORT_OFFLINE/SDEV_OFFLINE, > scsi_device_set_state() does not. > So add the missing state transition to scsi_device_set_state(). > > Signed-off-by: Hannes Reinecke > --- > drivers/scsi/scsi_lib.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c > index 41c19c7..1ae531b 100644 > --- a/drivers/scsi/scsi_lib.c > +++ b/drivers/scsi/scsi_lib.c > @@ -2599,6 +2599,7 @@ void scsi_exit_queue(void) > case SDEV_RUNNING: > case SDEV_QUIESCE: > case SDEV_BLOCK: > + case SDEV_CREATED_BLOCK: > break; > default: > goto illegal; This isn't quite good enough: a device that went CREATE_BLOCK -> OFFLINE, the queue will still be stopped meaning I/O will pile up in it forever and it won't restart when the device is brought online. It we need to set the queue running again so that the now offline device errors all pending I/O. It looks like this is a bug in BLOCK->OFFLINE too. We can't simply call scsi_start_queue() from within scsi_device_set_state() because we may have a lock recursion problem, so we might have to introduce a new state that allows the queue to be restarted in the caller. James
Re: Increased memory usage with scsi-mq
On Thu, Aug 10, 2017 at 02:53:58PM +0200, Paolo Bonzini wrote:
> can_queue and cmd_per_lun are different. can_queue should be set to the
> value of vq->vring.num where vq is the command virtqueue (the first one
> is okay if there's >1).
>
> If you want to change it, you'll have to do so in QEMU.
Here are a couple more patches I came up with, the first to Linux,
the second to qemu.
There are a few problems ...
(1) Setting virtqueue_size to < 128 causes a BUG_ON failure in
virtio_ring.c:virtqueue_add at:
BUG_ON(total_sg > vq->vring.num);
I initially thought that I should also set cmd_per_lun to the same
value, but that doesn't help. Is there some relationship between
virtqueue_size and other settings?
(2) Can/should the ctrl, event and cmd queue sizes be set to the same
values? Or should ctrl & event be left at 128?
(3) It seems like it might be a problem that virtqueue_size is not
passed through the virtio_scsi_conf struct (which is ABI between the
kernel and qemu AFAICT and so not easily modifiable). However I think
this is not a problem because virtqueue size is stored in the
Virtqueue Descriptor table, which is how the kernel gets the value.
Is that right?
Rich.
--- kernel patch ---
diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index 9be211d68b15..d6b4ff634c0d 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -972,6 +972,8 @@ static int virtscsi_probe(struct virtio_device *vdev)
if (err)
goto virtscsi_init_failed;
+ shost->can_queue = virtqueue_get_vring_size(vscsi->req_vqs[0].vq);
+
cmd_per_lun = virtscsi_config_get(vdev, cmd_per_lun) ?: 1;
shost->cmd_per_lun = min_t(u32, cmd_per_lun, shost->can_queue);
shost->max_sectors = virtscsi_config_get(vdev, max_sectors) ?: 0x;
--- qemu patch ---
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index eb639442d1..aadd99aad1 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -867,10 +867,10 @@ void virtio_scsi_common_realize(DeviceState *dev,
s->sense_size = VIRTIO_SCSI_SENSE_DEFAULT_SIZE;
s->cdb_size = VIRTIO_SCSI_CDB_DEFAULT_SIZE;
-s->ctrl_vq = virtio_add_queue(vdev, VIRTIO_SCSI_VQ_SIZE, ctrl);
-s->event_vq = virtio_add_queue(vdev, VIRTIO_SCSI_VQ_SIZE, evt);
+s->ctrl_vq = virtio_add_queue(vdev, s->conf.virtqueue_size, ctrl);
+s->event_vq = virtio_add_queue(vdev, s->conf.virtqueue_size, evt);
for (i = 0; i < s->conf.num_queues; i++) {
-s->cmd_vqs[i] = virtio_add_queue(vdev, VIRTIO_SCSI_VQ_SIZE, cmd);
+s->cmd_vqs[i] = virtio_add_queue(vdev, s->conf.virtqueue_size, cmd);
}
}
@@ -917,6 +917,7 @@ static void virtio_scsi_device_unrealize(DeviceState *dev,
Error **errp)
static Property virtio_scsi_properties[] = {
DEFINE_PROP_UINT32("num_queues", VirtIOSCSI, parent_obj.conf.num_queues,
1),
+DEFINE_PROP_UINT32("virtqueue_size", VirtIOSCSI,
parent_obj.conf.virtqueue_size, 128),
DEFINE_PROP_UINT32("max_sectors", VirtIOSCSI, parent_obj.conf.max_sectors,
0x),
DEFINE_PROP_UINT32("cmd_per_lun", VirtIOSCSI, parent_obj.conf.cmd_per_lun,
diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h
index de6ae5a9f6..e30a92d3e7 100644
--- a/include/hw/virtio/virtio-scsi.h
+++ b/include/hw/virtio/virtio-scsi.h
@@ -48,6 +48,7 @@ typedef struct virtio_scsi_config VirtIOSCSIConfig;
struct VirtIOSCSIConf {
uint32_t num_queues;
+uint32_t virtqueue_size;
uint32_t max_sectors;
uint32_t cmd_per_lun;
#ifdef CONFIG_VHOST_SCSI
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html
Re: [PATCH RESEND 0/6] hpsa: support legacy boards
On 08/10/2017 04:06 PM, James Bottomley wrote: > On Thu, 2017-08-10 at 09:09 +0200, Christoph Hellwig wrote: >> No device support in Linux is unsupported, sorry. I think we're >> getting into the corporate bullshit game a little too much here. > > I think there are two different definitions of supported here. To us, > any device to which the driver attaches is "supported". However, if > it's never been tested before it may not work very well. In the Linux > way, we'll try to fix the bugs when they're reported and in that sense > we support the device until nothing in the kernel attaches to its ids > anymore. > > In the corporate world "supported" means we'll sell you a contract > giving you certain rights to report bugs and have us fix them. There > are definite reasons why corporations only support a small range of new > devices, even though devices not on this list may still be attached to > by the driver and thus we (Linux Community) would try to fix the bug > reports for. > > I think what you're basically asking for is a different name for the > flag, which is fine? how about 'legacy' instead? > Sure, no problem with that. Don? Cheers, Hannes -- Dr. Hannes ReineckeTeamlead Storage & Networking h...@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg)
Re: [PATCH RESEND 0/6] hpsa: support legacy boards
On Thu, 2017-08-10 at 09:09 +0200, Christoph Hellwig wrote: > No device support in Linux is unsupported, sorry. I think we're > getting into the corporate bullshit game a little too much here. I think there are two different definitions of supported here. To us, any device to which the driver attaches is "supported". However, if it's never been tested before it may not work very well. In the Linux way, we'll try to fix the bugs when they're reported and in that sense we support the device until nothing in the kernel attaches to its ids anymore. In the corporate world "supported" means we'll sell you a contract giving you certain rights to report bugs and have us fix them. There are definite reasons why corporations only support a small range of new devices, even though devices not on this list may still be attached to by the driver and thus we (Linux Community) would try to fix the bug reports for. I think what you're basically asking for is a different name for the flag, which is fine? how about 'legacy' instead? James
[PATCH] qedi: Limit number for CQ queues.
[qed_sp_iscsi_func_start:189(host_7-0)]Cannot satisfy CQ amount. Queues
requested 8, CQs available 4. Aborting function start
Above condition will resolve as management firmware is capable of telling
us the number of CQs available for a given PF, qed will communicate the
same number to qedi, So that qedi will know how much CQs are allowed.
Signed-off-by: Manish Rangankar
---
drivers/scsi/qedi/qedi.h | 5 ++---
drivers/scsi/qedi/qedi_iscsi.c | 2 +-
drivers/scsi/qedi/qedi_main.c | 10 +++---
3 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/drivers/scsi/qedi/qedi.h b/drivers/scsi/qedi/qedi.h
index 91d2f51..b8b22ce 100644
--- a/drivers/scsi/qedi/qedi.h
+++ b/drivers/scsi/qedi/qedi.h
@@ -54,8 +54,8 @@
/* MAX Length for cached SGL */
#define MAX_SGLEN_FOR_CACHESGL ((1U << 16) - 1)
-#define MAX_NUM_MSIX_PF 8
-#define MIN_NUM_CPUS_MSIX(x) min((x)->msix_count, num_online_cpus())
+#define MIN_NUM_CPUS_MSIX(x) min_t(u32, x->dev_info.num_cqs, \
+ num_online_cpus())
#define QEDI_LOCAL_PORT_MIN 6
#define QEDI_LOCAL_PORT_MAX 61024
@@ -301,7 +301,6 @@ struct qedi_ctx {
u16 bdq_prod_idx;
u16 rq_num_entries;
- u32 msix_count;
u32 max_sqes;
u8 num_queues;
u32 max_active_conns;
diff --git a/drivers/scsi/qedi/qedi_iscsi.c b/drivers/scsi/qedi/qedi_iscsi.c
index 37da9a8..a02b34e 100644
--- a/drivers/scsi/qedi/qedi_iscsi.c
+++ b/drivers/scsi/qedi/qedi_iscsi.c
@@ -534,7 +534,7 @@ static int qedi_iscsi_offload_conn(struct qedi_endpoint
*qedi_ep)
SET_FIELD(conn_info->tcp_flags, TCP_OFFLOAD_PARAMS_DA_CNT_EN, 1);
SET_FIELD(conn_info->tcp_flags, TCP_OFFLOAD_PARAMS_KA_EN, 1);
- conn_info->default_cq = (qedi_ep->fw_cid % 8);
+ conn_info->default_cq = (qedi_ep->fw_cid % qedi->num_queues);
conn_info->ka_max_probe_cnt = DEF_KA_MAX_PROBE_COUNT;
conn_info->dup_ack_theshold = 3;
diff --git a/drivers/scsi/qedi/qedi_main.c b/drivers/scsi/qedi/qedi_main.c
index 2c37836..c4a470b 100644
--- a/drivers/scsi/qedi/qedi_main.c
+++ b/drivers/scsi/qedi/qedi_main.c
@@ -794,13 +794,14 @@ static int qedi_set_iscsi_pf_param(struct qedi_ctx *qedi)
u32 log_page_size;
int rval = 0;
- QEDI_INFO(&qedi->dbg_ctx, QEDI_LOG_DISC, "Min number of MSIX %d\n",
- MIN_NUM_CPUS_MSIX(qedi));
num_sq_pages = (MAX_OUSTANDING_TASKS_PER_CON * 8) / PAGE_SIZE;
qedi->num_queues = MIN_NUM_CPUS_MSIX(qedi);
+ QEDI_INFO(&qedi->dbg_ctx, QEDI_LOG_INFO,
+ "Number of CQ count is %d\n", qedi->num_queues);
+
memset(&qedi->pf_params.iscsi_pf_params, 0,
sizeof(qedi->pf_params.iscsi_pf_params));
@@ -2179,9 +2180,12 @@ static int __qedi_probe(struct pci_dev *pdev, int mode)
goto free_host;
}
- qedi->msix_count = MAX_NUM_MSIX_PF;
atomic_set(&qedi->link_state, QEDI_LINK_DOWN);
+ rc = qedi_ops->fill_dev_info(qedi->cdev, &qedi->dev_info);
+ if (rc)
+ goto free_host;
+
if (mode != QEDI_MODE_RECOVERY) {
rc = qedi_set_iscsi_pf_param(qedi);
if (rc) {
--
1.8.3.1
Re: Increased memory usage with scsi-mq
On 10/08/2017 14:22, Richard W.M. Jones wrote: > On Wed, Aug 09, 2017 at 06:50:10PM +0200, Paolo Bonzini wrote: >> On 09/08/2017 18:01, Christoph Hellwig wrote: >>> On Mon, Aug 07, 2017 at 03:07:48PM +0200, Paolo Bonzini wrote: can_queue should depend on the virtqueue size, which unfortunately can vary for each virtio-scsi device in theory. The virtqueue size is retrieved by drivers/virtio prior to calling vring_create_virtqueue, and in QEMU it is the second argument to virtio_add_queue. >>> >>> Why is that unfortunate? We don't even have to set can_queue in >>> the host template, we can dynamically set it on per-host basis. >> >> Ah, cool, I thought allocations based on can_queue happened already in >> scsi_host_alloc, but they happen at scsi_add_host time. > > I think I've decoded all that information into the patch below. > > I tested it, and it appears to work: when I set cmd_per_lun on the > qemu command line, I see that the guest can add more disks: > > With scsi-mq enabled: 175 disks > cmd_per_lun not set:177 disks * > cmd_per_lun=16: 776 disks * > cmd_per_lun=4: 1160 disks * > With scsi-mq disabled: 1755 disks > * = new result > > From my point of view, this is a good result, but you should be warned > that I don't fully understand what's going on here and I may have made > obvious or not-so-obvious mistakes. can_queue and cmd_per_lun are different. can_queue should be set to the value of vq->vring.num where vq is the command virtqueue (the first one is okay if there's >1). If you want to change it, you'll have to do so in QEMU. Paolo > I tested the performance impact and it's not noticable in the > libguestfs case even with very small cmd_per_lun settings, but > libguestfs is largely serial and so this result won't be applicable to > guests in general. > > Also, should the guest kernel validate cmd_per_lun to make sure it's > not too small or large? And if so, what would the limits be? > > Rich. > > From e923e49846189b2f55f3f02b70a290d4be237ed5 Mon Sep 17 00:00:00 2001 > From: "Richard W.M. Jones" > Date: Thu, 10 Aug 2017 12:21:47 +0100 > Subject: [PATCH] scsi: virtio_scsi: Set can_queue based on cmd_per_lun passed > by hypervisor. > > Signed-off-by: Richard W.M. Jones > --- > drivers/scsi/virtio_scsi.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c > index 9be211d68b15..b22591e9b16b 100644 > --- a/drivers/scsi/virtio_scsi.c > +++ b/drivers/scsi/virtio_scsi.c > @@ -973,7 +973,7 @@ static int virtscsi_probe(struct virtio_device *vdev) > goto virtscsi_init_failed; > > cmd_per_lun = virtscsi_config_get(vdev, cmd_per_lun) ?: 1; > - shost->cmd_per_lun = min_t(u32, cmd_per_lun, shost->can_queue); > + shost->cmd_per_lun = shost->can_queue = cmd_per_lun; > shost->max_sectors = virtscsi_config_get(vdev, max_sectors) ?: 0x; > > /* LUNs > 256 are reported with format 1, so they go in the range >
Re: nvmf question - synchronization between target/initiator regarding partitions
On 08/10/2017 06:16 AM, Christoph Hellwig wrote: > On Mon, Aug 07, 2017 at 02:29:47PM -0300, Guilherme G. Piccoli wrote: >> Thanks for your feedback Hannes, agreed! > > And btw, you'll see similar results with the SCSI target or nbd, > so it's not really nvme specific. Thanks, I agree - noticed the same stuff. I've used nvmf as a "trigger" to the subject, in order to discuss a possible generic solution. But since everything is working as expected, no need to pursue a "fix" heheh
Re: Increased memory usage with scsi-mq
On Wed, Aug 09, 2017 at 06:50:10PM +0200, Paolo Bonzini wrote: > On 09/08/2017 18:01, Christoph Hellwig wrote: > > On Mon, Aug 07, 2017 at 03:07:48PM +0200, Paolo Bonzini wrote: > >> can_queue should depend on the virtqueue size, which unfortunately can > >> vary for each virtio-scsi device in theory. The virtqueue size is > >> retrieved by drivers/virtio prior to calling vring_create_virtqueue, and > >> in QEMU it is the second argument to virtio_add_queue. > > > > Why is that unfortunate? We don't even have to set can_queue in > > the host template, we can dynamically set it on per-host basis. > > Ah, cool, I thought allocations based on can_queue happened already in > scsi_host_alloc, but they happen at scsi_add_host time. I think I've decoded all that information into the patch below. I tested it, and it appears to work: when I set cmd_per_lun on the qemu command line, I see that the guest can add more disks: With scsi-mq enabled: 175 disks cmd_per_lun not set:177 disks * cmd_per_lun=16: 776 disks * cmd_per_lun=4: 1160 disks * With scsi-mq disabled: 1755 disks * = new result >From my point of view, this is a good result, but you should be warned that I don't fully understand what's going on here and I may have made obvious or not-so-obvious mistakes. I tested the performance impact and it's not noticable in the libguestfs case even with very small cmd_per_lun settings, but libguestfs is largely serial and so this result won't be applicable to guests in general. Also, should the guest kernel validate cmd_per_lun to make sure it's not too small or large? And if so, what would the limits be? Rich. >From e923e49846189b2f55f3f02b70a290d4be237ed5 Mon Sep 17 00:00:00 2001 From: "Richard W.M. Jones" Date: Thu, 10 Aug 2017 12:21:47 +0100 Subject: [PATCH] scsi: virtio_scsi: Set can_queue based on cmd_per_lun passed by hypervisor. Signed-off-by: Richard W.M. Jones --- drivers/scsi/virtio_scsi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c index 9be211d68b15..b22591e9b16b 100644 --- a/drivers/scsi/virtio_scsi.c +++ b/drivers/scsi/virtio_scsi.c @@ -973,7 +973,7 @@ static int virtscsi_probe(struct virtio_device *vdev) goto virtscsi_init_failed; cmd_per_lun = virtscsi_config_get(vdev, cmd_per_lun) ?: 1; - shost->cmd_per_lun = min_t(u32, cmd_per_lun, shost->can_queue); + shost->cmd_per_lun = shost->can_queue = cmd_per_lun; shost->max_sectors = virtscsi_config_get(vdev, max_sectors) ?: 0x; /* LUNs > 256 are reported with format 1, so they go in the range -- 2.13.1 -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW
Re: [RFC PATCH 6/6] bsg: reduce unnecessary arguments for blk_complete_sgv4_hdr_rq()
Looks good, Reviewed-by: Christoph Hellwig
Re: [RFC PATCH 5/6] bsg: reduce unnecessary arguments for bsg_map_hdr()
On Wed, Aug 09, 2017 at 04:11:19PM +0200, Benjamin Block wrote: > Since struct bsg_command is now used in every calling case, we don't > need separation of arguments anymore that are contained in the same > bsg_command. > > Signed-off-by: Benjamin Block > --- > block/bsg.c | 13 ++--- > 1 file changed, 6 insertions(+), 7 deletions(-) > > diff --git a/block/bsg.c b/block/bsg.c > index 8517361a9b3f..6ee2ffca808a 100644 > --- a/block/bsg.c > +++ b/block/bsg.c > @@ -212,11 +212,12 @@ bsg_validate_sgv4_hdr(struct sg_io_v4 *hdr, int *op) > * map sg_io_v4 to a request. > */ > static struct request * > -bsg_map_hdr(struct bsg_device *bd, struct sg_io_v4 *hdr, fmode_t > has_write_perm, > - u8 *reply_buffer) > +bsg_map_hdr(struct bsg_device *bd, struct bsg_command *bc, > + fmode_t has_write_perm) I wish we could just rename the argument to mode and pass on the whole file->f_mode while you are cleaning up this code. That should be a separate patch, though. Reviewed-by: Christoph Hellwig
Re: [RFC PATCH 4/6] bsg: refactor ioctl to use regular BSG-command infrastructure for SG_IO
On Thu, Aug 10, 2017 at 10:24:56AM +0200, Johannes Thumshirn wrote: > On Wed, Aug 09, 2017 at 04:11:18PM +0200, Benjamin Block wrote: > > + return 0 == (bc->hdr.flags & BSG_FLAG_Q_AT_TAIL); > > return !(bc->hdr.flags & BSG_FLAG_Q_AT_TAIL); and make the function return > bool? I have to admit, this is the 1st time I have seen the above construct. It's a somewhat odd style. I agree with your comment, but otherwise the patch looks ok to me.
Re: [RFC PATCH 3/6] bsg: scsi-transport: add compile-tests to prevent reply-buffer overflows
Looks fine, Reviewed-by: Christoph Hellwig
Re: [RFC PATCH 2/6] bsg: assign sense_len instead of fixed SCSI_SENSE_BUFFERSIZE
Looks fine, Reviewed-by: Christoph Hellwig
Re: [RFC PATCH 1/6] bsg: fix kernel panic resulting from missing allocation of a reply-buffer
We can't use an on-stack buffer for the sense data, as drivers will dma to it. So we should reuse the SCSI init_rq_fn() for the BSG queues and/or implement the same scheme.
Re: [PATCH 3/3] scsi: make 'state' device attribute pollable
Looks good, Reviewed-by: Christoph Hellwig
Re: [PATCH 2/3] scsi_lib: rework scsi_internal_device_unblock_nowait()
On Thu, Aug 10, 2017 at 09:05:30AM +0200, Hannes Reinecke wrote:
> Rework scsi_internal_device_unblock_nowait() into using a
> switch statement.
> No functional changes.
>
> Signed-off-by: Hannes Reinecke
> ---
> drivers/scsi/scsi_lib.c | 18 --
> 1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index 1ae531b..035aa4c 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -3074,19 +3074,25 @@ int scsi_internal_device_unblock_nowait(struct
> scsi_device *sdev,
>* Try to transition the scsi device to SDEV_RUNNING or one of the
>* offlined states and goose the device queue if successful.
>*/
> - if ((sdev->sdev_state == SDEV_BLOCK) ||
> - (sdev->sdev_state == SDEV_TRANSPORT_OFFLINE))
> + switch (sdev->sdev_state) {
> + case SDEV_BLOCK:
> + case SDEV_TRANSPORT_OFFLINE:
> sdev->sdev_state = new_state;
> - else if (sdev->sdev_state == SDEV_CREATED_BLOCK) {
> + break;
> + case SDEV_CREATED_BLOCK:
> if (new_state == SDEV_TRANSPORT_OFFLINE ||
> new_state == SDEV_OFFLINE)
> sdev->sdev_state = new_state;
> else
> sdev->sdev_state = SDEV_CREATED;
> - } else if (sdev->sdev_state != SDEV_CANCEL &&
> - sdev->sdev_state != SDEV_OFFLINE)
> + break;
> + case SDEV_TRANSPORT_OFFLINE:
> + case SDEV_CANCEL:
> + case SDEV_OFFLINE:
> + break;
> + default:
> return -EINVAL;
This changes ok by default to reject by default and instead lists
the ok states. Which probably is the right thing to do for future
proofing against new states, so:
Reviewed-by: Christoph Hellwig
