Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Mon, 30 Mar 2020 15:45:54 -0700
From:   Bjorn Andersson <bjorn.andersson@linaro.org>
To:     Mathieu Poirier <mathieu.poirier@linaro.org>
Cc:     Rishabh Bhatnagar <rishabhb@codeaurora.org>,
        linux-kernel@vger.kernel.org, linux-remoteproc@vger.kernel.org,
        ohad@wizery.com, psodagud@codeaurora.org, tsoni@codeaurora.org,
        sidgup@codeaurora.org
Subject: Re: [PATCH 1/2] remoteproc: Add userspace char device driver
Message-ID: <20200330224554.GD215915@minitux>
References: <1585241440-7572-1-git-send-email-rishabhb@codeaurora.org>
 <1585241440-7572-2-git-send-email-rishabhb@codeaurora.org>
 <20200330221245.GA17782@xps15>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20200330221245.GA17782@xps15>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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.

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.

[1] https://github.com/andersson/rmtfs

Regards,
Bjorn