Re: [PATCH] crypto: virtio-crypto: call finalize with bh disabled

2023-11-06 Thread Herbert Xu
On Thu, Nov 02, 2023 at 01:01:09PM +, Gonglei (Arei) wrote: > > > So I think the core of this question is: Can we call > > crypto_finalize_request() in > > the upper half of the interrupt? The finalize path originates from the network stack. So the conditions are the same as that of the

RE: [PATCH] crypto: virtio-crypto: call finalize with bh disabled

2023-11-02 Thread Gonglei (Arei) via Virtualization
sts.linux-foundation.org; linux-ker...@vger.kernel.org; > pizhen...@bytedance.com; Cornelia Huck > Subject: RE: [PATCH] crypto: virtio-crypto: call finalize with bh disabled > > > > > -Original Message- > > From: Halil Pasic [mailto:pa...@linux.ibm.com] > > Sen

RE: Re: [PATCH] crypto: virtio-crypto: call finalize with bh disabled

2023-09-27 Thread Gonglei (Arei) via Virtualization
Wang > ; virtualization@lists.linux-foundation.org; > linux-ker...@vger.kernel.org; Cornelia Huck > Subject: Re: Re: [PATCH] crypto: virtio-crypto: call finalize with bh disabled > > Hi Michael & Lei, > > I volunteer to fix this by workqueue. > Thanks, patches are alwa

Re: Re: [PATCH] crypto: virtio-crypto: call finalize with bh disabled

2023-09-27 Thread zhenwei pi via Virtualization
Hi Michael & Lei, I volunteer to fix this by workqueue. I also notice that device drivers use workqueue to handle config-changed again and again, what about re-implement __virtio_config_changed() by kicking workqueue instead? By the way, balloon dirvers uses

Re: [PATCH] crypto: virtio-crypto: call finalize with bh disabled

2023-09-27 Thread Halil Pasic
On Tue, 26 Sep 2023 18:41:58 +0200 Halil Pasic wrote: > > + local_bh_disable(); > > crypto_finalize_akcipher_request(vc_akcipher_req->base.dataq->engine, > > req, err); > > + local_bh_enable(); > > Thanks Gonglei! > > I did this a quick spin, and it does not seem to be sufficient on

Re: [PATCH] crypto: virtio-crypto: call finalize with bh disabled

2023-09-27 Thread Halil Pasic
On Wed, 27 Sep 2023 09:24:09 + "Gonglei (Arei)" wrote: > > On a related note, config change callback is also handled incorrectly in > > this > > driver, it takes a mutex from interrupt context. > > Good catch. Will fix it. Thanks Gonglei! Sorry I first misunderstood this as a problem

Re: [PATCH] crypto: virtio-crypto: call finalize with bh disabled

2023-09-27 Thread Halil Pasic
On Wed, 27 Sep 2023 14:12:19 +0200 Cornelia Huck wrote: > On Wed, Sep 27 2023, Halil Pasic wrote: > > > On Wed, 27 Sep 2023 12:08:43 +0200 > > Cornelia Huck wrote: > > > >> > On the other hand virtio_airq_handler() calls vring_interrupt() with > >> > interrupts enabled. (While

Re: [PATCH] crypto: virtio-crypto: call finalize with bh disabled

2023-09-27 Thread Cornelia Huck
On Wed, Sep 27 2023, Halil Pasic wrote: > On Wed, 27 Sep 2023 12:08:43 +0200 > Cornelia Huck wrote: > >> > On the other hand virtio_airq_handler() calls vring_interrupt() with >> > interrupts enabled. (While vring_interrupt() is called in a (read) >> > critical section in virtio_airq_handler()

Re: [PATCH] crypto: virtio-crypto: call finalize with bh disabled

2023-09-27 Thread Halil Pasic
On Wed, 27 Sep 2023 12:08:43 +0200 Cornelia Huck wrote: > > On the other hand virtio_airq_handler() calls vring_interrupt() with > > interrupts enabled. (While vring_interrupt() is called in a (read) > > critical section in virtio_airq_handler() we use read_lock() and > > not read_lock_irqsave()

Re: [PATCH] crypto: virtio-crypto: call finalize with bh disabled

2023-09-27 Thread Cornelia Huck
On Tue, Sep 26 2023, Halil Pasic wrote: > [..] >> --- a/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c >> +++ b/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c >> @@ -61,8 +61,9 @@ static void virtio_crypto_akcipher_finalize_req( >> vc_akcipher_req->src_buf = NULL; >>

Re: [PATCH] crypto: virtio-crypto: call finalize with bh disabled

2023-09-27 Thread Halil Pasic
On Tue, 26 Sep 2023 13:13:51 -0400 "Michael S. Tsirkin" wrote: > > On the other hand virtio_airq_handler() calls vring_interrupt() with > > interrupts enabled. (While vring_interrupt() is called in a (read) > > critical section in virtio_airq_handler() we use read_lock() and > > not

RE: [PATCH] crypto: virtio-crypto: call finalize with bh disabled

2023-09-27 Thread Gonglei (Arei) via Virtualization
sts.linux-foundation.org; > linux-ker...@vger.kernel.org; pizhen...@bytedance.com; Cornelia Huck > > Subject: Re: [PATCH] crypto: virtio-crypto: call finalize with bh disabled > > On Tue, Sep 26, 2023 at 06:41:58PM +0200, Halil Pasic wrote: > > [..] > > > --- a/drivers/c

RE: [PATCH] crypto: virtio-crypto: call finalize with bh disabled

2023-09-27 Thread Gonglei (Arei) via Virtualization
sts.linux-foundation.org; linux-ker...@vger.kernel.org; > pizhen...@bytedance.com; Halil Pasic ; Cornelia Huck > > Subject: Re: [PATCH] crypto: virtio-crypto: call finalize with bh disabled > > [..] > > --- a/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c > > +++ b/

Re: [PATCH] crypto: virtio-crypto: call finalize with bh disabled

2023-09-26 Thread Michael S. Tsirkin
On Tue, Sep 26, 2023 at 06:41:58PM +0200, Halil Pasic wrote: > [..] > > --- a/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c > > +++ b/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c > > @@ -61,8 +61,9 @@ static void virtio_crypto_akcipher_finalize_req( > > vc_akcipher_req->src_buf =

Re: [PATCH] crypto: virtio-crypto: call finalize with bh disabled

2023-09-26 Thread Halil Pasic
[..] > --- a/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c > +++ b/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c > @@ -61,8 +61,9 @@ static void virtio_crypto_akcipher_finalize_req( > vc_akcipher_req->src_buf = NULL; > vc_akcipher_req->dst_buf = NULL; >