Re: [PATCH 1/2] vfs: make real_lookup do dentry revalidation with

Re: [PATCH 1/2] vfs: make real_lookup do dentry revalidation with

[Jim Garlick]

On Thu, Sep 17, 2009 at 06:36:49AM +0800, Ian Kent wrote:
> So, it's probably time to post my patch series to get some more eyes
> looking at them. Who on the recipient list of this mail should I include
> for the post?

I'm keen to see Sage's patch land in -mm.  I'd be happy to help review/test
your changes if it would help move things along.

Thanks, 

Jim

Re: [PATCH 1/2] vfs: make real_lookup do dentry revalidation with

[Ian Kent]

Jim Garlick wrote:
> On Thu, Sep 17, 2009 at 06:36:49AM +0800, Ian Kent wrote:
>> So, it's probably time to post my patch series to get some more eyes
>> looking at them. Who on the recipient list of this mail should I include
>> for the post?
> 
> I'm keen to see Sage's patch land in -mm.  I'd be happy to help review/test
> your changes if it would help move things along.

Wow, at last some interest.

I was beginning to think my painstaking effort had been wasted.

We will need to verify the patch I've used for the VFS locking is
adequate because I had some difficulty working out which of the several
originally posted were the ones needed and at least one didn't even
apply. It will be included in the series I post.

Since no-one else has replied I'll post the patch series and copy
everyone on the cc list of the original discussion and yourself.

Ian

Re: [PATCH 1/2] vfs: make real_lookup do dentry revalidation with

[Al Viro]

On Thu, Sep 24, 2009 at 11:50:55AM +0800, Ian Kent wrote:
> Jim Garlick wrote:
> > On Thu, Sep 17, 2009 at 06:36:49AM +0800, Ian Kent wrote:
> >> So, it's probably time to post my patch series to get some more eyes
> >> looking at them. Who on the recipient list of this mail should I include
> >> for the post?
> > 
> > I'm keen to see Sage's patch land in -mm.  I'd be happy to help review/test
> > your changes if it would help move things along.
> 
> Wow, at last some interest.
> 
> I was beginning to think my painstaking effort had been wasted.
> 
> We will need to verify the patch I've used for the VFS locking is
> adequate because I had some difficulty working out which of the several
> originally posted were the ones needed and at least one didn't even
> apply. It will be included in the series I post.
> 
> Since no-one else has replied I'll post the patch series and copy
> everyone on the cc list of the original discussion and yourself.

It's definitely not wasted.  I have a patch series massaging the pathname
resolution sitting in the local tree and once I'm done with the misc stuff
(tonight, hopefully) it'll be time for that one.  I was going to ask you
to post once I get to that, since it clearly needs to be integrated.
So if you have the patch series against the current mainline, please post
it and I'll deal with that.

Re: [PATCH 1/2] vfs: make real_lookup do dentry revalidation with

[Ian Kent]

Al Viro wrote:
> On Thu, Sep 24, 2009 at 11:50:55AM +0800, Ian Kent wrote:
>> Jim Garlick wrote:
>>> On Thu, Sep 17, 2009 at 06:36:49AM +0800, Ian Kent wrote:
>>>> So, it's probably time to post my patch series to get some more eyes
>>>> looking at them. Who on the recipient list of this mail should I include
>>>> for the post?
>>> I'm keen to see Sage's patch land in -mm.  I'd be happy to help review/test
>>> your changes if it would help move things along.
>> Wow, at last some interest.
>>
>> I was beginning to think my painstaking effort had been wasted.
>>
>> We will need to verify the patch I've used for the VFS locking is
>> adequate because I had some difficulty working out which of the several
>> originally posted were the ones needed and at least one didn't even
>> apply. It will be included in the series I post.
>>
>> Since no-one else has replied I'll post the patch series and copy
>> everyone on the cc list of the original discussion and yourself.
> 
> It's definitely not wasted.  I have a patch series massaging the pathname
> resolution sitting in the local tree and once I'm done with the misc stuff
> (tonight, hopefully) it'll be time for that one.  I was going to ask you
> to post once I get to that, since it clearly needs to be integrated.
> So if you have the patch series against the current mainline, please post
> it and I'll deal with that.

OK, I'll pull the latest changes into my local tree and check there are
no surprises, then post the series. Unfortunately, the series is not
trivial so review will be difficult for those not familiar with the
issues. But it does need review before going further.

I'm not sure what to do about the autofs module, namely the removal and
rename of autofs4 to autofs, since that will require a fair amount of
experimentation to make it, at least as much as is possible, a seamless
change wrt. to the module usage. OTOH, there aren't many users, if any
at all, of the autofs module nowadays. Perhaps people with embedded
devices are still using it, I don't know.

Thoughts?

Ian

Re: [PATCH 1/2] vfs: make real_lookup do dentry revalidation with

[Ian Kent]

Ian Kent wrote:
> Al Viro wrote:
>> On Thu, Sep 24, 2009 at 11:50:55AM +0800, Ian Kent wrote:
>>> Jim Garlick wrote:
>>>> On Thu, Sep 17, 2009 at 06:36:49AM +0800, Ian Kent wrote:
>>>>> So, it's probably time to post my patch series to get some more eyes
>>>>> looking at them. Who on the recipient list of this mail should I include
>>>>> for the post?
>>>> I'm keen to see Sage's patch land in -mm.  I'd be happy to help review/test
>>>> your changes if it would help move things along.
>>> Wow, at last some interest.
>>>
>>> I was beginning to think my painstaking effort had been wasted.
>>>
>>> We will need to verify the patch I've used for the VFS locking is
>>> adequate because I had some difficulty working out which of the several
>>> originally posted were the ones needed and at least one didn't even
>>> apply. It will be included in the series I post.
>>>
>>> Since no-one else has replied I'll post the patch series and copy
>>> everyone on the cc list of the original discussion and yourself.
>> It's definitely not wasted.  I have a patch series massaging the pathname
>> resolution sitting in the local tree and once I'm done with the misc stuff
>> (tonight, hopefully) it'll be time for that one.  I was going to ask you
>> to post once I get to that, since it clearly needs to be integrated.
>> So if you have the patch series against the current mainline, please post
>> it and I'll deal with that.
> 
> OK, I'll pull the latest changes into my local tree and check there are
> no surprises, then post the series. Unfortunately, the series is not
> trivial so review will be difficult for those not familiar with the
> issues. But it does need review before going further.
> 
> I'm not sure what to do about the autofs module, namely the removal and
> rename of autofs4 to autofs, since that will require a fair amount of
> experimentation to make it, at least as much as is possible, a seamless
> change wrt. to the module usage. OTOH, there aren't many users, if any
> at all, of the autofs module nowadays. Perhaps people with embedded
> devices are still using it, I don't know.
> 
> Thoughts?

I still need to deal with the autofs module.

I'm reluctant to remove it and do the rename at the same time the other
changes are going in.

I thought a better idea would be to leave the autofs module in place for
the moment and change the Kconfig help message to describe what is going
to happen and alert users to the fact it won't work and also change all
the defconfig files that select autofs to select autofs4.

Thoughts please?

Ian

Re: [PATCH 1/2] vfs: make real_lookup do dentry revalidation with

[Jeff Moyer]

Ian Kent <raven@themaw.net> writes:
>
> I still need to deal with the autofs module.
>
> I'm reluctant to remove it and do the rename at the same time the other
> changes are going in.
>
> I thought a better idea would be to leave the autofs module in place for
> the moment and change the Kconfig help message to describe what is going
> to happen and alert users to the fact it won't work and also change all
> the defconfig files that select autofs to select autofs4.
>
> Thoughts please?

I think it's safe to remove fs/autofs.  There's no sense in keeping
around code that doesn't work, and we don't really fix bugs in autofs3
anyway.  Heck, when was the last time you got a bug report for it?  I
haven't seen one in probably 5 years!

I'm not so sure what the implications are of renaming autofs4 to autofs.
At the very least, the autofs init script itself tries to load the
autofs4 kernel module.  This would cause issues when updating a kernel,
so it sounds like a bad idea to me.  If there was a module alias causing
autofs to load when autofs4 is requested on newer kernels, I guess that
would be okay.  But I think that sort of thing is managed by the
userspace configuration.  The other option, then, is to ship an autofs
with an init script that knows which module to load.  Then, after that's
been in the wild for some time (a year?), make the switch.

These sorts of things are always painful.

Cheers,
Jeff

Re: [PATCH 1/2] vfs: make real_lookup do dentry revalidation with

[Ian Kent]

Jeff Moyer wrote:
> Ian Kent <raven@themaw.net> writes:
>> I still need to deal with the autofs module.
>>
>> I'm reluctant to remove it and do the rename at the same time the other
>> changes are going in.
>>
>> I thought a better idea would be to leave the autofs module in place for
>> the moment and change the Kconfig help message to describe what is going
>> to happen and alert users to the fact it won't work and also change all
>> the defconfig files that select autofs to select autofs4.
>>
>> Thoughts please?
> 
> I think it's safe to remove fs/autofs.  There's no sense in keeping
> around code that doesn't work, and we don't really fix bugs in autofs3
> anyway.  Heck, when was the last time you got a bug report for it?  I
> haven't seen one in probably 5 years!

Agreed, that's not really the issue, the sort of things below are the worry.

> 
> I'm not so sure what the implications are of renaming autofs4 to autofs.
> At the very least, the autofs init script itself tries to load the
> autofs4 kernel module.  This would cause issues when updating a kernel,
> so it sounds like a bad idea to me.  If there was a module alias causing
> autofs to load when autofs4 is requested on newer kernels, I guess that
> would be okay.  But I think that sort of thing is managed by the
> userspace configuration.  The other option, then, is to ship an autofs
> with an init script that knows which module to load.  Then, after that's
> been in the wild for some time (a year?), make the switch.

Clearly we can't account for people using absolute paths so that will
cause pain for some.

Some time ago Christoph suggested registering both autofs and autofs4
but I'm not sure about that since both modules have always only
registered autofs as the file system name.

We can add a MODULE_ALIAS() to the module source but that doesn't
completely work, I think because the user space tools then don't get the
directory right. Changing the user space configuration is also
problematic because booting from a kernel with and without would require
a configuration change every time.

The obvious simple solution would be to use symlinks to make the
directory and module appear to be present, set about a process of user
awareness and remove them after some pre-defined number of subsequent
releases but I'm not sure how that approach would be received? We could
even write a module stub that issues a warning message to syslog and
then loads the autofs module but I haven't tried that yet.

Please, folks, some suggestions.

Ian

Re: [PATCH 1/2] vfs: make real_lookup do dentry revalidation with

[Ian Kent]

Ian Kent wrote:
> Jeff Moyer wrote:
>> Ian Kent <raven@themaw.net> writes:
>>> I still need to deal with the autofs module.
>>>
>>> I'm reluctant to remove it and do the rename at the same time the other
>>> changes are going in.
>>>
>>> I thought a better idea would be to leave the autofs module in place for
>>> the moment and change the Kconfig help message to describe what is going
>>> to happen and alert users to the fact it won't work and also change all
>>> the defconfig files that select autofs to select autofs4.
>>>
>>> Thoughts please?
>> I think it's safe to remove fs/autofs.  There's no sense in keeping
>> around code that doesn't work, and we don't really fix bugs in autofs3
>> anyway.  Heck, when was the last time you got a bug report for it?  I
>> haven't seen one in probably 5 years!
> 
> Agreed, that's not really the issue, the sort of things below are the worry.

Another thing that is a bit of a worry is, for the reasons above, we
haven't actually tested usage with version 3 for a long time and there
have been many changes since.

> 
>> I'm not so sure what the implications are of renaming autofs4 to autofs.
>> At the very least, the autofs init script itself tries to load the
>> autofs4 kernel module.  This would cause issues when updating a kernel,
>> so it sounds like a bad idea to me.  If there was a module alias causing
>> autofs to load when autofs4 is requested on newer kernels, I guess that
>> would be okay.  But I think that sort of thing is managed by the
>> userspace configuration.  The other option, then, is to ship an autofs
>> with an init script that knows which module to load.  Then, after that's
>> been in the wild for some time (a year?), make the switch.
> 
> Clearly we can't account for people using absolute paths so that will
> cause pain for some.
> 
> Some time ago Christoph suggested registering both autofs and autofs4
> but I'm not sure about that since both modules have always only
> registered autofs as the file system name.
> 
> We can add a MODULE_ALIAS() to the module source but that doesn't
> completely work, I think because the user space tools then don't get the
> directory right. Changing the user space configuration is also
> problematic because booting from a kernel with and without would require
> a configuration change every time.
> 
> The obvious simple solution would be to use symlinks to make the
> directory and module appear to be present, set about a process of user
> awareness and remove them after some pre-defined number of subsequent
> releases but I'm not sure how that approach would be received? We could
> even write a module stub that issues a warning message to syslog and
> then loads the autofs module but I haven't tried that yet.
> 
> Please, folks, some suggestions.
> 
> Ian
> 

Re: [PATCH 1/2] vfs: make real_lookup do dentry revalidation with

[Jeff Moyer]

Ian Kent <raven@themaw.net> writes:

> We could even write a module stub that issues a warning message to
> syslog and then loads the autofs module but I haven't tried that yet.

That sounds like the best idea so far.

Cheers,
Jeff

Re: [PATCH 1/2] vfs: make real_lookup do dentry revalidation with

[Christoph Hellwig]

On Wed, Oct 14, 2009 at 10:34:12AM +0800, Ian Kent wrote:
> Some time ago Christoph suggested registering both autofs and autofs4
> but I'm not sure about that since both modules have always only
> registered autofs as the file system name.

Oops, I didn't notice that.

> We can add a MODULE_ALIAS() to the module source but that doesn't
> completely work, I think because the user space tools then don't get the
> directory right. Changing the user space configuration is also
> problematic because booting from a kernel with and without would require
> a configuration change every time.
> 
> The obvious simple solution would be to use symlinks to make the
> directory and module appear to be present, set about a process of user
> awareness and remove them after some pre-defined number of subsequent
> releases but I'm not sure how that approach would be received? We could
> even write a module stub that issues a warning message to syslog and
> then loads the autofs module but I haven't tried that yet.
> 
> Please, folks, some suggestions.

Just build two modules using the same source code?  That quite ugly, but
if the userspace is really that messed up I can't think of any better
idea.

Re: [PATCH 1/2] vfs: make real_lookup do dentry revalidation with

[Ian Kent]

Christoph Hellwig wrote:
> On Wed, Oct 14, 2009 at 10:34:12AM +0800, Ian Kent wrote:
>> Some time ago Christoph suggested registering both autofs and autofs4
>> but I'm not sure about that since both modules have always only
>> registered autofs as the file system name.
> 
> Oops, I didn't notice that.
> 
>> We can add a MODULE_ALIAS() to the module source but that doesn't
>> completely work, I think because the user space tools then don't get the
>> directory right. Changing the user space configuration is also
>> problematic because booting from a kernel with and without would require
>> a configuration change every time.
>>
>> The obvious simple solution would be to use symlinks to make the
>> directory and module appear to be present, set about a process of user
>> awareness and remove them after some pre-defined number of subsequent
>> releases but I'm not sure how that approach would be received? We could
>> even write a module stub that issues a warning message to syslog and
>> then loads the autofs module but I haven't tried that yet.
>>
>> Please, folks, some suggestions.
> 
> Just build two modules using the same source code?  That quite ugly, but
> if the userspace is really that messed up I can't think of any better
> idea.

Yep, that's what I've ended up doing for now, (after the autofs4 source
has been copied) autofs4 will build from autofs source, along with a
Kconfig help message explaining autofs4 is going to be removed.

Ian