Excerpts from Haren Myneni's message of June 18, 2021 5:49 pm:

On Fri, 2021-06-18 at 09:22 +1000, Nicholas Piggin wrote:

quoted

Excerpts from Haren Myneni's message of June 18, 2021 6:36 am:

quoted

This patch adds VAS window allocatioa/close with the corresponding
hcalls. Also changes to integrate with the existing user space VAS
API and provide register/unregister functions to NX pseries driver.

The driver register function is used to create the user space
interface (/dev/crypto/nx-gzip) and unregister to remove this
entry.

The user space process opens this device node and makes an ioctl
to allocate VAS window. The close interface is used to deallocate
window.

Signed-off-by: Haren Myneni <haren@linux.ibm.com>

Reviewed-by: Nicholas Piggin <npiggin@gmail.com>

Unless there is some significant performance reason it might be
simplest
to take the mutex for the duration of the allocate and frees rather
than 
taking it several times, covering the atomic with the lock instead.

You have a big lock, might as well use it and not have to wonder what
if 
things race here or there.

Using mutex to protect allocate/deallocate window and setup/free IRQ,
also to protect updating the list. We do not need lock for modify
window hcall and other things. Hence taking mutex several times.

Right, at which point you have to consider what happens with 
interleaving allocates and deallocates. I'm not saying it's wrong, just 
that if you do credential allocation, hcall allocation, irq allocation, 
and list insertion all under the one lock, and remoe it all under the 
one lock, concurrency requires less attention.

Also
used atomic for counters (used_lpar_creds) which can be exported in
sysfs (this patch will be added later in next enhancement seris). 

That's okay you can use mutexes for that too if that's how you're
protecting them.

Genarlly applications open window initially, do continuous copy/paste
operations and close window later. But possible that the library /
application to open/close window for each request. Also may be opening
or closing multiple windows (say 1000 depends on cores on the system)
at the same time. These cases may affect the application performance.

It definitely could if you have a lot of concurrent open/close, but
the code as is won't handle it all that well either, so there's the
question of what is reasonable to do and what is reasonable to add
concurrency complexity for.

As I said, you've got it working and seem to have covered all cases now 
so let's get the series in first. But something to consider changing
IMO.

Thanks,
Nick