From: rishabhb@codeaurora.org
To: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
linux-remoteproc <linux-remoteproc@vger.kernel.org>,
Ohad Ben-Cohen <ohad@wizery.com>,
psodagud@codeaurora.org, tsoni@codeaurora.org,
Siddharth Gupta <sidgup@codeaurora.org>,
linux-remoteproc-owner@vger.kernel.org
Subject: Re: [PATCH 1/2] remoteproc: Add userspace char device driver
Date: Tue, 31 Mar 2020 10:38:24 -0700 [thread overview]
Message-ID: <30f8b41df8831d19ce6efd0a28862708@codeaurora.org> (raw)
In-Reply-To: <CANLsYkxEA66kGZh1rToSn79fpgPHqEjMZsSw74Rx3OLwG2k35w@mail.gmail.com>
On 2020-03-31 09:47, Mathieu Poirier wrote:
> On Mon, 30 Mar 2020 at 16:45, Bjorn Andersson
> <bjorn.andersson@linaro.org> wrote:
>>
>> On Mon 30 Mar 15:12 PDT 2020, Mathieu Poirier wrote:
>> [..]
>> > > + struct rproc *rproc;
>> > > +
>> > > + rproc = container_of(inode->i_cdev, struct rproc, char_dev);
>> > > + if (!rproc)
>> > > + return -EINVAL;
>> > > +
>> > > + rproc_shutdown(rproc);
>> >
>> > The scenario I see here is that a userspace program will call
>> > open(/dev/rproc_xyz, SOME_OPTION) when it is launched. The file stays open
>> > until either the application shuts down, in which case it calls close() or it
>> > crashes. In that case the system will automatically close all file descriptors
>> > that were open by the application, which will also call rproc_shutdown().
>> >
>> > To me the same functionality can be achieved with the current functionality
>> > provided by sysfs.
>> >
>> > When the application starts it needs to read
>> > "/sys/class/remoteproc/remoteprocX/state". If the state is "offline" then
>> > "start" should be written to "/sys/.../state". If the state is "running" the
>> > application just crashed and got restarted. In which case it needs to stop the
>> > remote processor and start it again.
>> >
>>
>> A case when this would be useful is the Qualcomm modem, which relies
>> on
>> disk access through a "remote file system service" [1].
>>
>> Today we register the service (a few layers ontop of rpmsg/GLINK) then
>> find the modem remoteproc and write "start" into the state sysfs file.
>>
>> When we get a signal for termination we write "stop" into state to
>> stop
>> the remoteproc before exiting.
>>
>> There is however no way for us to indicate to the modem that rmtfs
>> just
>> died, e.g. a kill -9 on the process will result in the modem continue
>> and the next IO request will fail which in most cases will be fatal.
I have this scenario written down in the cover letter for this patchset
"[PATCH 0/2] Add character device interface to remoteproc"
I'll add it to the commit text as well.
>
> The modem will crash when attempting an IO while rmtfs is down?
>
>>
>> So instead having rmtfs holding /dev/rproc_foo open would upon its
>> termination cause the modem to be stopped automatically, and as the
>> system respawns rmtfs the modem would be started anew and the two
>> sides
>> would be synced up again.
>
> I have a better idea of what is going on now - thanks for writing this
> up.
>
> I would make this feature a kernel configurable option as some people
> may not want it. I also think having "/dev/remoteprocX" is fine, so
> no need to change anything currently visible in sysfs.
Ok. Makes sense.
>
> Mathieu
>
>>
>> [1] https://github.com/andersson/rmtfs
>>
>> Regards,
>> Bjorn
next prev parent reply other threads:[~2020-03-31 17:38 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-26 16:50 [PATCH 0/2] Add character device interface to remoteproc Rishabh Bhatnagar
2020-03-26 16:50 ` [PATCH 1/2] remoteproc: Add userspace char device driver Rishabh Bhatnagar
2020-03-26 17:37 ` Clément Leger
2020-03-28 0:09 ` rishabhb
2020-03-30 10:42 ` Clément Leger
2020-03-27 4:00 ` kbuild test robot
2020-03-30 22:12 ` Mathieu Poirier
2020-03-30 22:45 ` Bjorn Andersson
2020-03-31 16:47 ` Mathieu Poirier
2020-03-31 17:38 ` rishabhb [this message]
2020-03-31 17:55 ` Bjorn Andersson
2020-03-26 16:50 ` [PATCH 2/2] remoteproc: core: Register the character device interface Rishabh Bhatnagar
2020-03-30 22:13 ` Mathieu Poirier
-- strict thread matches above, loose matches on Subject: below --
2020-03-20 23:36 [PATCH 0/2] Add character device interface to remoteproc Rishabh Bhatnagar
2020-03-20 23:36 ` [PATCH 1/2] remoteproc: Add userspace char device driver Rishabh Bhatnagar
2020-03-25 4:10 ` Bjorn Andersson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=30f8b41df8831d19ce6efd0a28862708@codeaurora.org \
--to=rishabhb@codeaurora.org \
--cc=bjorn.andersson@linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-remoteproc-owner@vger.kernel.org \
--cc=linux-remoteproc@vger.kernel.org \
--cc=mathieu.poirier@linaro.org \
--cc=ohad@wizery.com \
--cc=psodagud@codeaurora.org \
--cc=sidgup@codeaurora.org \
--cc=tsoni@codeaurora.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).