All of lore.kernel.org
 help / color / mirror / Atom feed
* Important for fs devs: rcu-walk merged upstream
@ 2011-01-08  2:54 Nick Piggin
  2011-01-08  9:10 ` Marco Stornelli
                   ` (3 more replies)
  0 siblings, 4 replies; 21+ messages in thread
From: Nick Piggin @ 2011-01-08  2:54 UTC (permalink / raw)
  To: linux-fsdevel, linux-kernel

The vfs-scale branch is now upstream. If you haven't
looked yet, your filesystem is likely to have been
touched, so check it out.

Also look at Documentation/filesystems/porting and
path-lookup.txt.

The dcache_lock stuff should have been all done for you
(for in-tree filesystems, I can help out of tree fses with
conversions there if you ping me offline).

The rcu-walk stuff can be more tricky for your filesystem
to take advantage of.

If you supply a .d_revalidate, .permission, or .check_acl,
then path walking is going to be slow and unscalable for
you.

Out of tree filesystems: you _have_ to at least add a line
of code to the above functions in order to specify that
you don't want to participate in rcu-walk.

Otherwise, you don't have to care about rcu-walk if you
have a legacy or special filesystem like configfs then I'd
advise against anything fancy. But if you have a
userbase and you expect them to actually do any path
lookups into your filesystem, please take a look.

This is a big and complex change by any measure, so
please don't be afraid to ask for help or clarification. I'd
also really like to be able to update documentation
based on questions from fs maintainers (in and out of
tree) who are trying to follow it and bring their code up to
speed.

Thanks,
Nick

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: Important for fs devs: rcu-walk merged upstream
  2011-01-08  2:54 Important for fs devs: rcu-walk merged upstream Nick Piggin
@ 2011-01-08  9:10 ` Marco Stornelli
  2011-01-11  9:57     ` Nick Piggin
  2011-01-10 14:38 ` J. R. Okajima
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 21+ messages in thread
From: Marco Stornelli @ 2011-01-08  9:10 UTC (permalink / raw)
  To: Nick Piggin; +Cc: linux-fsdevel, linux-kernel

Il 08/01/2011 03:54, Nick Piggin ha scritto:
> The vfs-scale branch is now upstream. If you haven't
> looked yet, your filesystem is likely to have been
> touched, so check it out.
> 
> Also look at Documentation/filesystems/porting and
> path-lookup.txt.

Nick, in the porting file I read:

..........hazards on dentries and inodes (see
Documentation/filesystems/path-walk.txt). ..........
 ^^^^^^^^^^^^^

Should it be "path-lookup.txt"?

Marco

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: Important for fs devs: rcu-walk merged upstream
  2011-01-08  2:54 Important for fs devs: rcu-walk merged upstream Nick Piggin
  2011-01-08  9:10 ` Marco Stornelli
@ 2011-01-10 14:38 ` J. R. Okajima
  2011-01-11 12:57     ` Nick Piggin
  2011-01-11  1:08 ` Joel Becker
  2011-01-13 14:58 ` Miklos Szeredi
  3 siblings, 1 reply; 21+ messages in thread
From: J. R. Okajima @ 2011-01-10 14:38 UTC (permalink / raw)
  To: Nick Piggin; +Cc: linux-fsdevel, linux-kernel


Nick Piggin:
> This is a big and complex change by any measure, so
> please don't be afraid to ask for help or clarification. I'd
> also really like to be able to update documentation
> based on questions from fs maintainers (in and out of
> tree) who are trying to follow it and bring their code up to
> speed.

Question about what d_lock protects.
Can we skip d_lock when we access d_inode and d_name during its parent
i_mutex is held?
Should these BUG_ON be placed after d_lock?

void dentry_update_name_case(struct dentry *dentry, struct qstr *name)
{
	BUG_ON(!mutex_is_locked(&dentry->d_inode->i_mutex));
	BUG_ON(dentry->d_name.len != name->len); /* d_lookup gives this */

	spin_lock(&dentry->d_lock);
	:::


J. R. Okajima

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: Important for fs devs: rcu-walk merged upstream
  2011-01-08  2:54 Important for fs devs: rcu-walk merged upstream Nick Piggin
  2011-01-08  9:10 ` Marco Stornelli
  2011-01-10 14:38 ` J. R. Okajima
@ 2011-01-11  1:08 ` Joel Becker
  2011-01-11 13:06     ` Nick Piggin
  2011-01-13 14:58 ` Miklos Szeredi
  3 siblings, 1 reply; 21+ messages in thread
From: Joel Becker @ 2011-01-11  1:08 UTC (permalink / raw)
  To: Nick Piggin; +Cc: linux-fsdevel, linux-kernel

On Sat, Jan 08, 2011 at 01:54:47PM +1100, Nick Piggin wrote:
> The rcu-walk stuff can be more tricky for your filesystem
> to take advantage of.
> 
> If you supply a .d_revalidate, .permission, or .check_acl,
> then path walking is going to be slow and unscalable for
> you.

	Do you mean "as slow and unscalable as it has always been" or
"even slower now"?  A quick look suggests the former, but I wanted to be
sure.

Joel

-- 

"But all my words come back to me
 In shades of mediocrity.
 Like emptiness in harmony
 I need someone to comfort me."

			http://www.jlbec.org/
			jlbec@evilplan.org

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: Important for fs devs: rcu-walk merged upstream
  2011-01-08  9:10 ` Marco Stornelli
@ 2011-01-11  9:57     ` Nick Piggin
  0 siblings, 0 replies; 21+ messages in thread
From: Nick Piggin @ 2011-01-11  9:57 UTC (permalink / raw)
  To: Marco Stornelli; +Cc: linux-fsdevel, linux-kernel

On Saturday, 8 January 2011, Marco Stornelli <marco.stornelli@gmail.com> wrote:
> Il 08/01/2011 03:54, Nick Piggin ha scritto:
>> The vfs-scale branch is now upstream. If you haven't
>> looked yet, your filesystem is likely to have been
>> touched, so check it out.
>>
>> Also look at Documentation/filesystems/porting and
>> path-lookup.txt.
>
> Nick, in the porting file I read:
>
> ..........hazards on dentries and inodes (see
> Documentation/filesystems/path-walk.txt). ..........
>  ^^^^^^^^^^^^^
>
> Should it be "path-lookup.txt"?
>
> Marco
>

Hi, yes thanks I'll fix it

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: Important for fs devs: rcu-walk merged upstream
@ 2011-01-11  9:57     ` Nick Piggin
  0 siblings, 0 replies; 21+ messages in thread
From: Nick Piggin @ 2011-01-11  9:57 UTC (permalink / raw)
  To: Marco Stornelli; +Cc: linux-fsdevel, linux-kernel

On Saturday, 8 January 2011, Marco Stornelli <marco.stornelli@gmail.com> wrote:
> Il 08/01/2011 03:54, Nick Piggin ha scritto:
>> The vfs-scale branch is now upstream. If you haven't
>> looked yet, your filesystem is likely to have been
>> touched, so check it out.
>>
>> Also look at Documentation/filesystems/porting and
>> path-lookup.txt.
>
> Nick, in the porting file I read:
>
> ..........hazards on dentries and inodes (see
> Documentation/filesystems/path-walk.txt). ..........
>  ^^^^^^^^^^^^^
>
> Should it be "path-lookup.txt"?
>
> Marco
>

Hi, yes thanks I'll fix it
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: Important for fs devs: rcu-walk merged upstream
  2011-01-11  9:57     ` Nick Piggin
  (?)
@ 2011-01-11 11:08     ` Miklos Szeredi
  -1 siblings, 0 replies; 21+ messages in thread
From: Miklos Szeredi @ 2011-01-11 11:08 UTC (permalink / raw)
  To: Nick Piggin; +Cc: marco.stornelli, linux-fsdevel, linux-kernel

On Tue, 11 Jan 2011, Nick Piggin wrote:
> On Saturday, 8 January 2011, Marco Stornelli <marco.stornelli@gmail.com> wrote:
> > Il 08/01/2011 03:54, Nick Piggin ha scritto:
> >> The vfs-scale branch is now upstream. If you haven't
> >> looked yet, your filesystem is likely to have been
> >> touched, so check it out.
> >>
> >> Also look at Documentation/filesystems/porting and
> >> path-lookup.txt.
> >
> > Nick, in the porting file I read:
> >
> > ..........hazards on dentries and inodes (see
> > Documentation/filesystems/path-walk.txt). ..........
> >  ^^^^^^^^^^^^^
> >
> > Should it be "path-lookup.txt"?
> >
> > Marco
> >
> 
> Hi, yes thanks I'll fix it

Also you use IPERM_RCU instead of IPERM_FLAG_RCU in the documentation.

Thanks,
Miklos


diff --git a/Documentation/filesystems/porting b/Documentation/filesystems/porting
index 07a32b4..c2819ee 100644
--- a/Documentation/filesystems/porting
+++ b/Documentation/filesystems/porting
@@ -383,5 +383,5 @@ Documentation/filesystems/vfs.txt for more details.
 
 	permission and check_acl are inode permission checks that are called
 on many or all directory inodes on the way down a path walk (to check for
-exec permission). These must now be rcu-walk aware (flags & IPERM_RCU). See
-Documentation/filesystems/vfs.txt for more details.
+exec permission). These must now be rcu-walk aware (flags & IPERM_FLAG_RCU).
+See Documentation/filesystems/vfs.txt for more details.
diff --git a/Documentation/filesystems/vfs.txt b/Documentation/filesystems/vfs.txt
index fbb324e..de8a967 100644
--- a/Documentation/filesystems/vfs.txt
+++ b/Documentation/filesystems/vfs.txt
@@ -415,9 +415,9 @@ otherwise noted.
   permission: called by the VFS to check for access rights on a POSIX-like
   	filesystem.
 
-	May be called in rcu-walk mode (flags & IPERM_RCU). If in rcu-walk
-	mode, the filesystem must check the permission without blocking or
-	storing to the inode.
+	May be called in rcu-walk mode (flags & IPERM_FLAG_RCU). If in
+	rcu-walk mode, the filesystem must check the permission without
+	blocking or storing to the inode.
 
 	If a situation is encountered that rcu-walk cannot handle, return
 	-ECHILD and it will be called again in ref-walk mode.
5B5B

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: Important for fs devs: rcu-walk merged upstream
  2011-01-10 14:38 ` J. R. Okajima
@ 2011-01-11 12:57     ` Nick Piggin
  0 siblings, 0 replies; 21+ messages in thread
From: Nick Piggin @ 2011-01-11 12:57 UTC (permalink / raw)
  To: J. R. Okajima; +Cc: linux-fsdevel, linux-kernel

On Tuesday, January 11, 2011, J. R. Okajima <hooanon05@yahoo.co.jp> wrote:
>
> Nick Piggin:
>> This is a big and complex change by any measure, so
>> please don't be afraid to ask for help or clarification. I'd
>> also really like to be able to update documentation
>> based on questions from fs maintainers (in and out of
>> tree) who are trying to follow it and bring their code up to
>> speed.
>
> Question about what d_lock protects.
> Can we skip d_lock when we access d_inode and d_name during its parent
> i_mutex is held?

That is a good observation. I think we are ok here because parent
mutex should stabilize children names and linkages.

But the documentation for a lot of locking is not complete. It would
be nice to improve.


> Should these BUG_ON be placed after d_lock?
>
> void dentry_update_name_case(struct dentry *dentry, struct qstr *name)
> {
>         BUG_ON(!mutex_is_locked(&dentry->d_inode->i_mutex));
>         BUG_ON(dentry->d_name.len != name->len); /* d_lookup gives this */
>
>         spin_lock(&dentry->d_lock);
>         :::
>
>
> J. R. Okajima
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: Important for fs devs: rcu-walk merged upstream
@ 2011-01-11 12:57     ` Nick Piggin
  0 siblings, 0 replies; 21+ messages in thread
From: Nick Piggin @ 2011-01-11 12:57 UTC (permalink / raw)
  To: J. R. Okajima; +Cc: linux-fsdevel, linux-kernel

On Tuesday, January 11, 2011, J. R. Okajima <hooanon05@yahoo.co.jp> wrote:
>
> Nick Piggin:
>> This is a big and complex change by any measure, so
>> please don't be afraid to ask for help or clarification. I'd
>> also really like to be able to update documentation
>> based on questions from fs maintainers (in and out of
>> tree) who are trying to follow it and bring their code up to
>> speed.
>
> Question about what d_lock protects.
> Can we skip d_lock when we access d_inode and d_name during its parent
> i_mutex is held?

That is a good observation. I think we are ok here because parent
mutex should stabilize children names and linkages.

But the documentation for a lot of locking is not complete. It would
be nice to improve.


> Should these BUG_ON be placed after d_lock?
>
> void dentry_update_name_case(struct dentry *dentry, struct qstr *name)
> {
>         BUG_ON(!mutex_is_locked(&dentry->d_inode->i_mutex));
>         BUG_ON(dentry->d_name.len != name->len); /* d_lookup gives this */
>
>         spin_lock(&dentry->d_lock);
>         :::
>
>
> J. R. Okajima
>
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: Important for fs devs: rcu-walk merged upstream
  2011-01-11  1:08 ` Joel Becker
@ 2011-01-11 13:06     ` Nick Piggin
  0 siblings, 0 replies; 21+ messages in thread
From: Nick Piggin @ 2011-01-11 13:06 UTC (permalink / raw)
  To: Nick Piggin, linux-fsdevel, linux-kernel

On Tuesday, January 11, 2011, Joel Becker <jlbec@evilplan.org> wrote:
> On Sat, Jan 08, 2011 at 01:54:47PM +1100, Nick Piggin wrote:
>> The rcu-walk stuff can be more tricky for your filesystem
>> to take advantage of.
>>
>> If you supply a .d_revalidate, .permission, or .check_acl,
>> then path walking is going to be slow and unscalable for
>> you.
>
>         Do you mean "as slow and unscalable as it has always been" or
> "even slower now"?  A quick look suggests the former, but I wanted to be
> sure.

Yeah it should be about the same. Bit more complex code so it might be
a bit slower. A few code and branch and cache improvements so it might
be a bit faster. Refcounts on the same dentry become a bit less
scalable for using lock instead of atomic. On the other hand dropping
the ref on leaf denty doesn't require global lock.

It should not be urgent for 2.6.38.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: Important for fs devs: rcu-walk merged upstream
@ 2011-01-11 13:06     ` Nick Piggin
  0 siblings, 0 replies; 21+ messages in thread
From: Nick Piggin @ 2011-01-11 13:06 UTC (permalink / raw)
  To: Nick Piggin, linux-fsdevel, linux-kernel

On Tuesday, January 11, 2011, Joel Becker <jlbec@evilplan.org> wrote:
> On Sat, Jan 08, 2011 at 01:54:47PM +1100, Nick Piggin wrote:
>> The rcu-walk stuff can be more tricky for your filesystem
>> to take advantage of.
>>
>> If you supply a .d_revalidate, .permission, or .check_acl,
>> then path walking is going to be slow and unscalable for
>> you.
>
>         Do you mean "as slow and unscalable as it has always been" or
> "even slower now"?  A quick look suggests the former, but I wanted to be
> sure.

Yeah it should be about the same. Bit more complex code so it might be
a bit slower. A few code and branch and cache improvements so it might
be a bit faster. Refcounts on the same dentry become a bit less
scalable for using lock instead of atomic. On the other hand dropping
the ref on leaf denty doesn't require global lock.

It should not be urgent for 2.6.38.
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: Important for fs devs: rcu-walk merged upstream
  2011-01-08  2:54 Important for fs devs: rcu-walk merged upstream Nick Piggin
                   ` (2 preceding siblings ...)
  2011-01-11  1:08 ` Joel Becker
@ 2011-01-13 14:58 ` Miklos Szeredi
  2011-01-13 16:18   ` Miklos Szeredi
  3 siblings, 1 reply; 21+ messages in thread
From: Miklos Szeredi @ 2011-01-13 14:58 UTC (permalink / raw)
  To: Nick Piggin; +Cc: linux-fsdevel, linux-kernel

On Sat, 8 Jan 2011, Nick Piggin wrote:
> The vfs-scale branch is now upstream. If you haven't
> looked yet, your filesystem is likely to have been
> touched, so check it out.
> 
> Also look at Documentation/filesystems/porting and
> path-lookup.txt.
> 
> The dcache_lock stuff should have been all done for you
> (for in-tree filesystems, I can help out of tree fses with
> conversions there if you ping me offline).
> 
> The rcu-walk stuff can be more tricky for your filesystem
> to take advantage of.
> 
> If you supply a .d_revalidate, .permission, or .check_acl,
> then path walking is going to be slow and unscalable for
> you.
> 
> Out of tree filesystems: you _have_ to at least add a line
> of code to the above functions in order to specify that
> you don't want to participate in rcu-walk.
> 
> Otherwise, you don't have to care about rcu-walk if you
> have a legacy or special filesystem like configfs then I'd
> advise against anything fancy. But if you have a
> userbase and you expect them to actually do any path
> lookups into your filesystem, please take a look.

One other thing: I know ECHILD is safe since no sane filesystem will
return it in its permission or revalidate callbacks, and even if it
does that's just a loss of optimization.

But it would be better from a readability and grepability standpoint
if it was a kernel internal errno (ERESTARTNORCU or whatever)
similarly to ERESTARTSYS and the like.

Thanks,
Miklos

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: Important for fs devs: rcu-walk merged upstream
  2011-01-13 14:58 ` Miklos Szeredi
@ 2011-01-13 16:18   ` Miklos Szeredi
  2011-01-14  1:21       ` Nick Piggin
  0 siblings, 1 reply; 21+ messages in thread
From: Miklos Szeredi @ 2011-01-13 16:18 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: npiggin, linux-fsdevel, linux-kernel

On Thu, 13 Jan 2011, Miklos Szeredi wrote:
> On Sat, 8 Jan 2011, Nick Piggin wrote:
> > The vfs-scale branch is now upstream. If you haven't
> > looked yet, your filesystem is likely to have been
> > touched, so check it out.
> > 
> > Also look at Documentation/filesystems/porting and
> > path-lookup.txt.
> > 
> > The dcache_lock stuff should have been all done for you
> > (for in-tree filesystems, I can help out of tree fses with
> > conversions there if you ping me offline).
> > 
> > The rcu-walk stuff can be more tricky for your filesystem
> > to take advantage of.
> > 
> > If you supply a .d_revalidate, .permission, or .check_acl,
> > then path walking is going to be slow and unscalable for
> > you.
> > 
> > Out of tree filesystems: you _have_ to at least add a line
> > of code to the above functions in order to specify that
> > you don't want to participate in rcu-walk.
> > 
> > Otherwise, you don't have to care about rcu-walk if you
> > have a legacy or special filesystem like configfs then I'd
> > advise against anything fancy. But if you have a
> > userbase and you expect them to actually do any path
> > lookups into your filesystem, please take a look.
> 
> One other thing: I know ECHILD is safe since no sane filesystem will
> return it in its permission or revalidate callbacks, and even if it
> does that's just a loss of optimization.

And it's not entirely safe either.  A fuse filesystem returning ECHILD
would make nameidata_dentry_drop_rcu() to BUG.  So some sort of errno
filter is necessary in the fuse kernel module.

Thanks,
Miklos

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: Important for fs devs: rcu-walk merged upstream
  2011-01-13 16:18   ` Miklos Szeredi
@ 2011-01-14  1:21       ` Nick Piggin
  0 siblings, 0 replies; 21+ messages in thread
From: Nick Piggin @ 2011-01-14  1:21 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: linux-fsdevel, linux-kernel

On Fri, Jan 14, 2011 at 3:18 AM, Miklos Szeredi <miklos@szeredi.hu> wrote:
> On Thu, 13 Jan 2011, Miklos Szeredi wrote:
>> On Sat, 8 Jan 2011, Nick Piggin wrote:
>> > The vfs-scale branch is now upstream. If you haven't
>> > looked yet, your filesystem is likely to have been
>> > touched, so check it out.
>> >
>> > Also look at Documentation/filesystems/porting and
>> > path-lookup.txt.
>> >
>> > The dcache_lock stuff should have been all done for you
>> > (for in-tree filesystems, I can help out of tree fses with
>> > conversions there if you ping me offline).
>> >
>> > The rcu-walk stuff can be more tricky for your filesystem
>> > to take advantage of.
>> >
>> > If you supply a .d_revalidate, .permission, or .check_acl,
>> > then path walking is going to be slow and unscalable for
>> > you.
>> >
>> > Out of tree filesystems: you _have_ to at least add a line
>> > of code to the above functions in order to specify that
>> > you don't want to participate in rcu-walk.
>> >
>> > Otherwise, you don't have to care about rcu-walk if you
>> > have a legacy or special filesystem like configfs then I'd
>> > advise against anything fancy. But if you have a
>> > userbase and you expect them to actually do any path
>> > lookups into your filesystem, please take a look.
>>
>> One other thing: I know ECHILD is safe since no sane filesystem will
>> return it in its permission or revalidate callbacks, and even if it
>> does that's just a loss of optimization.
>
> And it's not entirely safe either.  A fuse filesystem returning ECHILD
> would make nameidata_dentry_drop_rcu() to BUG.  So some sort of errno
> filter is necessary in the fuse kernel module.

Surely you'd need some filtering anyway? I don't think any function
involving path lookup could sanely return -ECHILD.

That said, it probably is a good idea to have a new errno.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: Important for fs devs: rcu-walk merged upstream
@ 2011-01-14  1:21       ` Nick Piggin
  0 siblings, 0 replies; 21+ messages in thread
From: Nick Piggin @ 2011-01-14  1:21 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: linux-fsdevel, linux-kernel

On Fri, Jan 14, 2011 at 3:18 AM, Miklos Szeredi <miklos@szeredi.hu> wrote:
> On Thu, 13 Jan 2011, Miklos Szeredi wrote:
>> On Sat, 8 Jan 2011, Nick Piggin wrote:
>> > The vfs-scale branch is now upstream. If you haven't
>> > looked yet, your filesystem is likely to have been
>> > touched, so check it out.
>> >
>> > Also look at Documentation/filesystems/porting and
>> > path-lookup.txt.
>> >
>> > The dcache_lock stuff should have been all done for you
>> > (for in-tree filesystems, I can help out of tree fses with
>> > conversions there if you ping me offline).
>> >
>> > The rcu-walk stuff can be more tricky for your filesystem
>> > to take advantage of.
>> >
>> > If you supply a .d_revalidate, .permission, or .check_acl,
>> > then path walking is going to be slow and unscalable for
>> > you.
>> >
>> > Out of tree filesystems: you _have_ to at least add a line
>> > of code to the above functions in order to specify that
>> > you don't want to participate in rcu-walk.
>> >
>> > Otherwise, you don't have to care about rcu-walk if you
>> > have a legacy or special filesystem like configfs then I'd
>> > advise against anything fancy. But if you have a
>> > userbase and you expect them to actually do any path
>> > lookups into your filesystem, please take a look.
>>
>> One other thing: I know ECHILD is safe since no sane filesystem will
>> return it in its permission or revalidate callbacks, and even if it
>> does that's just a loss of optimization.
>
> And it's not entirely safe either.  A fuse filesystem returning ECHILD
> would make nameidata_dentry_drop_rcu() to BUG.  So some sort of errno
> filter is necessary in the fuse kernel module.

Surely you'd need some filtering anyway? I don't think any function
involving path lookup could sanely return -ECHILD.

That said, it probably is a good idea to have a new errno.
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: Important for fs devs: rcu-walk merged upstream
  2011-01-14  1:21       ` Nick Piggin
@ 2011-01-14  8:45         ` Miklos Szeredi
  -1 siblings, 0 replies; 21+ messages in thread
From: Miklos Szeredi @ 2011-01-14  8:45 UTC (permalink / raw)
  To: Nick Piggin; +Cc: miklos, linux-fsdevel, linux-kernel

On Fri, 14 Jan 2011, Nick Piggin wrote:
> On Fri, Jan 14, 2011 at 3:18 AM, Miklos Szeredi <miklos@szeredi.hu> wrote:
> > On Thu, 13 Jan 2011, Miklos Szeredi wrote:
> >> On Sat, 8 Jan 2011, Nick Piggin wrote:
> >> > The vfs-scale branch is now upstream. If you haven't
> >> > looked yet, your filesystem is likely to have been
> >> > touched, so check it out.
> >> >
> >> > Also look at Documentation/filesystems/porting and
> >> > path-lookup.txt.
> >> >
> >> > The dcache_lock stuff should have been all done for you
> >> > (for in-tree filesystems, I can help out of tree fses with
> >> > conversions there if you ping me offline).
> >> >
> >> > The rcu-walk stuff can be more tricky for your filesystem
> >> > to take advantage of.
> >> >
> >> > If you supply a .d_revalidate, .permission, or .check_acl,
> >> > then path walking is going to be slow and unscalable for
> >> > you.
> >> >
> >> > Out of tree filesystems: you _have_ to at least add a line
> >> > of code to the above functions in order to specify that
> >> > you don't want to participate in rcu-walk.
> >> >
> >> > Otherwise, you don't have to care about rcu-walk if you
> >> > have a legacy or special filesystem like configfs then I'd
> >> > advise against anything fancy. But if you have a
> >> > userbase and you expect them to actually do any path
> >> > lookups into your filesystem, please take a look.
> >>
> >> One other thing: I know ECHILD is safe since no sane filesystem will
> >> return it in its permission or revalidate callbacks, and even if it
> >> does that's just a loss of optimization.
> >
> > And it's not entirely safe either.  A fuse filesystem returning ECHILD
> > would make nameidata_dentry_drop_rcu() to BUG.  So some sort of errno
> > filter is necessary in the fuse kernel module.
> 
> Surely you'd need some filtering anyway? I don't think any function
> involving path lookup could sanely return -ECHILD.

No, but not filtering doesn't normally hurt.  And it's not quite
trivial deciding what should be allowed and what shoudln't, and the
filter would have to be updated for each addition of a new errno.  So
I'm not sure I want to go there.

> That said, it probably is a good idea to have a new errno.

Yeah, that makes the fitering much easier.

Thanks,
Miklos

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: Important for fs devs: rcu-walk merged upstream
@ 2011-01-14  8:45         ` Miklos Szeredi
  0 siblings, 0 replies; 21+ messages in thread
From: Miklos Szeredi @ 2011-01-14  8:45 UTC (permalink / raw)
  To: Nick Piggin; +Cc: miklos, linux-fsdevel, linux-kernel

On Fri, 14 Jan 2011, Nick Piggin wrote:
> On Fri, Jan 14, 2011 at 3:18 AM, Miklos Szeredi <miklos@szeredi.hu> wrote:
> > On Thu, 13 Jan 2011, Miklos Szeredi wrote:
> >> On Sat, 8 Jan 2011, Nick Piggin wrote:
> >> > The vfs-scale branch is now upstream. If you haven't
> >> > looked yet, your filesystem is likely to have been
> >> > touched, so check it out.
> >> >
> >> > Also look at Documentation/filesystems/porting and
> >> > path-lookup.txt.
> >> >
> >> > The dcache_lock stuff should have been all done for you
> >> > (for in-tree filesystems, I can help out of tree fses with
> >> > conversions there if you ping me offline).
> >> >
> >> > The rcu-walk stuff can be more tricky for your filesystem
> >> > to take advantage of.
> >> >
> >> > If you supply a .d_revalidate, .permission, or .check_acl,
> >> > then path walking is going to be slow and unscalable for
> >> > you.
> >> >
> >> > Out of tree filesystems: you _have_ to at least add a line
> >> > of code to the above functions in order to specify that
> >> > you don't want to participate in rcu-walk.
> >> >
> >> > Otherwise, you don't have to care about rcu-walk if you
> >> > have a legacy or special filesystem like configfs then I'd
> >> > advise against anything fancy. But if you have a
> >> > userbase and you expect them to actually do any path
> >> > lookups into your filesystem, please take a look.
> >>
> >> One other thing: I know ECHILD is safe since no sane filesystem will
> >> return it in its permission or revalidate callbacks, and even if it
> >> does that's just a loss of optimization.
> >
> > And it's not entirely safe either.  A fuse filesystem returning ECHILD
> > would make nameidata_dentry_drop_rcu() to BUG.  So some sort of errno
> > filter is necessary in the fuse kernel module.
> 
> Surely you'd need some filtering anyway? I don't think any function
> involving path lookup could sanely return -ECHILD.

No, but not filtering doesn't normally hurt.  And it's not quite
trivial deciding what should be allowed and what shoudln't, and the
filter would have to be updated for each addition of a new errno.  So
I'm not sure I want to go there.

> That said, it probably is a good idea to have a new errno.

Yeah, that makes the fitering much easier.

Thanks,
Miklos
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: Important for fs devs: rcu-walk merged upstream
  2011-01-14  8:45         ` Miklos Szeredi
@ 2011-01-14  8:52           ` Nick Piggin
  -1 siblings, 0 replies; 21+ messages in thread
From: Nick Piggin @ 2011-01-14  8:52 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: linux-fsdevel, linux-kernel

On Fri, Jan 14, 2011 at 7:45 PM, Miklos Szeredi <miklos@szeredi.hu> wrote:
> On Fri, 14 Jan 2011, Nick Piggin wrote:
>> On Fri, Jan 14, 2011 at 3:18 AM, Miklos Szeredi <miklos@szeredi.hu> wrote:

>> Surely you'd need some filtering anyway? I don't think any function
>> involving path lookup could sanely return -ECHILD.
>
> No, but not filtering doesn't normally hurt.  And it's not quite
> trivial deciding what should be allowed and what shoudln't, and the
> filter would have to be updated for each addition of a new errno.  So
> I'm not sure I want to go there.

Well if you allow untrusted filesystems it is possible that
-ECHILD return will do something a bit silly. So it would be
good to filter it I guess.


>> That said, it probably is a good idea to have a new errno.
>
> Yeah, that makes the fitering much easier.

How so? Would -ECHILD ever be sane to return? I'm not
arguing against changing it but I just want to know what
the issue is there.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: Important for fs devs: rcu-walk merged upstream
@ 2011-01-14  8:52           ` Nick Piggin
  0 siblings, 0 replies; 21+ messages in thread
From: Nick Piggin @ 2011-01-14  8:52 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: linux-fsdevel, linux-kernel

On Fri, Jan 14, 2011 at 7:45 PM, Miklos Szeredi <miklos@szeredi.hu> wrote:
> On Fri, 14 Jan 2011, Nick Piggin wrote:
>> On Fri, Jan 14, 2011 at 3:18 AM, Miklos Szeredi <miklos@szeredi.hu> wrote:

>> Surely you'd need some filtering anyway? I don't think any function
>> involving path lookup could sanely return -ECHILD.
>
> No, but not filtering doesn't normally hurt.  And it's not quite
> trivial deciding what should be allowed and what shoudln't, and the
> filter would have to be updated for each addition of a new errno.  So
> I'm not sure I want to go there.

Well if you allow untrusted filesystems it is possible that
-ECHILD return will do something a bit silly. So it would be
good to filter it I guess.


>> That said, it probably is a good idea to have a new errno.
>
> Yeah, that makes the fitering much easier.

How so? Would -ECHILD ever be sane to return? I'm not
arguing against changing it but I just want to know what
the issue is there.
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: Important for fs devs: rcu-walk merged upstream
  2011-01-14  8:52           ` Nick Piggin
@ 2011-01-14  9:38             ` Miklos Szeredi
  -1 siblings, 0 replies; 21+ messages in thread
From: Miklos Szeredi @ 2011-01-14  9:38 UTC (permalink / raw)
  To: Nick Piggin; +Cc: miklos, linux-fsdevel, linux-kernel

On Fri, 14 Jan 2011, Nick Piggin wrote:
> On Fri, Jan 14, 2011 at 7:45 PM, Miklos Szeredi <miklos@szeredi.hu> wrote:
> > On Fri, 14 Jan 2011, Nick Piggin wrote:
> >> On Fri, Jan 14, 2011 at 3:18 AM, Miklos Szeredi <miklos@szeredi.hu> wrote:
> 
> >> Surely you'd need some filtering anyway? I don't think any function
> >> involving path lookup could sanely return -ECHILD.
> >
> > No, but not filtering doesn't normally hurt.  And it's not quite
> > trivial deciding what should be allowed and what shoudln't, and the
> > filter would have to be updated for each addition of a new errno.  So
> > I'm not sure I want to go there.
> 
> Well if you allow untrusted filesystems it is possible that
> -ECHILD return will do something a bit silly. So it would be
> good to filter it I guess.

What could it do (other than resulting in a silly error printout)?

If we'd want errno filtering for fuse how would you define "sane"?

> >> That said, it probably is a good idea to have a new errno.
> >
> > Yeah, that makes the fitering much easier.
> 
> How so? Would -ECHILD ever be sane to return? I'm not
> arguing against changing it but I just want to know what
> the issue is there.

All kernel private errnos are >=512 (see <linux/errno.h), filtering
out those is quite easy and clearly desirable.

Thanks,
Miklos

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: Important for fs devs: rcu-walk merged upstream
@ 2011-01-14  9:38             ` Miklos Szeredi
  0 siblings, 0 replies; 21+ messages in thread
From: Miklos Szeredi @ 2011-01-14  9:38 UTC (permalink / raw)
  To: Nick Piggin; +Cc: miklos, linux-fsdevel, linux-kernel

On Fri, 14 Jan 2011, Nick Piggin wrote:
> On Fri, Jan 14, 2011 at 7:45 PM, Miklos Szeredi <miklos@szeredi.hu> wrote:
> > On Fri, 14 Jan 2011, Nick Piggin wrote:
> >> On Fri, Jan 14, 2011 at 3:18 AM, Miklos Szeredi <miklos@szeredi.hu> wrote:
> 
> >> Surely you'd need some filtering anyway? I don't think any function
> >> involving path lookup could sanely return -ECHILD.
> >
> > No, but not filtering doesn't normally hurt.  And it's not quite
> > trivial deciding what should be allowed and what shoudln't, and the
> > filter would have to be updated for each addition of a new errno.  So
> > I'm not sure I want to go there.
> 
> Well if you allow untrusted filesystems it is possible that
> -ECHILD return will do something a bit silly. So it would be
> good to filter it I guess.

What could it do (other than resulting in a silly error printout)?

If we'd want errno filtering for fuse how would you define "sane"?

> >> That said, it probably is a good idea to have a new errno.
> >
> > Yeah, that makes the fitering much easier.
> 
> How so? Would -ECHILD ever be sane to return? I'm not
> arguing against changing it but I just want to know what
> the issue is there.

All kernel private errnos are >=512 (see <linux/errno.h), filtering
out those is quite easy and clearly desirable.

Thanks,
Miklos
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2011-01-14  9:38 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-01-08  2:54 Important for fs devs: rcu-walk merged upstream Nick Piggin
2011-01-08  9:10 ` Marco Stornelli
2011-01-11  9:57   ` Nick Piggin
2011-01-11  9:57     ` Nick Piggin
2011-01-11 11:08     ` Miklos Szeredi
2011-01-10 14:38 ` J. R. Okajima
2011-01-11 12:57   ` Nick Piggin
2011-01-11 12:57     ` Nick Piggin
2011-01-11  1:08 ` Joel Becker
2011-01-11 13:06   ` Nick Piggin
2011-01-11 13:06     ` Nick Piggin
2011-01-13 14:58 ` Miklos Szeredi
2011-01-13 16:18   ` Miklos Szeredi
2011-01-14  1:21     ` Nick Piggin
2011-01-14  1:21       ` Nick Piggin
2011-01-14  8:45       ` Miklos Szeredi
2011-01-14  8:45         ` Miklos Szeredi
2011-01-14  8:52         ` Nick Piggin
2011-01-14  8:52           ` Nick Piggin
2011-01-14  9:38           ` Miklos Szeredi
2011-01-14  9:38             ` Miklos Szeredi

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.