[PATCH v3 3/3] nvmet-rdma: support 16K inline data

[swise@opengridcomputing.com (Steve Wise)]

> -----Original Message-----
> From: Max Gurtovoy <maxg at mellanox.com>
> Sent: Monday, June 4, 2018 8:58 AM
> To: Steve Wise <swise at opengridcomputing.com>; axboe at kernel.dk;
> hch at lst.de; keith.busch at intel.com; sagi at grimberg.me; linux-
> nvme at lists.infradead.org
> Cc: parav at mellanox.com; linux-rdma at vger.kernel.org
> Subject: Re: [PATCH v3 3/3] nvmet-rdma: support 16K inline data
> 
> 
> 
> On 6/3/2018 9:25 PM, Steve Wise wrote:
> >
> >
> >> -----Original Message-----
> >> From: Max Gurtovoy <maxg at mellanox.com>
> >> Sent: Sunday, June 3, 2018 3:40 AM
> >> To: Steve Wise <swise at opengridcomputing.com>; axboe at kernel.dk;
> >> hch at lst.de; keith.busch at intel.com; sagi at grimberg.me; linux-
> >> nvme at lists.infradead.org
> >> Cc: parav at mellanox.com; linux-rdma at vger.kernel.org
> >> Subject: Re: [PATCH v3 3/3] nvmet-rdma: support 16K inline data
> >>
> >>
> >>
> >> On 5/29/2018 9:25 PM, Steve Wise wrote:
> >>> Add a new configfs port attribute, called inline_data_size,
> >>> to allow configuring the size of inline data for a given port.
> >>> The maximum size allowed is still enforced by nvmet-rdma with
> >>> NVMET_RDMA_MAX_INLINE_DATA_SIZE, which is increased to
> max(16KB,
> >>> PAGE_SIZE).  And the default size, if not specified via configfs,
> >>> is still PAGE_SIZE.  This preserves the existing behavior, but allows
> >>> larger inline sizes.
> >>>
> >>> Also support a configuration where inline_data_size is 0, which
disables
> >>> using inline data.
> >>>
> >>> Signed-off-by: Steve Wise <swise at opengridcomputing.com>
> >>> ---
> >>>    drivers/nvme/target/admin-cmd.c |  4 ++--
> >>>    drivers/nvme/target/configfs.c  | 31 ++++++++++++++++++++++++++++
> >>>    drivers/nvme/target/core.c      |  4 ++++
> >>>    drivers/nvme/target/discovery.c |  2 +-
> >>>    drivers/nvme/target/nvmet.h     |  2 +-
> >>>    drivers/nvme/target/rdma.c      | 45 ++++++++++++++++++++++++++++-
> ----
> >> --------
> >>>    6 files changed, 70 insertions(+), 18 deletions(-)
> >>
> >> snip..
> >>
> >>>
> >>> diff --git a/drivers/nvme/target/rdma.c b/drivers/nvme/target/rdma.c
> >>> index 52e0c5d..2f0b08e 100644
> >>> --- a/drivers/nvme/target/rdma.c
> >>> +++ b/drivers/nvme/target/rdma.c
> >>> @@ -33,9 +33,10 @@
> >>>    #include "nvmet.h"
> >>>
> >>>    /*
> >>> - * We allow up to a page of inline data to go with the SQE
> >>> + * We allow at least 1 page, and up to 16KB of inline data to go with
> > the
> >> SQE
> >>>     */
> >>> -#define NVMET_RDMA_INLINE_DATA_SIZE	PAGE_SIZE
> >>> +#define NVMET_RDMA_DEFAULT_INLINE_DATA_SIZE	PAGE_SIZE
> >>> +#define NVMET_RDMA_MAX_INLINE_DATA_SIZE		max_t(int,
> >> SZ_16K, PAGE_SIZE)
> >>
> >> why not use SZ_16K ? why we need to mention the PAGE_SIZE ?
> >>
> >
> > The idea is to allow at least 1 page.  So for, say, a 64K page system,
we'll
> > allow 64K since we're allocating a page minimum for the buffer.
> 
> IMO you want to support upto 16K inline data and not upto 64k (also in
> PowerPC system)...

Why?