From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: multipart/mixed; boundary="===============0290157864981121931==" MIME-Version: 1.0 From: Walker, Benjamin Subject: Re: [SPDK] BDEV-IO Lifecycle - Need your input. Date: Mon, 09 Jul 2018 17:52:13 +0000 Message-ID: In-Reply-To: 89862ECB-1D2A-4AA3-841C-2FF700941252@netapp.com List-ID: To: spdk@lists.01.org --===============0290157864981121931== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable On Mon, 2018-07-09 at 16:18 +0000, Meneghini, John wrote: > Hi Jim. > = > This is the patch series I believe Ben is proposing to replace Srikanth's= : ht > tps://review.gerrithub.io/#/c/spdk/spdk/+/415860/ > = > pick a21bbcf2 nvmf: Move data buffer pool to generic layer > pick 09586e54 bdev: Add a zero copy I/O path > pick 1cc9fe95 bdev: Make malloc bdev use the new zero copy mechanism > pick 40a1f62b bdevperf: Use new zcopy API for reads > pick eba6f35a bdev: Emulate zero copy support when necessary > = > Is this correct? My patch series doesn't entirely replace Srikanth's set of patches. There a= re two separate things necessary to implement zero copy. First is the infrastructure within the bdev layer to make requests of a bdev module to o= btain or release a suitable buffer for a zero copy operation. Second is actually making the NVMe-oF target use that mechanism, which includes extending the bdev_io lifetime. My patch series only addresses the first part. Thanks, Ben > = > /John > = > =EF=BB=BFOn 7/9/18, 11:31 AM, "Harris, James R" wrote: > = > Hi Sriram: > = > Ben has some later patches in this series with some examples: > = > https://review.gerrithub.io/#/c/spdk/spdk/+/386167/ - implements this= zero > copy API in the simple =E2=80=98malloc=E2=80=99 bdev module > https://review.gerrithub.io/#/c/spdk/spdk/+/416579/ - uses the zero c= opy > API in the SPDK bdevperf utility > = > -Jim > = > = > On 7/9/18, 3:34 AM, "Popuri, Sriram" wro= te: > = > Sorry was not focusing on this change. Just give me a day or two = to > get back. > From a quick glance I didn't understand how zcopy start/end fits = into > a read/write life cycle. Is there an example on how zcopy start/end is > consumed or someone can do give me a quick dump on how its envisioned to = be > used. > = > Regards, > ~Sriram > = > -----Original Message----- > From: Meneghini, John = > Sent: Friday, July 6, 2018 10:16 PM > To: Walker, Benjamin ; Verkamp, Dani= el iel.verkamp(a)intel.com>; Harris, James R ; K= aligotla, > Srikanth > Cc: raju.gottumukkala(a)broadcom.com; spdk(a)lists.01.org; Rodrig= uez, > Edwin ; Pai, Madhu ; NGC- > john.barnard-broadcom.com ; Popuri, Sriram <= Sriram. > Popuri(a)netapp.com> > Subject: Re: BDEV-IO Lifecycle - Need your input. > = > I'm adding Sriram to this thread. Sriram is another NetApp engin= eer > who is working on this stuff internally. > = > /John > = > On 7/6/18, 12:41 PM, "Walker, Benjamin" > wrote: > = > Hi Srikanth, > = > I wanted to check in to see if you had any additional feedbac= k on > the zero copy > operations in the bdev layer here: > = > https://review.gerrithub.io/#/c/spdk/spdk/+/386166/ > = > This does not address extending the lifetime of the bdev_io i= n the > NVMe-oF > target, but I think this is looking like the right mechanism = for > the bdev layer. > = > Thanks, > Ben > = > On Thu, 2018-06-21 at 18:11 -0700, Harris, James R wrote: > > Thanks Srikanth. Sounds like the spdk_bdev_io pool sizing = along > with > > spdk_bdev_queue_io_wait() meets your needs then regarding t= he > spdk_bdev_io > > memory. > > = > > Regarding zcopy_start/zcopy_end =E2=80=93 it looks like you= =E2=80=99ve already > added a bunch > > of comments to Ben=E2=80=99s patch on GerritHub. For now I= =E2=80=99d say let=E2=80=99s > continue our > > discussion there. I=E2=80=99ve responded to a couple of si= milar > questions there and > > I=E2=80=99m sure Ben will have more replies tomorrow. > > = > > -Jim > > = > > = > > From: "Kaligotla, Srikanth" > > Date: Thursday, June 21, 2018 at 5:01 PM > > To: James Harris , "Walker, Ben= jamin" > > er(a)intel.com>, Daniel Verkamp > > Cc: "raju.gottumukkala(a)broadcom.com" .com>, > > "Meneghini, John" , "Rodriguez, > Edwin" > z(a)netapp.com>, "Pai, Madhu" = , "NGC- > john.barnard- > > broadcom.com" , "spdk(a)lists.= 01.org" < > spdk(a)lists.01. > > org> > > Subject: RE: BDEV-IO Lifecycle - Need your input. > > = > > Hi Jim, > > = > > I wish I joined the community meeting, I also missed the > > spdk_bdev_queue_io_wait(). > > = > > So, there are 2 issues I am intending to solve, > > = > > 1. We want to ensure that an instance of bdev-io is > acquired prior to or > > along with I/O data buffer. This allows for better error > handling when bdev_io > > pool is exhausted. Yes, we can solve it via sizing the pool > right. The changes > > queue-io-wait also addresses the problem. > > 2. Most importantly, by extending the life of bdev-io > (Same life span as > > nvmf_request structure), abort use case and other use cases= that > involve > > cleaning up of IO data buffer are accomplished effectively.= Let > me elaborate; > > bdev-io is the fabric that connects the nvme command and > operation with the > > backend. The driver context and data buffer context is stor= ed in > bdev-io. The > > freeing up of bdev_io resource is pushed to the end so the = I/O > cleanup can > > happen after controller has transmitted the data to the hos= t. In > absence of > > bdev_io, we will have end up adding more and more void cont= ext > in request > > structure. Hence the push to extend the life of bdev_io. > > = > > I see Ben=E2=80=99s recent patch =E2=80=9Czcopy_start=E2=80= =9D and =E2=80=9Czcopy_end=E2=80=9D; We can > make that work > > as long as the bdev-io allocated/acquired stays till the en= d. > One of the > > challenges I see with that patch is defining a callback for= the > I/O > > submission. For instance, zopy_start will allocate bdev_io = and > submits the I/O > > to the bdev device. Underlying implementation can be synchr= onous > or can be > > asynchronous. The callback for this submission should check= to > see if it is a > > BUFFER-READY or PENDING and accordingly relay it back to > transport. Next phase > > is actual I/O submission. Let=E2=80=99s say, it reuses the = bdev-io > obtained in ZCOPY- > > START, now the callback should determine if it is a success= or > failure. > > Finally when ZCOPY-END is invoked the supplied bdev_io will= have > all necessary > > data to release the WRITE buffer or unlock the read buffer = based > on the > > operation performed. > > = > > I hope I=E2=80=99m making sense. I guess, my effort is to e= xtend the > lifespan of bdev- > > io and let it continue to host the driver context and buffer > context populated > > during the BEGIN phase. > > = > > Regards, > > Srikanth > > = > > From: Harris, James R = > > Sent: Thursday, June 21, 2018 2:21 PM > > To: Kaligotla, Srikanth ; = Walker, > Benjamin > jamin.walker(a)intel.com>; Verkamp, Daniel om> > > Cc: raju.gottumukkala(a)broadcom.com; Meneghini, John ini(a)netapp.com > > >; Rodriguez, Edwin ; Pai, Madhu= sudan.Pai(a)neta > > pp.com>; NGC-john.barnard-broadcom.com m>; spdk(a)lists > > .01.org > > Subject: Re: BDEV-IO Lifecycle - Need your input. > > = > > Hi Srikanth, > > = > > Following up on this thread and the discussion in yesterday= =E2=80=99s > community > > meeting. > > = > > The recent spdk_bdev_queue_io_wait() changes allow an SPDK > application to work > > better in general when the spdk_bdev_io buffer pool is > exhausted. We still > > need to make changes to the NVMe-oF target to use this new = API > but that should > > be done soon-ish. > > = > > With that in place, the spdk_bdev_io buffer pool itself can= be > configured up > > or down when the application starts. Currently the default= is > 64K > > spdk_bdev_io buffers. sizeof(struct spdk_bdev_io) =3D=3D 2= 16 plus > the per-IO > > context size allocated for the bdev module. This can be up= to > 192 bytes > > (virtio bdev module) but is likely much smaller for you > depending on the > > context size for your ontap bdev module. > > = > > Let=E2=80=99s assume your per-IO context size is 64 bytes. = 64K x (192 + > 64) =3D 16MB. > > = > > I=E2=80=99m not sure how many spdk_bdev_io you need in flig= ht at any > given time. 64K > > seems like a lot but I=E2=80=99d be curious to hear your th= oughts on > this. If this is > > the right number, then worst case, there would be about 16M= B of > DRAM that > > would sit unused if the NVMe-oF target in your system was n= ot > active. Is that > > too burdensome for your application? > > = > > Thanks, > > = > > -Jim > > = > > = > > = > > From: "Kaligotla, Srikanth" > > Date: Wednesday, June 20, 2018 at 12:47 PM > > To: "Walker, Benjamin" , James= Harris > > is(a)intel.com>, Daniel Verkamp > > Cc: "raju.gottumukkala(a)broadcom.com" .com>, > > "Meneghini, John" , "Rodriguez, > Edwin" > z(a)netapp.com>, "Pai, Madhu" = , "NGC- > john.barnard- > > broadcom.com" , "spdk(a)lists.= 01.org" < > spdk(a)lists.01. > > org> > > Subject: RE: BDEV-IO Lifecycle - Need your input. > > = > > Hello, > > = > > First revision of changes to extend the lifecycle of bdev_i= o are > available for > > review. I would like to solicit your input on the proposed > API/Code flow > > changes. > > = > > https://review.gerrithub.io/c/spdk/spdk/+/415860 > > = > > Thanks, > > Srikanth > > = > > = > > From: "Kaligotla, Srikanth" > > Date: Friday, May 11, 2018 at 2:27 PM > > To: "Walker, Benjamin" , "Harr= is, > James R" > .harris(a)intel.com> > > Cc: "raju.gottumukkala(a)broadcom.com" .com>, > > "Meneghini, John" , "Rodriguez, > Edwin" > z(a)netapp.com>, "Pai, Madhu" = , "NGC- > john.barnard- > > broadcom.com" , "spdk(a)lists.= 01.org" < > spdk(a)lists.01. > > org> > > Subject: RE: BDEV-IO Lifecycle - Need your input. > > = > > CC: List > > = > > Hi Ben, > > = > > Your proposal to interface with the backend to acquire and > release buffers is > > good. You have accurately stated that the challenge is in > developing intuitive > > semantics. And that has been my struggle. To me, there are = two > problem > > statements; > > = > > 1. It is expected that bdev_io pool is sized correctl= y so > the call to > > get bdev_io succeeds. Failure to acquire bdev_io will resul= t in > DEVICE-ERROR. > > The transport is already capable of handling temporary memo= ry > failures by > > moving the request to PENDING-Queue. Hence the proposal to > change the bdev_io > > lifecycle and perhaps connect it with the spdk_nvmf_request > object. Thus all > > buffer needs are addressed at the beginning of I/O request. > > 2. I/O Data buffers are sourced and managed by the > backend. One of the > > challenges I see with your proposed interface is the lack of > details like if > > resource is being acquired for a READ operation or WRITE > operation. The > > handling is quite different in each case. Since bdev_io->ty= pe is > overloaded > > the type of I/O operation is lost. I suppose one can cast t= he > cb_arg > > (nvmf_request) and then proceed. Also, the bdev_io should be > present to > > RELEASE the buffer. Zero copy semantics warrants that data > buffer stays until > > controller-to-host has occurred. In other words, bdev_io li= ves > till REQUEST > > come to COMPLETE state. > > = > > What are your thoughts in introducing spdk_bdev_init() and > spdk_bdev_fini() as > > an alternative approach to extend the lifecyle of bdev_io a= nd > allow data > > buffer management via bdev fn_table ? > > = > > I hope I=E2=80=99m making sense=E2=80=A6 > > = > > Thanks, > > Srikanth > > = > > From: Walker, Benjamin = > > Sent: Friday, May 11, 2018 12:28 PM > > To: Harris, James R ; Kaligotla, > Srikanth > Kaligotla(a)netapp.com> > > Cc: raju.gottumukkala(a)broadcom.com; Meneghini, John ini(a)netapp.com > > >; Rodriguez, Edwin ; Pai, Madhu= sudan.Pai(a)neta > > pp.com>; NGC-john.barnard-broadcom.com m> > > Subject: Re: BDEV-IO Lifecycle - Need your input. > > = > > Hi Srikanth, > > = > > Yes - we'll need to introduce some way to acquire and relea= se > buffers from the > > bdev layer earlier on in the state machine that processes an > NVMe-oF request. > > I've had this patch out for review for several months as a > proposal for this > > scenario: > > = > > https://review.gerrithub.io/#/c/spdk/spdk/+/386166/ > > = > > It doesn't pass the tests - it's just a proposal for the > interface. 90% of the > > challenge here is in developing intuitive semantics. > > = > > Thanks, > > Ben > > = > > P.S. This is the kind of discussion that would fit perfectl= y on > the mailing > > list. > > = > > On Wed, 2018-05-09 at 20:35 +0000, Kaligotla, Srikanth wrot= e: > > > Hi Ben, Hi James, > > > = > > > I would like to solicit opinions on the lifecycle for bde= v-io > resource > > > object. Attached is the image of RDMA state machine in its > current > > > implementation. When the REQUEST enters the NEED-BUFFER s= tate, > the buffers > > > necessary for carrying out I/O operation are > allocated/acquired from the > > > memory pool. An instance of BDEV-IO comes into existence = after > REQUEST > > > reaches READY-TO-EXECUTE state. The BDEV-IO is teared dow= n as > soon the > > > backend returns. From a BDEV perspective, BDEV-IO is simp= ly a > translation > > > unit that facilitates I/O buffers from the backend. The d= river > context > > > embedded within bdev_io holds great deal of information > pertaining to the > > > I/O under execution. It assists in error handling, > dereferencing the buffers > > > upon I/O completion and in abort handling. In summary, the > bdev_io stays > > > alive until request has come to COMPLETE state. I=E2=80= =99d like to > hear peoples > > > thoughts in introducing the plumbing to acquire BDEV-IO > resource in REQUEST- > > > NEED-BUFFER state and release it in REQUEST-COMPLETE stat= e. I > will shortly > > > have patch available for review that introduces spdk_bdev= _init > and > > > spdk_bdev_fini which in turn invokes the corresponding bd= ev > fn_table to > > > initialize/cleanup. = > > > = > > > I wanted to use this email to communicate our intent and > solicit your > > > feedback. We have a working implementation of the above > proposal and prior > > > to pushing it upstream for review would like to hear your > thoughts. These > > > proposed changes to upstream are a result of FC transport= work > in > > > collaboration with the Broadcom team who are also copied = to > this mail. > > > Myself and John will be at SPDK Dev conference and if req= uired > we can > > > elaborate further on this proposal. > > > = > > > Thanks, > > > Srikanth > > = > > = > = > = > = > = > = >=20 --===============0290157864981121931==--