On Wed, Sep 07, 2005 at 12:26:02PM +0300, Ronen Shitrit ([EMAIL PROTECTED]) wrote: > I always assume that there won't be any errors ;)
It could be too boring to live in a perfect world :) > I fixed it, Thanks for the review. > I even decide to be a little too cautious now, and in case r<0 the > kcryptd_do_work task will go to sleep for 1/2 sec before freeing the io, > > to give the read requests a chance to finish (If there are any). > See attached. I do not think it is needed, better to either switch to sync mode or complete BIO with error, such sleeping will only hide possible problems and make debugging much more complex. > Regards > > Ronen Shitrit > Marvell Semiconductor Israel Ltd > > -----Original Message----- > From: Evgeniy Polyakov [mailto:[EMAIL PROTECTED] > Sent: Wednesday, September 07, 2005 11:19 AM > To: Ronen Shitrit > Cc: [EMAIL PROTECTED]; linux-crypto@vger.kernel.org > Subject: Re: Dm-crypt patch for OCF > > On Wed, Sep 07, 2005 at 11:07:14AM +0300, Ronen Shitrit > ([EMAIL PROTECTED]) wrote: > > I don't think so, > > > > In the kcrypt_do_work I call dec_pending only in case return value is > > error. > > But you return from crypto code with zero (OK) status when > crypto_dispatch() can fail, and this BIO will be lost. > I was confused by error processing, so described problem in a wrong way > first time, sorry. > > > Other from this for each context of convert_crypt which is might be > > breaked into some sectors decryption requests, I will call the > > dec_pending only in the last read callback of the last sector. > > > > -> I assume that the order of the insertion to the OCF Qs, is the > > -> order > > of the completion. > > > > If I'm wrong please point me in the code, sorry. > > > > Regards > > > > Ronen Shitrit > > Marvell Semiconductor Israel Ltd > > > > > > -----Original Message----- > > From: Evgeniy Polyakov [mailto:[EMAIL PROTECTED] > > Sent: Wednesday, September 07, 2005 10:57 AM > > To: Ronen Shitrit > > Cc: [EMAIL PROTECTED]; linux-crypto@vger.kernel.org > > Subject: Re: Dm-crypt patch for OCF > > > > On Wed, Sep 07, 2005 at 10:44:01AM +0300, Ronen Shitrit > > ([EMAIL PROTECTED]) wrote: > > > Hi > > > > > > I don't think there is any problem to let the read request to flow > > > through, since in the original code, In order to decrypt the read > > > requests, we create a new task (workingQ) that perform the decrypt, > > > and doesn't notify any other task when it finish, except for the > > > dec_pending(io,r) which I moved to the read callback. > > > > No, dm-crypt only calls dec_pending() with BIO with decrypted data, > > but with your code it can be called before read callback is invoked > > and even before BIO is touched in crypto code. > > > > > Regards > > > > > > Ronen Shitrit > > > Marvell Semiconductor Israel Ltd > > > > > > > > > -----Original Message----- > > > From: Evgeniy Polyakov [mailto:[EMAIL PROTECTED] > > > Sent: Wednesday, September 07, 2005 10:36 AM > > > To: Ronen Shitrit > > > Cc: [EMAIL PROTECTED]; linux-crypto@vger.kernel.org > > > Subject: Re: Dm-crypt patch for OCF > > > > > > On Tue, Sep 06, 2005 at 09:20:42PM +0300, Ronen Shitrit > > > ([EMAIL PROTECTED]) wrote: > > > > Hi > > > > > > Hello, Ronen. > > > I have some doubts - you only wait on write request, but allow read > > > request to flow through - when doind read request it does not mean, > > > that dm-crypt is the last target, it can be even freed when your > > > read callback will be called. > > > > > > And according to crypto sessions - I was confused by this name, and > > > did not understood you from the first point. > > > I want to say, that acrypto does not have such crypto session like > > > OCF > > > > > has. It only has crypto requests in OCF terminology, since it does > > > not > > > > > require special controlling strucutre on top of it. > > > I will describe it in more details in different thread. > > > > > > > I improved the first patch, see attached. > > > > > > > > Dm-crypt: > > > > -)On the encrypt(write) side we first allocate a new buffer then > > > > we encrypt (using crypt_convert) the src buffer to the new buffer, > > > > > we send it > > > > to be written through the generic_make_request(clone), then when > > > > > the > > > > clone->bi_end_io is called we free the buffer and the io. > > > > -)On the decrypt(read) side we first read the buffer to the source > > > > > through the generic_make_request(clone), then when the > > > clone->bi_end_io > > > > is called, we create a new working thread which will decrypt > > > > (using > > > > crypt_convert) the buffer and free the io. > > > > > > > > In the first patch, for each crypt_convert operation we send all > > > > the > > > > > > sectors of the context to be encrypt/decrypt to the OCF, and then > > > > we > > > > > > are waiting for A completion of all of these sectors before > > > > returning from crypt_convert, i.e. we get a limitation that only > > > > one > > > > > > encrypt/decrypt crypt_convert operation can occur in parallel. > > > > > > > > In the attached new patch, I removed this limitation for the > > > > decrypt(read) crypt_convert, > > > > By doing this I can see that when running the Bonnie benchmark, I > > > > get better performance for the read/rewrite tests. > > > > > > > > The same approach can be used for the encrypt(write) > > > > crypt_convert, it > > > > > > > is a little bit more complicated then the decrypt(read), maybe I > > > > will try to implement it in future patches. > > > > > > > > I also noticed that sometimes I get 2 encrypt(write) crypt_convert > > > > > in parallel?!?!, that why I moved the wr_pending into a separate > > > > structure, > > > > > > > > which is allocated per write request. > > > > This change affected the write performance only in a bit, less > > > > then > > > 1%. > > > > > > > > btw - this patch is a patch for kernel 2.6.12, with OCF 20050630 > > > > patch > > > > > > > applied on it. > > > > same tests were used as for the first patch. > > > > > > > > Regards > > > > > > > > Ronen Shitrit > > > > Marvell Semiconductor Israel Ltd > > > > > > > > > > > > > > > > -----Original Message----- > > > > From: [EMAIL PROTECTED] > > > > [mailto:[EMAIL PROTECTED] On Behalf Of Ronen > > > > Shitrit > > > > Sent: Monday, September 05, 2005 6:33 PM > > > > To: [EMAIL PROTECTED]; linux-crypto@vger.kernel.org > > > > Cc: Evgeniy Polyakov; David McCullough > > > > Subject: Dm-crypt patch for OCF > > > > > > > > Hi all > > > > > > > > Attached is a patch for the dm_crypt HD encryption which use the > > OCF. > > > > The patch includes a new Kconfig configuration option for > > > > OCF_DM_CRYPT, When choosing this option the dm_crypt will use the > > > > OCF for dm_crypt sector encryption/decryption, When using the > > > > essiv mode, the essiv generation will use the kernel cryptoAPIs. > > > > Currently this patch support the following encryption algorithms: > > > > DES-CBC, 3DES-CBC and AES-CBC. > > > > > > > > I tested this driver using AES-CBC, with OCF SW driver, it seems > > > stable. > > > > I used the Bonnie benchmark to get some statistics: > > > > http://www.textuality.com/bonnie/ > > > > The bandwidth performance are much better when using the OCF > > dm_crypt. > > > > > > > This might be explained since Bonnie is using a large blocks of io > > > > > (crypt_convert get contexts of 512byte * 256), which cause the > > > > dm_crypt to Q few requests at a time, and this "multi tasking", > > > > cause that the HD and the CPU "bandwidth" are exploit in a better > > > > way. (I > > > > assume) > > > > > > > > When using mkfs.ext2 on large partition I see that the OCF > > > > dm_crypt requires about 7% more time then when using the standard > dm_crypt. > > > > This can be explained since the mkfs.ext2 is mostly using writes > > > > of small blocks (crypt_convert get contexts of 512byte * 8), Which > > > > > cause that we gets less "multi tasking", and as explained below > > > > the write request are not optimized in this patch. > > > > > > > > Currently the dm_crypt is implemented in a way that: for > > > > decryption > > > > > > (read requests), it is using the source buffer itself, While for > > > > the encryption (write requests), it is using a different buffer. > > > > The current implementation of the OCF only support encryption on > > > > the > > > > > > source buffer, which is not efficient for this case, therefore > > > > this patch has overhead of copying the buffers to be encrypted. > > > > > > > > > > > > Thanks to Evgeniy Polyakov for the Acrypto patch reference. > > > > > > > > Regards > > > > > > > > Ronen Shitrit > > > > Marvell Semiconductor Israel Ltd > > > > > > > > > > > > > > _______________________________________________ > > > > > > > > Subscription: http://lists.logix.cz/mailman/listinfo/cryptoapi > > > > List archive: http://lists.logix.cz/pipermail/cryptoapi > > > > > > -- > > > Evgeniy Polyakov > > > > -- > > Evgeniy Polyakov > > - > > To unsubscribe from this list: send the line "unsubscribe > > linux-crypto" in the body of a message to [EMAIL PROTECTED] > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > -- > Evgeniy Polyakov -- Evgeniy Polyakov - To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
