From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: multipart/mixed; boundary="===============6049937832938673508==" MIME-Version: 1.0 From: Walker, Benjamin Subject: Re: [SPDK] BDEV-IO Lifecycle - Need your input. Date: Fri, 06 Jul 2018 16:41:32 +0000 Message-ID: <53179e83a40fd945ba54a563325a3ea73410412f.camel@intel.com> In-Reply-To: 17807346-A730-4DEC-ADC8-F68B471BDC07@intel.com List-ID: To: spdk@lists.01.org --===============6049937832938673508== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Hi Srikanth, I wanted to check in to see if you had any additional feedback 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 in the NVMe-oF target, but I think this is looking like the right mechanism for the bdev l= ayer. 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 the spdk_bdev_io > memory. > = > Regarding zcopy_start/zcopy_end =E2=80=93 it looks like you=E2=80=99ve al= ready 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 similar question= s 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, Benjamin" er(a)intel.com>, Daniel Verkamp > Cc: "raju.gottumukkala(a)broadcom.com" , > "Meneghini, John" , "Rodriguez, Edwin" z(a)netapp.com>, "Pai, Madhu" , "NGC-john.ba= rnard- > broadcom.com" , "spdk(a)lists.01.org" 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 bd= ev_io > pool is exhausted. Yes, we can solve it via sizing the pool right. The ch= anges > queue-io-wait also addresses the problem. > 2. Most importantly, by extending the life of bdev-io (Same life sp= an as > nvmf_request structure), abort use case and other use cases that involve > cleaning up of IO data buffer are accomplished effectively. Let me elabor= ate; > bdev-io is the fabric that connects the nvme command and operation with t= he > backend. The driver context and data buffer context is stored 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 host. In absence = of > bdev_io, we will have end up adding more and more void context 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 end. 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 th= e I/O > to the bdev device. Underlying implementation can be synchronous 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 obtain= ed 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 nece= ssary > 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 extend the life= span of bdev- > io and let it continue to host the driver context and buffer context popu= lated > during the BEGIN phase. > = > Regards, > Srikanth > = > From: Harris, James R = > Sent: Thursday, June 21, 2018 2:21 PM > To: Kaligotla, Srikanth ; Walker, Benjam= in jamin.walker(a)intel.com>; Verkamp, Daniel > Cc: raju.gottumukkala(a)broadcom.com; Meneghini, John >; Rodriguez, Edwin ; Pai, Madhu pp.com>; NGC-john.barnard-broadcom.com ; spd= k(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 com= munity > 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 sti= ll > need to make changes to the NVMe-oF target to use this new API but that s= hould > 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 216 plus the pe= r-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 flight at any give= n time. 64K > seems like a lot but I=E2=80=99d be curious to hear your thoughts on this= . If this is > the right number, then worst case, there would be about 16MB of DRAM that > would sit unused if the NVMe-oF target in your system was not 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" , > "Meneghini, John" , "Rodriguez, Edwin" z(a)netapp.com>, "Pai, Madhu" , "NGC-john.ba= rnard- > broadcom.com" , "spdk(a)lists.01.org" org> > Subject: RE: BDEV-IO Lifecycle - Need your input. > = > Hello, > = > First revision of changes to extend the lifecycle of bdev_io are availabl= e 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" , "Harris, James R" <= james.r > .harris(a)intel.com> > Cc: "raju.gottumukkala(a)broadcom.com" , > "Meneghini, John" , "Rodriguez, Edwin" z(a)netapp.com>, "Pai, Madhu" , "NGC-john.ba= rnard- > broadcom.com" , "spdk(a)lists.01.org" org> > Subject: RE: BDEV-IO Lifecycle - Need your input. > = > CC: List > = > Hi Ben, > = > Your proposal to interface with the backend to acquire and release buffer= s is > good. You have accurately stated that the challenge is in developing intu= itive > semantics. And that has been my struggle. To me, there are two problem > statements; > = > 1. It is expected that bdev_io pool is sized correctly so the call = to > get bdev_io succeeds. Failure to acquire bdev_io will result in DEVICE-ER= ROR. > The transport is already capable of handling temporary memory failures by > moving the request to PENDING-Queue. Hence the proposal to change the bde= v_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->type is overload= ed > the type of I/O operation is lost. I suppose one can cast the 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 u= ntil > controller-to-host has occurred. In other words, bdev_io lives till REQUE= ST > 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 and 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 >; Rodriguez, Edwin ; Pai, Madhu pp.com>; NGC-john.barnard-broadcom.com > Subject: Re: BDEV-IO Lifecycle - Need your input. > = > Hi Srikanth, > = > Yes - we'll need to introduce some way to acquire and release buffers fro= m the > bdev layer earlier on in the state machine that processes an NVMe-oF requ= est. > I've had this patch out for review for several months as a proposal for t= his > scenario: > = > https://review.gerrithub.io/#/c/spdk/spdk/+/386166/ > = > It doesn't pass the tests - it's just a proposal for the interface. 90% o= f the > challenge here is in developing intuitive semantics. > = > Thanks, > Ben > = > P.S. This is the kind of discussion that would fit perfectly on the maili= ng > list. > = > On Wed, 2018-05-09 at 20:35 +0000, Kaligotla, Srikanth wrote: > > Hi Ben, Hi James, > > = > > I would like to solicit opinions on the lifecycle for bdev-io resource > > object. Attached is the image of RDMA state machine in its current > > implementation. When the REQUEST enters the NEED-BUFFER state, the buff= ers > > 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 down as soon the > > backend returns. From a BDEV perspective, BDEV-IO is simply a translati= on > > unit that facilitates I/O buffers from the backend. The driver context > > embedded within bdev_io holds great deal of information pertaining to t= he > > I/O under execution. It assists in error handling, dereferencing the bu= ffers > > 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 hea= r peoples > > thoughts in introducing the plumbing to acquire BDEV-IO resource in REQ= UEST- > > NEED-BUFFER state and release it in REQUEST-COMPLETE state. I will sho= rtly > > have patch available for review that introduces spdk_bdev_init and > > spdk_bdev_fini which in turn invokes the corresponding bdev 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 pr= ior > > to pushing it upstream for review would like to hear your thoughts. The= se > > 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 required we can > > elaborate further on this proposal. > > = > > Thanks, > > Srikanth > = > =20 --===============6049937832938673508==--