Re: getting rid of filp search in fs_may_remount_ro()

[Christoph Hellwig]

On Mon, Dec 31, 2007 at 11:54:26AM -0800, Dave Hansen wrote:
> > Btw, I just noticed in current -mm fs_may_remount_ro() is still around
> > and not replaced by ther per-sb writers count.  That surely sounds like
> > some kind of mismerge..
> 
> I was actually leaving that for later.  Getting rid of the filp search
> is a great benefit of the r/o bind patches, but it isn't strictly
> necessary and it doesn't really hurt anything to keep it.

Well, it's one of the primary arguments for the expensive infrastructure
behind the per-mount r/o patches, as checkin the counter here means
we can now do exact tracking of the writable status of the superblock
and thus avoid the remount r/o races.

But there's another argument for it which is unrelated to the per-mount
r/o patches.  fs_may_remount_ro is the last non-trivial user of
sb->s_files and the file_list_lock.  Peter Zijlstra identified the
latter as scalabilty problem, and I'd really prefer to get rid of the
list and the lock instead of hacking around these issues.  Removing
sb->s_files will also allow removing a list_head from struct file
with some additional hackery to the tty core which should be a nice
memory safer on big systems.

> The reason that it was contentious was that we need some way to be able
> to do an sb-to-mount mapping.  When remounting the sb, we need to
> determine whether *any* of the mounts of that sb have any writers.
> 
> We don't currently have any mechanisms to do direct lookups from sb to
> mount.  The only alternative I can see right now is to walk over all
> tasks, then walk over all vfs namespaces, and walk each mount tree to
> see if any mounts are of the sb we're looking for.  This needs to be
> done while already holding the mnt_writers[] locks so that no new mnt
> writers can come in.
> 
> *THAT* is going to be a heavyweight operation.  I need to go look in
> detail at how the mount trees are kept, and we'll need some kind of
> mechanism to keep track of which vfs namespaces we've looked at during
> the search so we don't search them twice. 
> 
> Can you think of a simpler way to do it?

Well, the first revisions of the patches added a list of vfsmounts
to the superblock, and I wasn't quite happy with that because I though
we could get around that easily.  Given that we can't get around it
easily I'd be happy to take my comments back.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/