* [patch] inotify: make our sysfs files show up
@ 2004-11-17 16:57 Robert Love
2004-11-17 18:02 ` [patch] inotify: vfs_permission was replaced Robert Love
2004-11-18 19:05 ` [patch] inotify: grab right lock Robert Love
0 siblings, 2 replies; 16+ messages in thread
From: Robert Love @ 2004-11-17 16:57 UTC (permalink / raw)
To: ttb; +Cc: linux-kernel
Hey, John.
On top of the patch I just posted (see "[patch] add class_device to
miscdevice") this patch uses the newly attainable class_device value to
correctly add our sysfs attributes.
The problem with the current solution is that we were adding attributes
to the device. But we did not have a device entry, only the fake
inotify entry in class/misc. So we need to attach the attributes to the
class device and not the nonexistent physical device.
In order to attach attributes to the class_device, I needed to make the
changes I sent to Greg. I would suggest merging that patch and this and
hopefully the miscdevice bits will find their way into mainline and we
can then drop them from inotify.
Thanks,
Robert Love
Make our /sys/class/misc/inotify entries show up and work.
Signed-Off-By: Robert Love <rml@novell.com>
inotify.c | 57 +++++++++++++++++++++++----------------------------------
1 files changed, 23 insertions(+), 34 deletions(-)
diff -u linux/drivers/char/inotify.c linux/drivers/char/inotify.c
--- linux/drivers/char/inotify.c 2004-11-15 15:28:34.951248696 -0500
+++ linux/drivers/char/inotify.c 2004-11-16 14:42:11.929575168 -0500
@@ -80,60 +80,48 @@
#define inotify_watch_i_list(pos) list_entry((pos), struct inotify_watch, i_list)
#define inotify_watch_u_list(pos) list_entry((pos), struct inotify_watch, u_list)
-static ssize_t show_max_queued_events(struct device *dev, char *buf)
+static ssize_t show_max_queued_events(struct class_device *class, char *buf)
{
sprintf(buf, "%d", sysfs_attrib_max_queued_events);
-
return strlen(buf) + 1;
}
-static ssize_t store_max_queued_events(struct device *dev, const char *buf,
- size_t count)
+static ssize_t store_max_queued_events(struct class_device *class,
+ const char *buf, size_t count)
{
return 0;
}
-static ssize_t show_max_user_devices(struct device *dev, char *buf)
+static ssize_t show_max_user_devices(struct class_device *class, char *buf)
{
sprintf(buf, "%d", sysfs_attrib_max_user_devices);
-
return strlen(buf) + 1;
}
-static ssize_t store_max_user_devices(struct device *dev, const char *buf,
- size_t count)
+static ssize_t store_max_user_devices(struct class_device *class,
+ const char *buf, size_t count)
{
return 0;
}
-static ssize_t show_max_user_watches(struct device *dev, char *buf)
+static ssize_t show_max_user_watches(struct class_device *class, char *buf)
{
sprintf(buf, "%d", sysfs_attrib_max_user_watches);
-
return strlen(buf) + 1;
}
-static ssize_t store_max_user_watches(struct device *dev, const char *buf,
- size_t count)
+static ssize_t store_max_user_watches(struct class_device *class,
+ const char *buf, size_t count)
{
return 0;
}
-
-static DEVICE_ATTR(max_queued_events, S_IRUGO | S_IWUSR, show_max_queued_events,
- store_max_queued_events);
-static DEVICE_ATTR(max_user_devices, S_IRUGO | S_IWUSR, show_max_user_devices,
- store_max_user_devices);
-static DEVICE_ATTR(max_user_watches, S_IRUGO | S_IWUSR, show_max_user_watches,
- store_max_user_watches);
-
-static struct device_attribute *inotify_device_attrs[] = {
- &dev_attr_max_queued_events,
- &dev_attr_max_user_devices,
- &dev_attr_max_user_watches,
- NULL
-};
-
+static CLASS_DEVICE_ATTR(max_queued_events, S_IRUGO | S_IWUSR,
+ show_max_queued_events, store_max_queued_events);
+static CLASS_DEVICE_ATTR(max_user_devices, S_IRUGO | S_IWUSR,
+ show_max_user_devices, store_max_user_devices);
+static CLASS_DEVICE_ATTR(max_user_watches, S_IRUGO | S_IWUSR,
+ show_max_user_watches, store_max_user_watches);
/*
* A list of these is attached to each instance of the driver
@@ -307,7 +295,8 @@
{
int ret;
- if (atomic_read(&dev->user->inotify_watches) >= sysfs_attrib_max_user_watches)
+ if (atomic_read(&dev->user->inotify_watches) >=
+ sysfs_attrib_max_user_watches)
return -ENOSPC;
repeat:
@@ -968,7 +957,8 @@
static int __init inotify_init(void)
{
- int ret,i;
+ struct class_device *class;
+ int ret;
ret = misc_register(&inotify_device);
if (ret)
@@ -978,11 +968,10 @@
sysfs_attrib_max_user_devices = 64;
sysfs_attrib_max_user_watches = 16384;
- for (i = 0; inotify_device_attrs[i]; i++) {
- int res;
- res = device_create_file (inotify_device.dev, inotify_device_attrs[i]);
- printk(KERN_INFO "Inotify sysfs create file = %d\n", res);
- }
+ class = inotify_device.class;
+ class_device_create_file(class, &class_device_attr_max_queued_events);
+ class_device_create_file(class, &class_device_attr_max_user_devices);
+ class_device_create_file(class, &class_device_attr_max_user_watches);
atomic_set(&inotify_cookie, 0);
^ permalink raw reply [flat|nested] 16+ messages in thread
* [patch] inotify: vfs_permission was replaced
2004-11-17 16:57 [patch] inotify: make our sysfs files show up Robert Love
@ 2004-11-17 18:02 ` Robert Love
2004-11-17 19:08 ` Christoph Hellwig
2004-11-17 20:10 ` [patch] inotify: add sysfs store support Robert Love
2004-11-18 19:05 ` [patch] inotify: grab right lock Robert Love
1 sibling, 2 replies; 16+ messages in thread
From: Robert Love @ 2004-11-17 18:02 UTC (permalink / raw)
To: ttb; +Cc: linux-kernel
John,
In 2.6.10-rc, vfs_permission() was replaced by generic_permission(),
which also has a slightly changed behavior and argument list.
Robert Love
vfs_permission was replaced by generic_permission in 2.6.10-rc.
Signed-Off-By: Robert Love <rml@novell.com>
inotify.c | 2 +-
1 files changed, 1 insertion(+), 1 deletion(-)
diff -u linux/drivers/char/inotify.c linux/drivers/char/inotify.c
--- linux/drivers/char/inotify.c 2004-11-15 15:28:34.951248696 -0500
+++ linux/drivers/char/inotify.c 2004-11-16 14:42:11.929575168 -0500
@@ -163,7 +151,7 @@
inode = nd.dentry->d_inode;
/* you can only watch an inode if you have read permissions on it */
- error = vfs_permission(inode, MAY_READ);
+ error = generic_permission(inode, MAY_READ, NULL);
if (error) {
inode = ERR_PTR(error);
goto release_and_out;
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [patch] inotify: vfs_permission was replaced
2004-11-17 18:02 ` [patch] inotify: vfs_permission was replaced Robert Love
@ 2004-11-17 19:08 ` Christoph Hellwig
2004-11-17 19:10 ` Robert Love
2004-11-17 20:10 ` [patch] inotify: add sysfs store support Robert Love
1 sibling, 1 reply; 16+ messages in thread
From: Christoph Hellwig @ 2004-11-17 19:08 UTC (permalink / raw)
To: Robert Love; +Cc: ttb, linux-kernel
On Wed, Nov 17, 2004 at 01:02:40PM -0500, Robert Love wrote:
> John,
>
> In 2.6.10-rc, vfs_permission() was replaced by generic_permission(),
> which also has a slightly changed behavior and argument list.
And using either of them was/is totally bogus. It's a helper for filesystems,
not a general API.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [patch] inotify: vfs_permission was replaced
2004-11-17 19:08 ` Christoph Hellwig
@ 2004-11-17 19:10 ` Robert Love
2004-11-17 19:18 ` Christoph Hellwig
0 siblings, 1 reply; 16+ messages in thread
From: Robert Love @ 2004-11-17 19:10 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: ttb, linux-kernel
On Wed, 2004-11-17 at 19:08 +0000, Christoph Hellwig wrote:
> And using either of them was/is totally bogus. It's a helper for filesystems,
> not a general API.
Is there some specific reason you have a problem with its use? It works
quite nicely for what we need.
Robert Love
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [patch] inotify: vfs_permission was replaced
2004-11-17 19:18 ` Christoph Hellwig
@ 2004-11-17 19:17 ` Robert Love
2004-11-17 20:09 ` Mike Waychison
2004-11-17 20:19 ` [patch] inotify: vfs_permission was replaced Al Viro
0 siblings, 2 replies; 16+ messages in thread
From: Robert Love @ 2004-11-17 19:17 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: ttb, linux-kernel
On Wed, 2004-11-17 at 19:18 +0000, Christoph Hellwig wrote:
> No it doesn't. Please try to understand the APIs before you're using them.
> Just looking at the callers should give you an immediate clue.
Maybe you should look at the code in question. We actually want to
perform the exact same sort of permission checks that, say, read
performs.
Robert Love
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [patch] inotify: vfs_permission was replaced
2004-11-17 19:10 ` Robert Love
@ 2004-11-17 19:18 ` Christoph Hellwig
2004-11-17 19:17 ` Robert Love
0 siblings, 1 reply; 16+ messages in thread
From: Christoph Hellwig @ 2004-11-17 19:18 UTC (permalink / raw)
To: Robert Love; +Cc: Christoph Hellwig, ttb, linux-kernel
On Wed, Nov 17, 2004 at 02:10:01PM -0500, Robert Love wrote:
> On Wed, 2004-11-17 at 19:08 +0000, Christoph Hellwig wrote:
>
> > And using either of them was/is totally bogus. It's a helper for filesystems,
> > not a general API.
>
> Is there some specific reason you have a problem with its use? It works
> quite nicely for what we need.
No it doesn't. Please try to understand the APIs before you're using them.
Just looking at the callers should give you an immediate clue.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [patch] inotify: vfs_permission was replaced
2004-11-17 19:17 ` Robert Love
@ 2004-11-17 20:09 ` Mike Waychison
2004-11-17 20:17 ` [patch] inotify: use permission not vfs_permission Robert Love
2004-11-17 20:19 ` [patch] inotify: vfs_permission was replaced Al Viro
1 sibling, 1 reply; 16+ messages in thread
From: Mike Waychison @ 2004-11-17 20:09 UTC (permalink / raw)
To: Robert Love; +Cc: Christoph Hellwig, ttb, linux-kernel
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Robert Love wrote:
> On Wed, 2004-11-17 at 19:18 +0000, Christoph Hellwig wrote:
>
>
>>No it doesn't. Please try to understand the APIs before you're using them.
>>Just looking at the callers should give you an immediate clue.
>
>
> Maybe you should look at the code in question. We actually want to
> perform the exact same sort of permission checks that, say, read
> performs.
>
> Robert Love
>
>
> -
> 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/
use permission()
- --
Mike Waychison
Sun Microsystems, Inc.
1 (650) 352-5299 voice
1 (416) 202-8336 voice
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
NOTICE: The opinions expressed in this email are held by me,
and may not represent the views of Sun Microsystems, Inc.
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.5 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org
iD8DBQFBm6/hdQs4kOxk3/MRAjFPAJ9tmyglXGvlhP4aYFzbX4uAmXIZkwCgjs86
mUksZfBEDIdncxVMmutvVGA=
=ei8c
-----END PGP SIGNATURE-----
^ permalink raw reply [flat|nested] 16+ messages in thread
* [patch] inotify: add sysfs store support
2004-11-17 18:02 ` [patch] inotify: vfs_permission was replaced Robert Love
2004-11-17 19:08 ` Christoph Hellwig
@ 2004-11-17 20:10 ` Robert Love
2004-11-18 1:00 ` John McCutchan
1 sibling, 1 reply; 16+ messages in thread
From: Robert Love @ 2004-11-17 20:10 UTC (permalink / raw)
To: ttb; +Cc: linux-kernel
Attached patch implements the final chunk of our sysfs solution: store
support. I added basic write support with some simple checking (do not
allow zero) to the existing sysfs attributes.
I made a few other changes. I added a newline after the values in the
show function. The other sysfs attributes do this and it makes
sense--do a "cat *" in our sysfs directory before and after. I also
just return sprintf() directly instead of the strlen(). I also made
max_queued_events unsigned, since dev->max_events is unsigned. If we
don't do this we need to add checking in store_max_queued_events to
ensure that the given value is less than or equal to INT_MAX so this
seems easier and more optimal anyhow. The other values I kept at int
since that is the range of atomic_t's.
I am running it now. I can read and write the values fine. Works
great.
Robert Love
Add store support to our sysfs attributes and a few other changes.
inotify.c | 35 +++++++++++++++++++++++++----------
1 files changed, 25 insertions(+), 10 deletions(-)
diff -u linux/drivers/char/inotify.c linux/drivers/char/inotify.c
--- linux/drivers/char/inotify.c 2004-11-16 14:42:11.929575168 -0500
+++ linux/drivers/char/inotify.c 2004-11-17 12:28:27.921136656 -0500
@@ -40,7 +40,7 @@
static int sysfs_attrib_max_user_devices;
static int sysfs_attrib_max_user_watches;
-static int sysfs_attrib_max_queued_events;
+static unsigned int sysfs_attrib_max_queued_events;
/*
* struct inotify_device - represents an open instance of an inotify device
@@ -82,38 +82,53 @@
static ssize_t show_max_queued_events(struct class_device *class, char *buf)
{
- sprintf(buf, "%d", sysfs_attrib_max_queued_events);
- return strlen(buf) + 1;
+ return sprintf(buf, "%d\n", sysfs_attrib_max_queued_events);
}
static ssize_t store_max_queued_events(struct class_device *class,
const char *buf, size_t count)
{
- return 0;
+ unsigned int max;
+
+ if (sscanf(buf, "%u", &max) > 0 && max > 0) {
+ sysfs_attrib_max_queued_events = max;
+ return strlen(buf);
+ }
+ return -EINVAL;
}
static ssize_t show_max_user_devices(struct class_device *class, char *buf)
{
- sprintf(buf, "%d", sysfs_attrib_max_user_devices);
- return strlen(buf) + 1;
+ return sprintf(buf, "%d\n", sysfs_attrib_max_user_devices);
}
static ssize_t store_max_user_devices(struct class_device *class,
const char *buf, size_t count)
{
- return 0;
+ int max;
+
+ if (sscanf(buf, "%d", &max) > 0 && max > 0) {
+ sysfs_attrib_max_user_devices = max;
+ return strlen(buf);
+ }
+ return -EINVAL;
}
static ssize_t show_max_user_watches(struct class_device *class, char *buf)
{
- sprintf(buf, "%d", sysfs_attrib_max_user_watches);
- return strlen(buf) + 1;
+ return sprintf(buf, "%d\n", sysfs_attrib_max_user_watches);
}
static ssize_t store_max_user_watches(struct class_device *class,
const char *buf, size_t count)
{
- return 0;
+ int max;
+
+ if (sscanf(buf, "%d", &max) > 0 && max > 0) {
+ sysfs_attrib_max_user_watches = max;
+ return strlen(buf);
+ }
+ return -EINVAL;
}
static CLASS_DEVICE_ATTR(max_queued_events, S_IRUGO | S_IWUSR,
^ permalink raw reply [flat|nested] 16+ messages in thread
* [patch] inotify: use permission not vfs_permission
2004-11-17 20:09 ` Mike Waychison
@ 2004-11-17 20:17 ` Robert Love
2004-11-17 20:25 ` Mike Waychison
0 siblings, 1 reply; 16+ messages in thread
From: Robert Love @ 2004-11-17 20:17 UTC (permalink / raw)
To: Mike Waychison; +Cc: Christoph Hellwig, ttb, linux-kernel
On Wed, 2004-11-17 at 15:09 -0500, Mike Waychison wrote:
> use permission()
I appreciate the constructive comment.
John, attached patch replaces vfs_permission() with permission().
Thanks, Mike.
Robert Love
Use permission() instead of generic_permission().
Signed-Off-By: Robert Love <rml@novell.com>
diff -u linux/drivers/char/inotify.c linux/drivers/char/inotify.c
--- linux/drivers/char/inotify.c 2004-11-17 12:28:27.921136656 -0500
+++ linux/drivers/char/inotify.c 2004-11-17 12:28:27.921136656 -0500
@@ -166,7 +166,7 @@
inode = nd.dentry->d_inode;
/* you can only watch an inode if you have read permissions on it */
- error = generic_permission(inode, MAY_READ, NULL);
+ error = permission(inode, MAY_READ);
if (error) {
inode = ERR_PTR(error);
goto release_and_out;
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [patch] inotify: vfs_permission was replaced
2004-11-17 19:17 ` Robert Love
2004-11-17 20:09 ` Mike Waychison
@ 2004-11-17 20:19 ` Al Viro
1 sibling, 0 replies; 16+ messages in thread
From: Al Viro @ 2004-11-17 20:19 UTC (permalink / raw)
To: Robert Love; +Cc: Christoph Hellwig, ttb, linux-kernel
On Wed, Nov 17, 2004 at 02:17:32PM -0500, Robert Love wrote:
> On Wed, 2004-11-17 at 19:18 +0000, Christoph Hellwig wrote:
>
> > No it doesn't. Please try to understand the APIs before you're using them.
> > Just looking at the callers should give you an immediate clue.
>
> Maybe you should look at the code in question. We actually want to
> perform the exact same sort of permission checks that, say, read
> performs.
Maybe you should look at the code in kernel - e.g. aforementioned sys_read().
Or sys_open().
The only fs-independent code that has any business calling that puppy is
permission(9).
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [patch] inotify: use permission not vfs_permission
2004-11-17 20:17 ` [patch] inotify: use permission not vfs_permission Robert Love
@ 2004-11-17 20:25 ` Mike Waychison
2004-11-17 20:28 ` Robert Love
0 siblings, 1 reply; 16+ messages in thread
From: Mike Waychison @ 2004-11-17 20:25 UTC (permalink / raw)
To: Robert Love; +Cc: Christoph Hellwig, ttb, linux-kernel
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Robert Love wrote:
> On Wed, 2004-11-17 at 15:09 -0500, Mike Waychison wrote:
>
>
>>use permission()
>
>
> I appreciate the constructive comment.
>
> John, attached patch replaces vfs_permission() with permission().
>
> Thanks, Mike.
>
> Robert Love
>
>
> Use permission() instead of generic_permission().
>
> Signed-Off-By: Robert Love <rml@novell.com>
>
> diff -u linux/drivers/char/inotify.c linux/drivers/char/inotify.c
> --- linux/drivers/char/inotify.c 2004-11-17 12:28:27.921136656 -0500
> +++ linux/drivers/char/inotify.c 2004-11-17 12:28:27.921136656 -0500
> @@ -166,7 +166,7 @@
> inode = nd.dentry->d_inode;
>
> /* you can only watch an inode if you have read permissions on it */
> - error = generic_permission(inode, MAY_READ, NULL);
> + error = permission(inode, MAY_READ);
> if (error) {
> inode = ERR_PTR(error);
> goto release_and_out;
>
>
permission() still takes 3 arguments though. I think it is safe to pass
NULL for the nameidata.
- --
Mike Waychison
Sun Microsystems, Inc.
1 (650) 352-5299 voice
1 (416) 202-8336 voice
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
NOTICE: The opinions expressed in this email are held by me,
and may not represent the views of Sun Microsystems, Inc.
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.5 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org
iD8DBQFBm7O2dQs4kOxk3/MRApYgAJ0c1XGD99i8Yir2LYTAtosJrOtvIACfT5BE
C3T8DDq+L9NskVe1sI47IKM=
=BZA8
-----END PGP SIGNATURE-----
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [patch] inotify: use permission not vfs_permission
2004-11-17 20:25 ` Mike Waychison
@ 2004-11-17 20:28 ` Robert Love
2004-11-18 0:59 ` John McCutchan
0 siblings, 1 reply; 16+ messages in thread
From: Robert Love @ 2004-11-17 20:28 UTC (permalink / raw)
To: Mike Waychison; +Cc: Christoph Hellwig, ttb, linux-kernel
On Wed, 2004-11-17 at 15:25 -0500, Mike Waychison wrote:
> permission() still takes 3 arguments though. I think it is safe to pass
> NULL for the nameidata.
Ugh, I compile-tested the wrong tree. Thanks.
It is safe to pass NULL.
Patch below.
Robert Love
Use permission() instead of generic_permission().
Signed-Off-By: Robert Love <rml@novell.com>
diff -u linux/drivers/char/inotify.c linux/drivers/char/inotify.c
--- linux/drivers/char/inotify.c 2004-11-17 12:28:27.921136656 -0500
+++ linux/drivers/char/inotify.c 2004-11-17 12:28:27.921136656 -0500
@@ -166,7 +166,7 @@
inode = nd.dentry->d_inode;
/* you can only watch an inode if you have read permissions on it */
- error = vfs_permission(inode, MAY_READ);
+ error = permission(inode, MAY_READ, NULL);
if (error) {
inode = ERR_PTR(error);
goto release_and_out;
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [patch] inotify: use permission not vfs_permission
2004-11-17 20:28 ` Robert Love
@ 2004-11-18 0:59 ` John McCutchan
0 siblings, 0 replies; 16+ messages in thread
From: John McCutchan @ 2004-11-18 0:59 UTC (permalink / raw)
To: Robert Love; +Cc: Mike Waychison, Christoph Hellwig, linux-kernel
This looks good. Thanks.
On Wed, 2004-11-17 at 15:28 -0500, Robert Love wrote:
> On Wed, 2004-11-17 at 15:25 -0500, Mike Waychison wrote:
>
> > permission() still takes 3 arguments though. I think it is safe to pass
> > NULL for the nameidata.
>
> Ugh, I compile-tested the wrong tree. Thanks.
>
> It is safe to pass NULL.
>
> Patch below.
>
> Robert Love
>
>
> Use permission() instead of generic_permission().
>
> Signed-Off-By: Robert Love <rml@novell.com>
>
> diff -u linux/drivers/char/inotify.c linux/drivers/char/inotify.c
> --- linux/drivers/char/inotify.c 2004-11-17 12:28:27.921136656 -0500
> +++ linux/drivers/char/inotify.c 2004-11-17 12:28:27.921136656 -0500
> @@ -166,7 +166,7 @@
> inode = nd.dentry->d_inode;
>
> /* you can only watch an inode if you have read permissions on it */
> - error = vfs_permission(inode, MAY_READ);
> + error = permission(inode, MAY_READ, NULL);
> if (error) {
> inode = ERR_PTR(error);
> goto release_and_out;
>
>
--
John McCutchan <ttb@tentacle.dhs.org>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [patch] inotify: add sysfs store support
2004-11-17 20:10 ` [patch] inotify: add sysfs store support Robert Love
@ 2004-11-18 1:00 ` John McCutchan
2004-11-18 1:24 ` Robert Love
0 siblings, 1 reply; 16+ messages in thread
From: John McCutchan @ 2004-11-18 1:00 UTC (permalink / raw)
To: Robert Love; +Cc: linux-kernel
Awesome work! But I am going to hold off applying this, until the
mainline kernel gets the misc device changes.
John
On Wed, 2004-11-17 at 15:10 -0500, Robert Love wrote:
> Attached patch implements the final chunk of our sysfs solution: store
> support. I added basic write support with some simple checking (do not
> allow zero) to the existing sysfs attributes.
>
> I made a few other changes. I added a newline after the values in the
> show function. The other sysfs attributes do this and it makes
> sense--do a "cat *" in our sysfs directory before and after. I also
> just return sprintf() directly instead of the strlen(). I also made
> max_queued_events unsigned, since dev->max_events is unsigned. If we
> don't do this we need to add checking in store_max_queued_events to
> ensure that the given value is less than or equal to INT_MAX so this
> seems easier and more optimal anyhow. The other values I kept at int
> since that is the range of atomic_t's.
>
> I am running it now. I can read and write the values fine. Works
> great.
>
> Robert Love
>
>
> Add store support to our sysfs attributes and a few other changes.
>
> inotify.c | 35 +++++++++++++++++++++++++----------
> 1 files changed, 25 insertions(+), 10 deletions(-)
>
> diff -u linux/drivers/char/inotify.c linux/drivers/char/inotify.c
> --- linux/drivers/char/inotify.c 2004-11-16 14:42:11.929575168 -0500
> +++ linux/drivers/char/inotify.c 2004-11-17 12:28:27.921136656 -0500
> @@ -40,7 +40,7 @@
>
> static int sysfs_attrib_max_user_devices;
> static int sysfs_attrib_max_user_watches;
> -static int sysfs_attrib_max_queued_events;
> +static unsigned int sysfs_attrib_max_queued_events;
>
> /*
> * struct inotify_device - represents an open instance of an inotify device
> @@ -82,38 +82,53 @@
>
> static ssize_t show_max_queued_events(struct class_device *class, char *buf)
> {
> - sprintf(buf, "%d", sysfs_attrib_max_queued_events);
> - return strlen(buf) + 1;
> + return sprintf(buf, "%d\n", sysfs_attrib_max_queued_events);
> }
>
> static ssize_t store_max_queued_events(struct class_device *class,
> const char *buf, size_t count)
> {
> - return 0;
> + unsigned int max;
> +
> + if (sscanf(buf, "%u", &max) > 0 && max > 0) {
> + sysfs_attrib_max_queued_events = max;
> + return strlen(buf);
> + }
> + return -EINVAL;
> }
>
> static ssize_t show_max_user_devices(struct class_device *class, char *buf)
> {
> - sprintf(buf, "%d", sysfs_attrib_max_user_devices);
> - return strlen(buf) + 1;
> + return sprintf(buf, "%d\n", sysfs_attrib_max_user_devices);
> }
>
> static ssize_t store_max_user_devices(struct class_device *class,
> const char *buf, size_t count)
> {
> - return 0;
> + int max;
> +
> + if (sscanf(buf, "%d", &max) > 0 && max > 0) {
> + sysfs_attrib_max_user_devices = max;
> + return strlen(buf);
> + }
> + return -EINVAL;
> }
>
> static ssize_t show_max_user_watches(struct class_device *class, char *buf)
> {
> - sprintf(buf, "%d", sysfs_attrib_max_user_watches);
> - return strlen(buf) + 1;
> + return sprintf(buf, "%d\n", sysfs_attrib_max_user_watches);
> }
>
> static ssize_t store_max_user_watches(struct class_device *class,
> const char *buf, size_t count)
> {
> - return 0;
> + int max;
> +
> + if (sscanf(buf, "%d", &max) > 0 && max > 0) {
> + sysfs_attrib_max_user_watches = max;
> + return strlen(buf);
> + }
> + return -EINVAL;
> }
>
> static CLASS_DEVICE_ATTR(max_queued_events, S_IRUGO | S_IWUSR,
>
>
--
John McCutchan <ttb@tentacle.dhs.org>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [patch] inotify: add sysfs store support
2004-11-18 1:00 ` John McCutchan
@ 2004-11-18 1:24 ` Robert Love
0 siblings, 0 replies; 16+ messages in thread
From: Robert Love @ 2004-11-18 1:24 UTC (permalink / raw)
To: John McCutchan; +Cc: linux-kernel
On Wed, 2004-11-17 at 20:00 -0500, John McCutchan wrote:
> Awesome work!
Thanks.
> But I am going to hold off applying this, until the
> mainline kernel gets the misc device changes.
OK, but it is going to be awhile. It is too late for 2.6.10, so it
won't go in until 2.6.11.
Robert Love
^ permalink raw reply [flat|nested] 16+ messages in thread
* [patch] inotify: grab right lock
2004-11-17 16:57 [patch] inotify: make our sysfs files show up Robert Love
2004-11-17 18:02 ` [patch] inotify: vfs_permission was replaced Robert Love
@ 2004-11-18 19:05 ` Robert Love
1 sibling, 0 replies; 16+ messages in thread
From: Robert Love @ 2004-11-18 19:05 UTC (permalink / raw)
To: ttb; +Cc: linux-kernel
John,
Ah, another part of my locking rewrite I remember and should pull out
and send now.
We need to grab inode_lock before calling __iget().
Robert Love
need to hold inode_lock around __iget()
drivers/char/inotify.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletion(-)
diff -u linux/drivers/char/inotify.c linux/drivers/char/inotify.c
--- linux/drivers/char/inotify.c 2004-11-18 12:31:14.294242616 -0500
+++ linux/drivers/char/inotify.c 2004-11-18 13:59:33.400655992 -0500
@@ -171,7 +171,9 @@
goto release_and_out;
}
+ spin_lock(&inode_lock);
__iget(inode);
+ spin_unlock(&inode_lock);
release_and_out:
path_release(&nd);
out:
@@ -790,7 +792,7 @@
*/
static void inotify_release_all_watches(struct inotify_device *dev)
{
- struct inotify_watch *watch,*next;
+ struct inotify_watch *watch, *next;
list_for_each_entry_safe(watch, next, &dev->watches, d_list)
ignore_helper(watch, 0);
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2004-11-19 0:44 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-11-17 16:57 [patch] inotify: make our sysfs files show up Robert Love
2004-11-17 18:02 ` [patch] inotify: vfs_permission was replaced Robert Love
2004-11-17 19:08 ` Christoph Hellwig
2004-11-17 19:10 ` Robert Love
2004-11-17 19:18 ` Christoph Hellwig
2004-11-17 19:17 ` Robert Love
2004-11-17 20:09 ` Mike Waychison
2004-11-17 20:17 ` [patch] inotify: use permission not vfs_permission Robert Love
2004-11-17 20:25 ` Mike Waychison
2004-11-17 20:28 ` Robert Love
2004-11-18 0:59 ` John McCutchan
2004-11-17 20:19 ` [patch] inotify: vfs_permission was replaced Al Viro
2004-11-17 20:10 ` [patch] inotify: add sysfs store support Robert Love
2004-11-18 1:00 ` John McCutchan
2004-11-18 1:24 ` Robert Love
2004-11-18 19:05 ` [patch] inotify: grab right lock Robert Love
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox