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

From: Arun MURTHY
Date: Mon Oct 01 2012 - 05:12:42 EST


> >> >> > +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.
> >> > 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.
> >> > 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.
> >> >
> >> > 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.
> >
> > 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
-----------------
N‹§²æìr¸›yúèšØb²X¬¶ÇvØ^–)Þ{.nÇ+‰·¥Š{±‘êçzX§¶›¡Ü}©ž²ÆzÚ&j:+v‰¨¾«‘êçzZ+€Ê+zf£¢·hšˆ§~†­†Ûiÿûàz¹®w¥¢¸?™¨è­Ú&¢)ßf”ù^jÇy§m…á@A«a¶Úÿ 0¶ìh®å’i