Re: [PATCH v2 01/11] crypto: introduce crypto wait for async op

2017-06-10 Thread Herbert Xu
On Sat, Jun 10, 2017 at 11:05:39AM +0300, Gilad Ben-Yossef wrote:
>
> I guess there is a question if it really is important to know that
> your request ended up
> on the backlog, rather than being handled.I can imagine it can be used
> as back pressure
> indication but I wonder if someone is using that.

Oh yes we do want it to return EBUSY if we put it on the backlog
because in that case we want the user to stop sending us new
requests.

It's the other case where we dropped the request and returned
EBUSY where I think we could return something other than EBUSY
and get rid of the ambiguity.

Thanks,
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 01/11] crypto: introduce crypto wait for async op

2017-06-10 Thread Gilad Ben-Yossef
On Sat, Jun 10, 2017 at 6:43 AM, Herbert Xu  wrote:
> On Mon, May 29, 2017 at 11:22:48AM +0300, Gilad Ben-Yossef wrote:
>>
>> +static inline int crypto_wait_req(int err, struct crypto_wait *wait)
>> +{
>> + switch (err) {
>> + case -EINPROGRESS:
>> + case -EBUSY:
>> + wait_for_completion(&wait->completion);
>> + reinit_completion(&wait->completion);
>> + err = wait->err;
>> + break;
>> + };
>> +
>> + return err;
>> +}
>
> This assumes that the request is used with backlog.  For non-backlog
> requests this would result in a memory leak as EBUSY in that case is
> a fatal error.
>
> So this API can't be used without backlog.

You are right, of course. I did not take that into account.

>
> We could introduce a flag to indicate whether we want backlog or not,
> or maybe we should change our API so that in the non-backlog case we
> return something other than EBUSY.
>
> Opinions?

I guess there is a question if it really is important to know that
your request ended up
on the backlog, rather than being handled.I can imagine it can be used
as back pressure
indication but I wonder if someone is using that.

If not, maybe we can simplify things and use EINPROGRESS asindication
of a request
being accepted by the next layer (either being processed or queued in
the back log), whereas
EBUSY would indicate failure.

It does have a potential to make things simpler, I think.

Gilad

>
> Thanks,
> --
> Email: Herbert Xu 
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt



-- 
Gilad Ben-Yossef
Chief Coffee Drinker

"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
 -- Jean-Baptiste Queru
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 01/11] crypto: introduce crypto wait for async op

2017-06-09 Thread Herbert Xu
On Mon, May 29, 2017 at 11:22:48AM +0300, Gilad Ben-Yossef wrote:
>
> +static inline int crypto_wait_req(int err, struct crypto_wait *wait)
> +{
> + switch (err) {
> + case -EINPROGRESS:
> + case -EBUSY:
> + wait_for_completion(&wait->completion);
> + reinit_completion(&wait->completion);
> + err = wait->err;
> + break;
> + };
> +
> + return err;
> +}

This assumes that the request is used with backlog.  For non-backlog
requests this would result in a memory leak as EBUSY in that case is
a fatal error.

So this API can't be used without backlog.

We could introduce a flag to indicate whether we want backlog or not,
or maybe we should change our API so that in the non-backlog case we
return something other than EBUSY.

Opinions?

Thanks,
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 01/11] crypto: introduce crypto wait for async op

2017-05-29 Thread Gilad Ben-Yossef
Invoking a possibly async. crypto op and waiting for completion
while correctly handling backlog processing is a common task
in the crypto API implementation and outside users of it.

This patch adds a generic implementation for doing so in
preparation for using it across the board instead of hand
rolled versions.

Signed-off-by: Gilad Ben-Yossef 
CC: Eric Biggers 
CC: Ofir Drang 
---
 crypto/api.c   | 13 +
 include/linux/crypto.h | 41 +
 2 files changed, 54 insertions(+)

diff --git a/crypto/api.c b/crypto/api.c
index 941cd4c..2a2479d 100644
--- a/crypto/api.c
+++ b/crypto/api.c
@@ -24,6 +24,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "internal.h"
 
 LIST_HEAD(crypto_alg_list);
@@ -595,5 +596,17 @@ int crypto_has_alg(const char *name, u32 type, u32 mask)
 }
 EXPORT_SYMBOL_GPL(crypto_has_alg);
 
+void crypto_req_done(struct crypto_async_request *req, int err)
+{
+   struct crypto_wait *wait = req->data;
+
+   if (err == -EINPROGRESS)
+   return;
+
+   wait->err = err;
+   complete(&wait->completion);
+}
+EXPORT_SYMBOL_GPL(crypto_req_done);
+
 MODULE_DESCRIPTION("Cryptographic core API");
 MODULE_LICENSE("GPL");
diff --git a/include/linux/crypto.h b/include/linux/crypto.h
index 84da997..bb00186 100644
--- a/include/linux/crypto.h
+++ b/include/linux/crypto.h
@@ -24,6 +24,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /*
  * Autoloaded crypto modules should only use a prefixed name to avoid allowing
@@ -468,6 +469,45 @@ struct crypto_alg {
 } CRYPTO_MINALIGN_ATTR;
 
 /*
+ * A helper struct for waiting for completion of async crypto ops
+ */
+struct crypto_wait {
+   struct completion completion;
+   int err;
+};
+
+/*
+ * Macro for declaring a crypto op async wait object on stack
+ */
+#define DECLARE_CRYPTO_WAIT(_wait) \
+   struct crypto_wait _wait = { \
+   COMPLETION_INITIALIZER_ONSTACK((_wait).completion), 0 }
+
+/*
+ * Async ops completion helper functioons
+ */
+void crypto_req_done(struct crypto_async_request *req, int err);
+
+static inline int crypto_wait_req(int err, struct crypto_wait *wait)
+{
+   switch (err) {
+   case -EINPROGRESS:
+   case -EBUSY:
+   wait_for_completion(&wait->completion);
+   reinit_completion(&wait->completion);
+   err = wait->err;
+   break;
+   };
+
+   return err;
+}
+
+static inline void crypto_init_wait(struct crypto_wait *wait)
+{
+   init_completion(&wait->completion);
+}
+
+/*
  * Algorithm registration interface.
  */
 int crypto_register_alg(struct crypto_alg *alg);
@@ -1604,5 +1644,6 @@ static inline int crypto_comp_decompress(struct 
crypto_comp *tfm,
src, slen, dst, dlen);
 }
 
+
 #endif /* _LINUX_CRYPTO_H */
 
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html