[PATCH] support polling of /proc/swaps

[PATCH] support polling of /proc/swaps

[Kay Sievers]

From: Kay Sievers <kay.sievers@vrfy.org>
Subject: support polling of /proc/swaps

System management wants to subscribe to changes in swap
configuration. Make /proc/swaps pollable like /proc/mounts.

Signed-Off-By: Kay Sievers <kay.sievers@vrfy.org>
---
 mm/swapfile.c |   48 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 47 insertions(+), 1 deletion(-)

--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -30,6 +30,7 @@
 #include <linux/capability.h>
 #include <linux/syscalls.h>
 #include <linux/memcontrol.h>
+#include <linux/poll.h>
 
 #include <asm/pgtable.h>
 #include <asm/tlbflush.h>
@@ -58,6 +59,9 @@ static struct swap_info_struct *swap_inf
 
 static DEFINE_MUTEX(swapon_mutex);
 
+static DECLARE_WAIT_QUEUE_HEAD(proc_poll_wait);
+static int proc_poll_event;
+
 static inline unsigned char swap_count(unsigned char ent)
 {
 	return ent & ~SWAP_HAS_CACHE;	/* may include SWAP_HAS_CONT flag */
@@ -1680,6 +1684,8 @@ SYSCALL_DEFINE1(swapoff, const char __us
 	}
 	filp_close(swap_file, NULL);
 	err = 0;
+	proc_poll_event++;
+	wake_up_interruptible(&proc_poll_wait);
 
 out_dput:
 	filp_close(victim, NULL);
@@ -1688,6 +1694,25 @@ out:
 }
 
 #ifdef CONFIG_PROC_FS
+struct proc_swaps {
+	struct seq_file seq;
+	int event;
+};
+
+static unsigned swaps_poll(struct file *file, poll_table *wait)
+{
+	struct proc_swaps *s = file->private_data;
+
+	poll_wait(file, &proc_poll_wait, wait);
+
+	if (s->event != proc_poll_event) {
+		s->event = proc_poll_event;
+		return POLLIN | POLLRDNORM | POLLERR | POLLPRI;
+	}
+
+	return POLLIN | POLLRDNORM;
+}
+
 /* iterator */
 static void *swap_start(struct seq_file *swap, loff_t *pos)
 {
@@ -1771,7 +1796,24 @@ static const struct seq_operations swaps
 
 static int swaps_open(struct inode *inode, struct file *file)
 {
-	return seq_open(file, &swaps_op);
+	struct proc_swaps *s;
+	int ret;
+
+	s = kmalloc(sizeof(struct proc_swaps), GFP_KERNEL);
+	if (!s)
+		return -ENOMEM;
+
+	file->private_data = &s->seq;
+
+	ret = seq_open(file, &swaps_op);
+	if (ret) {
+		kfree(s);
+		return ret;
+	}
+
+	s->seq.private = s;
+	s->event = proc_poll_event;
+	return ret;
 }
 
 static const struct file_operations proc_swaps_operations = {
@@ -1779,6 +1821,7 @@ static const struct file_operations proc
 	.read		= seq_read,
 	.llseek		= seq_lseek,
 	.release	= seq_release,
+	.poll		= swaps_poll,
 };
 
 static int __init procswaps_init(void)
@@ -2084,6 +2127,9 @@ SYSCALL_DEFINE2(swapon, const char __use
 		swap_info[prev]->next = type;
 	spin_unlock(&swap_lock);
 	mutex_unlock(&swapon_mutex);
+	proc_poll_event++;
+	wake_up_interruptible(&proc_poll_wait);
+
 	error = 0;
 	goto out;
 bad_swap:

Re: [PATCH] support polling of /proc/swaps

[Peter Zijlstra]

On Tue, 2010-10-19 at 11:19 +0200, Kay Sievers wrote:
> From: Kay Sievers <kay.sievers@vrfy.org>
> Subject: support polling of /proc/swaps
> 
> System management wants to subscribe to changes in swap
> configuration. Make /proc/swaps pollable like /proc/mounts.

And yet you didn't cc any of the mm/ folks on this patch...

> Signed-Off-By: Kay Sievers <kay.sievers@vrfy.org>
> ---
>  mm/swapfile.c |   48 +++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 47 insertions(+), 1 deletion(-)
> 
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -30,6 +30,7 @@
>  #include <linux/capability.h>
>  #include <linux/syscalls.h>
>  #include <linux/memcontrol.h>
> +#include <linux/poll.h>
>  
>  #include <asm/pgtable.h>
>  #include <asm/tlbflush.h>
> @@ -58,6 +59,9 @@ static struct swap_info_struct *swap_inf
>  
>  static DEFINE_MUTEX(swapon_mutex);
>  
> +static DECLARE_WAIT_QUEUE_HEAD(proc_poll_wait);
> +static int proc_poll_event;
> +
>  static inline unsigned char swap_count(unsigned char ent)
>  {
>  	return ent & ~SWAP_HAS_CACHE;	/* may include SWAP_HAS_CONT flag */
> @@ -1680,6 +1684,8 @@ SYSCALL_DEFINE1(swapoff, const char __us
>  	}
>  	filp_close(swap_file, NULL);
>  	err = 0;
> +	proc_poll_event++;
> +	wake_up_interruptible(&proc_poll_wait);
>  
>  out_dput:
>  	filp_close(victim, NULL);
> @@ -1688,6 +1694,25 @@ out:
>  }
>  
>  #ifdef CONFIG_PROC_FS
> +struct proc_swaps {
> +	struct seq_file seq;
> +	int event;
> +};
> +
> +static unsigned swaps_poll(struct file *file, poll_table *wait)
> +{
> +	struct proc_swaps *s = file->private_data;
> +
> +	poll_wait(file, &proc_poll_wait, wait);
> +
> +	if (s->event != proc_poll_event) {
> +		s->event = proc_poll_event;
> +		return POLLIN | POLLRDNORM | POLLERR | POLLPRI;
> +	}
> +
> +	return POLLIN | POLLRDNORM;
> +}
> +
>  /* iterator */
>  static void *swap_start(struct seq_file *swap, loff_t *pos)
>  {
> @@ -1771,7 +1796,24 @@ static const struct seq_operations swaps
>  
>  static int swaps_open(struct inode *inode, struct file *file)
>  {
> -	return seq_open(file, &swaps_op);
> +	struct proc_swaps *s;
> +	int ret;
> +
> +	s = kmalloc(sizeof(struct proc_swaps), GFP_KERNEL);
> +	if (!s)
> +		return -ENOMEM;
> +
> +	file->private_data = &s->seq;
> +
> +	ret = seq_open(file, &swaps_op);
> +	if (ret) {
> +		kfree(s);
> +		return ret;
> +	}
> +
> +	s->seq.private = s;
> +	s->event = proc_poll_event;
> +	return ret;
>  }
>  
>  static const struct file_operations proc_swaps_operations = {
> @@ -1779,6 +1821,7 @@ static const struct file_operations proc
>  	.read		= seq_read,
>  	.llseek		= seq_lseek,
>  	.release	= seq_release,
> +	.poll		= swaps_poll,
>  };
>  
>  static int __init procswaps_init(void)
> @@ -2084,6 +2127,9 @@ SYSCALL_DEFINE2(swapon, const char __use
>  		swap_info[prev]->next = type;
>  	spin_unlock(&swap_lock);
>  	mutex_unlock(&swapon_mutex);
> +	proc_poll_event++;
> +	wake_up_interruptible(&proc_poll_wait);
> +
>  	error = 0;
>  	goto out;
>  bad_swap:
> 
> 
> --
> 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/

Re: [PATCH] support polling of /proc/swaps

[Jonathan Corbet]

> From: Kay Sievers <kay.sievers@vrfy.org>
> Subject: support polling of /proc/swaps
> 
> System management wants to subscribe to changes in swap
> configuration. Make /proc/swaps pollable like /proc/mounts.

Please forgive me...I'm probably undercaffeinated and even dumber than
usual, but...

Here we have this:

> +static unsigned swaps_poll(struct file *file, poll_table *wait)
> +{
> +	struct proc_swaps *s = file->private_data;

But here I see:

>  static int swaps_open(struct inode *inode, struct file *file)
>  {
> -	return seq_open(file, &swaps_op);
> +	struct proc_swaps *s;
> +	int ret;
> +
> +	s = kmalloc(sizeof(struct proc_swaps), GFP_KERNEL);
> +	if (!s)
> +		return -ENOMEM;
> +
> +	file->private_data = &s->seq;

It sure looks to me like private_data is a struct seq_file pointer, not a
struct proc_swaps pointer.  What am I missing?

Thanks,

jon

Re: [PATCH] support polling of /proc/swaps

[Kay Sievers]

On Tue, Oct 19, 2010 at 18:11, Jonathan Corbet <corbet@lwn.net> wrote:
> Here we have this:
>
>> +static unsigned swaps_poll(struct file *file, poll_table *wait)
>> +{
>> +     struct proc_swaps *s = file->private_data;
>
> But here I see:
>
>>  static int swaps_open(struct inode *inode, struct file *file)
>>  {
>> -     return seq_open(file, &swaps_op);
>> +     struct proc_swaps *s;
>> +     int ret;
>> +
>> +     s = kmalloc(sizeof(struct proc_swaps), GFP_KERNEL);
>> +     if (!s)
>> +             return -ENOMEM;
>> +
>> +     file->private_data = &s->seq;
>
> It sure looks to me like private_data is a struct seq_file pointer, not a
> struct proc_swaps pointer.  What am I missing?

Right, that looks weird. It's the same pointer though, because it's
the first element. I'll correct that.

Thanks a lot for the sharp eyes,
Kay

Re: [PATCH] support polling of /proc/swaps

[Andrew Morton]

On Tue, 19 Oct 2010 11:19:16 +0200
Kay Sievers <kay.sievers@vrfy.org> wrote:

> From: Kay Sievers <kay.sievers@vrfy.org>
> Subject: support polling of /proc/swaps
> 
> System management wants to subscribe to changes in swap
> configuration. Make /proc/swaps pollable like /proc/mounts.
> 

It's a bit sad that we have to add quite a pile of infrastructure to
make a procfs file pollable.  I wonder if it's possible to provide some
core support for this, and reduce the amount of code at each particular
handler site.

Also, I wonder how we are to communicate the existence of this feature
to our users.  Nobody will look in Documentation/filesystems/.  Is
there a manpage?  Seems not...

> ---
>  mm/swapfile.c |   48 +++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 47 insertions(+), 1 deletion(-)
> 
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -30,6 +30,7 @@
>  #include <linux/capability.h>
>  #include <linux/syscalls.h>
>  #include <linux/memcontrol.h>
> +#include <linux/poll.h>
>  
>  #include <asm/pgtable.h>
>  #include <asm/tlbflush.h>
> @@ -58,6 +59,9 @@ static struct swap_info_struct *swap_inf
>  
>  static DEFINE_MUTEX(swapon_mutex);
>  
> +static DECLARE_WAIT_QUEUE_HEAD(proc_poll_wait);
> +static int proc_poll_event;

Please pick a lock to protect proc_poll_event.  Then document that
locking here, when you also document proc_poll_event ;)

>  static inline unsigned char swap_count(unsigned char ent)
>  {
>  	return ent & ~SWAP_HAS_CACHE;	/* may include SWAP_HAS_CONT flag */
> @@ -1680,6 +1684,8 @@ SYSCALL_DEFINE1(swapoff, const char __us
>  	}
>  	filp_close(swap_file, NULL);
>  	err = 0;
> +	proc_poll_event++;

Then fix this race.

> +	wake_up_interruptible(&proc_poll_wait);
>  
>  out_dput:
>  	filp_close(victim, NULL);

Re: [PATCH] support polling of /proc/swaps

[Kay Sievers]

On Tue, 2010-10-19 at 15:31 -0700, Andrew Morton wrote:
> On Tue, 19 Oct 2010 11:19:16 +0200
> Kay Sievers <kay.sievers@vrfy.org> wrote:

> It's a bit sad that we have to add quite a pile of infrastructure to
> make a procfs file pollable.  I wonder if it's possible to provide some
> core support for this, and reduce the amount of code at each particular
> handler site.

You mean something like adding the event counter to the seq_file? There
is /proc/self/mounts,mountinfo and /proc/swaps so far, I think.

> Also, I wonder how we are to communicate the existence of this feature
> to our users.  Nobody will look in Documentation/filesystems/.  Is
> there a manpage?  Seems not...

Hmm, 'man 5 proc'?

> > +static DECLARE_WAIT_QUEUE_HEAD(proc_poll_wait);
> > +static int proc_poll_event;
> 
> Please pick a lock to protect proc_poll_event.

An atomic_t should do it too, right?

> Then document that
> locking here, when you also document proc_poll_event ;)

The actual value has no meaning at all, it just tells that something
happened if it has changed.



From: Kay Sievers <kay.sievers@vrfy.org>
Subject: support polling of /proc/swaps

System management wants to subscribe to changes in swap
configuration. Make /proc/swaps pollable like /proc/mounts.

Signed-Off-By: Kay Sievers <kay.sievers@vrfy.org>
---
 mm/swapfile.c |   48 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 47 insertions(+), 1 deletion(-)

--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -30,6 +30,7 @@
 #include <linux/capability.h>
 #include <linux/syscalls.h>
 #include <linux/memcontrol.h>
+#include <linux/poll.h>
 
 #include <asm/pgtable.h>
 #include <asm/tlbflush.h>
@@ -58,6 +59,9 @@ static struct swap_info_struct *swap_inf
 
 static DEFINE_MUTEX(swapon_mutex);
 
+static DECLARE_WAIT_QUEUE_HEAD(proc_poll_wait);
+static atomic_t proc_poll_event = ATOMIC_INIT(0);
+
 static inline unsigned char swap_count(unsigned char ent)
 {
 	return ent & ~SWAP_HAS_CACHE;	/* may include SWAP_HAS_CONT flag */
@@ -1680,6 +1684,8 @@ SYSCALL_DEFINE1(swapoff, const char __us
 	}
 	filp_close(swap_file, NULL);
 	err = 0;
+	atomic_inc(&proc_poll_event);
+	wake_up_interruptible(&proc_poll_wait);
 
 out_dput:
 	filp_close(victim, NULL);
@@ -1688,6 +1694,25 @@ out:
 }
 
 #ifdef CONFIG_PROC_FS
+struct proc_swaps {
+	struct seq_file seq;
+	int event;
+};
+
+static unsigned swaps_poll(struct file *file, poll_table *wait)
+{
+	struct proc_swaps *s = file->private_data;
+
+	poll_wait(file, &proc_poll_wait, wait);
+
+	if (s->event != atomic_read(&proc_poll_event)) {
+		s->event = atomic_read(&proc_poll_event);
+		return POLLIN | POLLRDNORM | POLLERR | POLLPRI;
+	}
+
+	return POLLIN | POLLRDNORM;
+}
+
 /* iterator */
 static void *swap_start(struct seq_file *swap, loff_t *pos)
 {
@@ -1771,7 +1796,24 @@ static const struct seq_operations swaps
 
 static int swaps_open(struct inode *inode, struct file *file)
 {
-	return seq_open(file, &swaps_op);
+	struct proc_swaps *s;
+	int ret;
+
+	s = kmalloc(sizeof(struct proc_swaps), GFP_KERNEL);
+	if (!s)
+		return -ENOMEM;
+
+	file->private_data = s;
+
+	ret = seq_open(file, &swaps_op);
+	if (ret) {
+		kfree(s);
+		return ret;
+	}
+
+	s->seq.private = s;
+	s->event = atomic_read(&proc_poll_event);
+	return ret;
 }
 
 static const struct file_operations proc_swaps_operations = {
@@ -1779,6 +1821,7 @@ static const struct file_operations proc
 	.read		= seq_read,
 	.llseek		= seq_lseek,
 	.release	= seq_release,
+	.poll		= swaps_poll,
 };
 
 static int __init procswaps_init(void)
@@ -2084,6 +2127,9 @@ SYSCALL_DEFINE2(swapon, const char __use
 		swap_info[prev]->next = type;
 	spin_unlock(&swap_lock);
 	mutex_unlock(&swapon_mutex);
+	atomic_inc(&proc_poll_event);
+	wake_up_interruptible(&proc_poll_wait);
+
 	error = 0;
 	goto out;
 bad_swap:

Re: [PATCH] support polling of /proc/swaps

[Andrew Morton]

On Wed, 20 Oct 2010 01:25:47 +0200
Kay Sievers <kay.sievers@vrfy.org> wrote:

> On Tue, 2010-10-19 at 15:31 -0700, Andrew Morton wrote:
> > On Tue, 19 Oct 2010 11:19:16 +0200
> > Kay Sievers <kay.sievers@vrfy.org> wrote:
> 
> > It's a bit sad that we have to add quite a pile of infrastructure to
> > make a procfs file pollable.  I wonder if it's possible to provide some
> > core support for this, and reduce the amount of code at each particular
> > handler site.
> 
> You mean something like adding the event counter to the seq_file? There
> is /proc/self/mounts,mountinfo and /proc/swaps so far, I think.

Don't know - I was just waving hands about wondering if we really need
to add 48 new lines of code each time we want to make a procfs file
pollable.

> > Also, I wonder how we are to communicate the existence of this feature
> > to our users.  Nobody will look in Documentation/filesystems/.  Is
> > there a manpage?  Seems not...
> 
> Hmm, 'man 5 proc'?

yes, that mentions /proc/swaps.

> > > +static DECLARE_WAIT_QUEUE_HEAD(proc_poll_wait);
> > > +static int proc_poll_event;
> > 
> > Please pick a lock to protect proc_poll_event.
> 
> An atomic_t should do it too, right?

It does.

> > Then document that
> > locking here, when you also document proc_poll_event ;)
> 
> The actual value has no meaning at all, it just tells that something
> happened if it has changed.
> 

bah.

--- a/mm/swapfile.c~proc-swaps-support-polling-fix
+++ a/mm/swapfile.c
@@ -60,6 +60,7 @@ static struct swap_info_struct *swap_inf
 static DEFINE_MUTEX(swapon_mutex);
 
 static DECLARE_WAIT_QUEUE_HEAD(proc_poll_wait);
+/* Activity counter to indicate that a swapon or swapoff has occurred */
 static atomic_t proc_poll_event = ATOMIC_INIT(0);
 
 static inline unsigned char swap_count(unsigned char ent)
_

Re: [PATCH] support polling of /proc/swaps

[Neil Brown]

On Wed, 20 Oct 2010 01:25:47 +0200
Kay Sievers <kay.sievers@vrfy.org> wrote:

> On Tue, 2010-10-19 at 15:31 -0700, Andrew Morton wrote:
> > On Tue, 19 Oct 2010 11:19:16 +0200
> > Kay Sievers <kay.sievers@vrfy.org> wrote:
> 
> > It's a bit sad that we have to add quite a pile of infrastructure to
> > make a procfs file pollable.  I wonder if it's possible to provide some
> > core support for this, and reduce the amount of code at each particular
> > handler site.
> 
> You mean something like adding the event counter to the seq_file? There
> is /proc/self/mounts,mountinfo and /proc/swaps so far, I think.

And /proc/mdstat.

In /proc/mdstat I do something a little bit different.  I only update the
per-file event number when reading the first byte of the file, rather than
when poll returns POLL_PRI.
This is somewhat more robust.  In general select/poll will continue returning
a 'ready' status until the program performs some action (typically read or
write) which makes the fd not ready any more.

With your code (which I think is the same as /proc/mounts) you get at most 1
notification per change so you have to be careful not to miss it.  It is like
an edge triggered interrupt rather than level triggered.

So if this were extracted into the seqfile library (which is probably a good
idea), I'd like to see a suitably robust version used.

Thanks,
NeilBrown


> 
> > Also, I wonder how we are to communicate the existence of this feature
> > to our users.  Nobody will look in Documentation/filesystems/.  Is
> > there a manpage?  Seems not...
> 
> Hmm, 'man 5 proc'?
> 
> > > +static DECLARE_WAIT_QUEUE_HEAD(proc_poll_wait);
> > > +static int proc_poll_event;
> > 
> > Please pick a lock to protect proc_poll_event.
> 
> An atomic_t should do it too, right?
> 
> > Then document that
> > locking here, when you also document proc_poll_event ;)
> 
> The actual value has no meaning at all, it just tells that something
> happened if it has changed.
> 
> 
> 
> From: Kay Sievers <kay.sievers@vrfy.org>
> Subject: support polling of /proc/swaps
> 
> System management wants to subscribe to changes in swap
> configuration. Make /proc/swaps pollable like /proc/mounts.
> 
> Signed-Off-By: Kay Sievers <kay.sievers@vrfy.org>
> ---
>  mm/swapfile.c |   48 +++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 47 insertions(+), 1 deletion(-)
> 
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -30,6 +30,7 @@
>  #include <linux/capability.h>
>  #include <linux/syscalls.h>
>  #include <linux/memcontrol.h>
> +#include <linux/poll.h>
>  
>  #include <asm/pgtable.h>
>  #include <asm/tlbflush.h>
> @@ -58,6 +59,9 @@ static struct swap_info_struct *swap_inf
>  
>  static DEFINE_MUTEX(swapon_mutex);
>  
> +static DECLARE_WAIT_QUEUE_HEAD(proc_poll_wait);
> +static atomic_t proc_poll_event = ATOMIC_INIT(0);
> +
>  static inline unsigned char swap_count(unsigned char ent)
>  {
>  	return ent & ~SWAP_HAS_CACHE;	/* may include SWAP_HAS_CONT flag */
> @@ -1680,6 +1684,8 @@ SYSCALL_DEFINE1(swapoff, const char __us
>  	}
>  	filp_close(swap_file, NULL);
>  	err = 0;
> +	atomic_inc(&proc_poll_event);
> +	wake_up_interruptible(&proc_poll_wait);
>  
>  out_dput:
>  	filp_close(victim, NULL);
> @@ -1688,6 +1694,25 @@ out:
>  }
>  
>  #ifdef CONFIG_PROC_FS
> +struct proc_swaps {
> +	struct seq_file seq;
> +	int event;
> +};
> +
> +static unsigned swaps_poll(struct file *file, poll_table *wait)
> +{
> +	struct proc_swaps *s = file->private_data;
> +
> +	poll_wait(file, &proc_poll_wait, wait);
> +
> +	if (s->event != atomic_read(&proc_poll_event)) {
> +		s->event = atomic_read(&proc_poll_event);
> +		return POLLIN | POLLRDNORM | POLLERR | POLLPRI;
> +	}
> +
> +	return POLLIN | POLLRDNORM;
> +}
> +
>  /* iterator */
>  static void *swap_start(struct seq_file *swap, loff_t *pos)
>  {
> @@ -1771,7 +1796,24 @@ static const struct seq_operations swaps
>  
>  static int swaps_open(struct inode *inode, struct file *file)
>  {
> -	return seq_open(file, &swaps_op);
> +	struct proc_swaps *s;
> +	int ret;
> +
> +	s = kmalloc(sizeof(struct proc_swaps), GFP_KERNEL);
> +	if (!s)
> +		return -ENOMEM;
> +
> +	file->private_data = s;
> +
> +	ret = seq_open(file, &swaps_op);
> +	if (ret) {
> +		kfree(s);
> +		return ret;
> +	}
> +
> +	s->seq.private = s;
> +	s->event = atomic_read(&proc_poll_event);
> +	return ret;
>  }
>  
>  static const struct file_operations proc_swaps_operations = {
> @@ -1779,6 +1821,7 @@ static const struct file_operations proc
>  	.read		= seq_read,
>  	.llseek		= seq_lseek,
>  	.release	= seq_release,
> +	.poll		= swaps_poll,
>  };
>  
>  static int __init procswaps_init(void)
> @@ -2084,6 +2127,9 @@ SYSCALL_DEFINE2(swapon, const char __use
>  		swap_info[prev]->next = type;
>  	spin_unlock(&swap_lock);
>  	mutex_unlock(&swapon_mutex);
> +	atomic_inc(&proc_poll_event);
> +	wake_up_interruptible(&proc_poll_wait);
> +
>  	error = 0;
>  	goto out;
>  bad_swap:
> 
> 
> --
> 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/

Re: [PATCH] support polling of /proc/swaps

[Kay Sievers]

On Mon, Nov 15, 2010 at 04:44, Neil Brown <neilb@suse.de> wrote:
> On Wed, 20 Oct 2010 01:25:47 +0200 Kay Sievers <kay.sievers@vrfy.org> wrote:
>> On Tue, 2010-10-19 at 15:31 -0700, Andrew Morton wrote:
>> > On Tue, 19 Oct 2010 11:19:16 +0200
>> > Kay Sievers <kay.sievers@vrfy.org> wrote:
>>
>> > It's a bit sad that we have to add quite a pile of infrastructure to
>> > make a procfs file pollable.  I wonder if it's possible to provide some
>> > core support for this, and reduce the amount of code at each particular
>> > handler site.
>>
>> You mean something like adding the event counter to the seq_file? There
>> is /proc/self/mounts,mountinfo and /proc/swaps so far, I think.
>
> And /proc/mdstat.
>
> In /proc/mdstat I do something a little bit different.  I only update the
> per-file event number when reading the first byte of the file, rather than
> when poll returns POLL_PRI.
> This is somewhat more robust.  In general select/poll will continue returning
> a 'ready' status until the program performs some action (typically read or
> write) which makes the fd not ready any more.
>
> With your code (which I think is the same as /proc/mounts) you get at most 1
> notification per change so you have to be careful not to miss it.  It is like
> an edge triggered interrupt rather than level triggered.
>
> So if this were extracted into the seqfile library (which is probably a good
> idea), I'd like to see a suitably robust version used.

Sure, sounds good to me to have that available in the seq file. The
current logic works fine so far, but always returning from poll(),
until data is actually read(), sounds nice.

You want to give it a try how stuff moved into seq_file would look like?

Kay