Re: [PATCH v4 02/10] ufs: sysfs: device descriptor
On Fri, Feb 02, 2018 at 12:25:46AM +, Bart Van Assche wrote:
> On Thu, 2018-02-01 at 18:15 +0200, Stanislav Nijnikov wrote:
> > +enum ufs_desc_param_size {
> > + UFS_PARAM_BYTE_SIZE = 1,
> > + UFS_PARAM_WORD_SIZE = 2,
> > + UFS_PARAM_DWORD_SIZE= 4,
> > + UFS_PARAM_QWORD_SIZE= 8,
> > +};
>
> Please do not copy bad naming choices from the Windows kernel into the Linux
> kernel. Using names like WORD / DWORD / QWORD is much less readable than using
> the numeric constants 2, 4, 8. Hence my proposal to leave out the above enum
> completely.
Are you sure those do not come from the spec itself? It's been a while
since I last read it, but for some reason I remember those types of
names being in there. But I might be confusing specs here.
thanks,
greg k-h
Re: [PATCH 1/1] scsi: ufs: make sure all interrupts are processed
On 1/31/2018 1:09 PM, Avri Altman wrote:
Hi,
Can you elaborate how this can even happen?
Isn't the interrupt aggregation capability should attend for those cases?
Thanks,
Avri
-Original Message-
From: linux-scsi-ow...@vger.kernel.org [mailto:linux-scsi-
ow...@vger.kernel.org] On Behalf Of Asutosh Das
Sent: Tuesday, January 30, 2018 6:54 AM
To: subha...@codeaurora.org; c...@codeaurora.org;
vivek.gau...@codeaurora.org; rna...@codeaurora.org;
vinholika...@gmail.com; j...@linux.vnet.ibm.com;
martin.peter...@oracle.com
Cc: linux-scsi@vger.kernel.org; Venkat Gopalakrishnan
; Asutosh Das ; open
list
Subject: [PATCH 1/1] scsi: ufs: make sure all interrupts are processed
From: Venkat Gopalakrishnan
As multiple requests are submitted to the ufs host controller in parallel there
could be instances where the command completion interrupt arrives later for a
request that is already processed earlier as the corresponding doorbell was
cleared when handling the previous interrupt. Read the interrupt status in a
loop after processing the received interrupt to catch such interrupts and handle
it.
Signed-off-by: Venkat Gopalakrishnan
Signed-off-by: Asutosh Das
---
drivers/scsi/ufs/ufshcd.c | 27 +++
1 file changed, 19 insertions(+), 8 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index
8af2af3..58d81de 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -5357,19 +5357,30 @@ static irqreturn_t ufshcd_intr(int irq, void *__hba)
u32 intr_status, enabled_intr_status;
irqreturn_t retval = IRQ_NONE;
struct ufs_hba *hba = __hba;
+ int retries = hba->nutrs;
spin_lock(hba->host->host_lock);
intr_status = ufshcd_readl(hba, REG_INTERRUPT_STATUS);
- enabled_intr_status =
- intr_status & ufshcd_readl(hba, REG_INTERRUPT_ENABLE);
- if (intr_status)
- ufshcd_writel(hba, intr_status, REG_INTERRUPT_STATUS);
+ /*
+* There could be max of hba->nutrs reqs in flight and in worst case
+* if the reqs get finished 1 by 1 after the interrupt status is
+* read, make sure we handle them by checking the interrupt status
+* again in a loop until we process all of the reqs before returning.
+*/
+ do {
+ enabled_intr_status =
+ intr_status & ufshcd_readl(hba,
REG_INTERRUPT_ENABLE);
+ if (intr_status)
+ ufshcd_writel(hba, intr_status,
REG_INTERRUPT_STATUS);
+ if (enabled_intr_status) {
+ ufshcd_sl_intr(hba, enabled_intr_status);
+ retval = IRQ_HANDLED;
+ }
+
+ intr_status = ufshcd_readl(hba, REG_INTERRUPT_STATUS);
+ } while (intr_status && --retries);
- if (enabled_intr_status) {
- ufshcd_sl_intr(hba, enabled_intr_status);
- retval = IRQ_HANDLED;
- }
spin_unlock(hba->host->host_lock);
return retval;
}
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center,
Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux
Foundation Collaborative Project.
Hi
yes - interrupt aggregation makes sense here. But there were some
performance concerns with it; well, I don't have the data to back that
up now though.
However, I can code it up and check it.
Will post it in some time.
-asd
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a
Linux Foundation Collaborative Project
Re: [LSF/MM TOPIC] irq affinity handling for high CPU count machines
Hello Kashyap, On Thu, Feb 01, 2018 at 10:29:22PM +0530, Kashyap Desai wrote: > > -Original Message- > > From: Hannes Reinecke [mailto:h...@suse.de] > > Sent: Thursday, February 1, 2018 9:50 PM > > To: Ming Lei > > Cc: lsf...@lists.linux-foundation.org; linux-scsi@vger.kernel.org; linux- > > n...@lists.infradead.org; Kashyap Desai > > Subject: Re: [LSF/MM TOPIC] irq affinity handling for high CPU count > > machines > > > > On 02/01/2018 04:05 PM, Ming Lei wrote: > > > Hello Hannes, > > > > > > On Mon, Jan 29, 2018 at 10:08:43AM +0100, Hannes Reinecke wrote: > > >> Hi all, > > >> > > >> here's a topic which came up on the SCSI ML (cf thread '[RFC 0/2] > > >> mpt3sas/megaraid_sas: irq poll and load balancing of reply queue'). > > >> > > >> When doing I/O tests on a machine with more CPUs than MSIx vectors > > >> provided by the HBA we can easily setup a scenario where one CPU is > > >> submitting I/O and the other one is completing I/O. Which will result > > >> in the latter CPU being stuck in the interrupt completion routine for > > >> basically ever, resulting in the lockup detector kicking in. > > > > > > Today I am looking at one megaraid_sas related issue, and found > > > pci_alloc_irq_vectors(PCI_IRQ_AFFINITY) is used in the driver, so > > > looks each reply queue has been handled by more than one CPU if there > > > are more CPUs than MSIx vectors in the system, which is done by > > > generic irq affinity code, please see kernel/irq/affinity.c. > > Yes. That is a problematic area. If CPU and MSI-x(reply queue) is 1:1 > mapped, we don't have any issue. I guess the problematic area is similar with the following link: https://marc.info/?l=linux-kernel&m=151748144730409&w=2 otherwise could you explain a bit about the area? > > > > > > > Also IMO each reply queue may be treated as blk-mq's hw queue, then > > > megaraid may benefit from blk-mq's MQ framework, but one annoying > > > thing is that both legacy and blk-mq path need to be handled inside > > > driver. > > Both MR and IT driver is (due to H/W design.) is using blk-mq frame work but > it is really a single h/w queue. > IT and MR HBA has single submission queue and multiple reply queue. It should have been covered by MQ, but just we need to share tags among hctxs, like what Hannes posted long time ago. > > > > > > The megaraid driver is a really strange beast;, having layered two > > different > > interfaces (the 'legacy' MFI interface and that from from > > mpt3sas) on top of each other. > > I had been thinking of converting it to scsi-mq, too (as my mpt3sas patch > > finally went in), but I'm not sure if we can benefit from it as we're > > still be > > bound by the HBA-wide tag pool. > > It's on my todo list, albeit pretty far down :-) > > Hannes, this is typically same in both MR (megaraid_sas) and IT (mpt3sas). > Both the driver is using shared HBA-wide tag pool. > Both MR and IT driver use request->tag to get command from free pool. Seems a generic thing, same with HPSA too. Thanks, Ming
Re: [LSF/MM TOPIC] irq affinity handling for high CPU count machines
Hello Hannes, On Thu, Feb 01, 2018 at 05:20:26PM +0100, Hannes Reinecke wrote: > On 02/01/2018 04:05 PM, Ming Lei wrote: > > Hello Hannes, > > > > On Mon, Jan 29, 2018 at 10:08:43AM +0100, Hannes Reinecke wrote: > >> Hi all, > >> > >> here's a topic which came up on the SCSI ML (cf thread '[RFC 0/2] > >> mpt3sas/megaraid_sas: irq poll and load balancing of reply queue'). > >> > >> When doing I/O tests on a machine with more CPUs than MSIx vectors > >> provided by the HBA we can easily setup a scenario where one CPU is > >> submitting I/O and the other one is completing I/O. Which will result in > >> the latter CPU being stuck in the interrupt completion routine for > >> basically ever, resulting in the lockup detector kicking in. > > > > Today I am looking at one megaraid_sas related issue, and found > > pci_alloc_irq_vectors(PCI_IRQ_AFFINITY) is used in the driver, so looks > > each reply queue has been handled by more than one CPU if there are more > > CPUs than MSIx vectors in the system, which is done by generic irq affinity > > code, please see kernel/irq/affinity.c. > > > > Also IMO each reply queue may be treated as blk-mq's hw queue, then > > megaraid may benefit from blk-mq's MQ framework, but one annoying thing is > > that both legacy and blk-mq path need to be handled inside driver. > > > The megaraid driver is a really strange beast;, having layered two > different interfaces (the 'legacy' MFI interface and that from from > mpt3sas) on top of each other. > I had been thinking of converting it to scsi-mq, too (as my mpt3sas > patch finally went in), but I'm not sure if we can benefit from it as > we're still be bound by the HBA-wide tag pool. Actually current SCSI_MQ works at this mode of HBA-wide tag pool too, please see scsi_host_queue_ready() which is called in scsi_queue_rq(), same with scsi_mq_get_budget(). Seems it is weird for real MQ cases, even the tag is allocated from per-hctx tags, the host wide queue depth still need to be respected, finally it is just like HBA-wide tag pool. That is something which need to discuss too. Also I remembered you posted the patch for sharing tags among hctx, that should help to convert reply queues into scsi_mq/blk_mq's hctx. > It's on my todo list, albeit pretty far down :-) > > >> > >> How should these situations be handled? > >> Should it be made the responsibility of the drivers, ensuring that the > >> interrupt completion routine is terminated after a certain time? > >> Should it be made the resposibility of the upper layers? > >> Should it be the responsibility of the interrupt mapping code? > >> Can/should interrupt polling be used in these situations? > > > > Yeah, I guess interrupt polling may improve these situations, especially > > KPTI introduces some extra cost in interrupt handling. > > > The question is not so much if one should be doing irq polling, but > rather if we can come up with some guidance or even infrastructure to > make this happen automatically. > Having to rely on individual drivers to get this right is probably not > the best option. Agree. Thanks, Ming
Re: [dm-devel] [PATCH V4] blk-mq: introduce BLK_STS_DEV_RESOURCE
On Thu, 2018-02-01 at 19:26 -0500, John Stoffel wrote: > Doesn't this argue that you really want some sort of completions to be > run in this case instead? Instead of busy looping or waiting for a > set amount of time, just fire off a callback to run once you have the > resources available, no? Hello John, Rerunning the queue when the resource we ran out of is available again would be ideal. However, I'm not sure it is possible to implement this. If a driver performs memory allocation while executing a request and kmalloc() returns -ENOMEM then I don't know which mechanism should be used to trigger a queue rerun when sufficient memory is available again. Another example is the SCSI subsystem. If a the .queuecommand() implementation of a SCSI LLD returns SCSI_MLQUEUE_*_BUSY because a certain HBA condition occurred and the HBA does not trigger an interrupt when that condition is cleared, how to figure out whether or not a request can be executed other than by retrying, which means rerunning the queue? Thanks, Bart.
Re: [PATCH v4 08/10] ufs: sysfs: unit descriptor
On Thu, 2018-02-01 at 18:15 +0200, Stanislav Nijnikov wrote:
> diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
> index cbc0fe2..69a6a1f 100644
> --- a/drivers/scsi/scsi_sysfs.c
> +++ b/drivers/scsi/scsi_sysfs.c
> @@ -1309,6 +1309,14 @@ int scsi_sysfs_add_sdev(struct scsi_device *sdev)
> }
> }
>
> + if (sdev->host->hostt->sdev_groups) {
> + error = sysfs_create_groups(&sdev->sdev_gendev.kobj,
> + (const struct attribute_group **)
> + sdev->host->hostt->sdev_groups);
> + if (error)
> + return error;
> + }
> +
> scsi_autopm_put_device(sdev);
> return error;
> }
> @@ -1326,6 +1334,12 @@ void __scsi_remove_device(struct scsi_device *sdev)
> if (sdev->sdev_state == SDEV_DEL)
> return;
>
> + if (sdev->host->hostt->sdev_groups) {
> + sysfs_remove_groups(&sdev->sdev_gendev.kobj,
> + (const struct attribute_group **)
> + sdev->host->hostt->sdev_groups);
> + }
> +
> if (sdev->is_visible) {
> /*
>* If scsi_internal_target_block() is running concurrently,
Please split this patch into two patches, namely one patch that introduces
the sdev_groups attribute in the SCSI core and a second patch that does not
modify the SCSI core but only the UFS driver. That will make these changes
easier to review.
Additionally, it seems wrong to me to cast the third argument of
sysfs_create_groups() and sysfs_remove_groups() from a non-const to a const
pointer. Please consider to declare the sdev_groups member const and also
the corresponding data structures in the UFS driver.
Thanks,
Bart.
Re: [dm-devel] [PATCH V4] blk-mq: introduce BLK_STS_DEV_RESOURCE
> "Bart" == Bart Van Assche writes: Bart> On Mon, 2018-01-29 at 16:44 -0500, Mike Snitzer wrote: >> But regardless of which wins the race, the queue will have been run. >> Which is all we care about right? Bart> Running the queue is not sufficient. With this patch applied it can happen Bart> that the block driver returns BLK_STS_DEV_RESOURCE, that the two or more Bart> concurrent queue runs finish before sufficient device resources are available Bart> to execute the request and that blk_mq_delay_run_hw_queue() does not get Bart> called at all. If no other activity triggers a queue run, e.g. request Bart> completion, this will result in a queue stall. Doesn't this argue that you really want some sort of completions to be run in this case instead? Instead of busy looping or waiting for a set amount of time, just fire off a callback to run once you have the resources available, no? I can't provide any code examples, I don't know the code base nearly well enough. John
Re: [PATCH v4 02/10] ufs: sysfs: device descriptor
On Thu, 2018-02-01 at 18:15 +0200, Stanislav Nijnikov wrote:
> +ssize_t ufs_sysfs_read_desc_param(struct ufs_hba *hba,
> + enum desc_idn desc_id,
> + u8 desc_index,
> + u8 param_offset,
> + u8 *sysfs_buf,
> + u8 param_size)
> +{
> + u8 desc_buf[UFS_PARAM_QWORD_SIZE] = {0};
> + int ret;
> +
> + if (param_size > UFS_PARAM_QWORD_SIZE)
> + return -EINVAL;
> +
> + ret = ufshcd_read_desc_param(hba, desc_id, desc_index,
> + param_offset, desc_buf, param_size);
> + if (ret)
> + return -EINVAL;
> + switch (param_size) {
> + case UFS_PARAM_BYTE_SIZE:
> + ret = sprintf(sysfs_buf, "0x%02X\n", *desc_buf);
> + break;
> + case UFS_PARAM_WORD_SIZE:
> + ret = sprintf(sysfs_buf, "0x%04X\n",
> + be16_to_cpu(*((u16 *)desc_buf)));
> + break;
> + case UFS_PARAM_DWORD_SIZE:
> + ret = sprintf(sysfs_buf, "0x%08X\n",
> + be32_to_cpu(*((u32 *)desc_buf)));
> + break;
> + case UFS_PARAM_QWORD_SIZE:
> + ret = sprintf(sysfs_buf, "0x%016llX\n",
> + be64_to_cpu(*((u64 *)desc_buf)));
> + break;
> + }
> +
> + return ret;
> +}
Seeing code like this makes me wonder whether this patch series has been
verified
with sparse? I think sparse will complain about all three be*_to_cpu() casts
above.
Please use get_unaligned_be*() instead of open-coding these functions.
Thanks,
Bart.
Re: [PATCH v4 02/10] ufs: sysfs: device descriptor
On Thu, 2018-02-01 at 18:15 +0200, Stanislav Nijnikov wrote:
> +enum ufs_desc_param_size {
> + UFS_PARAM_BYTE_SIZE = 1,
> + UFS_PARAM_WORD_SIZE = 2,
> + UFS_PARAM_DWORD_SIZE= 4,
> + UFS_PARAM_QWORD_SIZE= 8,
> +};
Please do not copy bad naming choices from the Windows kernel into the Linux
kernel. Using names like WORD / DWORD / QWORD is much less readable than using
the numeric constants 2, 4, 8. Hence my proposal to leave out the above enum
completely.
Thanks,
Bart.
test results
The test-case results with PATCH v2. scsih_abort() = Without patch: [ 362.669743] setting logging_level(0x1000) [ 362.705074] mpt3sas_cm0: skip free_smid/scsi_done scmd(c01fd4f2bd40) [ 363.956579] sd 16:0:1:0: [sdf] Synchronizing SCSI cache [ 363.956844] sd 16:0:0:0: [sde] Synchronizing SCSI cache [ 363.957147] mpt3sas_cm0: sending diag reset !! [ 365.073568] mpt3sas_cm0: diag reset: SUCCESS [ 365.090316] mpt3sas_cm0: sleep on shutdown [ 366.120209] mpt3sas_cm0: sleep on shutdown ... [ 373.419954] sd 16:0:1:0: attempting task abort! scmd(c01fd4f2bd40) ... [ 373.420256] mpt3sas_scsih_issue_tm: mpt3sas_cm0: host reset in progress! [ 373.420300] sd 16:0:1:0: task abort: FAILED scmd(c01fd4f2bd40) [ 373.459961] sd 16:0:1:0: attempting device reset! scmd(c01fd4f2bd40) ... [ 373.460271] mpt3sas_scsih_issue_tm: mpt3sas_cm0: host reset in progress! [ 373.460315] sd 16:0:1:0: device reset: FAILED scmd(c01fd4f2bd40) [ 373.460362] scsi target16:0:1: attempting target reset! scmd(c01fd4f2bd40) ... [ 373.460628] mpt3sas_scsih_issue_tm: mpt3sas_cm0: host reset in progress! [ 373.460673] scsi target16:0:1: target reset: FAILED scmd(c01fd4f2bd40) [ 373.460720] mpt3sas_cm0: attempting host reset! scmd(c01fd4f2bd40) [ 373.460764] sd 16:0:1:0: [sdf] tag#0 CDB: Read(10) 28 00 00 00 00 00 00 00 01 00 [ 373.460821] Unable to handle kernel paging request for data at address 0xd0003800817d [ 373.460876] Faulting instruction address: 0xd00017b169b8 [ 373.460921] Oops: Kernel access of bad area, sig: 11 [#1] ... [ 373.462028] NIP [d00017b169b8] mpt3sas_base_get_iocstate+0x38/0xb0 [mpt3sas] [ 373.462085] LR [d00017b1a3f0] mpt3sas_base_hard_reset_handler+0x1a0/0x6d0 [mpt3sas] [ 373.462138] Call Trace: [ 373.462160] [c01fdf5f7ab0] [d00017b1a3f0] mpt3sas_base_hard_reset_handler+0x1a0/0x6d0 [mpt3sas] [ 373.462225] [c01fdf5f7b80] [d00017b20f6c] scsih_host_reset+0x7c/0x100 [mpt3sas] [ 373.462281] [c01fdf5f7c00] [c076f34c] scsi_try_host_reset+0x5c/0x140 [ 373.462335] [c01fdf5f7c40] [c077107c] scsi_eh_ready_devs+0x73c/0x960 [ 373.462390] [c01fdf5f7d10] [c0772800] scsi_error_handler+0x4c0/0x4e0 [ 373.462444] [c01fdf5f7dc0] [c0107ed4] kthread+0x164/0x1b0 [ 373.462490] [c01fdf5f7e30] [c000b5a0] ret_from_kernel_thread+0x5c/0xbc [ 373.468473] Instruction dump: With patch: [ 332.994654] setting logging_level(0x1000) [ 333.024246] mpt3sas_cm0: skip free_smid/scsi_done scmd(c03c97348140) [ 334.232041] sd 16:0:1:0: [sdf] Synchronizing SCSI cache [ 334.232324] sd 16:0:0:0: [sde] Synchronizing SCSI cache [ 334.232598] mpt3sas_cm0: sending diag reset !! [ 335.352533] mpt3sas_cm0: diag reset: SUCCESS [ 335.372075] mpt3sas_cm0: sleep on shutdown [ 336.440544] mpt3sas_cm0: sleep on shutdown ... [ 343.100179] sd 16:0:1:0: attempting task abort! scmd(c03c97348140) ... [ 343.100488] sd 16:0:1:0: device been deleted! scmd(c03c97348140) [ 343.100532] sd 16:0:1:0: task abort: SUCCESS scmd(c03c97348140) [ 343.140185] sd 16:0:1:0: [sdf] tag#0 FAILED Result: hostbyte=DID_NO_CONNECT driverbyte=DRIVER_OK [ 343.140275] sd 16:0:1:0: [sdf] tag#0 CDB: Read(10) 28 00 00 00 00 00 00 00 01 00 [ 343.140338] print_req_error: I/O error, dev sdf, sector 0 scsih_dev/target/host_reset() === Without patch: [ 101.077946] setting logging_level(0x1100) [ 101.110029] mpt3sas_cm0: skip free_smid/scsi_done scmd(c01fb7a2dd40) [ 102.356108] sd 16:0:1:0: [sdf] Synchronizing SCSI cache [ 102.356396] sd 16:0:0:0: [sde] Synchronizing SCSI cache [ 102.356647] mpt3sas_cm0: sending diag reset !! [ 103.476380] mpt3sas_cm0: diag reset: SUCCESS [ 103.492696] mpt3sas_cm0: sleep on shutdown [ 104.512914] mpt3sas_cm0: sleep on shutdown ... [ 111.732912] sd 16:0:1:0: attempting task abort! scmd(c01fb7a2dd40) ... [ 111.733202] mpt3sas_cm0: fail task abort scmd(c01fb7a2dd40) [ 111.733246] sd 16:0:1:0: task abort: FAILED scmd(c01fb7a2dd40) [ 111.772919] sd 16:0:1:0: attempting device reset! scmd(c01fb7a2dd40) ... [ 111.773177] mpt3sas_scsih_issue_tm: mpt3sas_cm0: host reset in progress! [ 111.773222] sd 16:0:1:0: device reset: FAILED scmd(c01fb7a2dd40) [ 111.773266] scsi target16:0:1: attempting target reset! scmd(c01fb7a2dd40) ... [ 111.773528] mpt3sas_scsih_issue_tm: mpt3sas_cm0: host reset in progress! [ 111.773574] scsi target16:0:1: target reset: FAILED scmd(c01fb7a2dd40) [ 111.773617] mpt3sas_cm0: attempting host reset! scmd(c01fb7a2dd40) [ 111.773662] sd 16:0:1:0: [sdf] tag#0 CDB: Read(1
[PATCH v2] scsi: mpt3sas: fix oops in error handlers after shutdown/unload
This patch adds checks for 'ioc->remove_host' in the SCSI error
handlers, so not to access pointers/resources potentially freed
in the PCI shutdown/module unload path. The error handlers may
be invoked after shutdown/unload, depending on other components.
This problem was observed with kexec on a system with a mpt3sas
based adapter and an infiniband adapter which takes long enough
to shutdown:
The mpt3sas driver finished shutting down / disabled interrupt
handling, thus some commands have not finished and timed out.
Since the system was still running (waiting for the infiniband
adapter to shutdown), the scsi error handler for task abort of
mpt3sas was invoked, and hit an oops -- either in scsih_abort()
because 'ioc->scsi_lookup' was NULL (without commit dbec4c9040ed
("scsi: mpt3sas: lockless command submission")), or later up in
scsih_host_reset() (with or without that commit), because it
eventually called mpt3sas_base_get_iocstate().
After that commit, the oops in scsih_abort() does not occur
anymore (_scsih_scsi_lookup_find_by_scmd() is no longer called),
but that commit is too big and out of the scope of linux-stable,
where this patch might help, so still go for the changes.
Also, this might help to prevent similar errors in the future,
in case code changes and possibly tries to access freed stuff.
Note the fix in scsih_host_reset() is still important anyway.
Signed-off-by: Mauricio Faria de Oliveira
---
v2:
- rebase on top of mkp/scsi.git's fixes branch
- insert check for 'ioc->remove_host' in existing
checks which already set DID_NO_CONNECT instead
of duplicating that code. (helps with backports)
- update commit message about that commit.
drivers/scsi/mpt3sas/mpt3sas_scsih.c | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index 74fca18..5ab3caf 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -2835,7 +2835,8 @@ int mpt3sas_scsih_issue_locked_tm(struct MPT3SAS_ADAPTER
*ioc, u16 handle,
_scsih_tm_display_info(ioc, scmd);
sas_device_priv_data = scmd->device->hostdata;
- if (!sas_device_priv_data || !sas_device_priv_data->sas_target) {
+ if (!sas_device_priv_data || !sas_device_priv_data->sas_target ||
+ ioc->remove_host) {
sdev_printk(KERN_INFO, scmd->device,
"device been deleted! scmd(%p)\n", scmd);
scmd->result = DID_NO_CONNECT << 16;
@@ -2898,7 +2899,8 @@ int mpt3sas_scsih_issue_locked_tm(struct MPT3SAS_ADAPTER
*ioc, u16 handle,
_scsih_tm_display_info(ioc, scmd);
sas_device_priv_data = scmd->device->hostdata;
- if (!sas_device_priv_data || !sas_device_priv_data->sas_target) {
+ if (!sas_device_priv_data || !sas_device_priv_data->sas_target ||
+ ioc->remove_host) {
sdev_printk(KERN_INFO, scmd->device,
"device been deleted! scmd(%p)\n", scmd);
scmd->result = DID_NO_CONNECT << 16;
@@ -2961,7 +2963,8 @@ int mpt3sas_scsih_issue_locked_tm(struct MPT3SAS_ADAPTER
*ioc, u16 handle,
_scsih_tm_display_info(ioc, scmd);
sas_device_priv_data = scmd->device->hostdata;
- if (!sas_device_priv_data || !sas_device_priv_data->sas_target) {
+ if (!sas_device_priv_data || !sas_device_priv_data->sas_target ||
+ ioc->remove_host) {
starget_printk(KERN_INFO, starget, "target been deleted!
scmd(%p)\n",
scmd);
scmd->result = DID_NO_CONNECT << 16;
@@ -3019,7 +3022,7 @@ int mpt3sas_scsih_issue_locked_tm(struct MPT3SAS_ADAPTER
*ioc, u16 handle,
ioc->name, scmd);
scsi_print_command(scmd);
- if (ioc->is_driver_loading) {
+ if (ioc->is_driver_loading || ioc->remove_host) {
pr_info(MPT3SAS_FMT "Blocking the host reset\n",
ioc->name);
r = FAILED;
--
1.8.3.1
Re: [LSF/MM TOPIC] KPTI effect on IO performance
On 01/31/18 00:23, Ming Lei wrote: After KPTI is merged, there is extra load introduced to context switch between user space and kernel space. It is observed on my laptop that one syscall takes extra ~0.15us[1] compared with 'nopti'. IO performance is affected too, it is observed that IOPS drops by 32% in my test[2] on null_blk compared with 'nopti': randread IOPS on latest linus tree: - | randread IOPS | randread IOPS with 'nopti'| | 928K | 1372K | Two paths are affected, one is IO submission(read, write,... syscall), another is the IO completion path in which interrupt may be triggered from user space, and context switch is needed. So is there something we can do for decreasing the effect on IO performance? This effect may make Hannes's issue[3] worse, and maybe 'irq poll' should be used more widely for all high performance IO device, even some optimization should be considered for KPTI's effect. For what kind of workload would you like to improve I/O performance? Desktop-style workloads where the only third party code is the code that runs in the webbrowser and in the e-mail client or datacenter workloads where code from multiple customers runs on the same server? I'm asking this because the per-task KPTI work seems very useful to me for improving I/O performance for desktop-style workloads. I'm not sure however whether that work will be as useful for datacenter workloads. See also Willy Tarreau, [PATCH RFC 0/4] Per-task PTI activation (https://lkml.org/lkml/2018/1/8/568). Bart.
Re: [PATCH] test-case
On 01/31/2018 08:50 PM, Bart Van Assche wrote: I think it would be useful to have some variant of the above code in the kernel tree. Are you familiar with the fault injection framework (see also and Documentation/fault-injection/fault-injection.txt)? No, not yet. That's very interesting. Do you think that framework would be appropriate for controlling whether or not the above code gets executed? Yes, apparently it might be done w/ fail attributes to control the wait-loop in the shutdown/unload hook, whether to call scsi_done(), and whether to fail an error handler so it escalates to the next. I'd be happy to take a look at it, if you/others are interested. (just to make sure it does not end up rejected later because of others' opinion. :-) cheers, Mauricio
Re: [PATCH] scsi: mpt3sas: fix oops in error handlers after shutdown/unload
On 01/31/2018 08:59 PM, Bart Van Assche wrote: On Wed, 2018-01-31 at 17:48 -0200, Mauricio Faria de Oliveira wrote: On 01/31/2018 05:06 PM, Bart Van Assche wrote: Sorry but I think this patch introduces new race conditions. Have you Can you detail the race conditions? As far as I can see, the only race condition would be when an error handler is invoked very close in time to the shutdown/unload handler, and as such miss the 'ioc->remove_host' assignment (thus it continues running anyway, and hit the oops). That's indeed the race I was referring to. [snip] Okay. I'm not sure that point invalidates this patch; let me explain. First, IMHO that problem already exists in mpt3sas as it is now. There are 17 checks for 'ioc->remove_host' to return early, in the same manner. AFAIK, those are subject to this same race condition. $ grep -r 'ioc->remove_host[^=]*$' drivers/scsi/mpt3sas/ | wc -l 17 Second, I don't think this patch makes the driver any worse. Even with the potential race condition (which already exists elsewhere in the driver), it reduces the chance to hit an oops from _every time_ to only when the race condition occurs _and_ the error handler looses. So, even not being completely safe (like other parts of the driver), it still does help. BTW, are you aware that your patch does not seem to apply on Martin's tree (the fixes branch of git://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git): > [snip] Can you rebase this patch on top of Martin's tree? Yes, sure. I'll rebase, test, and send v2. Thanks for the pointer. cheers, Mauricio
[PATCH 1/2] qla2xxx: Fix double free bug after firmware timeout
From: Quinn Tran
This patch is based on Max's original patch.
When the qla2xxx firmware is unavailable, eventually
qla2x00_sp_timeout() is reached, which calls the timeout function and
frees the srb_t instance.
The timeout function always resolves to qla2x00_async_iocb_timeout(),
which invokes another callback function called "done". All of these
qla2x00_*_sp_done() callbacks also free the srb_t instance; after
returning to qla2x00_sp_timeout(), it is freed again.
The fix is to remove the "sp->free(sp)" call from qla2x00_sp_timeout()
and add it to those code paths in qla2x00_async_iocb_timeout() which
do not already free the object.
This is how it looks like with KASAN:
BUG: KASAN: use-after-free in qla2x00_sp_timeout+0x228/0x250
Read of size 8 at addr 88278147a590 by task swapper/2/0
Allocated by task 1502:
save_stack+0x33/0xa0
kasan_kmalloc+0xa0/0xd0
kmem_cache_alloc+0xb8/0x1c0
mempool_alloc+0xd6/0x260
qla24xx_async_gnl+0x3c5/0x1100
Freed by task 0:
save_stack+0x33/0xa0
kasan_slab_free+0x72/0xc0
kmem_cache_free+0x75/0x200
qla24xx_async_gnl_sp_done+0x556/0x9e0
qla2x00_async_iocb_timeout+0x1c7/0x420
qla2x00_sp_timeout+0x16d/0x250
call_timer_fn+0x36/0x200
The buggy address belongs to the object at 88278147a440
which belongs to the cache qla2xxx_srbs of size 344
The buggy address is located 336 bytes inside of
344-byte region [88278147a440, 88278147a598)
Reported-by: Max Kellermann
Signed-off-by: Quinn Tran
Signed-off-by: Himanshu Madhani
Cc: Max Kellermann
---
drivers/scsi/qla2xxx/qla_init.c | 23 +++
1 file changed, 3 insertions(+), 20 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
index aececf664654..2dea1129d396 100644
--- a/drivers/scsi/qla2xxx/qla_init.c
+++ b/drivers/scsi/qla2xxx/qla_init.c
@@ -59,8 +59,6 @@ qla2x00_sp_timeout(struct timer_list *t)
req->outstanding_cmds[sp->handle] = NULL;
iocb = &sp->u.iocb_cmd;
iocb->timeout(sp);
- if (sp->type != SRB_ELS_DCMD)
- sp->free(sp);
spin_unlock_irqrestore(&vha->hw->hardware_lock, flags);
}
@@ -102,7 +100,6 @@ qla2x00_async_iocb_timeout(void *data)
srb_t *sp = data;
fc_port_t *fcport = sp->fcport;
struct srb_iocb *lio = &sp->u.iocb_cmd;
- struct event_arg ea;
if (fcport) {
ql_dbg(ql_dbg_disc, fcport->vha, 0x2071,
@@ -117,25 +114,13 @@ qla2x00_async_iocb_timeout(void *data)
switch (sp->type) {
case SRB_LOGIN_CMD:
- if (!fcport)
- break;
/* Retry as needed. */
lio->u.logio.data[0] = MBS_COMMAND_ERROR;
lio->u.logio.data[1] = lio->u.logio.flags & SRB_LOGIN_RETRIED ?
QLA_LOGIO_LOGIN_RETRIED : 0;
- memset(&ea, 0, sizeof(ea));
- ea.event = FCME_PLOGI_DONE;
- ea.fcport = sp->fcport;
- ea.data[0] = lio->u.logio.data[0];
- ea.data[1] = lio->u.logio.data[1];
- ea.sp = sp;
- qla24xx_handle_plogi_done_event(fcport->vha, &ea);
+ sp->done(sp, QLA_FUNCTION_TIMEOUT);
break;
case SRB_LOGOUT_CMD:
- if (!fcport)
- break;
- qlt_logo_completion_handler(fcport, QLA_FUNCTION_TIMEOUT);
- break;
case SRB_CT_PTHRU_CMD:
case SRB_MB_IOCB:
case SRB_NACK_PLOGI:
@@ -235,12 +220,10 @@ static void
qla2x00_async_logout_sp_done(void *ptr, int res)
{
srb_t *sp = ptr;
- struct srb_iocb *lio = &sp->u.iocb_cmd;
sp->fcport->flags &= ~(FCF_ASYNC_SENT | FCF_ASYNC_ACTIVE);
- if (!test_bit(UNLOADING, &sp->vha->dpc_flags))
- qla2x00_post_async_logout_done_work(sp->vha, sp->fcport,
- lio->u.logio.data);
+ sp->fcport->login_gen++;
+ qlt_logo_completion_handler(sp->fcport, res);
sp->free(sp);
}
--
2.12.0
[PATCH 2/2] qla2xxx: Fix incorrect handle for abort IOCB
This patch fixes incorrect handle used for abort IOCB.
Fixes: b027a5ace443 ("scsi: qla2xxx: Fix queue ID for async abort with
Multiqueue")
Signed-off-by: Darren Trapp
Signed-off-by: Himanshu Madhani
---
drivers/scsi/qla2xxx/qla_iocb.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_iocb.c b/drivers/scsi/qla2xxx/qla_iocb.c
index 1b62e943ec49..8d00d559bd26 100644
--- a/drivers/scsi/qla2xxx/qla_iocb.c
+++ b/drivers/scsi/qla2xxx/qla_iocb.c
@@ -3275,12 +3275,11 @@ qla24xx_abort_iocb(srb_t *sp, struct abort_entry_24xx
*abt_iocb)
memset(abt_iocb, 0, sizeof(struct abort_entry_24xx));
abt_iocb->entry_type = ABORT_IOCB_TYPE;
abt_iocb->entry_count = 1;
- abt_iocb->handle =
-cpu_to_le32(MAKE_HANDLE(aio->u.abt.req_que_no,
-aio->u.abt.cmd_hndl));
+ abt_iocb->handle = cpu_to_le32(MAKE_HANDLE(req->id, sp->handle));
abt_iocb->nport_handle = cpu_to_le16(sp->fcport->loop_id);
abt_iocb->handle_to_abort =
- cpu_to_le32(MAKE_HANDLE(req->id, aio->u.abt.cmd_hndl));
+ cpu_to_le32(MAKE_HANDLE(aio->u.abt.req_que_no,
+ aio->u.abt.cmd_hndl));
abt_iocb->port_id[0] = sp->fcport->d_id.b.al_pa;
abt_iocb->port_id[1] = sp->fcport->d_id.b.area;
abt_iocb->port_id[2] = sp->fcport->d_id.b.domain;
--
2.12.0
[PATCH 0/2] qla2xxx: Bug fixes for driver
Hi Martin, These are couple of bug fixes for the driver. Patch#1 is the issue reported by Max using KASAN tool. Patch#2 fixes use of wrong queue handle for abort IOCB. Please apply them to 4.16/scsi-fixes at your earliest convenience. Thanks, Himanshu Himanshu Madhani (1): qla2xxx: Fix incorrect handle for abort IOCB Quinn Tran (1): qla2xxx: Fix double free bug after firmware timeout drivers/scsi/qla2xxx/qla_init.c | 23 +++ drivers/scsi/qla2xxx/qla_iocb.c | 7 +++ 2 files changed, 6 insertions(+), 24 deletions(-) -- 2.12.0
Re: [PATCH v4 10/10] ufs: sysfs: attributes
On Thu, Feb 01, 2018 at 06:15:46PM +0200, Stanislav Nijnikov wrote:
> +#define UFS_LUN_ATTRIBUTE(_name, _uname)
> \
> +static ssize_t _name##_attribute_show(struct device *dev,
> \
> + struct device_attribute *attr, char *buf) \
> +{
> \
> + u32 value;\
> + struct scsi_device *sdev = to_scsi_device(dev); \
> + struct ufs_hba *hba = shost_priv(sdev->host); \
> + u8 lun = ufshcd_scsi_to_upiu_lun(sdev->lun); \
> + if (ufshcd_query_attr(hba, UPIU_QUERY_OPCODE_READ_ATTR, \
> + QUERY_ATTR_IDN##_uname, lun, 0, &value)) \
> + return -EINVAL; \
> + return sprintf(buf, "0x%08X\n", value); \
> +}
> \
> +static DEVICE_ATTR_RO(_name##_attribute)
> +
> +UFS_LUN_ATTRIBUTE(dyn_cap_needed, _DYN_CAP_NEEDED);
Why create a macro when you only have one instance of its use?
thanks,
greg k-h
Re: [PATCH v4 03/10] ufs: sysfs: interconnect descriptor
On Thu, Feb 01, 2018 at 06:15:39PM +0200, Stanislav Nijnikov wrote: > This patch introduces a sysfs group entry for the UFS interconnect > descriptor parameters. The group adds "interconnect_descriptor" folder > under the UFS driver sysfs entry (/sys/bus/platform/drivers/ufshcd/*). > The parameters are shown as hexadecimal numbers. The full information > about the parameters could be found at UFS specifications 2.1. > > Signed-off-by: Stanislav Nijnikov > > Reviewed-by: Greg Kroah-Hartman Nit, you should not have blank lines between these two statements, otherwise tools can get confused. You do that in later patches as well. thanks, greg k-h
Re: [PATCH v4 02/10] ufs: sysfs: device descriptor
On Thu, Feb 01, 2018 at 06:15:38PM +0200, Stanislav Nijnikov wrote:
> +#define UFS_DESC_PARAM(_name, _puname, _duname, _size)
> \
> +static ssize_t _name##_show(struct device *dev,
> \
> + struct device_attribute *attr, char *buf) \
> +{
> \
> + struct ufs_hba *hba = dev_get_drvdata(dev); \
> + return ufs_sysfs_read_desc_param(hba, QUERY_DESC_IDN_##_duname, \
> + 0, _duname##_DESC_PARAM##_puname, \
> + buf, UFS_PARAM_##_size##_SIZE); \
> +}
> \
> +static DEVICE_ATTR_RO(_name)
Nit, use tabs in your lines here to line up the trailing \
Same for other places in this patch series.
thanks,
greg k-h
Re: [PATCH v4 01/10] ufs: sysfs: attribute group for existing sysfs entries.
On Thu, Feb 01, 2018 at 06:15:37PM +0200, Stanislav Nijnikov wrote:
> This patch introduces attribute group to show existing sysfs entries.
>
> Signed-off-by: Stanislav Nijnikov
> ---
> drivers/scsi/ufs/Makefile| 3 +-
> drivers/scsi/ufs/ufs-sysfs.c | 164
> +++
> drivers/scsi/ufs/ufs-sysfs.h | 22 ++
> drivers/scsi/ufs/ufshcd.c| 156 ++--
> drivers/scsi/ufs/ufshcd.h| 2 +
> 5 files changed, 194 insertions(+), 153 deletions(-)
> create mode 100644 drivers/scsi/ufs/ufs-sysfs.c
> create mode 100644 drivers/scsi/ufs/ufs-sysfs.h
>
> diff --git a/drivers/scsi/ufs/Makefile b/drivers/scsi/ufs/Makefile
> index 9310c6c..918f579 100644
> --- a/drivers/scsi/ufs/Makefile
> +++ b/drivers/scsi/ufs/Makefile
> @@ -3,6 +3,7 @@
> obj-$(CONFIG_SCSI_UFS_DWC_TC_PCI) += tc-dwc-g210-pci.o ufshcd-dwc.o
> tc-dwc-g210.o
> obj-$(CONFIG_SCSI_UFS_DWC_TC_PLATFORM) += tc-dwc-g210-pltfrm.o ufshcd-dwc.o
> tc-dwc-g210.o
> obj-$(CONFIG_SCSI_UFS_QCOM) += ufs-qcom.o
> -obj-$(CONFIG_SCSI_UFSHCD) += ufshcd.o
> +obj-$(CONFIG_SCSI_UFSHCD) += ufshcd-core.o
> +ufshcd-core-objs := ufshcd.o ufs-sysfs.o
> obj-$(CONFIG_SCSI_UFSHCD_PCI) += ufshcd-pci.o
> obj-$(CONFIG_SCSI_UFSHCD_PLATFORM) += ufshcd-pltfrm.o
> diff --git a/drivers/scsi/ufs/ufs-sysfs.c b/drivers/scsi/ufs/ufs-sysfs.c
> new file mode 100644
> index 000..cc68a90
> --- /dev/null
> +++ b/drivers/scsi/ufs/ufs-sysfs.c
> @@ -0,0 +1,164 @@
> +//SPDX-License-Identifier: GPL-2.0-only
> +//Copyright (C) 2018 Western Digital Corporation
> +//This program is free software; you can redistribute it and/or modify it
> +//under the terms of the GNU General Public License as published by the
> +//Free Software Foundation; version 2.
> +//
> +//This program is distributed in the hope that it will be useful, but
> +//WITHOUT ANY WARRANTY; without even the implied warranty of
> +//MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> +//General Public License for more details.
No need to put the whole "This program" crud in the file here if you
have the SPDX line already. We are going through the kernel tree and
removing all of the 700+ different ways we have this boilerplate code in
the tree, please do not add new ones.
Also, please put a ' ' after "//", it just looks ugly like this, don't
you think so?
Please fix this for all of the files you add in this patch series.
> +#include
> +#include
> +
> +#include "ufs-sysfs.h"
> +
> +static const char *ufschd_uic_link_state_to_string(
> + enum uic_link_state state)
> +{
> + switch (state) {
> + case UIC_LINK_OFF_STATE:return "OFF";
> + case UIC_LINK_ACTIVE_STATE: return "ACTIVE";
> + case UIC_LINK_HIBERN8_STATE:return "HIBERN8";
> + default:return "UNKNOWN";
> + }
> +}
> +
> +static const char *ufschd_ufs_dev_pwr_mode_to_string(
> + enum ufs_dev_pwr_mode state)
> +{
> + switch (state) {
> + case UFS_ACTIVE_PWR_MODE: return "ACTIVE";
> + case UFS_SLEEP_PWR_MODE:return "SLEEP";
> + case UFS_POWERDOWN_PWR_MODE:return "POWERDOWN";
> + default:return "UNKNOWN";
> + }
> +}
> +
> +static inline ssize_t ufs_sysfs_pm_lvl_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count,
> + bool rpm)
> +{
> + struct ufs_hba *hba = dev_get_drvdata(dev);
> + unsigned long flags, value;
> +
> + if (kstrtoul(buf, 0, &value))
> + return -EINVAL;
> +
> + if (value >= UFS_PM_LVL_MAX)
> + return -EINVAL;
> +
> + spin_lock_irqsave(hba->host->host_lock, flags);
> + if (rpm)
> + hba->rpm_lvl = value;
> + else
> + hba->spm_lvl = value;
> + spin_unlock_irqrestore(hba->host->host_lock, flags);
> + return count;
> +}
> +
> +static ssize_t rpm_lvl_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct ufs_hba *hba = dev_get_drvdata(dev);
> + int curr_len;
> + u8 lvl;
> +
> + curr_len = snprintf(buf, PAGE_SIZE,
> + "\nCurrent Runtime PM level [%d] => dev_state [%s]
> link_state [%s]\n",
> + hba->rpm_lvl,
> + ufschd_ufs_dev_pwr_mode_to_string(
> + ufs_pm_lvl_states[hba->rpm_lvl].dev_state),
> + ufschd_uic_link_state_to_string(
> + ufs_pm_lvl_states[hba->rpm_lvl].link_state));
> +
> + curr_len += snprintf((buf + curr_len), (PAGE_SIZE - curr_len),
> + "\nAll available Runtime PM levels info:\n");
> + for (lvl = UFS_PM_LVL_0; lvl < UFS_PM_LVL_MAX; lvl++)
> + curr_len += snprintf((buf + curr_len), (PAGE_SIZE -
RE: [LSF/MM TOPIC] irq affinity handling for high CPU count machines
> -Original Message- > From: Hannes Reinecke [mailto:h...@suse.de] > Sent: Thursday, February 1, 2018 9:50 PM > To: Ming Lei > Cc: lsf...@lists.linux-foundation.org; linux-scsi@vger.kernel.org; linux- > n...@lists.infradead.org; Kashyap Desai > Subject: Re: [LSF/MM TOPIC] irq affinity handling for high CPU count > machines > > On 02/01/2018 04:05 PM, Ming Lei wrote: > > Hello Hannes, > > > > On Mon, Jan 29, 2018 at 10:08:43AM +0100, Hannes Reinecke wrote: > >> Hi all, > >> > >> here's a topic which came up on the SCSI ML (cf thread '[RFC 0/2] > >> mpt3sas/megaraid_sas: irq poll and load balancing of reply queue'). > >> > >> When doing I/O tests on a machine with more CPUs than MSIx vectors > >> provided by the HBA we can easily setup a scenario where one CPU is > >> submitting I/O and the other one is completing I/O. Which will result > >> in the latter CPU being stuck in the interrupt completion routine for > >> basically ever, resulting in the lockup detector kicking in. > > > > Today I am looking at one megaraid_sas related issue, and found > > pci_alloc_irq_vectors(PCI_IRQ_AFFINITY) is used in the driver, so > > looks each reply queue has been handled by more than one CPU if there > > are more CPUs than MSIx vectors in the system, which is done by > > generic irq affinity code, please see kernel/irq/affinity.c. Yes. That is a problematic area. If CPU and MSI-x(reply queue) is 1:1 mapped, we don't have any issue. > > > > Also IMO each reply queue may be treated as blk-mq's hw queue, then > > megaraid may benefit from blk-mq's MQ framework, but one annoying > > thing is that both legacy and blk-mq path need to be handled inside > > driver. Both MR and IT driver is (due to H/W design.) is using blk-mq frame work but it is really a single h/w queue. IT and MR HBA has single submission queue and multiple reply queue. > > > The megaraid driver is a really strange beast;, having layered two > different > interfaces (the 'legacy' MFI interface and that from from > mpt3sas) on top of each other. > I had been thinking of converting it to scsi-mq, too (as my mpt3sas patch > finally went in), but I'm not sure if we can benefit from it as we're > still be > bound by the HBA-wide tag pool. > It's on my todo list, albeit pretty far down :-) Hannes, this is typically same in both MR (megaraid_sas) and IT (mpt3sas). Both the driver is using shared HBA-wide tag pool. Both MR and IT driver use request->tag to get command from free pool. > > >> > >> How should these situations be handled? > >> Should it be made the responsibility of the drivers, ensuring that > >> the interrupt completion routine is terminated after a certain time? > >> Should it be made the resposibility of the upper layers? > >> Should it be the responsibility of the interrupt mapping code? > >> Can/should interrupt polling be used in these situations? > > > > Yeah, I guess interrupt polling may improve these situations, > > especially KPTI introduces some extra cost in interrupt handling. > > > The question is not so much if one should be doing irq polling, but rather > if we > can come up with some guidance or even infrastructure to make this happen > automatically. > Having to rely on individual drivers to get this right is probably not the > best > option. > > Cheers, > > Hannes > -- > Dr. Hannes Reinecke Teamlead 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 v4 00/10] ufs: sysfs: read-only access to device descriptors, attributes and flags
Can you please clarify the naming, at least as far as saying scsi/ufs. Universal Flash Storage is not the same thing as all as the Unix File System which has been using the acronym ufs for much longer. I saw this patchset and I was wondering what in the world does a filesystem need with sysfs entries. Eric Stanislav Nijnikov writes: > This patch introduces sysfs entries that will provide read-only access to > device management data that could be received with UFS query requests. > User-space applications will be able to read UFS device descriptors, > flags and attributes. This will allow to get full UFS device configuration > and its status. The descriptors are provided as set of files representing > its parameters. The flags are using "true"/"false" representation of > their value. The attributes are shown as hexadecimal value. The > descriptors, attributes and flags are placed in separate subfolders under > the UFS device sysfs entry (/sys/bus/platform/drivers/ufshcd/*/). The > string descriptor subfolder contains five string descriptors defined by > UFS specification 2.1. The LUN specific descriptor and attribute are > placed under corresponding SCSI device sysfs entries > (/sys/class/scsi_device/*/device/). > In addition the patch presents an additional field in the > scsi_host_template structure - struct attribute_group **sdev_group. > This field allows to define groups of attributes. It will provide an > ability to use binary attributes in addition to device attributes and > to group them under subfolders if necessary. > > Changelog: > v3 -> v4 > Additional patch the introduces default attributes group for the > existing ufs sysfs entries (rpm_lvl and spm_lvl) > The ufs_sysfs_read_desc_param function calls to ufshcd_read_desc_param > insted of ufshcd_query_descriptor_retry to avoid code duplication. > The code was updated to remove the checkpatch error "ERROR: Macros > with complex values should be enclosed in parentheses" > Added "_" to macros parameters to remove "#undef DEVICE_CLASS" > The legal information was updated to satisfy the SPDX requirements > The date in Documentation/ABI/testing/sysfs-driver-ufs was updated. > > v2 -> v3 > The Makefile is updated to make ufs-sysfs.c part of the ufshcd module. > The unnecessary EXPORT_SYMBOL were removed > Added a legal info header to the new files > The date in Documentation/ABI/testing/sysfs-driver-ufs was updated. > > v1 -> v2 > Provided additional description for the changes > > Stanislav Nijnikov (10): > ufs: sysfs: attribute group for existing sysfs entries. > ufs: sysfs: device descriptor > ufs: sysfs: interconnect descriptor > ufs: sysfs: geometry descriptor > ufs: sysfs: health descriptor > ufs: sysfs: power descriptor > ufs: sysfs: string descriptors > ufs: sysfs: unit descriptor > ufs: sysfs: flags > ufs: sysfs: attributes > > Documentation/ABI/testing/sysfs-driver-ufs | 804 > + > drivers/scsi/scsi_sysfs.c | 14 + > drivers/scsi/ufs/Makefile | 3 +- > drivers/scsi/ufs/ufs-sysfs.c | 757 +++ > drivers/scsi/ufs/ufs-sysfs.h | 25 + > drivers/scsi/ufs/ufs.h | 115 - > drivers/scsi/ufs/ufshcd.c | 218 ++-- > drivers/scsi/ufs/ufshcd.h | 34 ++ > include/scsi/scsi_host.h | 6 + > 9 files changed, 1785 insertions(+), 191 deletions(-) > create mode 100644 Documentation/ABI/testing/sysfs-driver-ufs > create mode 100644 drivers/scsi/ufs/ufs-sysfs.c > create mode 100644 drivers/scsi/ufs/ufs-sysfs.h
Re: scsi: sg: assorted memory corruptions
On Thu, Feb 1, 2018 at 5:17 PM, Ben Hutchings wrote: > On Thu, 2018-02-01 at 08:04 +0100, Dmitry Vyukov wrote: >> On Thu, Feb 1, 2018 at 7:03 AM, Douglas Gilbert >> wrote: >> > On 2018-01-30 07:22 AM, Dmitry Vyukov wrote: > [...] >> > > [1:0:0:0]cd/dvd QEMU QEMU DVD-ROM 2.0. /dev/sr0 /dev/sg1 >> > > >> > > # readlink /sys/class/scsi_generic/sg0 >> > > >> > > ../../devices/pci:00/:00:01.1/ata1/host0/target0:0:0/0:0:0:0/scsi_generic/sg0 >> > > >> > > # cat /sys/class/scsi_generic/sg0/device/vendor >> > > ATA >> > >> > >> > ^ >> > That subsystem is the culprit IMO, most likely libata. >> > >> > Until you can show this test failing on something other than an >> > ATA disk, then I will treat this issue as closed. >> >> Hi Doug, >> >> Why is bug in ATA not a bug? Is it long unused by everybody? I've got >> it by running qemu with default flags... > > If the bug is in libata then it's not on Doug to fix it since he's only > maintaining sg. Then I think we need to CC ata maintainers rather than treat it as closed. +Tejun, linux-ide@, you can see full thread here: https://groups.google.com/forum/#!topic/syzkaller/9RNr9Gu0MyY
Re: [LSF/MM TOPIC] irq affinity handling for high CPU count machines
On 02/01/2018 04:05 PM, Ming Lei wrote: > Hello Hannes, > > On Mon, Jan 29, 2018 at 10:08:43AM +0100, Hannes Reinecke wrote: >> Hi all, >> >> here's a topic which came up on the SCSI ML (cf thread '[RFC 0/2] >> mpt3sas/megaraid_sas: irq poll and load balancing of reply queue'). >> >> When doing I/O tests on a machine with more CPUs than MSIx vectors >> provided by the HBA we can easily setup a scenario where one CPU is >> submitting I/O and the other one is completing I/O. Which will result in >> the latter CPU being stuck in the interrupt completion routine for >> basically ever, resulting in the lockup detector kicking in. > > Today I am looking at one megaraid_sas related issue, and found > pci_alloc_irq_vectors(PCI_IRQ_AFFINITY) is used in the driver, so looks > each reply queue has been handled by more than one CPU if there are more > CPUs than MSIx vectors in the system, which is done by generic irq affinity > code, please see kernel/irq/affinity.c. > > Also IMO each reply queue may be treated as blk-mq's hw queue, then > megaraid may benefit from blk-mq's MQ framework, but one annoying thing is > that both legacy and blk-mq path need to be handled inside driver. > The megaraid driver is a really strange beast;, having layered two different interfaces (the 'legacy' MFI interface and that from from mpt3sas) on top of each other. I had been thinking of converting it to scsi-mq, too (as my mpt3sas patch finally went in), but I'm not sure if we can benefit from it as we're still be bound by the HBA-wide tag pool. It's on my todo list, albeit pretty far down :-) >> >> How should these situations be handled? >> Should it be made the responsibility of the drivers, ensuring that the >> interrupt completion routine is terminated after a certain time? >> Should it be made the resposibility of the upper layers? >> Should it be the responsibility of the interrupt mapping code? >> Can/should interrupt polling be used in these situations? > > Yeah, I guess interrupt polling may improve these situations, especially > KPTI introduces some extra cost in interrupt handling. > The question is not so much if one should be doing irq polling, but rather if we can come up with some guidance or even infrastructure to make this happen automatically. Having to rely on individual drivers to get this right is probably not the best option. 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)
[PATCH v4 03/10] ufs: sysfs: interconnect descriptor
This patch introduces a sysfs group entry for the UFS interconnect
descriptor parameters. The group adds "interconnect_descriptor" folder
under the UFS driver sysfs entry (/sys/bus/platform/drivers/ufshcd/*).
The parameters are shown as hexadecimal numbers. The full information
about the parameters could be found at UFS specifications 2.1.
Signed-off-by: Stanislav Nijnikov
Reviewed-by: Greg Kroah-Hartman
---
Documentation/ABI/testing/sysfs-driver-ufs | 19 +++
drivers/scsi/ufs/ufs-sysfs.c | 18 ++
drivers/scsi/ufs/ufs.h | 8
3 files changed, 45 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-driver-ufs
b/Documentation/ABI/testing/sysfs-driver-ufs
index 8da7b84..099e6fa 100644
--- a/Documentation/ABI/testing/sysfs-driver-ufs
+++ b/Documentation/ABI/testing/sysfs-driver-ufs
@@ -221,3 +221,22 @@ Description: This file shows the command maximum
timeout for a change
parameters. The full information about the descriptor could
be found at UFS specifications 2.1.
The file is read only.
+
+
+What:
/sys/bus/platform/drivers/ufshcd/*/interconnect_descriptor/unipro_version
+Date: February 2018
+Contact: Stanislav Nijnikov
+Description: This file shows the MIPI UniPro version number in BCD format.
+ This is one of the UFS interconnect descriptor parameters.
+ The full information about the descriptor could be found at
+ UFS specifications 2.1.
+ The file is read only.
+
+What:
/sys/bus/platform/drivers/ufshcd/*/interconnect_descriptor/mphy_version
+Date: February 2018
+Contact: Stanislav Nijnikov
+Description: This file shows the MIPI M-PHY version number in BCD format.
+ This is one of the UFS interconnect descriptor parameters.
+ The full information about the descriptor could be found at
+ UFS specifications 2.1.
+ The file is read only.
diff --git a/drivers/scsi/ufs/ufs-sysfs.c b/drivers/scsi/ufs/ufs-sysfs.c
index 372e281..460378b 100644
--- a/drivers/scsi/ufs/ufs-sysfs.c
+++ b/drivers/scsi/ufs/ufs-sysfs.c
@@ -264,9 +264,27 @@ static const struct attribute_group
ufs_sysfs_device_descriptor_group = {
.attrs = ufs_sysfs_device_descriptor,
};
+#define UFS_INTERCONNECT_DESC_PARAM(_name, _uname, _size) \
+ UFS_DESC_PARAM(_name, _uname, INTERCONNECT, _size)
+
+UFS_INTERCONNECT_DESC_PARAM(unipro_version, _UNIPRO_VER, WORD);
+UFS_INTERCONNECT_DESC_PARAM(mphy_version, _MPHY_VER, WORD);
+
+static struct attribute *ufs_sysfs_interconnect_descriptor[] = {
+ &dev_attr_unipro_version.attr,
+ &dev_attr_mphy_version.attr,
+ NULL,
+};
+
+static const struct attribute_group ufs_sysfs_interconnect_descriptor_group = {
+ .name = "interconnect_descriptor",
+ .attrs = ufs_sysfs_interconnect_descriptor,
+};
+
static const struct attribute_group *ufs_sysfs_groups[] = {
&ufs_sysfs_default_group,
&ufs_sysfs_device_descriptor_group,
+ &ufs_sysfs_interconnect_descriptor_group,
NULL,
};
diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index 6ae1e08..773c049 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -230,6 +230,14 @@ enum device_desc_param {
DEVICE_DESC_PARAM_PRDCT_REV = 0x2A,
};
+/* Interconnect descriptor parameters offsets in bytes*/
+enum interconnect_desc_param {
+ INTERCONNECT_DESC_PARAM_LEN = 0x0,
+ INTERCONNECT_DESC_PARAM_TYPE= 0x1,
+ INTERCONNECT_DESC_PARAM_UNIPRO_VER = 0x2,
+ INTERCONNECT_DESC_PARAM_MPHY_VER= 0x4,
+};
+
/*
* Logical Unit Write Protect
* 00h: LU not write protected
--
2.7.4
[PATCH v4 01/10] ufs: sysfs: attribute group for existing sysfs entries.
This patch introduces attribute group to show existing sysfs entries.
Signed-off-by: Stanislav Nijnikov
---
drivers/scsi/ufs/Makefile| 3 +-
drivers/scsi/ufs/ufs-sysfs.c | 164 +++
drivers/scsi/ufs/ufs-sysfs.h | 22 ++
drivers/scsi/ufs/ufshcd.c| 156 ++--
drivers/scsi/ufs/ufshcd.h| 2 +
5 files changed, 194 insertions(+), 153 deletions(-)
create mode 100644 drivers/scsi/ufs/ufs-sysfs.c
create mode 100644 drivers/scsi/ufs/ufs-sysfs.h
diff --git a/drivers/scsi/ufs/Makefile b/drivers/scsi/ufs/Makefile
index 9310c6c..918f579 100644
--- a/drivers/scsi/ufs/Makefile
+++ b/drivers/scsi/ufs/Makefile
@@ -3,6 +3,7 @@
obj-$(CONFIG_SCSI_UFS_DWC_TC_PCI) += tc-dwc-g210-pci.o ufshcd-dwc.o
tc-dwc-g210.o
obj-$(CONFIG_SCSI_UFS_DWC_TC_PLATFORM) += tc-dwc-g210-pltfrm.o ufshcd-dwc.o
tc-dwc-g210.o
obj-$(CONFIG_SCSI_UFS_QCOM) += ufs-qcom.o
-obj-$(CONFIG_SCSI_UFSHCD) += ufshcd.o
+obj-$(CONFIG_SCSI_UFSHCD) += ufshcd-core.o
+ufshcd-core-objs := ufshcd.o ufs-sysfs.o
obj-$(CONFIG_SCSI_UFSHCD_PCI) += ufshcd-pci.o
obj-$(CONFIG_SCSI_UFSHCD_PLATFORM) += ufshcd-pltfrm.o
diff --git a/drivers/scsi/ufs/ufs-sysfs.c b/drivers/scsi/ufs/ufs-sysfs.c
new file mode 100644
index 000..cc68a90
--- /dev/null
+++ b/drivers/scsi/ufs/ufs-sysfs.c
@@ -0,0 +1,164 @@
+//SPDX-License-Identifier: GPL-2.0-only
+//Copyright (C) 2018 Western Digital Corporation
+//This program is free software; you can redistribute it and/or modify it
+//under the terms of the GNU General Public License as published by the
+//Free Software Foundation; version 2.
+//
+//This program is distributed in the hope that it will be useful, but
+//WITHOUT ANY WARRANTY; without even the implied warranty of
+//MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+//General Public License for more details.
+
+#include
+#include
+
+#include "ufs-sysfs.h"
+
+static const char *ufschd_uic_link_state_to_string(
+ enum uic_link_state state)
+{
+ switch (state) {
+ case UIC_LINK_OFF_STATE:return "OFF";
+ case UIC_LINK_ACTIVE_STATE: return "ACTIVE";
+ case UIC_LINK_HIBERN8_STATE:return "HIBERN8";
+ default:return "UNKNOWN";
+ }
+}
+
+static const char *ufschd_ufs_dev_pwr_mode_to_string(
+ enum ufs_dev_pwr_mode state)
+{
+ switch (state) {
+ case UFS_ACTIVE_PWR_MODE: return "ACTIVE";
+ case UFS_SLEEP_PWR_MODE:return "SLEEP";
+ case UFS_POWERDOWN_PWR_MODE:return "POWERDOWN";
+ default:return "UNKNOWN";
+ }
+}
+
+static inline ssize_t ufs_sysfs_pm_lvl_store(struct device *dev,
+struct device_attribute *attr,
+const char *buf, size_t count,
+bool rpm)
+{
+ struct ufs_hba *hba = dev_get_drvdata(dev);
+ unsigned long flags, value;
+
+ if (kstrtoul(buf, 0, &value))
+ return -EINVAL;
+
+ if (value >= UFS_PM_LVL_MAX)
+ return -EINVAL;
+
+ spin_lock_irqsave(hba->host->host_lock, flags);
+ if (rpm)
+ hba->rpm_lvl = value;
+ else
+ hba->spm_lvl = value;
+ spin_unlock_irqrestore(hba->host->host_lock, flags);
+ return count;
+}
+
+static ssize_t rpm_lvl_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct ufs_hba *hba = dev_get_drvdata(dev);
+ int curr_len;
+ u8 lvl;
+
+ curr_len = snprintf(buf, PAGE_SIZE,
+ "\nCurrent Runtime PM level [%d] => dev_state [%s]
link_state [%s]\n",
+ hba->rpm_lvl,
+ ufschd_ufs_dev_pwr_mode_to_string(
+ ufs_pm_lvl_states[hba->rpm_lvl].dev_state),
+ ufschd_uic_link_state_to_string(
+ ufs_pm_lvl_states[hba->rpm_lvl].link_state));
+
+ curr_len += snprintf((buf + curr_len), (PAGE_SIZE - curr_len),
+"\nAll available Runtime PM levels info:\n");
+ for (lvl = UFS_PM_LVL_0; lvl < UFS_PM_LVL_MAX; lvl++)
+ curr_len += snprintf((buf + curr_len), (PAGE_SIZE - curr_len),
+"\tRuntime PM level [%d] => dev_state [%s]
link_state [%s]\n",
+ lvl,
+ ufschd_ufs_dev_pwr_mode_to_string(
+ ufs_pm_lvl_states[lvl].dev_state),
+ ufschd_uic_link_state_to_string(
+ ufs_pm_lvl_states[lvl].link_state));
+
+ return curr_len;
+}
+
+static ssize_t rpm_lvl_store(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+
[PATCH v4 04/10] ufs: sysfs: geometry descriptor
This patch introduces a sysfs group entry for the UFS geometry descriptor parameters. The group adds "geometry_descriptor" folder under the UFS driver sysfs entry (/sys/bus/platform/drivers/ufshcd/*). The parameters are shown as hexadecimal numbers. The full information about the parameters could be found at UFS specifications 2.1. Signed-off-by: Stanislav Nijnikov Reviewed-by: Greg Kroah-Hartman --- Documentation/ABI/testing/sysfs-driver-ufs | 173 + drivers/scsi/ufs/ufs-sysfs.c | 84 ++ drivers/scsi/ufs/ufs.h | 36 ++ 3 files changed, 293 insertions(+) diff --git a/Documentation/ABI/testing/sysfs-driver-ufs b/Documentation/ABI/testing/sysfs-driver-ufs index 099e6fa..6ea7613 100644 --- a/Documentation/ABI/testing/sysfs-driver-ufs +++ b/Documentation/ABI/testing/sysfs-driver-ufs @@ -240,3 +240,176 @@ Description: This file shows the MIPI M-PHY version number in BCD format. The full information about the descriptor could be found at UFS specifications 2.1. The file is read only. + + +What: /sys/bus/platform/drivers/ufshcd/*/geometry_descriptor/raw_device_capacity +Date: February 2018 +Contact: Stanislav Nijnikov +Description: This file shows the total memory quantity available to + the user to configure the device logical units. This is one + of the UFS geometry descriptor parameters. The full + information about the descriptor could be found at + UFS specifications 2.1. + The file is read only. + +What: /sys/bus/platform/drivers/ufshcd/*/geometry_descriptor/max_number_of_luns +Date: February 2018 +Contact: Stanislav Nijnikov +Description: This file shows the maximum number of logical units + supported by the UFS device. This is one of the UFS + geometry descriptor parameters. The full information about + the descriptor could be found at UFS specifications 2.1. + The file is read only. + +What: /sys/bus/platform/drivers/ufshcd/*/geometry_descriptor/segment_size +Date: February 2018 +Contact: Stanislav Nijnikov +Description: This file shows the segment size. This is one of the UFS + geometry descriptor parameters. The full information about + the descriptor could be found at UFS specifications 2.1. + The file is read only. + +What: /sys/bus/platform/drivers/ufshcd/*/geometry_descriptor/allocation_unit_size +Date: February 2018 +Contact: Stanislav Nijnikov +Description: This file shows the allocation unit size. This is one of + the UFS geometry descriptor parameters. The full information + about the descriptor could be found at UFS specifications 2.1. + The file is read only. + +What: /sys/bus/platform/drivers/ufshcd/*/geometry_descriptor/min_addressable_block_size +Date: February 2018 +Contact: Stanislav Nijnikov +Description: This file shows the minimum addressable block size. This + is one of the UFS geometry descriptor parameters. The full + information about the descriptor could be found at UFS + specifications 2.1. + The file is read only. + +What: /sys/bus/platform/drivers/ufshcd/*/geometry_descriptor/optimal_read_block_size +Date: February 2018 +Contact: Stanislav Nijnikov +Description: This file shows the optimal read block size. This is one + of the UFS geometry descriptor parameters. The full + information about the descriptor could be found at UFS + specifications 2.1. + The file is read only. + +What: /sys/bus/platform/drivers/ufshcd/*/geometry_descriptor/optimal_write_block_size +Date: February 2018 +Contact: Stanislav Nijnikov +Description: This file shows the optimal write block size. This is one + of the UFS geometry descriptor parameters. The full + information about the descriptor could be found at UFS + specifications 2.1. + The file is read only. + +What: /sys/bus/platform/drivers/ufshcd/*/geometry_descriptor/max_in_buffer_size +Date: February 2018 +Contact: Stanislav Nijnikov +Description: This file shows the maximum data-in buffer size. This + is one of the UFS geometry descriptor parameters. The full + information about the descriptor could be found at UFS + specifications 2.1. + The file is read only. + +What: /sys/bus/platform/drivers/ufshcd/*/geometry_descriptor/max_out_buffer_size +Date: February 2018 +Contact: Stanislav Nijnikov +Description: This file shows the maximum
[PATCH v4 06/10] ufs: sysfs: power descriptor
This patch introduces a sysfs group entry for the UFS power descriptor
parameters. The group adds "power_descriptor" folder under the UFS driver
sysfs entry (/sys/bus/platform/drivers/ufshcd/*). The parameters are shown
as hexadecimal numbers. The full information about the parameters could be
found at UFS specifications 2.1.
Signed-off-by: Stanislav Nijnikov
Reviewed-by: Greg Kroah-Hartman
---
Documentation/ABI/testing/sysfs-driver-ufs | 10 +++
drivers/scsi/ufs/ufs-sysfs.c | 118 +
2 files changed, 128 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-driver-ufs
b/Documentation/ABI/testing/sysfs-driver-ufs
index ddb012b..7460566 100644
--- a/Documentation/ABI/testing/sysfs-driver-ufs
+++ b/Documentation/ABI/testing/sysfs-driver-ufs
@@ -441,3 +441,13 @@ Description: This file shows indication of the
device life time
parameters. The full information about the descriptor
could be found at UFS specifications 2.1.
The file is read only.
+
+
+What:
/sys/bus/platform/drivers/ufshcd/*/power_descriptor/active_icc_levels_vcc*
+Date: February 2018
+Contact: Stanislav Nijnikov
+Description: This file shows maximum VCC, VCCQ and VCCQ2 value for
+ active ICC levels from 0 to 15. This is one of the UFS
+ power descriptor parameters. The full information about
+ the descriptor could be found at UFS specifications 2.1.
+ The file is read only.
diff --git a/drivers/scsi/ufs/ufs-sysfs.c b/drivers/scsi/ufs/ufs-sysfs.c
index 1c89009..d2e6b49 100644
--- a/drivers/scsi/ufs/ufs-sysfs.c
+++ b/drivers/scsi/ufs/ufs-sysfs.c
@@ -383,12 +383,130 @@ static const struct attribute_group
ufs_sysfs_health_descriptor_group = {
.attrs = ufs_sysfs_health_descriptor,
};
+#define UFS_POWER_DESC_PARAM(_name, _uname, _index) \
+static ssize_t _name##_index##_show(struct device *dev, \
+ struct device_attribute *attr, char *buf) \
+{ \
+ struct ufs_hba *hba = dev_get_drvdata(dev); \
+ return ufs_sysfs_read_desc_param(hba, QUERY_DESC_IDN_POWER, 0,\
+ PWR_DESC##_uname##_0 + _index * UFS_PARAM_WORD_SIZE, \
+ buf, UFS_PARAM_WORD_SIZE);\
+} \
+static DEVICE_ATTR_RO(_name##_index)
+
+UFS_POWER_DESC_PARAM(active_icc_levels_vcc, _ACTIVE_LVLS_VCC, 0);
+UFS_POWER_DESC_PARAM(active_icc_levels_vcc, _ACTIVE_LVLS_VCC, 1);
+UFS_POWER_DESC_PARAM(active_icc_levels_vcc, _ACTIVE_LVLS_VCC, 2);
+UFS_POWER_DESC_PARAM(active_icc_levels_vcc, _ACTIVE_LVLS_VCC, 3);
+UFS_POWER_DESC_PARAM(active_icc_levels_vcc, _ACTIVE_LVLS_VCC, 4);
+UFS_POWER_DESC_PARAM(active_icc_levels_vcc, _ACTIVE_LVLS_VCC, 5);
+UFS_POWER_DESC_PARAM(active_icc_levels_vcc, _ACTIVE_LVLS_VCC, 6);
+UFS_POWER_DESC_PARAM(active_icc_levels_vcc, _ACTIVE_LVLS_VCC, 7);
+UFS_POWER_DESC_PARAM(active_icc_levels_vcc, _ACTIVE_LVLS_VCC, 8);
+UFS_POWER_DESC_PARAM(active_icc_levels_vcc, _ACTIVE_LVLS_VCC, 9);
+UFS_POWER_DESC_PARAM(active_icc_levels_vcc, _ACTIVE_LVLS_VCC, 10);
+UFS_POWER_DESC_PARAM(active_icc_levels_vcc, _ACTIVE_LVLS_VCC, 11);
+UFS_POWER_DESC_PARAM(active_icc_levels_vcc, _ACTIVE_LVLS_VCC, 12);
+UFS_POWER_DESC_PARAM(active_icc_levels_vcc, _ACTIVE_LVLS_VCC, 13);
+UFS_POWER_DESC_PARAM(active_icc_levels_vcc, _ACTIVE_LVLS_VCC, 14);
+UFS_POWER_DESC_PARAM(active_icc_levels_vcc, _ACTIVE_LVLS_VCC, 15);
+UFS_POWER_DESC_PARAM(active_icc_levels_vccq, _ACTIVE_LVLS_VCCQ, 0);
+UFS_POWER_DESC_PARAM(active_icc_levels_vccq, _ACTIVE_LVLS_VCCQ, 1);
+UFS_POWER_DESC_PARAM(active_icc_levels_vccq, _ACTIVE_LVLS_VCCQ, 2);
+UFS_POWER_DESC_PARAM(active_icc_levels_vccq, _ACTIVE_LVLS_VCCQ, 3);
+UFS_POWER_DESC_PARAM(active_icc_levels_vccq, _ACTIVE_LVLS_VCCQ, 4);
+UFS_POWER_DESC_PARAM(active_icc_levels_vccq, _ACTIVE_LVLS_VCCQ, 5);
+UFS_POWER_DESC_PARAM(active_icc_levels_vccq, _ACTIVE_LVLS_VCCQ, 6);
+UFS_POWER_DESC_PARAM(active_icc_levels_vccq, _ACTIVE_LVLS_VCCQ, 7);
+UFS_POWER_DESC_PARAM(active_icc_levels_vccq, _ACTIVE_LVLS_VCCQ, 8);
+UFS_POWER_DESC_PARAM(active_icc_levels_vccq, _ACTIVE_LVLS_VCCQ, 9);
+UFS_POWER_DESC_PARAM(active_icc_levels_vccq, _ACTIVE_LVLS_VCCQ, 10);
+UFS_POWER_DESC_PARAM(active_icc_levels_vccq, _ACTIVE_LVLS_VCCQ, 11);
+UFS_POWER_DESC_PARAM(active_icc_levels_vccq, _ACTIVE_LVLS_VCCQ, 12);
+UFS_POWER_DESC_PARAM(active_icc_levels_vccq, _ACTIVE_LVLS_VCCQ, 13);
+UFS_POWER_DESC_PARAM(active_icc_levels_vccq, _ACTIVE_LVLS_VCCQ, 14);
+UFS_POWER_DESC_PARAM(active_icc_levels_vccq, _ACTIVE_LVLS_VCCQ, 15);
+UFS_POWER_DESC_PARAM(active_icc_levels_vccq2, _ACTIVE_LVLS_VCCQ2, 0);
+UFS_POWER_DESC_PARAM(active_icc_levels_vccq2, _ACTIVE_LVLS_VCCQ2, 1);
+UFS_POWER_DESC_PARAM(active_icc_
Re: scsi: sg: assorted memory corruptions
On Thu, 2018-02-01 at 08:04 +0100, Dmitry Vyukov wrote: > On Thu, Feb 1, 2018 at 7:03 AM, Douglas Gilbert wrote: > > On 2018-01-30 07:22 AM, Dmitry Vyukov wrote: [...] > > > [1:0:0:0]cd/dvd QEMU QEMU DVD-ROM 2.0. /dev/sr0 /dev/sg1 > > > > > > # readlink /sys/class/scsi_generic/sg0 > > > > > > ../../devices/pci:00/:00:01.1/ata1/host0/target0:0:0/0:0:0:0/scsi_generic/sg0 > > > > > > # cat /sys/class/scsi_generic/sg0/device/vendor > > > ATA > > > > > > ^ > > That subsystem is the culprit IMO, most likely libata. > > > > Until you can show this test failing on something other than an > > ATA disk, then I will treat this issue as closed. > > Hi Doug, > > Why is bug in ATA not a bug? Is it long unused by everybody? I've got > it by running qemu with default flags... If the bug is in libata then it's not on Doug to fix it since he's only maintaining sg. Ben. -- Ben Hutchings Software Developer, Codethink Ltd.
[PATCH v4 05/10] ufs: sysfs: health descriptor
This patch introduces a sysfs group entry for the UFS health descriptor
parameters. The group adds "health_descriptor" folder under the UFS driver
sysfs entry (/sys/bus/platform/drivers/ufshcd/*). The parameters are shown
as hexadecimal numbers. The full information about the parameters could be
found at UFS specifications 2.1.
Signed-off-by: Stanislav Nijnikov
Reviewed-by: Greg Kroah-Hartman
---
Documentation/ABI/testing/sysfs-driver-ufs | 28
drivers/scsi/ufs/ufs-sysfs.c | 20
drivers/scsi/ufs/ufs.h | 11 +++
drivers/scsi/ufs/ufshcd.c | 8
drivers/scsi/ufs/ufshcd.h | 1 +
5 files changed, 68 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-driver-ufs
b/Documentation/ABI/testing/sysfs-driver-ufs
index 6ea7613..ddb012b 100644
--- a/Documentation/ABI/testing/sysfs-driver-ufs
+++ b/Documentation/ABI/testing/sysfs-driver-ufs
@@ -413,3 +413,31 @@ Description: This file shows the memory capacity
adjustment factor for
descriptor parameters. The full information about the
descriptor could be found at UFS specifications 2.1.
The file is read only.
+
+
+What: /sys/bus/platform/drivers/ufshcd/*/health_descriptor/eol_info
+Date: February 2018
+Contact: Stanislav Nijnikov
+Description: This file shows preend of life information. This is one
+ of the UFS health descriptor parameters. The full
+ information about the descriptor could be found at
+ UFS specifications 2.1.
+ The file is read only.
+
+What:
/sys/bus/platform/drivers/ufshcd/*/health_descriptor/life_time_estimation_a
+Date: February 2018
+Contact: Stanislav Nijnikov
+Description: This file shows indication of the device life time
+ (method a). This is one of the UFS health descriptor
+ parameters. The full information about the descriptor
+ could be found at UFS specifications 2.1.
+ The file is read only.
+
+What:
/sys/bus/platform/drivers/ufshcd/*/health_descriptor/life_time_estimation_b
+Date: February 2018
+Contact: Stanislav Nijnikov
+Description: This file shows indication of the device life time
+ (method b). This is one of the UFS health descriptor
+ parameters. The full information about the descriptor
+ could be found at UFS specifications 2.1.
+ The file is read only.
diff --git a/drivers/scsi/ufs/ufs-sysfs.c b/drivers/scsi/ufs/ufs-sysfs.c
index 6427804..1c89009 100644
--- a/drivers/scsi/ufs/ufs-sysfs.c
+++ b/drivers/scsi/ufs/ufs-sysfs.c
@@ -364,11 +364,31 @@ static const struct attribute_group
ufs_sysfs_geometry_descriptor_group = {
.attrs = ufs_sysfs_geometry_descriptor,
};
+#define UFS_HEALTH_DESC_PARAM(_name, _uname, _size) \
+ UFS_DESC_PARAM(_name, _uname, HEALTH, _size)
+
+UFS_HEALTH_DESC_PARAM(eol_info, _EOL_INFO, BYTE);
+UFS_HEALTH_DESC_PARAM(life_time_estimation_a, _LIFE_TIME_EST_A, BYTE);
+UFS_HEALTH_DESC_PARAM(life_time_estimation_b, _LIFE_TIME_EST_B, BYTE);
+
+static struct attribute *ufs_sysfs_health_descriptor[] = {
+ &dev_attr_eol_info.attr,
+ &dev_attr_life_time_estimation_a.attr,
+ &dev_attr_life_time_estimation_b.attr,
+ NULL,
+};
+
+static const struct attribute_group ufs_sysfs_health_descriptor_group = {
+ .name = "health_descriptor",
+ .attrs = ufs_sysfs_health_descriptor,
+};
+
static const struct attribute_group *ufs_sysfs_groups[] = {
&ufs_sysfs_default_group,
&ufs_sysfs_device_descriptor_group,
&ufs_sysfs_interconnect_descriptor_group,
&ufs_sysfs_geometry_descriptor_group,
+ &ufs_sysfs_health_descriptor_group,
NULL,
};
diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index 04d41c8..6bfeedb 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -154,6 +154,7 @@ enum desc_idn {
QUERY_DESC_IDN_RFU_1= 0x6,
QUERY_DESC_IDN_GEOMETRY = 0x7,
QUERY_DESC_IDN_POWER= 0x8,
+ QUERY_DESC_IDN_HEALTH = 0x9,
QUERY_DESC_IDN_MAX,
};
@@ -169,6 +170,7 @@ enum ufs_desc_def_size {
QUERY_DESC_INTERCONNECT_DEF_SIZE= 0x06,
QUERY_DESC_GEOMETRY_DEF_SIZE= 0x44,
QUERY_DESC_POWER_DEF_SIZE = 0x62,
+ QUERY_DESC_HEALTH_DEF_SIZE = 0x25,
};
/* Unit descriptor parameters offsets in bytes*/
@@ -274,6 +276,15 @@ enum geometry_desc_param {
GEOMETRY_DESC_PARAM_OPT_LOG_BLK_SIZE= 0x44,
};
+/* Health descriptor parameters offsets in bytes*/
+enum health_desc_param {
+ HEALTH_DESC_PARAM_LEN = 0x0,
+ HEALTH_DESC_PARAM_TYPE = 0x1,
+ HEALTH_DESC_P
[PATCH v4 09/10] ufs: sysfs: flags
This patch introduces a sysfs group entry for the UFS flags. The group adds
"flags" folder under the UFS driver sysfs entry
(/sys/bus/platform/drivers/ufshcd/*). The flags are shown as boolean value
("true" or "false"). The full information about the UFS flags could be
found at UFS specifications 2.1.
Signed-off-by: Stanislav Nijnikov
---
Documentation/ABI/testing/sysfs-driver-ufs | 65 ++
drivers/scsi/ufs/ufs-sysfs.c | 39 ++
drivers/scsi/ufs/ufs.h | 14 +--
3 files changed, 115 insertions(+), 3 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-driver-ufs
b/Documentation/ABI/testing/sysfs-driver-ufs
index 57c6a90..f4f49e2 100644
--- a/Documentation/ABI/testing/sysfs-driver-ufs
+++ b/Documentation/ABI/testing/sysfs-driver-ufs
@@ -598,3 +598,68 @@ Description: This file shows the granularity of the
LUN. This is one of
the UFS unit descriptor parameters. The full information
about the descriptor could be found at UFS specifications 2.1.
The file is read only.
+
+
+What: /sys/bus/platform/drivers/ufshcd/*/flags/device_init
+Date: February 2018
+Contact: Stanislav Nijnikov
+Description: This file shows the device init status. The full information
+ about the flag could be found at UFS specifications 2.1.
+ The file is read only.
+
+What: /sys/bus/platform/drivers/ufshcd/*/flags/permanent_wpe
+Date: February 2018
+Contact: Stanislav Nijnikov
+Description: This file shows whether permanent write protection is enabled.
+ The full information about the flag could be found at
+ UFS specifications 2.1.
+ The file is read only.
+
+What: /sys/bus/platform/drivers/ufshcd/*/flags/power_on_wpe
+Date: February 2018
+Contact: Stanislav Nijnikov
+Description: This file shows whether write protection is enabled on all
+ logical units configured as power on write protected. The
+ full information about the flag could be found at
+ UFS specifications 2.1.
+ The file is read only.
+
+What: /sys/bus/platform/drivers/ufshcd/*/flags/bkops_enable
+Date: February 2018
+Contact: Stanislav Nijnikov
+Description: This file shows whether the device background operations are
+ enabled. The full information about the flag could be
+ found at UFS specifications 2.1.
+ The file is read only.
+
+What: /sys/bus/platform/drivers/ufshcd/*/flags/life_span_mode_enable
+Date: February 2018
+Contact: Stanislav Nijnikov
+Description: This file shows whether the device life span mode is enabled.
+ The full information about the flag could be found at
+ UFS specifications 2.1.
+ The file is read only.
+
+What: /sys/bus/platform/drivers/ufshcd/*/flags/phy_resource_removal
+Date: February 2018
+Contact: Stanislav Nijnikov
+Description: This file shows whether physical resource removal is enable.
+ The full information about the flag could be found at
+ UFS specifications 2.1.
+ The file is read only.
+
+What: /sys/bus/platform/drivers/ufshcd/*/flags/busy_rtc
+Date: February 2018
+Contact: Stanislav Nijnikov
+Description: This file shows whether the device is executing internal
+ operation related to real time clock. The full information
+ about the flag could be found at UFS specifications 2.1.
+ The file is read only.
+
+What: /sys/bus/platform/drivers/ufshcd/*/flags/disable_fw_update
+Date: February 2018
+Contact: Stanislav Nijnikov
+Description: This file shows whether the device FW update is permanently
+ disabled. The full information about the flag could be found
+ at UFS specifications 2.1.
+ The file is read only.
diff --git a/drivers/scsi/ufs/ufs-sysfs.c b/drivers/scsi/ufs/ufs-sysfs.c
index 265eca6..ab04205 100644
--- a/drivers/scsi/ufs/ufs-sysfs.c
+++ b/drivers/scsi/ufs/ufs-sysfs.c
@@ -553,6 +553,44 @@ static const struct attribute_group
ufs_sysfs_string_descriptors_group = {
.attrs = ufs_sysfs_string_descriptors,
};
+#define UFS_FLAG(_name, _uname) \
+static ssize_t _name##_show(struct device *dev, \
+ struct device_attribute *attr, char *buf) \
+{ \
+ bool flag;\
+ struct ufs_hba *hba = dev_get_drvdata(dev); \
+ if (ufshcd_query_flag(hba, UPIU_QUERY_OPCODE_
[PATCH v4 07/10] ufs: sysfs: string descriptors
This patch introduces a sysfs group entry for the UFS string descriptors.
The group adds "string_descriptors" folder under the UFS driver
sysfs entry (/sys/bus/platform/drivers/ufshcd/*). The folder will contain
5 files that will show string values defined by the UFS spec:
a manufacturer name, a product name, an OEM id, a serial number and a
product revision. The full information about the string descriptors
could be found at UFS specifications 2.1.
Signed-off-by: Stanislav Nijnikov
Reviewed-by: Greg Kroah-Hartman
---
Documentation/ABI/testing/sysfs-driver-ufs | 39 +
drivers/scsi/ufs/ufs-sysfs.c | 55 ++
drivers/scsi/ufs/ufshcd.c | 14
drivers/scsi/ufs/ufshcd.h | 9 +
4 files changed, 110 insertions(+), 7 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-driver-ufs
b/Documentation/ABI/testing/sysfs-driver-ufs
index 7460566..c17a968 100644
--- a/Documentation/ABI/testing/sysfs-driver-ufs
+++ b/Documentation/ABI/testing/sysfs-driver-ufs
@@ -451,3 +451,42 @@ Description: This file shows maximum VCC, VCCQ and
VCCQ2 value for
power descriptor parameters. The full information about
the descriptor could be found at UFS specifications 2.1.
The file is read only.
+
+
+What:
/sys/bus/platform/drivers/ufshcd/*/string_descriptors/manufacturer_name
+Date: February 2018
+Contact: Stanislav Nijnikov
+Description: This file contains a device manufactureer name string.
+ The full information about the descriptor could be found at
+ UFS specifications 2.1.
+ The file is read only.
+
+What:
/sys/bus/platform/drivers/ufshcd/*/string_descriptors/product_name
+Date: February 2018
+Contact: Stanislav Nijnikov
+Description: This file contains a product name string. The full information
+ about the descriptor could be found at UFS specifications 2.1.
+ The file is read only.
+
+What: /sys/bus/platform/drivers/ufshcd/*/string_descriptors/oem_id
+Date: February 2018
+Contact: Stanislav Nijnikov
+Description: This file contains a OEM ID string. The full information
+ about the descriptor could be found at UFS specifications 2.1.
+ The file is read only.
+
+What:
/sys/bus/platform/drivers/ufshcd/*/string_descriptors/serial_number
+Date: February 2018
+Contact: Stanislav Nijnikov
+Description: This file contains a device serial number string. The full
+ information about the descriptor could be found at
+ UFS specifications 2.1.
+ The file is read only.
+
+What:
/sys/bus/platform/drivers/ufshcd/*/string_descriptors/product_revision
+Date: February 2018
+Contact: Stanislav Nijnikov
+Description: This file contains a product revision string. The full
+ information about the descriptor could be found at
+ UFS specifications 2.1.
+ The file is read only.
diff --git a/drivers/scsi/ufs/ufs-sysfs.c b/drivers/scsi/ufs/ufs-sysfs.c
index d2e6b49..db5d2f8 100644
--- a/drivers/scsi/ufs/ufs-sysfs.c
+++ b/drivers/scsi/ufs/ufs-sysfs.c
@@ -500,6 +500,60 @@ static const struct attribute_group
ufs_sysfs_power_descriptor_group = {
.attrs = ufs_sysfs_power_descriptor,
};
+#define UFS_STRING_DESCRIPTOR(_name, _pname) \
+static ssize_t _name##_show(struct device *dev, \
+ struct device_attribute *attr, char *buf) \
+{ \
+ u8 index; \
+ struct ufs_hba *hba = dev_get_drvdata(dev); \
+ int ret; \
+ int desc_len = QUERY_DESC_MAX_SIZE; \
+ u8 *desc_buf; \
+ desc_buf = kzalloc(QUERY_DESC_MAX_SIZE, GFP_ATOMIC); \
+ if (!desc_buf)\
+ return -ENOMEM; \
+ ret = ufshcd_query_descriptor_retry(hba, UPIU_QUERY_OPCODE_READ_DESC, \
+ QUERY_DESC_IDN_DEVICE, 0, 0, desc_buf, &desc_len);\
+ if (ret) {\
+ ret = -EINVAL;\
+ goto out; \
+ } \
+ index = desc_buf[DEVICE_DESC_PARAM##_pname];
[PATCH v4 08/10] ufs: sysfs: unit descriptor
This patch introduces a sysfs group entry for the UFS unit descriptor parameters. The group adds "unit_descriptor" folder under the corresponding SCSI device sysfs entry (/sys/class/scsi_device/*/device/). The parameters are shown as hexadecimal numbers. The full information about the parameters could be found at UFS specifications 2.1. In addition the patch presents an additional field in the scsi_host_template structure - struct attribute_group **sdev_group. This field allows to define groups of attributes. It will provide an ability to use binary attributes in addition to device attributes and to group them under subfolders if necessary. Signed-off-by: Stanislav Nijnikov Reviewed-by: Greg Kroah-Hartman --- Documentation/ABI/testing/sysfs-driver-ufs | 108 + drivers/scsi/scsi_sysfs.c | 14 drivers/scsi/ufs/ufs-sysfs.c | 54 +++ drivers/scsi/ufs/ufs-sysfs.h | 2 + drivers/scsi/ufs/ufs.h | 11 +++ drivers/scsi/ufs/ufshcd.c | 23 ++ drivers/scsi/ufs/ufshcd.h | 15 include/scsi/scsi_host.h | 6 ++ 8 files changed, 217 insertions(+), 16 deletions(-) diff --git a/Documentation/ABI/testing/sysfs-driver-ufs b/Documentation/ABI/testing/sysfs-driver-ufs index c17a968..57c6a90 100644 --- a/Documentation/ABI/testing/sysfs-driver-ufs +++ b/Documentation/ABI/testing/sysfs-driver-ufs @@ -490,3 +490,111 @@ Description: This file contains a product revision string. The full information about the descriptor could be found at UFS specifications 2.1. The file is read only. + + +What: /sys/class/scsi_device/*/device/unit_descriptor/boot_lun_id +Date: February 2018 +Contact: Stanislav Nijnikov +Description: This file shows boot LUN information. This is one of + the UFS unit descriptor parameters. The full information + about the descriptor could be found at UFS specifications 2.1. + The file is read only. + +What: /sys/class/scsi_device/*/device/unit_descriptor/lun_write_protect +Date: February 2018 +Contact: Stanislav Nijnikov +Description: This file shows LUN write protection status. This is one of + the UFS unit descriptor parameters. The full information + about the descriptor could be found at UFS specifications 2.1. + The file is read only. + +What: /sys/class/scsi_device/*/device/unit_descriptor/lun_queue_depth +Date: February 2018 +Contact: Stanislav Nijnikov +Description: This file shows LUN queue depth. This is one of the UFS + unit descriptor parameters. The full information about + the descriptor could be found at UFS specifications 2.1. + The file is read only. + +What: /sys/class/scsi_device/*/device/unit_descriptor/psa_sensitive +Date: February 2018 +Contact: Stanislav Nijnikov +Description: This file shows PSA sensitivity. This is one of the UFS + unit descriptor parameters. The full information about + the descriptor could be found at UFS specifications 2.1. + The file is read only. + +What: /sys/class/scsi_device/*/device/unit_descriptor/lun_memory_type +Date: February 2018 +Contact: Stanislav Nijnikov +Description: This file shows LUN memory type. This is one of the UFS + unit descriptor parameters. The full information about + the descriptor could be found at UFS specifications 2.1. + The file is read only. + +What: /sys/class/scsi_device/*/device/unit_descriptor/data_reliability +Date: February 2018 +Contact: Stanislav Nijnikov +Description: This file defines the device behavior when a power failure + occurs during a write operation. This is one of the UFS + unit descriptor parameters. The full information about + the descriptor could be found at UFS specifications 2.1. + The file is read only. + +What: /sys/class/scsi_device/*/device/unit_descriptor/logical_block_size +Date: February 2018 +Contact: Stanislav Nijnikov +Description: This file shows the size of addressable logical blocks + (calculated as an exponent with base 2). This is one of + the UFS unit descriptor parameters. The full information about + the descriptor could be found at UFS specifications 2.1. + The file is read only. + +What: /sys/class/scsi_device/*/device/unit_descriptor/logical_block_count +Date: February 2018 +Contact: Stanislav Nijnikov +Description: This file shows total number of addressable logical blocks. + This is one of the UFS unit descripto
[PATCH v4 10/10] ufs: sysfs: attributes
This patch introduces a sysfs group entry for the UFS attributes. The group adds "attributes" folder under the UFS driver sysfs entry (/sys/bus/platform/drivers/ufshcd/*). The attributes are shown as hexadecimal numbers. The full information about the attributes could be found at UFS specifications 2.1. Signed-off-by: Stanislav Nijnikov --- Documentation/ABI/testing/sysfs-driver-ufs | 139 + drivers/scsi/ufs/ufs-sysfs.c | 82 + drivers/scsi/ufs/ufs-sysfs.h | 1 + drivers/scsi/ufs/ufs.h | 27 +- drivers/scsi/ufs/ufshcd.c | 5 +- drivers/scsi/ufs/ufshcd.h | 3 +- 6 files changed, 250 insertions(+), 7 deletions(-) diff --git a/Documentation/ABI/testing/sysfs-driver-ufs b/Documentation/ABI/testing/sysfs-driver-ufs index f4f49e2..07f1c2f 100644 --- a/Documentation/ABI/testing/sysfs-driver-ufs +++ b/Documentation/ABI/testing/sysfs-driver-ufs @@ -663,3 +663,142 @@ Description: This file shows whether the device FW update is permanently disabled. The full information about the flag could be found at UFS specifications 2.1. The file is read only. + + +What: /sys/bus/platform/drivers/ufshcd/*/attributes/boot_lun_enabled +Date: February 2018 +Contact: Stanislav Nijnikov +Description: This file provides the boot lun enabled UFS device attribute. + The full information about the attribute could be found at + UFS specifications 2.1. + The file is read only. + +What: /sys/bus/platform/drivers/ufshcd/*/attributes/current_power_mode +Date: February 2018 +Contact: Stanislav Nijnikov +Description: This file provides the current power mode UFS device attribute. + The full information about the attribute could be found at + UFS specifications 2.1. + The file is read only. + +What: /sys/bus/platform/drivers/ufshcd/*/attributes/active_icc_level +Date: February 2018 +Contact: Stanislav Nijnikov +Description: This file provides the active icc level UFS device attribute. + The full information about the attribute could be found at + UFS specifications 2.1. + The file is read only. + +What: /sys/bus/platform/drivers/ufshcd/*/attributes/ooo_data_enabled +Date: February 2018 +Contact: Stanislav Nijnikov +Description: This file provides the out of order data transfer enabled UFS + device attribute. The full information about the attribute + could be found at UFS specifications 2.1. + The file is read only. + +What: /sys/bus/platform/drivers/ufshcd/*/attributes/bkops_status +Date: February 2018 +Contact: Stanislav Nijnikov +Description: This file provides the background operations status UFS device + attribute. The full information about the attribute could + be found at UFS specifications 2.1. + The file is read only. + +What: /sys/bus/platform/drivers/ufshcd/*/attributes/purge_status +Date: February 2018 +Contact: Stanislav Nijnikov +Description: This file provides the purge operation status UFS device + attribute. The full information about the attribute could + be found at UFS specifications 2.1. + The file is read only. + +What: /sys/bus/platform/drivers/ufshcd/*/attributes/max_data_in_size +Date: February 2018 +Contact: Stanislav Nijnikov +Description: This file shows the maximum data size in a DATA IN + UPIU. The full information about the attribute could + be found at UFS specifications 2.1. + The file is read only. + +What: /sys/bus/platform/drivers/ufshcd/*/attributes/max_data_out_size +Date: February 2018 +Contact: Stanislav Nijnikov +Description: This file shows the maximum number of bytes that can be + requested with a READY TO TRANSFER UPIU. The full information + about the attribute could be found at UFS specifications 2.1. + The file is read only. + +What: /sys/bus/platform/drivers/ufshcd/*/attributes/reference_clock_frequency +Date: February 2018 +Contact: Stanislav Nijnikov +Description: This file provides the reference clock frequency UFS device + attribute. The full information about the attribute could + be found at UFS specifications 2.1. + The file is read only. + +What: /sys/bus/platform/drivers/ufshcd/*/attributes/configuration_descriptor_lock +Date: February 2018 +Contact: Stanislav Nijnikov +Description: This file shows whether the configuration descriptor is locked. +
[PATCH v4 00/10] ufs: sysfs: read-only access to device descriptors, attributes and flags
This patch introduces sysfs entries that will provide read-only access to device management data that could be received with UFS query requests. User-space applications will be able to read UFS device descriptors, flags and attributes. This will allow to get full UFS device configuration and its status. The descriptors are provided as set of files representing its parameters. The flags are using "true"/"false" representation of their value. The attributes are shown as hexadecimal value. The descriptors, attributes and flags are placed in separate subfolders under the UFS device sysfs entry (/sys/bus/platform/drivers/ufshcd/*/). The string descriptor subfolder contains five string descriptors defined by UFS specification 2.1. The LUN specific descriptor and attribute are placed under corresponding SCSI device sysfs entries (/sys/class/scsi_device/*/device/). In addition the patch presents an additional field in the scsi_host_template structure - struct attribute_group **sdev_group. This field allows to define groups of attributes. It will provide an ability to use binary attributes in addition to device attributes and to group them under subfolders if necessary. Changelog: v3 -> v4 Additional patch the introduces default attributes group for the existing ufs sysfs entries (rpm_lvl and spm_lvl) The ufs_sysfs_read_desc_param function calls to ufshcd_read_desc_param insted of ufshcd_query_descriptor_retry to avoid code duplication. The code was updated to remove the checkpatch error "ERROR: Macros with complex values should be enclosed in parentheses" Added "_" to macros parameters to remove "#undef DEVICE_CLASS" The legal information was updated to satisfy the SPDX requirements The date in Documentation/ABI/testing/sysfs-driver-ufs was updated. v2 -> v3 The Makefile is updated to make ufs-sysfs.c part of the ufshcd module. The unnecessary EXPORT_SYMBOL were removed Added a legal info header to the new files The date in Documentation/ABI/testing/sysfs-driver-ufs was updated. v1 -> v2 Provided additional description for the changes Stanislav Nijnikov (10): ufs: sysfs: attribute group for existing sysfs entries. ufs: sysfs: device descriptor ufs: sysfs: interconnect descriptor ufs: sysfs: geometry descriptor ufs: sysfs: health descriptor ufs: sysfs: power descriptor ufs: sysfs: string descriptors ufs: sysfs: unit descriptor ufs: sysfs: flags ufs: sysfs: attributes Documentation/ABI/testing/sysfs-driver-ufs | 804 + drivers/scsi/scsi_sysfs.c | 14 + drivers/scsi/ufs/Makefile | 3 +- drivers/scsi/ufs/ufs-sysfs.c | 757 +++ drivers/scsi/ufs/ufs-sysfs.h | 25 + drivers/scsi/ufs/ufs.h | 115 - drivers/scsi/ufs/ufshcd.c | 218 ++-- drivers/scsi/ufs/ufshcd.h | 34 ++ include/scsi/scsi_host.h | 6 + 9 files changed, 1785 insertions(+), 191 deletions(-) create mode 100644 Documentation/ABI/testing/sysfs-driver-ufs create mode 100644 drivers/scsi/ufs/ufs-sysfs.c create mode 100644 drivers/scsi/ufs/ufs-sysfs.h -- 2.7.4
[PATCH v4 02/10] ufs: sysfs: device descriptor
This patch introduces a sysfs group entry for the UFS device descriptor parameters. The group adds "device_descriptor" folder under the UFS driver sysfs entry (/sys/bus/platform/drivers/ufshcd/*). The parameters are shown as hexadecimal numbers. The full information about the parameters could be found at UFS specifications 2.1. Signed-off-by: Stanislav Nijnikov --- Documentation/ABI/testing/sysfs-driver-ufs | 223 + drivers/scsi/ufs/ufs-sysfs.c | 123 drivers/scsi/ufs/ufs.h | 8 ++ drivers/scsi/ufs/ufshcd.c | 12 +- drivers/scsi/ufs/ufshcd.h | 6 + 5 files changed, 366 insertions(+), 6 deletions(-) create mode 100644 Documentation/ABI/testing/sysfs-driver-ufs diff --git a/Documentation/ABI/testing/sysfs-driver-ufs b/Documentation/ABI/testing/sysfs-driver-ufs new file mode 100644 index 000..8da7b84 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-driver-ufs @@ -0,0 +1,223 @@ +What: /sys/bus/platform/drivers/ufshcd/*/device_descriptor/device_type +Date: February 2018 +Contact: Stanislav Nijnikov +Description: This file shows the device type. This is one of the UFS + device descriptor parameters. The full information about + the descriptor could be found at UFS specifications 2.1. + The file is read only. + +What: /sys/bus/platform/drivers/ufshcd/*/device_descriptor/device_class +Date: February 2018 +Contact: Stanislav Nijnikov +Description: This file shows the device class. This is one of the UFS + device descriptor parameters. The full information about + the descriptor could be found at UFS specifications 2.1. + The file is read only. + +What: /sys/bus/platform/drivers/ufshcd/*/device_descriptor/device_sub_class +Date: February 2018 +Contact: Stanislav Nijnikov +Description: This file shows the UFS storage subclass. This is one of + the UFS device descriptor parameters. The full information + about the descriptor could be found at UFS specifications 2.1. + The file is read only. + +What: /sys/bus/platform/drivers/ufshcd/*/device_descriptor/protocol +Date: February 2018 +Contact: Stanislav Nijnikov +Description: This file shows the protocol supported by an UFS device. + This is one of the UFS device descriptor parameters. + The full information about the descriptor could be found + at UFS specifications 2.1. + The file is read only. + +What: /sys/bus/platform/drivers/ufshcd/*/device_descriptor/number_of_luns +Date: February 2018 +Contact: Stanislav Nijnikov +Description: This file shows number of logical units. This is one of + the UFS device descriptor parameters. The full information + about the descriptor could be found at UFS specifications 2.1. + The file is read only. + +What: /sys/bus/platform/drivers/ufshcd/*/device_descriptor/number_of_wluns +Date: February 2018 +Contact: Stanislav Nijnikov +Description: This file shows number of well known logical units. + This is one of the UFS device descriptor parameters. + The full information about the descriptor could be found + at UFS specifications 2.1. + The file is read only. + +What: /sys/bus/platform/drivers/ufshcd/*/device_descriptor/boot_enable +Date: February 2018 +Contact: Stanislav Nijnikov +Description: This file shows value that indicates whether the device is + enabled for boot. This is one of the UFS device descriptor + parameters. The full information about the descriptor could + be found at UFS specifications 2.1. + The file is read only. + +What: /sys/bus/platform/drivers/ufshcd/*/device_descriptor/descriptor_access_enable +Date: February 2018 +Contact: Stanislav Nijnikov +Description: This file shows value that indicates whether the device + descriptor could be read after partial initialization phase + of the boot sequence. This is one of the UFS device descriptor + parameters. The full information about the descriptor could + be found at UFS specifications 2.1. + The file is read only. + +What: /sys/bus/platform/drivers/ufshcd/*/device_descriptor/initial_power_mode +Date: February 2018 +Contact: Stanislav Nijnikov +Description: This file shows value that defines the power mode after + device initialization or hardware reset. This is one of + the UFS device descriptor parameters. The full information + about the descriptor coul
Re: [PATCH] qedi: Fix truncation of name and secret
On Wed, 2018-01-31 at 22:57 -0800, Nilesh Javali wrote:
> Adjust the NULL byte added by snprintf.
Hello Nilesh,
Sorry but neither the title nor the description of this patch make clear what
the intention of this patch is. Please describe this more clearly.
> @@ -1840,7 +1840,7 @@ static ssize_t qedi_show_boot_ini_info(void *data, int
> type, char *buf)
>
> switch (type) {
> case ISCSI_BOOT_INI_INITIATOR_NAME:
> - rc = snprintf(str, NVM_ISCSI_CFG_ISCSI_NAME_MAX_LEN, "%s\n",
> + rc = snprintf(str, NVM_ISCSI_CFG_ISCSI_NAME_MAX_LEN + 1, "%s",
> initiator->initiator_name.byte);
> break;
> default:
The function qedi_show_boot_ini_info() is used to export information through
sysfs. Users expect ASCII data in sysfs to be terminated with a newline. So I
think removing the trailing newline is wrong. Additionally, an explanation of
why you think that the second argument of the snprintf() call should be
increased is missing. And since buf points at a buffer of size PAGE_SIZE,
have you consider to change the second snprintf() argument into PAGE_SIZE?
Thanks,
Bart.
Re: [LSF/MM TOPIC] irq affinity handling for high CPU count machines
Hello Hannes, On Mon, Jan 29, 2018 at 10:08:43AM +0100, Hannes Reinecke wrote: > Hi all, > > here's a topic which came up on the SCSI ML (cf thread '[RFC 0/2] > mpt3sas/megaraid_sas: irq poll and load balancing of reply queue'). > > When doing I/O tests on a machine with more CPUs than MSIx vectors > provided by the HBA we can easily setup a scenario where one CPU is > submitting I/O and the other one is completing I/O. Which will result in > the latter CPU being stuck in the interrupt completion routine for > basically ever, resulting in the lockup detector kicking in. Today I am looking at one megaraid_sas related issue, and found pci_alloc_irq_vectors(PCI_IRQ_AFFINITY) is used in the driver, so looks each reply queue has been handled by more than one CPU if there are more CPUs than MSIx vectors in the system, which is done by generic irq affinity code, please see kernel/irq/affinity.c. Also IMO each reply queue may be treated as blk-mq's hw queue, then megaraid may benefit from blk-mq's MQ framework, but one annoying thing is that both legacy and blk-mq path need to be handled inside driver. > > How should these situations be handled? > Should it be made the responsibility of the drivers, ensuring that the > interrupt completion routine is terminated after a certain time? > Should it be made the resposibility of the upper layers? > Should it be the responsibility of the interrupt mapping code? > Can/should interrupt polling be used in these situations? Yeah, I guess interrupt polling may improve these situations, especially KPTI introduces some extra cost in interrupt handling. Thanks, Ming
Re: [PATCH 0/2] genirq/affinity: try to make sure online CPU is assgined to irq vector
On Thu, Feb 01, 2018 at 02:53:35PM +, Don Brace wrote:
> > -Original Message-
> > From: Ming Lei [mailto:ming@redhat.com]
> > Sent: Thursday, February 01, 2018 4:37 AM
> > To: Don Brace
> > Cc: Laurence Oberman ; Thomas Gleixner
> > ; Christoph Hellwig ; Jens Axboe
> > ; linux-bl...@vger.kernel.org; linux-ker...@vger.kernel.org;
> > Mike Snitzer
> > Subject: Re: [PATCH 0/2] genirq/affinity: try to make sure online CPU is
> > assgined
> > to irq vector
> >
> > EXTERNAL EMAIL
> >
> >
> > On Tue, Jan 16, 2018 at 03:22:18PM +, Don Brace wrote:
> > > > -Original Message-
> > > > From: Laurence Oberman [mailto:lober...@redhat.com]
> > > > Sent: Tuesday, January 16, 2018 7:29 AM
> > > > To: Thomas Gleixner ; Ming Lei
> > > > Cc: Christoph Hellwig ; Jens Axboe ;
> > > > linux-bl...@vger.kernel.org; linux-ker...@vger.kernel.org; Mike Snitzer
> > > > ; Don Brace
> > > > Subject: Re: [PATCH 0/2] genirq/affinity: try to make sure online CPU is
> > assgined
> > > > to irq vector
> > > >
> > > > > > It is because of irq_create_affinity_masks().
> > > > >
> > > > > That still does not answer the question. If the interrupt for a queue
> > > > > is
> > > > > assigned to an offline CPU, then the queue should not be used and
> > > > > never
> > > > > raise an interrupt. That's how managed interrupts have been designed.
> > > > >
> > > > > Thanks,
> > > > >
> > > > > tglx
> > > > >
> > > > >
> > > > >
> > > > >
> > > >
> > > > I captured a full boot log for this issue for Microsemi, I will send it
> > > > to Don Brace.
> > > > I enabled all the HPSA debug and here is snippet
> > > >
> > > >
> > > > ..
> > > > ..
> > > > ..
> > > > 246.751135] INFO: task systemd-udevd:413 blocked for more than 120
> > > > seconds.
> > > > [ 246.788008] Tainted: G I 4.15.0-rc4.noming+ #1
> > > > [ 246.822380] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs"
> > > > disables this message.
> > > > [ 246.865594] systemd-udevd D0 413411 0x8004
> > > > [ 246.895519] Call Trace:
> > > > [ 246.909713] ? __schedule+0x340/0xc20
> > > > [ 246.930236] schedule+0x32/0x80
> > > > [ 246.947905] schedule_timeout+0x23d/0x450
> > > > [ 246.970047] ? find_held_lock+0x2d/0x90
> > > > [ 246.991774] ? wait_for_completion_io+0x108/0x170
> > > > [ 247.018172] io_schedule_timeout+0x19/0x40
> > > > [ 247.041208] wait_for_completion_io+0x110/0x170
> > > > [ 247.067326] ? wake_up_q+0x70/0x70
> > > > [ 247.086801] hpsa_scsi_do_simple_cmd+0xc6/0x100 [hpsa]
> > > > [ 247.114315] hpsa_scsi_do_simple_cmd_with_retry+0xb7/0x1c0 [hpsa]
> > > > [ 247.146629] hpsa_scsi_do_inquiry+0x73/0xd0 [hpsa]
> > > > [ 247.174118] hpsa_init_one+0x12cb/0x1a59 [hpsa]
> > >
> > > This trace comes from internally generated discovery commands. No SCSI
> > devices have
> > > been presented to the SML yet.
> > >
> > > At this point we should be running on only one CPU. These commands are
> > meant to use
> > > reply queue 0 which are tied to CPU 0. It's interesting that the patch
> > > helps.
> > >
> > > However, I was wondering if you could inspect the iLo IML logs and send
> > > the
> > > AHS logs for inspection.
> >
> > Hello Don,
> >
> > Now the patch has been merged to linus tree as:
> >
> > 84676c1f21e8ff54b ("genirq/affinity: assign vectors to all possible CPUs")
> >
> > and it breaks Laurence's machine completely, :-(
> >
> > I just take a look at HPSA's code, and found that reply queue is chosen
> > in the following way in most of code path:
> >
> > if (likely(reply_queue == DEFAULT_REPLY_QUEUE))
> > cp->ReplyQueue = smp_processor_id() % h->nreply_queues;
> >
> > h->nreply_queues is the msix vector number which is returned from
> > pci_alloc_irq_vectors(), and now some of vectors may be mapped to all
> > offline CPUs, for example, one processor isn't plugged to socket.
> >
> > If I understand correctly, 'cp->ReplyQueue' is aligned to one irq
> > vector, and the command is expected by handled via that irq vector,
> > is it right?
> >
> > If yes, now I guess this way can't work any more if number of online
> > CPUs is >= h->nreply_queues, and you may need to check the cpu affinity
> > of one vector before choosing the reply queue, and block/blk-mq-pci.c
> > may be helpful for you.
> >
> > Thanks,
> > Ming
>
> Thanks Ming,
> I start working up a patch.
Also the reply queue may be mapped to blk-mq's hw queue directly, then the
conversion may be done by blk-mq's MQ framework, but legacy path still need
the fix.
thanks
Ming
Re: [PATCH v3] scsi: sd: add new match array for cache_type
2018-01-31 11:22 GMT+08:00 Martin K. Petersen : > > Weiping, > >> add user friendly command strings sd_wr_cache to enable/disable >> write&read cache. user can enable both write and read cache by one of >> the following commands: > > I remain unconvinced that introducing redundant option strings is a > benefit. > > These shorthand forms may seem obvious to you, but not to everyone > else. Adding to the murkiness is that fact that write cache must be > *enabled* and read cache *disabled* in the protocol. The fact that the > ??rN and RCD columns below contain inverse values just emphasizes that > point that "0" and "1" are not a particularly good interface either. > > It is also confusing that the existing longhand forms will be printed > when the user subsequently queries the state instead of what they used > to toggle it. > Yes, indeed. > > This, however, is pretty much what I was looking for: > >> Encoding | WCE RCD | Write_cache Read_cache >> >> write through / w0r1 | 0 0 | off on >> none / w0r0 | 0 1 | off off >> write back / w1r1 | 1 0 | on on >> write back, no read (daft) / w1r0 | 1 1 | on off > > Please drop the w?r? options and submit a patch with this table and a > note about the "temporary" prefix. Put it in > Documentation/scsi/sd-parameters.txt or something to that effect. > It's ok for me. > Bonus points for also documenting the remaining sd sysfs attributes and > their options. I'll try. Thanks > > Thanks! > > -- > Martin K. Petersen Oracle Linux Engineering
