Re: [PATCH 09/26] make access() use mnt check

[Dave Hansen <haveblue@us.ibm.com>]

On Sat, 2007-06-23 at 08:45 +0100, Christoph Hellwig wrote:
> On Fri, Jun 22, 2007 at 01:03:14PM -0700, Dave Hansen wrote:
> > 
> > It is OK to let access() go without using a mnt_want/drop_write()
> > pair because it doesn't actually do writes to the filesystem,
> > and it is inherently racy anyway.  This is a rare case when it is
> > OK to use __mnt_is_readonly() directly.
> 
> You probably want to add a big comment explaining why it's fine here.

I've got this in the next set:

-
-       if(IS_RDONLY(nd.dentry->d_inode))
+       /*
+        * This is a rare case where using __mnt_is_readonly()
+        * is OK without a mnt_want/drop_write() pair.  Since
+        * not actual write to the fs is performed here, we do
+        * not need to telegraph to that to anyone.  Also, we
+        * accept that access is inherently racy, and know that
+        * the fs might be remounted between this syscall, and
+        * any action taken because of its result.
+        */
+       if (__mnt_is_readonly(nd.mnt))
                res = -EROFS;

> That reminds me of something else I had in mind to debug that the
> writer counts are okay:
> 
>  we should probably add a check in permission that we have an elevated
>  writercount on the vfsmount/sb.  Of course we'll need some way to
>  overrid it for access(), which means passing down a flag to it or
>  something.

This was already in the second to last patch in the series.  Good enough?

diff -puN fs/namei.c~numa_mnt_want_write fs/namei.c
--- lxc/fs/namei.c~numa_mnt_want_write  2007-06-25 11:05:50.000000000 -0700
+++ lxc-dave/fs/namei.c 2007-06-25 11:05:50.000000000 -0700
@@ -230,10 +230,12 @@ int permission(struct inode *inode, int
        int retval, submask;

        if (mask & MAY_WRITE) {
-
                /*
-                * Nobody gets write access to a read-only fs.
+                * If this WARN_ON() is hit, it likely means that
+                * there was a missed mnt_want_write() on the path
+                * leading here.
                 */
+               WARN_ON(__mnt_is_readonly(nd->mnt));
                if (IS_RDONLY(inode) &&


-- Dave