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

2015-08-13 Thread Michael Neuling
...@linux.vnet.ibm.com Signed-off-by: Manoj N. Kumar ma...@linux.vnet.ibm.com Thanks for incorporating my suggestions. One minor issue below but if you fix that and add the ioctl version advertising as we talked about offline, I'm happy if you add my reviewed by. Reviewed-by: Michael Neuling mi

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

2015-08-13 Thread Michael Neuling
...@linux.vnet.ibm.com Reviewed-by: Daniel Axtens d...@axtens.net Thanks for integrating my suggestions. Reviewed-by: Michael Neuling mi...@neuling.org --- drivers/scsi/cxlflash/Kconfig | 2 +- drivers/scsi/cxlflash/common.h | 12 ++- drivers/scsi/cxlflash/main.c | 174

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

2015-08-13 Thread Michael Neuling
reviewed by: Reviewed-by: Michael Neuling mi...@neuling.org --- Documentation/powerpc/cxlflash.txt | 63 +- drivers/scsi/cxlflash/Makefile |2 +- drivers/scsi/cxlflash/common.h |4 + drivers/scsi/cxlflash/lunmgt.c |3 + drivers/scsi/cxlflash/main.c | 13

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

2015-08-13 Thread Michael Neuling
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 7:03 AM, Michael Neuling wrote: Thanks for integrating my suggestions. create_context() has the same freeing bug as 2/3 but if you fix that I'm

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

2015-08-11 Thread Michael Neuling
Comments inline. 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. On Mon, 2015-08-10 at 12:09 -0500, Matthew R. Ochs wrote: Add support for physical LUN segmentation (virtual LUNs) to device driver supporting the

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

2015-08-11 Thread Michael Neuling
+ pr_debug(%s: Wait for user context to quiesce...\n, __func__); + wake_up_all(cfg-limbo_waitq); + ssleep(1); Why 1 sec and why in a loop? Can’t you poll/wait for completion somewhere? This routine is called when the device is being removed and we need to

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

2015-08-11 Thread Michael Neuling
Be good to do some clear sanity check the struct dk_cxlflash_resize *resize here. It's passed from userspace but then gets propogated to a bunch of other things here like nsectors, get_context etc who will all now be responsible for handling any dodgy data passed in. Same

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

2015-08-10 Thread Michael Neuling
Some comments inline On Mon, 2015-08-10 at 12:09 -0500, Matthew R. Ochs wrote: Add superpipe supporting infrastructure to device driver for the IBM CXL Flash adapter. This patch allows userspace applications to take advantage of the accelerated I/O features that this adapter provides and

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

2015-08-10 Thread Michael Neuling
On Mon, 2015-08-10 at 12:09 -0500, Matthew R. Ochs wrote: Introduce support for enhanced I/O error handling. This needs more description of what you're doing. What's the overall approach? There seems to be a new limbo queue thats created stall things while the device is in error state. That

Re: [PATCH v4 0/3] CXL Flash Error Recovery and Superpipe

2015-08-10 Thread Michael Neuling
This patch set is intended for the 4.3 release and adds support for error recovery and the superpipe features provided by the IBM CXL Flash adapter. The superpipe function was originally presented in an RFC patch set in late April. To aid with the review of the superpipe portion of these

Re: [PATCH 1/2] cxlflash: Base superpipe support

2015-07-02 Thread Michael Neuling
On Fri, 2015-06-19 at 17:37 -0500, Matthew R. Ochs wrote: Add superpipe supporting infrastructure to device driver for the IBM CXL Flash adapter. This patch allows userspace applications to take advantage of the accelerated I/O features that this adapter provides and bypass the traditional

Re: [PATCH 1/2] cxlflash: Base superpipe support

2015-07-02 Thread Michael Neuling
+struct dk_cxlflash_attach { + struct dk_cxlflash_hdr hdr; /* Common fields */ + __u64 num_interrupts; /* Requested number of interrupts */ + __u64 context_id; /* Returned context */ Matt, These ioctls seem to take a context id. Why isn't that assumed

Re: [PATCH 1/2] cxlflash: Base superpipe support

2015-07-02 Thread Michael Neuling
On Thu, 2015-07-02 at 17:00 +1000, Michael Neuling wrote: +struct dk_cxlflash_attach { + struct dk_cxlflash_hdr hdr; /* Common fields */ + __u64 num_interrupts; /* Requested number of interrupts */ + __u64 context_id; /* Returned context */ Matt

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

2015-06-08 Thread Michael Neuling
On Fri, 2015-06-05 at 16:46 -0500, Matthew R. Ochs wrote: SCSI device driver to support filesystem access on the IBM CXL Flash adapter. Few minor nits below but other than that it looks good to me. The CXL parts are good and the rest of the driver is looking decent. FWIW: Reviewed-by: Michael

Re: [PATCH RFC 1/2] cxlflash: Base support for IBM CXL Flash Adapter

2015-05-27 Thread Michael Neuling
Christoph, On Sun, Apr 26, 2015 at 11:50:20PM -0500, Matthew R. Ochs wrote: SCSI device driver to support filesystem access on the IBM CXL Flash adapter. Please drop all the code related to your weird direct access ioctls from this patch. Without that it looks mostly mergeable. What's

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

2015-05-21 Thread Michael Neuling
+#define CXLFLASH_MAX_CMDS 16 +#define CXLFLASH_MAX_CMDS_PER_LUN CXLFLASH_MAX_CMDS + +#define NOT_POW2(_x) ((_x) ((_x) ((_x) - 1))) include/linux/log2 has is_power_of_2() This is used for compile-time enforcement. The items in the log2 header are runtime