[PATCH] fs: Move attribute flags into non __KERNEL__ space

[PATCH] fs: Move attribute flags into non __KERNEL__ space

[Sasha Levin]

Attribute flags can be useful in userspace when working with filesystems
such as 9p.

Theres no reason to limit them to kernel space only.

Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: linux-fsdevel@vger.kernel.org
Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 include/linux/fs.h |   44 ++++++++++++++++++++++----------------------
 1 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/include/linux/fs.h b/include/linux/fs.h
index e0bc4ff..c51d344 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -374,6 +374,28 @@ struct inodes_stat_t {
 #define SYNC_FILE_RANGE_WRITE		2
 #define SYNC_FILE_RANGE_WAIT_AFTER	4
 
+/*
+ * Attribute flags.  These should be or-ed together to figure out what
+ * has been changed!
+ */
+#define ATTR_MODE	(1 << 0)
+#define ATTR_UID	(1 << 1)
+#define ATTR_GID	(1 << 2)
+#define ATTR_SIZE	(1 << 3)
+#define ATTR_ATIME	(1 << 4)
+#define ATTR_MTIME	(1 << 5)
+#define ATTR_CTIME	(1 << 6)
+#define ATTR_ATIME_SET	(1 << 7)
+#define ATTR_MTIME_SET	(1 << 8)
+#define ATTR_FORCE	(1 << 9) /* Not a change, but a change it */
+#define ATTR_ATTR_FLAG	(1 << 10)
+#define ATTR_KILL_SUID	(1 << 11)
+#define ATTR_KILL_SGID	(1 << 12)
+#define ATTR_FILE	(1 << 13)
+#define ATTR_KILL_PRIV	(1 << 14)
+#define ATTR_OPEN	(1 << 15) /* Truncating from open(O_TRUNC) */
+#define ATTR_TIMES_SET	(1 << 16)
+
 #ifdef __KERNEL__
 
 #include <linux/linkage.h>
@@ -429,28 +451,6 @@ typedef void (dio_iodone_t)(struct kiocb *iocb, loff_t offset,
 			bool is_async);
 
 /*
- * Attribute flags.  These should be or-ed together to figure out what
- * has been changed!
- */
-#define ATTR_MODE	(1 << 0)
-#define ATTR_UID	(1 << 1)
-#define ATTR_GID	(1 << 2)
-#define ATTR_SIZE	(1 << 3)
-#define ATTR_ATIME	(1 << 4)
-#define ATTR_MTIME	(1 << 5)
-#define ATTR_CTIME	(1 << 6)
-#define ATTR_ATIME_SET	(1 << 7)
-#define ATTR_MTIME_SET	(1 << 8)
-#define ATTR_FORCE	(1 << 9) /* Not a change, but a change it */
-#define ATTR_ATTR_FLAG	(1 << 10)
-#define ATTR_KILL_SUID	(1 << 11)
-#define ATTR_KILL_SGID	(1 << 12)
-#define ATTR_FILE	(1 << 13)
-#define ATTR_KILL_PRIV	(1 << 14)
-#define ATTR_OPEN	(1 << 15) /* Truncating from open(O_TRUNC) */
-#define ATTR_TIMES_SET	(1 << 16)
-
-/*
  * This is the Inode Attributes structure, used for notify_change().  It
  * uses the above definitions as flags, to know which values have changed.
  * Also, in this manner, a Filesystem can look at only the values it cares
-- 
1.7.8

Re: [PATCH] fs: Move attribute flags into non __KERNEL__ space

[Christoph Hellwig]

On Fri, Dec 16, 2011 at 09:47:06AM +0200, Sasha Levin wrote:
> Attribute flags can be useful in userspace when working with filesystems
> such as 9p.

No, they aren't.  They are kernel internal values and userspace has no
fucking business messing with them.

Re: [PATCH] fs: Move attribute flags into non __KERNEL__ space

[Sasha Levin]

On Fri, 2011-12-16 at 14:23 -0500, Christoph Hellwig wrote:
> On Fri, Dec 16, 2011 at 09:47:06AM +0200, Sasha Levin wrote:
> > Attribute flags can be useful in userspace when working with filesystems
> > such as 9p.
> 
> No, they aren't.  They are kernel internal values and userspace has no
> fucking business messing with them.
> 

They became userspace business once they got exposed through 9p.

Take a look at <linux/net/9p/9p.h>:

	/**
	 * struct p9_iattr_dotl - P9 inode attribute for setattr
	 * @valid: bitfield specifying which fields are valid
	 *         same as in struct iattr
	[...]

That structure is userspace facing, and it's using iattr values.

So either we expose them through fs.h, through 9p.h, or modify 9p code
to not use them directly. But claiming that they're kernel internal
values isn't entirely correct.

-- 

Sasha.

Re: [PATCH] fs: Move attribute flags into non __KERNEL__ space

[Al Viro]

On Fri, Dec 16, 2011 at 11:30:40PM +0200, Sasha Levin wrote:
> On Fri, 2011-12-16 at 14:23 -0500, Christoph Hellwig wrote:
> > On Fri, Dec 16, 2011 at 09:47:06AM +0200, Sasha Levin wrote:
> > > Attribute flags can be useful in userspace when working with filesystems
> > > such as 9p.
> > 
> > No, they aren't.  They are kernel internal values and userspace has no
> > fucking business messing with them.
> > 
> 
> They became userspace business once they got exposed through 9p.
> 
> Take a look at <linux/net/9p/9p.h>:
> 
> 	/**
> 	 * struct p9_iattr_dotl - P9 inode attribute for setattr
> 	 * @valid: bitfield specifying which fields are valid
> 	 *         same as in struct iattr
> 	[...]
> 
> That structure is userspace facing, and it's using iattr values.
> 
> So either we expose them through fs.h, through 9p.h, or modify 9p code
> to not use them directly. But claiming that they're kernel internal
> values isn't entirely correct.

They *are* kernel internal values and 9P is asking for trouble exposing
them.  Translation: tomorrow we might reassign those as we bloody wish
and any userland code that happens to rely on their values will break.
At which point we'll handle complaints by pointing and laughing.

It's a 9P bug; fix it there.  Turning random internal constants into a part
of ABI is not going to work.

Re: [PATCH] fs: Move attribute flags into non __KERNEL__ space

[Aneesh Kumar K.V]

On Fri, 16 Dec 2011 23:30:42 +0000, Al Viro <viro@ZenIV.linux.org.uk> wrote:
> On Fri, Dec 16, 2011 at 11:30:40PM +0200, Sasha Levin wrote:
> > On Fri, 2011-12-16 at 14:23 -0500, Christoph Hellwig wrote:
> > > On Fri, Dec 16, 2011 at 09:47:06AM +0200, Sasha Levin wrote:
> > > > Attribute flags can be useful in userspace when working with filesystems
> > > > such as 9p.
> > > 
> > > No, they aren't.  They are kernel internal values and userspace has no
> > > fucking business messing with them.
> > > 
> > 
> > They became userspace business once they got exposed through 9p.
> > 
> > Take a look at <linux/net/9p/9p.h>:
> > 
> > 	/**
> > 	 * struct p9_iattr_dotl - P9 inode attribute for setattr
> > 	 * @valid: bitfield specifying which fields are valid
> > 	 *         same as in struct iattr
> > 	[...]
> > 
> > That structure is userspace facing, and it's using iattr values.
> > 
> > So either we expose them through fs.h, through 9p.h, or modify 9p code
> > to not use them directly. But claiming that they're kernel internal
> > values isn't entirely correct.
> 
> They *are* kernel internal values and 9P is asking for trouble exposing
> them.  Translation: tomorrow we might reassign those as we bloody wish
> and any userland code that happens to rely on their values will break.
> At which point we'll handle complaints by pointing and laughing.
> 
> It's a 9P bug; fix it there.  Turning random internal constants into a part
> of ABI is not going to work.

I guess we would need to define them at the protocol level then. 
Something like f88657ce3f9713a0c62101dffb0e972a979e77b9. 

-aneesh

Re: [PATCH] fs: Move attribute flags into non __KERNEL__ space

[Al Viro]

On Sat, Dec 17, 2011 at 07:14:26PM +0530, Aneesh Kumar K.V wrote:

> > It's a 9P bug; fix it there.  Turning random internal constants into a part
> > of ABI is not going to work.
> 
> I guess we would need to define them at the protocol level then. 
> Something like f88657ce3f9713a0c62101dffb0e972a979e77b9. 

The question is which ones make sense at the protocol level...

Re: [PATCH] fs: Move attribute flags into non __KERNEL__ space

[Eric Van Hensbergen]

If these aren't normally exposed to user space, then how are they
typically set?  That interface may make more sense at the protocol
level (and perhaps more so from the server side implementation).

        -eric


On Sat, Dec 17, 2011 at 12:29 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> On Sat, Dec 17, 2011 at 07:14:26PM +0530, Aneesh Kumar K.V wrote:
>
>> > It's a 9P bug; fix it there.  Turning random internal constants into a part
>> > of ABI is not going to work.
>>
>> I guess we would need to define them at the protocol level then.
>> Something like f88657ce3f9713a0c62101dffb0e972a979e77b9.
>
> The question is which ones make sense at the protocol level...
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html