The idea of the client implementation is to be able to use it not only
from the vfs layer, but also without it. For example, creating a
device that connect to a remote 9P file server.
Thanks,
Lucho
On Wed, Oct 8, 2008 at 3:09 PM, Eric Van Hensbergen <[email protected]> wrote:
> Currently reading a directory is implemented in the client code.
> This function is not actually a wire operation, but a meta operation
> which calls read operations and processes the results.
>
> This patch moves this functionality to the fs layer and calls component
> wire operations instead of constructing their packets. This provides a
> cleaner separation and will help when we reorganize the client functions
> and protocol processing methods.
>
> Signed-off-by: Eric Van Hensbergen <[email protected]>
> ---
> fs/9p/vfs_dir.c | 54 +++++++++++++++++-------
> include/net/9p/client.h | 5 --
> net/9p/client.c | 103 -----------------------------------------------
> 3 files changed, 38 insertions(+), 124 deletions(-)
>
> diff --git a/fs/9p/vfs_dir.c b/fs/9p/vfs_dir.c
> index e298fe1..d7d0ac5 100644
> --- a/fs/9p/vfs_dir.c
> +++ b/fs/9p/vfs_dir.c
> @@ -69,32 +69,54 @@ static inline int dt_type(struct p9_stat *mistat)
> static int v9fs_dir_readdir(struct file *filp, void *dirent, filldir_t filldir)
> {
> int over;
> + struct p9_stat st;
> + int err;
> struct p9_fid *fid;
> - struct v9fs_session_info *v9ses;
> - struct inode *inode;
> - struct p9_stat *st;
> + int buflen;
> + char *statbuf;
> + int n, i = 0;
>
> P9_DPRINTK(P9_DEBUG_VFS, "name %s\n", filp->f_path.dentry->d_name.name);
> - inode = filp->f_path.dentry->d_inode;
> - v9ses = v9fs_inode2v9ses(inode);
> fid = filp->private_data;
> - while ((st = p9_client_dirread(fid, filp->f_pos)) != NULL) {
> - if (IS_ERR(st))
> - return PTR_ERR(st);
>
> - over = filldir(dirent, st->name.str, st->name.len, filp->f_pos,
> - v9fs_qid2ino(&st->qid), dt_type(st));
> + buflen = fid->clnt->msize - P9_IOHDRSZ;
> + statbuf = kmalloc(buflen, GFP_KERNEL);
> + if (!statbuf)
> + return -ENOMEM;
>
> - if (over)
> + while (1) {
> + err = v9fs_file_readn(filp, statbuf, NULL, fid->rdir_fpos,
> + buflen);
> + if (err <= 0)
> break;
>
> - filp->f_pos += st->size;
> - kfree(st);
> - st = NULL;
> + n = err;
> + while (i < n) {
> + err = p9_deserialize_stat(statbuf + i, buflen-i, &st,
> + fid->clnt->dotu);
> + if (!err) {
> + err = -EIO;
> + goto free_and_exit;
> + }
> +
> + i += err;
> + fid->rdir_fpos += err;
> +
> + over = filldir(dirent, st.name.str, st.name.len,
> + filp->f_pos, v9fs_qid2ino(&st.qid), dt_type(&st));
> +
> + filp->f_pos += st.size;
> +
> + if (over) {
> + err = 0;
> + goto free_and_exit;
> + }
> + }
> }
>
> - kfree(st);
> - return 0;
> +free_and_exit:
> + kfree(statbuf);
> + return err;
> }
>
>
> diff --git a/include/net/9p/client.h b/include/net/9p/client.h
> index bb8b0ed..eeb7d92 100644
> --- a/include/net/9p/client.h
> +++ b/include/net/9p/client.h
> @@ -163,8 +163,6 @@ struct p9_client {
> * @uid: the numeric uid of the local user who owns this handle
> * @aux: transport specific information (unused?)
> * @rdir_fpos: tracks offset of file position when reading directory contents
> - * @rdir_pos: (unused?)
> - * @rdir_fcall: holds response of last directory read request
> * @flist: per-client-instance fid tracking
> * @dlist: per-dentry fid tracking
> *
> @@ -181,8 +179,6 @@ struct p9_fid {
> void *aux;
>
> int rdir_fpos;
> - int rdir_pos;
> - struct p9_fcall *rdir_fcall;
> struct list_head flist;
> struct list_head dlist; /* list of all fids attached to a dentry */
> };
> @@ -207,7 +203,6 @@ int p9_client_write(struct p9_fid *fid, char *data, const char __user *udata,
> u64 offset, u32 count);
> struct p9_stat *p9_client_stat(struct p9_fid *fid);
> int p9_client_wstat(struct p9_fid *fid, struct p9_wstat *wst);
> -struct p9_stat *p9_client_dirread(struct p9_fid *fid, u64 offset);
>
> struct p9_req_t *p9_tag_lookup(struct p9_client *, u16);
> void p9_client_cb(struct p9_client *c, struct p9_req_t *req);
> diff --git a/net/9p/client.c b/net/9p/client.c
> index 89f877f..aa71ff3 100644
> --- a/net/9p/client.c
> +++ b/net/9p/client.c
> @@ -559,8 +559,6 @@ static struct p9_fid *p9_fid_create(struct p9_client *clnt)
> memset(&fid->qid, 0, sizeof(struct p9_qid));
> fid->mode = -1;
> fid->rdir_fpos = 0;
> - fid->rdir_pos = 0;
> - fid->rdir_fcall = NULL;
> fid->uid = current->fsuid;
> fid->clnt = clnt;
> fid->aux = NULL;
> @@ -586,7 +584,6 @@ static void p9_fid_destroy(struct p9_fid *fid)
> spin_lock(&clnt->lock);
> list_del(&fid->flist);
> spin_unlock(&clnt->lock);
> - kfree(fid->rdir_fcall);
> kfree(fid);
> }
>
> @@ -1261,103 +1258,3 @@ done:
> return err;
> }
> EXPORT_SYMBOL(p9_client_wstat);
> -
> -struct p9_stat *p9_client_dirread(struct p9_fid *fid, u64 offset)
> -{
> - int err, n, m;
> - struct p9_fcall *tc, *rc;
> - struct p9_client *clnt;
> - struct p9_stat st, *ret;
> -
> - P9_DPRINTK(P9_DEBUG_9P, "fid %d offset %llu\n", fid->fid,
> - (long long unsigned) offset);
> - err = 0;
> - tc = NULL;
> - rc = NULL;
> - ret = NULL;
> - clnt = fid->clnt;
> -
> - /* if the offset is below or above the current response, free it */
> - if (offset < fid->rdir_fpos || (fid->rdir_fcall &&
> - offset >= fid->rdir_fpos+fid->rdir_fcall->params.rread.count)) {
> - fid->rdir_pos = 0;
> - if (fid->rdir_fcall)
> - fid->rdir_fpos += fid->rdir_fcall->params.rread.count;
> -
> - kfree(fid->rdir_fcall);
> - fid->rdir_fcall = NULL;
> - if (offset < fid->rdir_fpos)
> - fid->rdir_fpos = 0;
> - }
> -
> - if (!fid->rdir_fcall) {
> - n = fid->iounit;
> - if (!n || n > clnt->msize-P9_IOHDRSZ)
> - n = clnt->msize - P9_IOHDRSZ;
> -
> - while (1) {
> - if (fid->rdir_fcall) {
> - fid->rdir_fpos +=
> - fid->rdir_fcall->params.rread.count;
> - kfree(fid->rdir_fcall);
> - fid->rdir_fcall = NULL;
> - }
> -
> - tc = p9_create_tread(fid->fid, fid->rdir_fpos, n);
> - if (IS_ERR(tc)) {
> - err = PTR_ERR(tc);
> - tc = NULL;
> - goto error;
> - }
> -
> - err = p9_client_rpc(clnt, tc, &rc);
> - if (err)
> - goto error;
> -
> - n = rc->params.rread.count;
> - if (n == 0)
> - goto done;
> -
> - fid->rdir_fcall = rc;
> - rc = NULL;
> - if (offset >= fid->rdir_fpos &&
> - offset < fid->rdir_fpos+n)
> - break;
> - }
> -
> - fid->rdir_pos = 0;
> - }
> -
> - m = offset - fid->rdir_fpos;
> - if (m < 0)
> - goto done;
> -
> - n = p9_deserialize_stat(fid->rdir_fcall->params.rread.data + m,
> - fid->rdir_fcall->params.rread.count - m, &st, clnt->dotu);
> -
> - if (!n) {
> - err = -EIO;
> - goto error;
> - }
> -
> - fid->rdir_pos += n;
> - st.size = n;
> - ret = p9_clone_stat(&st, clnt->dotu);
> - if (IS_ERR(ret)) {
> - err = PTR_ERR(ret);
> - ret = NULL;
> - goto error;
> - }
> -
> -done:
> - kfree(tc);
> - kfree(rc);
> - return ret;
> -
> -error:
> - kfree(tc);
> - kfree(rc);
> - kfree(ret);
> - return ERR_PTR(err);
> -}
> -EXPORT_SYMBOL(p9_client_dirread);
> --
> 1.5.4.3
>
>
> -------------------------------------------------------------------------
> This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
> Build the coolest Linux based applications with Moblin SDK & win great prizes
> Grand prize is a trip for two to an Open Source event anywhere in the world
> http://moblin-contest.org/redirect.php?banner_id=100&url=/
> _______________________________________________
> V9fs-developer mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/v9fs-developer
>
On Wed, Oct 8, 2008 at 5:10 PM, Latchesar Ionkov <[email protected]> wrote:
> The idea of the client implementation is to be able to use it not only
> from the vfs layer, but also without it. For example, creating a
> device that connect to a remote 9P file server.
>
That does make sense in principal -- although I'm not sure filldir
structures are the easiest things to work with either. The main idea
was to restrict the client.c code to wire operations, primarily
torwards the idea of treating the "client" as a proxy server once the
in-kernel-server code goes in (hopefully in 2.6.29).
Of course the other factor is that this patch-set is gearing up
towards an alternate extended-POSIX-support approach (what I've been
referring to as .L) -- that extension will include a wire operation
which more closely matches linux's concept of dirread (just grab the
files, not the associated stat) -- but whether or not that is useful
will largely depend on what sort of remote server you are accessing.
The Plan 9 dirread is one of those special cases where even though its
not a wire operation, it needs some intimate understanding of the
underlying protocol (at least how stats get marshalled) -- in some
ways if it were a client operation, it might make more sense to pass
it an array of p9_stat structs to populate with the results.
Do you have some code which used the old client interface I could look
at or were you just thinking towards the future?
-eric
I don't think that there is anything at the moment that uses dirread,
but the remote framebuffer that Abishek is working on uses some other
of the client operations.
Thanks,
Lucho
On Wed, Oct 8, 2008 at 5:12 PM, Eric Van Hensbergen <[email protected]> wrote:
> On Wed, Oct 8, 2008 at 5:10 PM, Latchesar Ionkov <[email protected]> wrote:
>> The idea of the client implementation is to be able to use it not only
>> from the vfs layer, but also without it. For example, creating a
>> device that connect to a remote 9P file server.
>>
>
> That does make sense in principal -- although I'm not sure filldir
> structures are the easiest things to work with either. The main idea
> was to restrict the client.c code to wire operations, primarily
> torwards the idea of treating the "client" as a proxy server once the
> in-kernel-server code goes in (hopefully in 2.6.29).
> Of course the other factor is that this patch-set is gearing up
> towards an alternate extended-POSIX-support approach (what I've been
> referring to as .L) -- that extension will include a wire operation
> which more closely matches linux's concept of dirread (just grab the
> files, not the associated stat) -- but whether or not that is useful
> will largely depend on what sort of remote server you are accessing.
>
> The Plan 9 dirread is one of those special cases where even though its
> not a wire operation, it needs some intimate understanding of the
> underlying protocol (at least how stats get marshalled) -- in some
> ways if it were a client operation, it might make more sense to pass
> it an array of p9_stat structs to populate with the results.
>
> Do you have some code which used the old client interface I could look
> at or were you just thinking towards the future?
>
> -eric
>
On Thu, Oct 9, 2008 at 8:40 AM, Latchesar Ionkov <[email protected]> wrote:
> I don't think that there is anything at the moment that uses dirread,
> but the remote framebuffer that Abishek is working on uses some other
> of the client operations.
>
Ah -- okay, I probably need to sit down down with him over email and
figure out what he's using and what he needs -- the client functions
interfaces haven't changed, but some of the ways in which reads and
writes work has which might mess him up...
-eric