Re: [PATCH 5/6] cxlflash: Remove adapter file descriptor cache

Acked-by: Manoj N. Kumar On 8/9/2016 6:40 PM, Matthew R. Ochs wrote: The adapter file descriptor was previously cached within the kernel for a given context in order to support performing a close on behalf of an application. This is no longer needed as applications

Re: [PATCH 6/6] cxlflash: Update documentation

Acked-by: Manoj N. Kumar On 8/9/2016 6:40 PM, Matthew R. Ochs wrote: Update the block library link in the API documentation. Signed-off-by: Matthew R. Ochs --- Documentation/powerpc/cxlflash.txt | 2 +- 1 file changed, 1 insertion(+), 1

Re: [PATCH 4/6] cxlflash: Transition to application close model

Acked-by: Manoj N. Kumar On 8/9/2016 6:39 PM, Matthew R. Ochs wrote: Caching the adapter file descriptor and performing a close on behalf of an application is a poor design. This is due to the fact that once a file descriptor in installed, it is free to be altered

Re: [PATCH 3/6] cxlflash: Add kref to context

Acked-by: Manoj N. Kumar On 8/9/2016 6:39 PM, Matthew R. Ochs wrote: Currently, context user references are tracked via the list of LUNs that have attached to the context. While convenient, this is not intuitive without a deep study of the code and is inconsistent

Re: [PATCH 2/6] cxlflash: Cache owning adapter within context

Acked-by: Manoj N. Kumar On 8/9/2016 6:39 PM, Matthew R. Ochs wrote: The context removal routine requires access to the owning adapter structure to reset the context within the AFU as part of the tear down sequence. In order to support kref adoption, the owning

Re: [PATCH 1/6] cxlflash: Avoid mutex when destroying context

Acked-by: Manoj N. Kumar On 8/9/2016 6:39 PM, Matthew R. Ochs wrote: Context information structures are protected by a mutex that is held when accessing/manipulating the context. When the code that manages these structures was authored, a decision was made to include

Re: [PATCH 3/3] cxlflash: Shutdown notify support for CXL Flash cards

On 6/15/2016 6:49 PM, Uma Krishnan wrote: Some CXL Flash cards need notification of device shutdown in order to flush pending I/Os. A PCI notification hook for shutdown has been added where the driver notifies the card and returns. When the device is removed in the PCI remove path, notification

Re: [PATCH 2/3] cxlflash: Add device dependent flags

On 6/15/2016 6:49 PM, Uma Krishnan wrote: Device dependent flags are needed to support functions that are specific to a particular device. Acked-by: Manoj N. Kumar -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message

Re: MAINTAINERS: use new email address for James Bottomley

Reviewed-by: Manoj Kumar <ma...@linux.vnet.ibm.com> --- Manoj Kumar On 3/14/2016 2:42 PM, James Bottomley wrote: The @odin.com one has been bouncing for a while now, so replace with new Employer email. Signed-off-by: James Bottomley <j...@linux.vnet.ibm.com> --- diff --git a/M

Re: [PATCHv2] scsi_transport_sas: add 'scsi_target_id' sysfs attribute

Hannes: Thanks for correcting the last argument. Reviewed-by: Manoj Kumar <ma...@linux.vnet.ibm.com> --- Manoj Kumar On 3/14/2016 4:43 AM, Hannes Reinecke wrote: There is no way to detect the scsi_target_id for any given SAS remote port, so add a new sysfs attribute 'scsi_target_id'.

Re: [PATCH] scsi_transport_sas: add 'scsi_target_id' sysfs attribute

s, "0x%016llx\n", unsigned long long); sas_rphy_simple_attr(identify.phy_identifier, phy_identifier, "%d\n", u8); +sas_rphy_simple_attr(scsi_target_id, scsi_target_id, "%d\n", u8); Hannes: Shouldn't the last argument be u32? - Manoj Kumar -- To unsub

Re: [patch v2] scsi_dh_alua: uninitialized variable in alua_check_vpd()

ata *h, { int rel_port = -1, group_id; struct alua_port_group *pg, *old_pg = NULL; - bool pg_updated; + bool pg_updated = false; unsigned long flags; Reviewed-by: Manoj Kumar <ma...@linux.vnet.ibm.com> --- Manoj Kumar -- To unsubscribe from this list: s

Re: [PATCH v6 19/20] cxlflash: Use new cxl_pci_read_adapter_vpd() API

Fred: Thanks for submitting this. Including linux-scsi. Acked-by: Manoj N. Kumar - Manoj On 3/4/2016 5:26 AM, Frederic Barrat wrote: To read the adapter VPD, drivers can't rely on pci config APIs, as it wouldn't work on powerVM. cxl introduced a new kernel API

Re: [PATCH v2 1/7] ibmvscsi: Correct values for several viosrp_crq_format enums

Reviewed-by: Manoj Kumar <ma...@linux.vnet.ibm.com> --- Manoj Kumar On 2/10/2016 7:32 PM, Tyrel Datwyler wrote: The enum values for VIOSRP_LINUX_FORMAT and VIOSRP_INLINE_FORMAT are off by one. They are currently defined as 0x06 and 0x07 respetively. These values are defined in PAPR cor

Re: [PATCH v2 2/7] ibmvscsi: Add and use enums for valid CRQ header values

Tyrel: Thanks for incorporating the suggestions. Reviewed-by: Manoj Kumar <ma...@linux.vnet.ibm.com> --- Manoj Kumar On 2/10/2016 7:32 PM, Tyrel Datwyler wrote: The PAPR defines four valid header values for the first byte of a CRQ message. Namely, an unused/empty message (0x00), a

Re: [PATCH v2 3/7] ibmvscsi: Replace magic values in set_adpater_info() with defines

Reviewed-by: Manoj Kumar <ma...@linux.vnet.ibm.com> --- Manoj Kumar On 2/10/2016 7:32 PM, Tyrel Datwyler wrote: Add defines for mad version and mad os_type, and replace the magic numbers in set_adapter_info() accordingly. Signed-off-by: Tyrel Datwyler <tyr...@linux.vne

Re: [PATCH 2/6] ibmvscsi: Add and use enums for valid CRQ header values

Yeah, I can see how that is confusing. Since, all three possible valid crq message types have the first bit set I think this was originally a cute hack to grab anything that was likely valid. Then in ibmvscsi_handle_crq() we explicitly match the full header value in a switch statement logging

Re: [PATCH 2/6] ibmvscsi: Add and use enums for valid CRQ header values

On 2/3/2016 5:28 PM, Tyrel Datwyler wrote: The PAPR defines four valid header values for the first byte of a CRQ message. Namely, an unused/empty message (0x00), a valid command/response entry (0x80), a valid initialization entry (0xC0), and a transport event (0xFF). Define these values as enums

Re: [PATCH v2 3/6] cxlflash: Removed driver date print

On 12/14/2015 3:06 PM, Uma Krishnan wrote: Having a date for the driver requires it to be updated quite often. Removing the date which is not necessary. Also made use of the existing symbol to print the driver name. Signed-off-by: Uma Krishnan --- Acked-by: Manoj

Re: [PATCH 6/6] cxlflash: Enable device id for future IBM CXL adapter

On 12/13/2015 9:47 PM, Andrew Donnellan wrote: On 11/12/15 09:54, Uma Krishnan wrote: From: Manoj Kumar <ma...@linux.vnet.ibm.com> This drop enables a future card with a device id of 0x0600 to be recognized by the cxlflash driver. No card specific programming has been added. Thes

Re: [PATCH 2/6] cxlflash: Fix to avoid virtual LUN failover failure

ry in a presence of an I/O failure. Signed-off-by: Matthew R. Ochs <mro...@linux.vnet.ibm.com> Acked-by: Manoj Kumar <ma...@linux.vnet.ibm.com> -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.k

Re: [PATCH RESEND] scsi: Set sg_tablesize to 1, for LLDDs that set SG_NONE

Hannes: A very valid point. Thanks for your suggestion. Will look at resolving the issue in blk-mq. Regards, - Manoj On 11/11/2015 1:28 AM, Hannes Reinecke wrote: Shouldn't we rather fixup blk-mq to properly support SG_NONE? Silently converting SG_NONE (=0) to 1 has a fair chance of breaking

[PATCH RESEND] scsi: Set sg_tablesize to 1, for LLDDs that set SG_NONE

Oops while testing blk_mq over the new cxlflash driver. [ 2960.817172] Oops: Kernel access of bad area, sig: 11 [#5] [ 2960.817309] NIP __blk_mq_run_hw_queue+0x278/0x4c0 [ 2960.817313] LR __blk_mq_run_hw_queue+0x2bc/0x4c0 [ 2960.817314] Call Trace: [ 2960.817320]

Re: [PATCH v2 03/27] hpsa: remove unused hpsa_tag_discard_error_bits

Don: Thanks for not redefining HPSA_PERF_ERROR_BITS. Reviewed-by: Manoj Kumar <ma...@linux.vnet.ibm.com> --- Manoj Kumar On 11/4/2015 3:50 PM, Don Brace wrote: This function is no longer used. Reviewed-by: Tomas Henzl <the...@redhat.com> Reviewed-by: Hannes Reinecke <h...@

Re: [PATCH v2 2/5] ipr: Don't set NO_ULEN_CHK bit when resource is a vset.

lve this in a future update. Reviewed-by: Manoj Kumar <ma...@linux.vnet.ibm.com> --- Manoj Kumar On 11/3/2015 12:26 PM, Gabriel Krisman Bertazi wrote: According to the IPR specification, Inhibit Underlength Checking bit must be disabled when issuing commands to vsets. Enabling it in this case might c

Re: [PATCH v4.3-rc7] be2iscsi : Fix bogus WARN_ON length check

Tim: Reviewed-by: Manoj Kumar <ma...@linux.vnet.ibm.com> --- Manoj Kumar On 10/30/2015 1:22 PM, tim.gard...@canonical.com wrote: From: Tim Gardner <tim.gard...@canonical.com> drivers/scsi/be2iscsi/be_main.c: In function 'be_sgl_create_contiguous': drivers/scsi/be2iscsi/be_ma

Re: [PATCH 2/5] ipr: Clear NO_ULEN_CHK bit when resource is a vset.

e same section of code. --- Manoj Kumar -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1 02/25] hpsa: remove unused hpsa_tag_discard_error_bits

Don: See comment below. - Manoj Kumar On 10/28/2015 5:04 PM, Don Brace wrote: This function is no longer used. diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c +#define HPSA_PERF_ERROR_BITS ((1 << DIRECT_LOOKUP_SHIFT) - 1) HPSA_PERF_ERROR_BITS seems to be only used in the fu

Re: [PATCH 1 03/25] hpsa: check for null arguments to dev_printk

On 10/28/2015 5:04 PM, Don Brace wrote: Check for NULLs. - int devtype; + unsigned int devtype; Don: Unrelated to the NULL argument check. Would have been preferable in a distinct patch. Reviewed-by: Manoj Kumar <ma...@linux.vnet.ibm.com> --- Manoj Kumar -- To unsub

[PATCH] scsi: Set sg_tablesize to 1, for LLDDs that set SG_NONE

Oops while testing blk_mq over the new cxlflash driver. [ 2960.817172] Oops: Kernel access of bad area, sig: 11 [#5] [ 2960.817309] NIP __blk_mq_run_hw_queue+0x278/0x4c0 [ 2960.817313] LR __blk_mq_run_hw_queue+0x2bc/0x4c0 [ 2960.817314] Call Trace: [ 2960.817320]

Re: [PATCH v6 35/37] cxlflash: Fix to avoid corrupting port selection mask

Acked-by: Manoj Kumar <ma...@linux.vnet.ibm.com> On 10/21/2015 3:16 PM, Matthew R. Ochs wrote: The port selection mask of a LUN can be corrupted when the manage LUN ioctl (DK_CXLFLASH_MANAGE_LUN) is issued more than once for any device. -- To unsubscribe from this list: send th

Re: [PATCH v6 37/37] cxlflash: Fix to avoid bypassing context cleanup

Acked-by: Manoj Kumar <ma...@linux.vnet.ibm.com> On 10/21/2015 3:16 PM, Matthew R. Ochs wrote: Contexts may be skipped over for cleanup in situations where contention for the adapter's table-list mutex is experienced in the presence of a signal during the execution of the release h

Re: [PATCH v6 36/37] cxlflash: Fix to avoid lock instrumentation rejection

Acked-by: Manoj Kumar <ma...@linux.vnet.ibm.com> On 10/21/2015 3:16 PM, Matthew R. Ochs wrote: When running with lock instrumentation (e.g. lockdep), some of the instrumentation can become disabled at probe time for a cxlflash adapter. This is due to a missing lock regist

Re: [PATCH 3/3] cxlflash: drop unlikely before IS_ERR_OR_NULL

Geliang: Thanks for catching this. - Manoj Acked-by: Manoj Kumar <ma...@linux.vnet.ibm.com> On 9/30/2015 9:55 PM, Geliang Tang wrote: IS_ERR_OR_NULL already contain an unlikely compiler flag. Drop it. Signed-off-by: Geliang Tang <geliangt...@163.com> --- drivers/scsi/cxlflash

Re: [PATCH v5 33/34] cxlflash: Fix to avoid leaving dangling interrupt resources

Acked-by: Manoj Kumar <ma...@linux.vnet.ibm.com> -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html

Re: [patch] cxlflash: a couple off by one bugs

Reviewed-by: Manoj Kumar <ma...@linux.vnet.ibm.com> --- Manoj Kumar On 9/22/2015 7:32 AM, Dan Carpenter wrote: The "> MAX_CONTEXT" should be ">= MAX_CONTEXT". Otherwise we go one step beyond the end of the cfg->ctx_tbl[] array. Signed-off-by: Dan Carpent

Re: [PATCH v6 1/3] cxlflash: Base error recovery support

James: Wondering whether there is anything else you were expecting from us before pulling this patch series in. Regards, - Manoj Kumar On 8/13/2015 9:47 PM, Matthew R. Ochs wrote: Introduce support for enhanced I/O error handling. A device state is added to track 3 possible states

Re: [PATCH v5 2/3] cxlflash: Superpipe support

Mikey: Good catch. Will resolve this in the v6 patch. Thanks, - Manoj On 8/13/2015 5:53 AM, Michael Neuling wrote: + + ctxi = kzalloc(sizeof(*ctxi), GFP_KERNEL); + lli = kzalloc((MAX_RHT_PER_CONTEXT * sizeof(*lli)), GFP_KERNEL); + if (unlikely(!ctxi || !lli)) { +

Re: [PATCH v5 3/3] cxlflash: Virtual LUN support

Mikey: Thanks for pointing this out. The patch for 2/3 should address this issue. Regards, - Manoj Kumar On 8/13/2015 8:08 PM, Michael Neuling wrote: On Thu, 2015-08-13 at 18:43 -0500, Manoj Kumar wrote: Mikey: Thanks for your review. See comment inline below. - Manoj Kumar On 8/13/2015

Re: [PATCH v4 3/3] cxlflash: Virtual LUN support

Mikey: Thanks for your review. Comments inline below. On 8/11/2015 5:54 AM, Michael Neuling wrote: I'm not keen on the numerous pr_err() in here. I think it'll make the driver chatty especially with a badly behaving userspace. Will look at all the pr_err() and limit them to errors that are

Re: [PATCH v4 2/3] cxlflash: Superpipe support

Ben: Comments inline below. On 8/11/2015 12:29 AM, Benjamin Herrenschmidt wrote: So in a similar vein to the previous review, I am missing a lot of context here but a few things did spring to me eyes: Thanks for your review. + list_for_each_entry_safe(lli, temp, cfg-lluns, list) +

Re: [PATCH v2 3/3] cxlflash: Virtual LUN support

Wendy: Thanks for taking the time to review this patch. Comments inline below. - Manoj Kumar On 7/29/2015 5:13 PM, wenxi...@linux.vnet.ibm.com wrote: +/* Update the free_curr_idx */ +if (bit_pos == 63) +lun_info-free_curr_idx = bit_word + 1; Predefined Macros for 63 and 64

Re: [PATCH v2 1/3] cxlflash: Base error recovery support

Wendy: Thanks for your review. Comment inline below. - Manoj Kumar On 7/29/2015 5:12 PM, wenxi...@linux.vnet.ibm.com wrote: + +cfg-eeh_active = EEH_STATE_NONE; +wake_up_all(cfg-eeh_waitq); +} + Do you need host-lock in these EEH callback functions? These are synchronous callbacks

Re: [PATCH v5] cxlflash: Base support for IBM CXL Flash Adapter

On 6/9/2015 6:29 AM, Brian King wrote: Pulled out going to sleep in the queuecommand path. udelay doesn't sleep, its a busy wait, so you can still use it in queuecommand, just don't spend too much time, and its probably better to udelay then to just re-read in a tight loop. Thanks for the

Re: [PATCH v4] cxlflash: Base support for IBM CXL Flash Adapter

Brian: Thank you for your review. Comments are inline. - Manoj On 6/8/2015 12:54 PM, Brian King wrote: Looking pretty good. A few more comments. Thanks, Brian + spin_lock_irqsave(cfg-tmf_waitq.lock, lock_flags); + if (cfg-tmf_active) +

Re: [PATCH v4] cxlflash: Base support for IBM CXL Flash Adapter

On 6/8/2015 12:54 PM, Brian King wrote: + + rcr = send_tmf(afu, scp, TMF_LUN_RESET); + if (unlikely(rcr)) + rc = FAILED; Do you need to wait for all commands to the LUN to be returned before returning from here? You could put a simple loop here, polling until there

Re: [PATCH v5] cxlflash: Base support for IBM CXL Flash Adapter

Brian: Thanks for the quick review. Comments below. - Manoj On 6/8/2015 5:56 PM, Brian King wrote: +retry: + newval = atomic64_dec_if_positive(afu-room); + if (!newval) { + do { + room = readq_be(afu-host_map-cmd_room); +

Re: [PATCH v3] cxlflash: Base support for IBM CXL Flash Adapter

Brian: Thanks for your review. Responses are inline below. - Manoj Kumar On 6/4/2015 9:38 AM, Brian King wrote: + + write_lock(cfg-tmf_lock); What is this lock protecting? The only thing it seems to be accomplishing is making sure one thread isn't sending a TMF and another thread