On Saturday 07 October 2006 01:04, Nic James Ferrier wrote:
> Blaisorblade <[EMAIL PROTECTED]> writes:
> >> Is there a reason that can't be done?
> >
> > No, it can be done. Produce a working patch and then that will be ok, and
> > I guess it will be appreciated (at least by me). It isn't IMHO that much
> > trivial however - the code will be probably be similar but not identical
> > to the userspace equivalent (using getdents()).
>
> Here's my current attempt. It doesn't work reliably yet and I'm not
> sure why:

That is not a bad attempt, and you show having some uncommon experience with 
kernel code. I'm not reviewing it line-by-line - I should check the APIs of 
VFS calls and usage examples, but I haven't the time.

I can only suggest reading the code of sys_getdents as a starting point. I 
think you could use without problems vfs_readdir instead of ->readdir since 
it accepts a filldir_t callback and does the needed locking you do not do.

You very likely race with process creation and exit - as /proc/<pid> folders 
appear and disappear you need some kind of locking.

In general, as you can see, we tried writing mconsole_proc() with direct 
access to VFS, but it is commented out and we now use sys_open() and friends, 
because that one was not reliable; you could try that for a first version do 
that too (save this version for future use), even it if looks as _this_ 
operation is simpler to reimplement (if you call vfs_readdir it is).

To show you what was happening there, after some time, Al Viro suggested that 
we were using them wrongly, for refcount issues:

        nd.mnt = NULL;

should be replaced by something as nd.mnt=mntget(proc_mountpoint) - in fact 
there is a corresponding mntput() done by the function we called, and that 
mntput() at the end freed the proc mountpoint (there was a similar issue with 
nd.dentry IIRC). The result was that after about 5 calls to mconsole proc 
(i.e. after removing all references to the proc mountpoint and freeing it), 
UML crashed.
PS: this is a recollection of ideas about the crash, but the fix was never 
completed so consider it as a reasonable approximation of actual problems.

>    @@ -209,19 +209,49 @@
>     }
>     #endif
>
>    +int mconsole_proc_filldir(void * __buf, const char * name, int namlen,
> loff_t offset, +                          ino_t ino, unsigned int d_type)
>    +{
>    +        struct mc_request *req = (struct mc_request *) __buf;
>    +        char buf[namlen+1];
>    +        memcpy(buf, name, namlen);
>    +        buf[namlen] = '\n';
>    +        mconsole_reply_len(req, buf, namlen +1, 0, 1);
>    +        return 0;
>    +}
>    +
>     void mconsole_proc(struct mc_request *req)
>     {
>            char path[64];
>    -  char *buf;
>    +  char *buf = NULL;
>            int len;
>            int fd;
>            int first_chunk = 1;
>    +        struct file *file;
>            char *ptr = req->request.data;
>
>            ptr += strlen("proc");
>            while(isspace(*ptr)) ptr++;
>            snprintf(path, sizeof(path), "/proc/%s", ptr);
>
>    +        printk("path: %s\n", path);
>    +
>    +        file = filp_open(path, 0, 0);
>    +
>    +        if (*file->f_op->readdir != NULL) {
>    +                int dir_result;
>    +                mconsole_reply(req, "\n", 0, 1);
>    +
>    +                do {
>    +                        dir_result = (*file->f_op->readdir)(file, req,
> mconsole_proc_filldir); +                }
>    +                while(dir_result > 0);
>    +                printk("dir_result: %d\n", dir_result);
>    +
>    +                mconsole_reply(req, "", 0, 0);
>    +                goto out;
>    +        }
>    +
>            fd = sys_open(path, 0, 0);
>            if (fd < 0) {
>                    mconsole_reply(req, "Failed to open file", 1, 0);
>    @@ -257,11 +287,12 @@
>            }
>
>      out_free:
>    -  kfree(buf);
>    +        kfree(buf);
>      out_close:
>            sys_close(fd);
>      out:
>    -  /* nothing */;
>    +        ;
>    +        //filp_close(file, NULL);
>     }
>
> I have yet to establish wny it doesn't work reliably.

-- 
Inform me of my mistakes, so I can keep imitating Homer Simpson's "Doh!".
Paolo Giarrusso, aka Blaisorblade
http://www.user-mode-linux.org/~blaisorblade
Chiacchiera con i tuoi amici in tempo reale! 
 http://it.yahoo.com/mail_it/foot/*http://it.messenger.yahoo.com 


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
User-mode-linux-user mailing list
User-mode-linux-user@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-user

Reply via email to