On Wed, 24 Jun 2009, Ian Kent wrote: > Ian Kent wrote: > > Sage Weil wrote: > >> Hi Ian, > >> > >> Have you had a chance to look at getting autofs4 lookup/revalidate > >> adjusted so that this real_lookup() fix[1] can go in? > >> > >> Please let me know if there is anything I can do to help here. If you're > >> still occupied, I'm happy to spin something up and send it your way... > >> just let me know. > > > > Sorry, I haven't had time to do more on this. > > There is also the issue of what to do about removing the autofs module > > and renaming autofs4 to autofs, as this will break the autofs module. > > > > I did start contacting people I think would want to know about this but > > haven't gone further than an initial mail. > > > > The other thing is that this patch was originally written quite a while > > ago and, although it appears to work ok, I'm not sure it's quite what we > > need. > > I'm continuing with this now, but there's a deadlock in there somewhere! Sorry, are you still working with the patch you posted a few months back? http://marc.info/?l=linux-fsdevel&m=123831685111213&w=2 Looking over it, the + unsigned int lock_held = mutex_is_locked(&dir->i_mutex); ... + if (lock_held) { + /* Already pending, send to ->lookup() */ + d_drop(dentry); bit looks highly suspect. I'm guessing revalidate should never sleep, and always kick things off to ->lookup() (to do any waiting on upcall completion or whatever else) if the dentry isn't valid now...? sage > > > > > Sorry for delaying you. > > > >> thanks- > >> sage > >> > >> > >> [1] http://marc.info/?l=linux-fsdevel&m=123749395609697&w=2 > >> > >> > >> On Fri, 3 Apr 2009, Ian Kent wrote: > >> > >>> Sage Weil wrote: > >>>>> Latest here works OK. > >>>>> > >>>>> I haven't finished checking yet but it looks like the patch below works > >>>>> OK. I started with a 2.6.29 build with your two patches but it was a > >>>>> little broken so I fell back to a Fedora 2.6.27 based kernel without the > >>>>> two revalidate pacthes to debug it. So I still need to test the result > >>>>> against 2.6.29 again. I also don't have any real way to test for the three > >>>>> process race we discussed where the revalidate isn't followed by a > >>>>> ->lookup() but with both of your patches applied that shouldn't be a > >>>>> problem (as we discussed). > >>>>> > >>>>> I've not run checkpatch.pl against the patch either at this stage. > >>>> That's good news... > >>> I'm still working on this too. > >>> I have some pressing work so it may be a while before I'm totally happy > >>> with the patch. Didn't you say you were expecting a 2.6.31 time frame > >>> for this? > >>> > >>>> > >>>>> There is a further issue and that is regarding the autofs module. > >>>>> > >>>>> I can't see updating autofs for this being practical (although I haven't > >>>>> actually looked yet). I suspect quite a bit of work would be needed. The > >>>>> fact is that autofs isn't used much any more and it really should be > >>>>> replaced with the autofs4 module at some point. But that's a fairly tricky > >>>>> exercise and will likely cause some user space breakage. It will require > >>>>> an updated module-init-tools to add "alais autofs4 autofs" for modprobe > >>>>> backward compatibility and will break for any explicit checks for the > >>>>> presence of the "autofs4" module. > >>>> Hmm. Well, I assume autofs needs to work properly before this gets > >>>> changed, though, right? Should I see what I can do with it? I took a > >>>> quick look, and I don't think it will take too much to make it behave. > >>>> It looks like the main thing is to make the lookup call to try_fill_dentry > >>>> return any existing dentry in place of the one the vfs provides. > >>> Yes, or be replaced by what is currently the autofs4 module. The autofs > >>> v2 communication protocol surely can't be being used any more and the > >>> autofs4 module supports versions 3, 4 and 5. In fact I received a mail > >>> from HPA recently suggesting he supports doing this. > >>> > >>> I had a quick look as well. I think you'll find it isn't quite as simple > >>> as that. I'll have a closer look as soon as I get a chance. > >>> > >>> > >>> Ian > >>> -- > >>> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in > >>> the body of a message to majordomo@xxxxxxxxxxxxxxx > >>> More majordomo info at http://vger.kernel.org/majordomo-info.html > >>> > >>> > > > > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html > > -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
