On 6/5/2018 3:52 AM, Max Gurtovoy wrote: > > > On 6/4/2018 5:18 PM, Steve Wise wrote: >> >> >>> -----Original Message----- >>> From: Max Gurtovoy <maxg@xxxxxxxxxxxx> >>> Sent: Monday, June 4, 2018 8:58 AM >>> To: Steve Wise <swise@xxxxxxxxxxxxxxxxxxxxx>; axboe@xxxxxxxxx; >>> hch@xxxxxx; keith.busch@xxxxxxxxx; sagi@xxxxxxxxxxx; linux- >>> nvme@xxxxxxxxxxxxxxxxxxx >>> Cc: parav@xxxxxxxxxxxx; linux-rdma@xxxxxxxxxxxxxxx >>> 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@xxxxxxxxxxxx> >>>>> Sent: Sunday, June 3, 2018 3:40 AM >>>>> To: Steve Wise <swise@xxxxxxxxxxxxxxxxxxxxx>; axboe@xxxxxxxxx; >>>>> hch@xxxxxx; keith.busch@xxxxxxxxx; sagi@xxxxxxxxxxx; linux- >>>>> nvme@xxxxxxxxxxxxxxxxxxx >>>>> Cc: parav@xxxxxxxxxxxx; linux-rdma@xxxxxxxxxxxxxxx >>>>> 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@xxxxxxxxxxxxxxxxxxxxx> >>>>>> --- >>>>>> 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? >> > > Ok we can use PAGE_SIZE from resource and perf perspective, but please > change the subject since it is confusing. I guess the subject can be > similar to the one you used in the initiator. > > I think we should consider clamping to NVMET_RDMA_MAX_INLINE_DATA_SIZE > in case port->inline_data_size > NVMET_RDMA_MAX_INLINE_DATA_SIZE > instead of failing the operation (and add an info print). > This sounds reasonable. In my yet-to-be sent new version of this, I have similar logic if the rdma device doesn't have enough sge entries to support the configured size. In that case, the inline data size is clamped to the max that can be supported by the device. Steve. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
