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
-
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
