RE: [PATCHv4 1/4] modem_shm: Add Modem Access Framework

quoted

quoted

quoted

quoted

quoted

+int modem_release(struct modem_desc *mdesc) {
+   if (!mdesc->release)
+           return -EFAULT;
+
+   if (modem_is_requested(mdesc)) {
+           atomic_dec(&mdesc->mclients->cnt);
+           if (atomic_read(&mdesc->use_cnt) == 1) {
+                   mdesc->release(mdesc);
+                   atomic_dec(&mdesc->use_cnt);
+           }

Eeek, why aren't you using the built-in reference counting that
the struct device provided to you, and instead are rolling your own?
This happens in many places, why?

My usage of counters over here is for each modem there are many

clients.

quoted

quoted

quoted

Each of the clients will have a ref to modem_desc. Each of them use
this for requesting and releasing the modem. One counter for
tracking the request and release for each client which is done by
variable 'cnt' in

struct clients.

quoted

The counter use_cnt is used for tracking the modem request/release
irrespective of the clients and counter cli_cnt is used for
restricting the modem_get to the no of clients defined in no_clients.

So totally 3 counter one for restricting the usage of modem_get by
clients, second for restricting modem request/release at top level,
and 3rd for restricting modem release/request for per client per
modem

basis.

quoted

Can you let me know if the same can be achieved by using built-in
ref counting?

Is this your model:
You have a modem device which can be requested by many clients.This
clients can register for a particular service which this modem
provides and then after that if it client doesn't need this service then it will

call un-register.

quoted

Let me correct a bit over here:
There are many clients, yes correct but the operations performed are
only two, i.e modem request and modem release. This is something like
waking up the modem and let modem to sleep.
The traffic of this request and release is too high.

So irrespective of the requests/releases made to the MAF framework,
the MAF should perform the operation request/release only once.

So each and every time handling list consumes time.
Let me brief the context, this is a single chip modem and ape,
basically used in mobile, tablets etc. So the traffic in ape-modem
communication is too high and also time critical. If it bound to
exceed the time, or delay might end up in buffer full. That’s the reason I

have made it as simple as possible.

So let me put it this way:
           Modem                 Client1     Client2    Client3    Client4
State  turn-on                   request
State  no-state-change                     request
State  no-state-change                                   request
State  no-state-change
request
State  no-state-change      release
State  no-state-change                     release
State  no-state-change                                   release
State   turn-off
      release

So eventhough all the clients have requested the modem it is being turned
on only once and when all of them have released then it will be turned-off.

In this case it makes sense to use the atomic variables to track the requests
and release but what will happen to sysfs incase the device is released when
the last call to release has come from client4?Won't it lead to kernel panic or
some unwanted behaviour?

Yes, you are right, so will add a check in modem_put and modem_unregister
to check if modem is requested and if so will release first and then go ahead.
But actually clients are not suppose to call modem_put/modem_unregister
unless modem is released but yes, in MAF we can take this precaution.

Thanks and Regards,
Arun R Murthy
-----------------