Re: [for-4.17 20/20] bcache: Fix a compiler warning in bcache_device_init()

[Coly Li]

On 22/03/2018 2:38 AM, Michael Lyle wrote:
> On 03/18/2018 06:32 PM, Coly Li wrote:
>> On 19/03/2018 8:36 AM, Michael Lyle wrote:
>>> From: Bart Van Assche 
>>>
>>> Avoid that building with W=1 triggers the following compiler warning:
>>>
>>> drivers/md/bcache/super.c:776:20: warning: comparison is always false due 
>>> to limited range of data type [-Wtype-limits]
>>>   d->nr_stripes > SIZE_MAX / sizeof(atomic_t)) {
>>> ^
>>>
>>> Signed-off-by: Bart Van Assche 
>>> Reviewed-by: Michael Lyle 
>>
>> There is a missing Reviewed-by: Coly Li  from me :-)
> 
> Hi Coly--- sorry that I lost these.

The major motivation is just making my employer know where they pay for
my time :-)

Thanks.

Coly Li

Re: [PATCH 08/12] lightnvm: implement get log report chunk helpers

[Javier Gonzalez]

> On 21 Mar 2018, at 20.27, Matias Bjørling  wrote:
> 
>> On 03/21/2018 03:36 PM, Keith Busch wrote:
>> On Wed, Mar 21, 2018 at 03:06:05AM -0700, Matias Bjørling wrote:
 outside of nvme core so that we can use it form lightnvm.
 
 Signed-off-by: Javier González 
 ---
   drivers/lightnvm/core.c  | 11 +++
   drivers/nvme/host/core.c |  6 ++--
   drivers/nvme/host/lightnvm.c | 74 
 
   drivers/nvme/host/nvme.h |  3 ++
   include/linux/lightnvm.h | 24 ++
   5 files changed, 115 insertions(+), 3 deletions(-)
 
 diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
 index 2e9e9f973a75..af642ce6ba69 100644
 --- a/drivers/nvme/host/core.c
 +++ b/drivers/nvme/host/core.c
 @@ -2127,9 +2127,9 @@ static int nvme_init_subsystem(struct nvme_ctrl 
 *ctrl, struct nvme_id_ctrl *id)
   return ret;
   }
   -static int nvme_get_log_ext(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
 -u8 log_page, void *log,
 -size_t size, size_t offset)
 +int nvme_get_log_ext(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
 + u8 log_page, void *log,
 + size_t size, size_t offset)
   {
   struct nvme_command c = { };
   unsigned long dwlen = size / 4 - 1;
 diff --git a/drivers/nvme/host/lightnvm.c b/drivers/nvme/host/lightnvm.c
 index 08f0f6b5bc06..ffd64a83c8c3 100644
 --- a/drivers/nvme/host/lightnvm.c
 +++ b/drivers/nvme/host/lightnvm.c
 @@ -35,6 +35,10 @@ enum nvme_nvm_admin_opcode {
   nvme_nvm_admin_set_bb_tbl= 0xf1,
   };
   
>>> 
>>> 
>>> 
   diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
 index 1ca08f4993ba..505f797f8c6c 100644
 --- a/drivers/nvme/host/nvme.h
 +++ b/drivers/nvme/host/nvme.h
 @@ -396,6 +396,9 @@ int nvme_reset_ctrl(struct nvme_ctrl *ctrl);
   int nvme_delete_ctrl(struct nvme_ctrl *ctrl);
   int nvme_delete_ctrl_sync(struct nvme_ctrl *ctrl);
   +int nvme_get_log_ext(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
 + u8 log_page, void *log, size_t size, size_t offset);
 +
   extern const struct attribute_group nvme_ns_id_attr_group;
   extern const struct block_device_operations nvme_ns_head_ops;
   
>>> 
>>> 
>>> Keith, Christoph, Sagi, Is it okay that these two changes that exposes
>>> the nvme_get_log_ext fn are carried through Jens' tree after the nvme
>>> tree for 4.17 has been pulled?
>> That's okay with me. Alteratively, if you want to split the generic nvme
>> part out, I can apply that immediately and the API will be in the first
>> nvme-4.17 pull request.
> 
> Will do. I've sent the patch in another mail. Thanks! :)

It’s fine with me.

Matias: do you take that part of the patch out directly on our tree?

Javier. 

Re: [PATCH 08/12] lightnvm: implement get log report chunk helpers

[Matias Bjørling]

On 03/21/2018 03:36 PM, Keith Busch wrote:

On Wed, Mar 21, 2018 at 03:06:05AM -0700, Matias Bjørling wrote:

outside of nvme core so that we can use it form lightnvm.

Signed-off-by: Javier González 
---
   drivers/lightnvm/core.c  | 11 +++
   drivers/nvme/host/core.c |  6 ++--
   drivers/nvme/host/lightnvm.c | 74 

   drivers/nvme/host/nvme.h |  3 ++
   include/linux/lightnvm.h | 24 ++
   5 files changed, 115 insertions(+), 3 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 2e9e9f973a75..af642ce6ba69 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -2127,9 +2127,9 @@ static int nvme_init_subsystem(struct nvme_ctrl *ctrl, 
struct nvme_id_ctrl *id)
return ret;
   }
   
-static int nvme_get_log_ext(struct nvme_ctrl *ctrl, struct nvme_ns *ns,

-   u8 log_page, void *log,
-   size_t size, size_t offset)
+int nvme_get_log_ext(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
+u8 log_page, void *log,
+size_t size, size_t offset)
   {
struct nvme_command c = { };
unsigned long dwlen = size / 4 - 1;
diff --git a/drivers/nvme/host/lightnvm.c b/drivers/nvme/host/lightnvm.c
index 08f0f6b5bc06..ffd64a83c8c3 100644
--- a/drivers/nvme/host/lightnvm.c
+++ b/drivers/nvme/host/lightnvm.c
@@ -35,6 +35,10 @@ enum nvme_nvm_admin_opcode {
nvme_nvm_admin_set_bb_tbl   = 0xf1,
   };
   




   
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h

index 1ca08f4993ba..505f797f8c6c 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -396,6 +396,9 @@ int nvme_reset_ctrl(struct nvme_ctrl *ctrl);
   int nvme_delete_ctrl(struct nvme_ctrl *ctrl);
   int nvme_delete_ctrl_sync(struct nvme_ctrl *ctrl);
   
+int nvme_get_log_ext(struct nvme_ctrl *ctrl, struct nvme_ns *ns,

+u8 log_page, void *log, size_t size, size_t offset);
+
   extern const struct attribute_group nvme_ns_id_attr_group;
   extern const struct block_device_operations nvme_ns_head_ops;
   



Keith, Christoph, Sagi, Is it okay that these two changes that exposes
the nvme_get_log_ext fn are carried through Jens' tree after the nvme
tree for 4.17 has been pulled?


That's okay with me. Alteratively, if you want to split the generic nvme
part out, I can apply that immediately and the API will be in the first
nvme-4.17 pull request.



Will do. I've sent the patch in another mail. Thanks! :)

Re: [for-4.17 20/20] bcache: Fix a compiler warning in bcache_device_init()

[Michael Lyle]

On 03/18/2018 06:32 PM, Coly Li wrote:
> On 19/03/2018 8:36 AM, Michael Lyle wrote:
>> From: Bart Van Assche 
>>
>> Avoid that building with W=1 triggers the following compiler warning:
>>
>> drivers/md/bcache/super.c:776:20: warning: comparison is always false due to 
>> limited range of data type [-Wtype-limits]
>>   d->nr_stripes > SIZE_MAX / sizeof(atomic_t)) {
>> ^
>>
>> Signed-off-by: Bart Van Assche 
>> Reviewed-by: Michael Lyle 
> 
> There is a missing Reviewed-by: Coly Li  from me :-)

Hi Coly--- sorry that I lost these.

Mike

Re: [PATCH] block: use 32-bit blk_status_t on Alpha

[Jens Axboe]

On 3/21/18 11:00 AM, Mikulas Patocka wrote:
> 
> 
> On Wed, 21 Mar 2018, Jens Axboe wrote:
> 
>> On 3/21/18 10:42 AM, Mikulas Patocka wrote:
>>> Early alpha processors cannot write a single byte or word; they read 8
>>> bytes, modify the value in registers and write back 8 bytes.
>>>
>>> The type blk_status_t is defined as one byte, it is often written
>>> asynchronously by I/O completion routines, this asynchronous modification
>>> can corrupt content of nearby bytes if these nearby bytes can be written
>>> simultaneously by another CPU.
>>>
>>> - one example of such corruption is the structure dm_io where
>>>   "blk_status_t status" is written by an asynchronous completion routine
>>>   and "atomic_t io_count" is modified synchronously
>>> - another example is the structure dm_buffer where "unsigned hold_count"
>>>   is modified synchronously from process context and "blk_status_t
>>>   write_error" is modified asynchronously from bio completion routine
>>>
>>> This patch fixes the bug by changing the type blk_status_t to 32 bits if
>>> we are on Alpha and if we are compiling for a processor that doesn't have
>>> the byte-word-extension.
>>
>> That's nasty. Is alpha the only problematic arch here?
> 
> Yes. All the other architectures supported by Linux have byte writes.
> 
>> As to the patch in question, normally I'd just say we should make it
>> unconditionally u32. But we pack so nicely in the bio, and I don't think
>> the bio itself has this issue as the rest of the members that share this
>> word are all set before the bio is submitted. But callers embedding
>> the status var in other structures don't necessarily have that
>> guarantee, as your dm examples show.
>>
>> -- 
>> Jens Axboe
> 
> Keeping blk_status_t 8-bit for most architectures will save a few bytes in 
> some of device mapper structures.

And more importantly, it won't screw up the bio layout, I'm somewhat more
concerned about that than random driver structures.

If alpha is the odd one out here, then I think your patch is fine as-is.

-- 
Jens Axboe

Re: [PATCH] block: use 32-bit blk_status_t on Alpha

[Mikulas Patocka]

On Wed, 21 Mar 2018, Jens Axboe wrote:

> On 3/21/18 10:42 AM, Mikulas Patocka wrote:
> > Early alpha processors cannot write a single byte or word; they read 8
> > bytes, modify the value in registers and write back 8 bytes.
> > 
> > The type blk_status_t is defined as one byte, it is often written
> > asynchronously by I/O completion routines, this asynchronous modification
> > can corrupt content of nearby bytes if these nearby bytes can be written
> > simultaneously by another CPU.
> > 
> > - one example of such corruption is the structure dm_io where
> >   "blk_status_t status" is written by an asynchronous completion routine
> >   and "atomic_t io_count" is modified synchronously
> > - another example is the structure dm_buffer where "unsigned hold_count"
> >   is modified synchronously from process context and "blk_status_t
> >   write_error" is modified asynchronously from bio completion routine
> > 
> > This patch fixes the bug by changing the type blk_status_t to 32 bits if
> > we are on Alpha and if we are compiling for a processor that doesn't have
> > the byte-word-extension.
> 
> That's nasty. Is alpha the only problematic arch here?

Yes. All the other architectures supported by Linux have byte writes.

> As to the patch in question, normally I'd just say we should make it
> unconditionally u32. But we pack so nicely in the bio, and I don't think
> the bio itself has this issue as the rest of the members that share this
> word are all set before the bio is submitted. But callers embedding
> the status var in other structures don't necessarily have that
> guarantee, as your dm examples show.
> 
> -- 
> Jens Axboe

Keeping blk_status_t 8-bit for most architectures will save a few bytes in 
some of device mapper structures.

Mikulas

Re: [PATCH v3 11/11] nvmet: Optionally use PCI P2P memory

[Logan Gunthorpe]

On 21/03/18 03:27 AM, Christoph Hellwig wrote:
>> +  const char *page, size_t count)
>> +{
>> +struct nvmet_port *port = to_nvmet_port(item);
>> +struct device *dev;
>> +struct pci_dev *p2p_dev = NULL;
>> +bool use_p2pmem;
>> +
>> +switch (page[0]) {
>> +case 'y':
>> +case 'Y':
>> +case 'a':
>> +case 'A':
>> +use_p2pmem = true;
>> +break;
>> +case 'n':
>> +case 'N':
>> +use_p2pmem = false;
>> +break;
>> +default:
>> +dev = bus_find_device_by_name(_bus_type, NULL, page);
>> +if (!dev) {
>> +pr_err("No such PCI device: %s\n", page);
>> +return -ENODEV;
>> +}
>> +
>> +use_p2pmem = true;
>> +p2p_dev = to_pci_dev(dev);
>> +
>> +if (!pci_has_p2pmem(p2p_dev)) {
>> +pr_err("PCI device has no peer-to-peer memory: %s\n",
>> +   page);
>> +pci_dev_put(p2p_dev);
>> +return -ENODEV;
>> +}
>> +}
> 
> Yikes.  Shouldn't auto just be the normal yes case instead of this
> string parsing mess?

Sorry, I don't follow. The code, as is, should automatically select the
device if the user  sets it to "yes" or "auto" or "y" or similar.
(Roughly similar to how kstrtobool() works, except '0' or '1' are not
accepted seeing they could overlap with PCI device names). In other
cases, it looks for the specific PCI device name to use exactly.

Are you saying it shouldn't work this way or are you saying the code to
implement it is too messy?

>> +if (rsp->req.sg != >cmd->inline_sg) {
>> +if (rsp->req.p2p_dev)
>> +pci_p2pmem_free_sgl(rsp->req.p2p_dev, rsp->req.sg,
>> +rsp->req.sg_cnt);
>> +else
>> +sgl_free(rsp->req.sg);
>> +}
> 
> Can we factor this into a helper, as the other target drivers (fc for now,
> tcp soon) using sgl allocatins should share the code?
> 
> (same for the alloc side)

Sure. Would the helpers belong in core.c?

Thanks,

Logan

Re: [PATCH] block: use 32-bit blk_status_t on Alpha

[Jens Axboe]

On 3/21/18 10:42 AM, Mikulas Patocka wrote:
> Early alpha processors cannot write a single byte or word; they read 8
> bytes, modify the value in registers and write back 8 bytes.
> 
> The type blk_status_t is defined as one byte, it is often written
> asynchronously by I/O completion routines, this asynchronous modification
> can corrupt content of nearby bytes if these nearby bytes can be written
> simultaneously by another CPU.
> 
> - one example of such corruption is the structure dm_io where
>   "blk_status_t status" is written by an asynchronous completion routine
>   and "atomic_t io_count" is modified synchronously
> - another example is the structure dm_buffer where "unsigned hold_count"
>   is modified synchronously from process context and "blk_status_t
>   write_error" is modified asynchronously from bio completion routine
> 
> This patch fixes the bug by changing the type blk_status_t to 32 bits if
> we are on Alpha and if we are compiling for a processor that doesn't have
> the byte-word-extension.

That's nasty. Is alpha the only problematic arch here?

As to the patch in question, normally I'd just say we should make it
unconditionally u32. But we pack so nicely in the bio, and I don't think
the bio itself has this issue as the rest of the members that share this
word are all set before the bio is submitted. But callers embedding
the status var in other structures don't necessarily have that
guarantee, as your dm examples show.

-- 
Jens Axboe

[PATCH] Fix slab name "biovec-(1<<(21-12))"

[Mikulas Patocka]

I'm getting a slab named "biovec-(1<<(21-12))". It is caused by unintended
expansion of the macro BIO_MAX_PAGES. This patch renames it to biovec-max.

Signed-off-by: Mikulas Patocka 
Cc: sta...@vger.kernel.org  # v4.14+

---
 block/bio.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Index: linux-2.6/block/bio.c
===
--- linux-2.6.orig/block/bio.c  2018-02-14 20:23:35.648255000 +0100
+++ linux-2.6/block/bio.c   2018-03-14 14:51:35.53000 +0100
@@ -43,9 +43,9 @@
  * break badly! cannot be bigger than what you can fit into an
  * unsigned short
  */
-#define BV(x) { .nr_vecs = x, .name = "biovec-"__stringify(x) }
+#define BV(x, n) { .nr_vecs = x, .name = "biovec-"#n }
 static struct biovec_slab bvec_slabs[BVEC_POOL_NR] __read_mostly = {
-   BV(1), BV(4), BV(16), BV(64), BV(128), BV(BIO_MAX_PAGES),
+   BV(1, 1), BV(4, 4), BV(16, 16), BV(64, 64), BV(128, 128), 
BV(BIO_MAX_PAGES, max),
 };
 #undef BV
 

[PATCH] block: use 32-bit blk_status_t on Alpha

[Mikulas Patocka]

Early alpha processors cannot write a single byte or word; they read 8
bytes, modify the value in registers and write back 8 bytes.

The type blk_status_t is defined as one byte, it is often written
asynchronously by I/O completion routines, this asynchronous modification
can corrupt content of nearby bytes if these nearby bytes can be written
simultaneously by another CPU.

- one example of such corruption is the structure dm_io where
  "blk_status_t status" is written by an asynchronous completion routine
  and "atomic_t io_count" is modified synchronously
- another example is the structure dm_buffer where "unsigned hold_count"
  is modified synchronously from process context and "blk_status_t
  write_error" is modified asynchronously from bio completion routine

This patch fixes the bug by changing the type blk_status_t to 32 bits if
we are on Alpha and if we are compiling for a processor that doesn't have
the byte-word-extension.

Signed-off-by: Mikulas Patocka 
Cc: sta...@vger.kernel.org  # 4.13+

---
 include/linux/blk_types.h |5 +
 1 file changed, 5 insertions(+)

Index: linux-2.6/include/linux/blk_types.h
===
--- linux-2.6.orig/include/linux/blk_types.h2018-02-14 20:24:42.038255000 
+0100
+++ linux-2.6/include/linux/blk_types.h 2018-03-21 15:04:54.96000 +0100
@@ -20,8 +20,13 @@ typedef void (bio_end_io_t) (struct bio
 
 /*
  * Block error status values.  See block/blk-core:blk_errors for the details.
+ * Alpha cannot write a byte atomically, so we need to use 32-bit value.
  */
+#if defined(CONFIG_ALPHA) && !defined(__alpha_bwx__)
+typedef u32 __bitwise blk_status_t;
+#else
 typedef u8 __bitwise blk_status_t;
+#endif
 #defineBLK_STS_OK 0
 #define BLK_STS_NOTSUPP((__force blk_status_t)1)
 #define BLK_STS_TIMEOUT((__force blk_status_t)2)

Re: [PATCH 08/12] lightnvm: implement get log report chunk helpers

[Keith Busch]

On Wed, Mar 21, 2018 at 03:06:05AM -0700, Matias Bjørling wrote:
> > outside of nvme core so that we can use it form lightnvm.
> > 
> > Signed-off-by: Javier González 
> > ---
> >   drivers/lightnvm/core.c  | 11 +++
> >   drivers/nvme/host/core.c |  6 ++--
> >   drivers/nvme/host/lightnvm.c | 74 
> > 
> >   drivers/nvme/host/nvme.h |  3 ++
> >   include/linux/lightnvm.h | 24 ++
> >   5 files changed, 115 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> > index 2e9e9f973a75..af642ce6ba69 100644
> > --- a/drivers/nvme/host/core.c
> > +++ b/drivers/nvme/host/core.c
> > @@ -2127,9 +2127,9 @@ static int nvme_init_subsystem(struct nvme_ctrl 
> > *ctrl, struct nvme_id_ctrl *id)
> > return ret;
> >   }
> >   
> > -static int nvme_get_log_ext(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
> > -   u8 log_page, void *log,
> > -   size_t size, size_t offset)
> > +int nvme_get_log_ext(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
> > +u8 log_page, void *log,
> > +size_t size, size_t offset)
> >   {
> > struct nvme_command c = { };
> > unsigned long dwlen = size / 4 - 1;
> > diff --git a/drivers/nvme/host/lightnvm.c b/drivers/nvme/host/lightnvm.c
> > index 08f0f6b5bc06..ffd64a83c8c3 100644
> > --- a/drivers/nvme/host/lightnvm.c
> > +++ b/drivers/nvme/host/lightnvm.c
> > @@ -35,6 +35,10 @@ enum nvme_nvm_admin_opcode {
> > nvme_nvm_admin_set_bb_tbl   = 0xf1,
> >   };
> >   
> 
> 
> 
> >   
> > diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
> > index 1ca08f4993ba..505f797f8c6c 100644
> > --- a/drivers/nvme/host/nvme.h
> > +++ b/drivers/nvme/host/nvme.h
> > @@ -396,6 +396,9 @@ int nvme_reset_ctrl(struct nvme_ctrl *ctrl);
> >   int nvme_delete_ctrl(struct nvme_ctrl *ctrl);
> >   int nvme_delete_ctrl_sync(struct nvme_ctrl *ctrl);
> >   
> > +int nvme_get_log_ext(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
> > +u8 log_page, void *log, size_t size, size_t offset);
> > +
> >   extern const struct attribute_group nvme_ns_id_attr_group;
> >   extern const struct block_device_operations nvme_ns_head_ops;
> >   
> 
> 
> Keith, Christoph, Sagi, Is it okay that these two changes that exposes 
> the nvme_get_log_ext fn are carried through Jens' tree after the nvme 
> tree for 4.17 has been pulled?

That's okay with me. Alteratively, if you want to split the generic nvme
part out, I can apply that immediately and the API will be in the first
nvme-4.17 pull request.

Re: [PATCH 08/12] lightnvm: implement get log report chunk helpers

[Matias Bjørling]

On 03/02/2018 04:21 PM, Javier González wrote:

The 2.0 spec provides a report chunk log page that can be retrieved
using the stangard nvme get log page. This replaces the dedicated
get/put bad block table in 1.2.

This patch implements the helper functions to allow targets retrieve the
chunk metadata using get log page. It makes nvme_get_log_ext available
outside of nvme core so that we can use it form lightnvm.

Signed-off-by: Javier González 
---
  drivers/lightnvm/core.c  | 11 +++
  drivers/nvme/host/core.c |  6 ++--
  drivers/nvme/host/lightnvm.c | 74 
  drivers/nvme/host/nvme.h |  3 ++
  include/linux/lightnvm.h | 24 ++
  5 files changed, 115 insertions(+), 3 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 2e9e9f973a75..af642ce6ba69 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -2127,9 +2127,9 @@ static int nvme_init_subsystem(struct nvme_ctrl *ctrl, 
struct nvme_id_ctrl *id)
return ret;
  }
  
-static int nvme_get_log_ext(struct nvme_ctrl *ctrl, struct nvme_ns *ns,

-   u8 log_page, void *log,
-   size_t size, size_t offset)
+int nvme_get_log_ext(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
+u8 log_page, void *log,
+size_t size, size_t offset)
  {
struct nvme_command c = { };
unsigned long dwlen = size / 4 - 1;
diff --git a/drivers/nvme/host/lightnvm.c b/drivers/nvme/host/lightnvm.c
index 08f0f6b5bc06..ffd64a83c8c3 100644
--- a/drivers/nvme/host/lightnvm.c
+++ b/drivers/nvme/host/lightnvm.c
@@ -35,6 +35,10 @@ enum nvme_nvm_admin_opcode {
nvme_nvm_admin_set_bb_tbl   = 0xf1,
  };
  




  
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h

index 1ca08f4993ba..505f797f8c6c 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -396,6 +396,9 @@ int nvme_reset_ctrl(struct nvme_ctrl *ctrl);
  int nvme_delete_ctrl(struct nvme_ctrl *ctrl);
  int nvme_delete_ctrl_sync(struct nvme_ctrl *ctrl);
  
+int nvme_get_log_ext(struct nvme_ctrl *ctrl, struct nvme_ns *ns,

+u8 log_page, void *log, size_t size, size_t offset);
+
  extern const struct attribute_group nvme_ns_id_attr_group;
  extern const struct block_device_operations nvme_ns_head_ops;
  



Keith, Christoph, Sagi, Is it okay that these two changes that exposes 
the nvme_get_log_ext fn are carried through Jens' tree after the nvme 
tree for 4.17 has been pulled?

Re: [PATCH v3 06/11] block: Introduce PCI P2P flags for request and request queue

[Christoph Hellwig]

Looks fine,

Reviewed-by: Christoph Hellwig 

Re: [PATCH v3 07/11] IB/core: Ensure we map P2P memory correctly in rdma_rw_ctx_[init|destroy]()

[Christoph Hellwig]

Looks fine,

Reviewed-by: Christoph Hellwig 

Re: [PATCH v3 11/11] nvmet: Optionally use PCI P2P memory

[Christoph Hellwig]

> +   const char *page, size_t count)
> +{
> + struct nvmet_port *port = to_nvmet_port(item);
> + struct device *dev;
> + struct pci_dev *p2p_dev = NULL;
> + bool use_p2pmem;
> +
> + switch (page[0]) {
> + case 'y':
> + case 'Y':
> + case 'a':
> + case 'A':
> + use_p2pmem = true;
> + break;
> + case 'n':
> + case 'N':
> + use_p2pmem = false;
> + break;
> + default:
> + dev = bus_find_device_by_name(_bus_type, NULL, page);
> + if (!dev) {
> + pr_err("No such PCI device: %s\n", page);
> + return -ENODEV;
> + }
> +
> + use_p2pmem = true;
> + p2p_dev = to_pci_dev(dev);
> +
> + if (!pci_has_p2pmem(p2p_dev)) {
> + pr_err("PCI device has no peer-to-peer memory: %s\n",
> +page);
> + pci_dev_put(p2p_dev);
> + return -ENODEV;
> + }
> + }

Yikes.  Shouldn't auto just be the normal yes case instead of this
string parsing mess?

> + if (rsp->req.sg != >cmd->inline_sg) {
> + if (rsp->req.p2p_dev)
> + pci_p2pmem_free_sgl(rsp->req.p2p_dev, rsp->req.sg,
> + rsp->req.sg_cnt);
> + else
> + sgl_free(rsp->req.sg);
> + }

Can we factor this into a helper, as the other target drivers (fc for now,
tcp soon) using sgl allocatins should share the code?

(same for the alloc side)

Re: [PATCH v3 09/11] nvme-pci: Add support for P2P memory in requests

[Christoph Hellwig]

Looks good,

Reviewed-by: Christoph Hellwig