All of lore.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@ftp.linux.org.uk>
To: Blaisorblade <blaisorblade@yahoo.it>
Cc: user-mode-linux-devel@lists.sourceforge.net,
	Al Viro <viro@zeniv.linux.org.uk>, Jeff Dike <jdike@addtoit.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [uml-devel] Re: [patch 1/2] Fixup symlink function pointers for hppfs [for 2.6.13]
Date: Wed, 21 Sep 2005 04:40:39 +0100	[thread overview]
Message-ID: <20050921034039.GV7992@ftp.linux.org.uk> (raw)
In-Reply-To: <200509181400.35765.blaisorblade@yahoo.it>

On Sun, Sep 18, 2005 at 02:00:35PM +0200, Blaisorblade wrote:
> Well, on this point I guess I'll need more help.

[snip]
Ugh.  What you need to do is
	* lock underlying directory (procfs one)
	* call lookup_one_len()
	* unlock
and be done with that.  And yes, ->d_revalidate() for your dentries should
call the underlying one *if* dentry is positive.  For negative ones you'll
obviously have to repeat lookup, so just return 0.

> > What the hell is going on with iget() calls, BTW? 
> 
> > Especially since all 
> > of them get the same inumber...  Looks completely broken.
> Why especially? You mean that ->lookup is not supposed to iget()? ext2 does 
> it, both for lookup and for fill_super.

Lookup is supposed to iget when there are useful inode numbers and a chance
to find something in icache.  You have neither...
 
> For the point of the same inumber...Argh... never realized how broken this 
> could be - until now. We're always reusing the *same* inode!
> 
> No idea, didn't write the code...
> 
> On using 0, in practice hostfs has been working almost perfectly (but 
> I'd underline *almost*) in the same way... I think it should be fixed but I 
> don't know how (we have an *intrusive* fix for hostfs).

Why bother them, anyway?  Just allocate a new inode upon ->lookup() and
have ->drop_inode = generic_delete_inode().

> However, since we often (not always) have the underlying procfs entry, maybe 
> we could reuse those inode numbers.

You want ->getattr() anyway, just call the underlying one or generic_fillattr()
if there's none (both for underlying inode).  That'll give you inumbers from
procfs among other things...

> Multiple lseek's giving one of the offsets is fully ok, but a corrupted offset 
> is not.
> 
> drivers/char/mem.c:memory_lseek()
>                         file->f_pos += offset;

... has
        down(&file->f_dentry->d_inode->i_sem);
in the very beginning.


-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server. Download
it for free - -and be entered to win a 42" plasma tv or your very own
Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

WARNING: multiple messages have this Message-ID (diff)
From: Al Viro <viro@ftp.linux.org.uk>
To: Blaisorblade <blaisorblade@yahoo.it>
Cc: user-mode-linux-devel@lists.sourceforge.net,
	Al Viro <viro@zeniv.linux.org.uk>, Jeff Dike <jdike@addtoit.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [uml-devel] Re: [patch 1/2] Fixup symlink function pointers for hppfs [for 2.6.13]
Date: Wed, 21 Sep 2005 04:40:39 +0100	[thread overview]
Message-ID: <20050921034039.GV7992@ftp.linux.org.uk> (raw)
In-Reply-To: <200509181400.35765.blaisorblade@yahoo.it>

On Sun, Sep 18, 2005 at 02:00:35PM +0200, Blaisorblade wrote:
> Well, on this point I guess I'll need more help.

[snip]
Ugh.  What you need to do is
	* lock underlying directory (procfs one)
	* call lookup_one_len()
	* unlock
and be done with that.  And yes, ->d_revalidate() for your dentries should
call the underlying one *if* dentry is positive.  For negative ones you'll
obviously have to repeat lookup, so just return 0.

> > What the hell is going on with iget() calls, BTW? 
> 
> > Especially since all 
> > of them get the same inumber...  Looks completely broken.
> Why especially? You mean that ->lookup is not supposed to iget()? ext2 does 
> it, both for lookup and for fill_super.

Lookup is supposed to iget when there are useful inode numbers and a chance
to find something in icache.  You have neither...
 
> For the point of the same inumber...Argh... never realized how broken this 
> could be - until now. We're always reusing the *same* inode!
> 
> No idea, didn't write the code...
> 
> On using 0, in practice hostfs has been working almost perfectly (but 
> I'd underline *almost*) in the same way... I think it should be fixed but I 
> don't know how (we have an *intrusive* fix for hostfs).

Why bother them, anyway?  Just allocate a new inode upon ->lookup() and
have ->drop_inode = generic_delete_inode().

> However, since we often (not always) have the underlying procfs entry, maybe 
> we could reuse those inode numbers.

You want ->getattr() anyway, just call the underlying one or generic_fillattr()
if there's none (both for underlying inode).  That'll give you inumbers from
procfs among other things...

> Multiple lseek's giving one of the offsets is fully ok, but a corrupted offset 
> is not.
> 
> drivers/char/mem.c:memory_lseek()
>                         file->f_pos += offset;

... has
        down(&file->f_dentry->d_inode->i_sem);
in the very beginning.

  parent reply	other threads:[~2005-09-21  3:40 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2005-08-26 14:57 [uml-devel] [patch 1/2] Fixup symlink function pointers for hppfs [for 2.6.13] blaisorblade
2005-08-26 14:57 ` blaisorblade
2005-08-26 19:03 ` [uml-devel] " Al Viro
2005-08-26 19:03   ` Al Viro
2005-08-26 20:04   ` [uml-devel] " Blaisorblade
2005-08-26 20:04     ` Blaisorblade
2005-08-26 21:48     ` [uml-devel] " Al Viro
2005-08-26 21:48       ` Al Viro
2005-09-18 12:00       ` [uml-devel] " Blaisorblade
2005-09-18 12:00         ` Blaisorblade
2005-09-21  3:23         ` Al Viro
2005-09-21  3:23           ` Al Viro
2005-09-21  3:40         ` Al Viro [this message]
2005-09-21  3:40           ` Al Viro

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=20050921034039.GV7992@ftp.linux.org.uk \
    --to=viro@ftp.linux.org.uk \
    --cc=blaisorblade@yahoo.it \
    --cc=jdike@addtoit.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=user-mode-linux-devel@lists.sourceforge.net \
    --cc=viro@zeniv.linux.org.uk \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.