On Fri, Mar 23, 2012 at 9:25 PM, Eric W. Biederman
<ebiederm@xxxxxxxxxxxx> wrote:
>
> Lucas can you take a look at the patch below. I don't have a test
> harness to test this but this should make poll generally useful
> for all sysctls as well as fix the module removal case.
>
> In particular if you could test to see if the code still works as
> expected for the hostname and domainname cases that would be great.
>
> I don't anticipate any problems but real world testing is always good.
>
> fs/proc/proc_sysctl.c | 34 +++++++++++++++++-----------------
> include/linux/sysctl.h | 22 +++-------------------
> kernel/utsname_sysctl.c | 14 ++++----------
> 3 files changed, 24 insertions(+), 46 deletions(-)
> ---
> diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
> index 47b474b..98fd5c9 100644
> --- a/fs/proc/proc_sysctl.c
> +++ b/fs/proc/proc_sysctl.c
> @@ -16,13 +16,15 @@ static const struct inode_operations proc_sys_inode_operations;
> static const struct file_operations proc_sys_dir_file_operations;
> static const struct inode_operations proc_sys_dir_operations;
>
> -void proc_sys_poll_notify(struct ctl_table_poll *poll)
> +static inline void *proc_sys_poll_event(struct ctl_table *table)
> {
> - if (!poll)
> - return;
> + return (void *)(unsigned long)atomic_read(&table->event);
> +}
>
> - atomic_inc(&poll->event);
> - wake_up_interruptible(&poll->wait);
> +void proc_sys_poll_notify(struct ctl_table_header *head, struct ctl_table *table)
> +{
> + atomic_inc(&table->event);
> + wake_up_interruptible(&head->wait);
> }
>
> static struct ctl_table root_table[] = {
> @@ -169,6 +171,7 @@ static void init_header(struct ctl_table_header *head,
> for (entry = table; entry->procname; entry++, node++) {
> rb_init_node(&node->node);
> node->header = head;
> + atomic_set(&entry->event, 1);
> }
> }
> }
> @@ -240,6 +243,8 @@ static void start_unregistering(struct ctl_table_header *p)
> /* anything non-NULL; we'll never dereference it */
> p->unregistering = ERR_PTR(-EINVAL);
> }
> + /* Don't let poll sleep forever on deleted entries */
> + wake_up_interruptible(&p->wait);
> /*
> * do not remove from the list until nobody holds it; walking the
> * list in do_sysctl() relies on that.
> @@ -505,6 +510,9 @@ static ssize_t proc_sys_call_handler(struct file *filp, void __user *buf,
> error = table->proc_handler(table, write, buf, &res, ppos);
> if (!error)
> error = res;
> +
> + if (write)
> + proc_sys_poll_notify(head, table);
> out:
> sysctl_head_finish(head);
>
> @@ -532,8 +540,7 @@ static int proc_sys_open(struct inode *inode, struct file *filp)
> if (IS_ERR(head))
> return PTR_ERR(head);
>
> - if (table->poll)
> - filp->private_data = proc_sys_poll_event(table->poll);
> + filp->private_data = proc_sys_poll_event(table);
>
> sysctl_head_finish(head);
>
> @@ -552,21 +559,14 @@ static unsigned int proc_sys_poll(struct file *filp, poll_table *wait)
> if (IS_ERR(head))
> return POLLERR | POLLHUP;
>
> - if (!table->proc_handler)
> - goto out;
> -
> - if (!table->poll)
> - goto out;
> -
> event = (unsigned long)filp->private_data;
> - poll_wait(filp, &table->poll->wait, wait);
> + poll_wait(filp, &head->wait, wait);
>
> - if (event != atomic_read(&table->poll->event)) {
> - filp->private_data = proc_sys_poll_event(table->poll);
> + if (event != atomic_read(&table->event)) {
> + filp->private_data = proc_sys_poll_event(table);
> ret = POLLIN | POLLRDNORM | POLLERR | POLLPRI;
> }
>
> -out:
> sysctl_head_finish(head);
>
> return ret;
> diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
> index c34b4c8..8647ee0 100644
> --- a/include/linux/sysctl.h
> +++ b/include/linux/sysctl.h
> @@ -992,34 +992,17 @@ extern int proc_do_large_bitmap(struct ctl_table *, int,
> * cover common cases.
> */
>
> -/* Support for userspace poll() to watch for changes */
> -struct ctl_table_poll {
> - atomic_t event;
> - wait_queue_head_t wait;
> -};
> -
> -static inline void *proc_sys_poll_event(struct ctl_table_poll *poll)
> -{
> - return (void *)(unsigned long)atomic_read(&poll->event);
> -}
> -
> -#define __CTL_TABLE_POLL_INITIALIZER(name) { \
> - .event = ATOMIC_INIT(0), \
> - .wait = __WAIT_QUEUE_HEAD_INITIALIZER(name.wait) }
> -
> -#define DEFINE_CTL_TABLE_POLL(name) \
> - struct ctl_table_poll name = __CTL_TABLE_POLL_INITIALIZER(name)
>
> /* A sysctl table is an array of struct ctl_table: */
> struct ctl_table
> {
> const char *procname; /* Text ID for /proc/sys, or zero */
> void *data;
> + atomic_t event;
> int maxlen;
> umode_t mode;
> struct ctl_table *child; /* Deprecated */
> proc_handler *proc_handler; /* Callback for text formatting */
> - struct ctl_table_poll *poll;
> void *extra1;
> void *extra2;
> };
> @@ -1042,6 +1025,7 @@ struct ctl_table_header
> };
> struct rcu_head rcu;
> };
> + wait_queue_head_t wait;
If you have a waitqueue per table instead of per entry you will get
spurious notifications when other entries change. The easiest way to
test this is to poll /proc/sys/kernel/hostname and change your
domainname.
I couldn't apply this patch to any tree (linus/master + my previous
patch, your master, 3.3 + my previous patch), so I couldn't test. On
top of your tree:
[lucas@vader kernel]$ git am /tmp/a.patch
Applying: Making poll generally useful for sysctls
error: patch failed: fs/proc/proc_sysctl.c:16
error: fs/proc/proc_sysctl.c: patch does not apply
error: patch failed: include/linux/sysctl.h:992
error: include/linux/sysctl.h: patch does not apply
Patch failed at 0001 Making poll generally useful for sysctls
Lucas De Marchi
--
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
