[PATCH 1/5] fsnotify: use fsnotify_create_event to allocate the q_overflow event

[PATCH 1/5] fsnotify: use fsnotify_create_event to allocate the q_overflow event

[Eric Paris]

Currently fsnotify defines a static fsnotify event which is sent when a
group overflows its allotted queue length.  This patch just allocates that
event from the event cache rather than defining it statically.  There is no
known reason that the current implementation is wrong, but this makes sure the
event is initialized and created like any other.

Signed-off-by: Eric Paris <eparis@redhat.com>
---

 fs/notify/notification.c |   11 +++++++----
 1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/fs/notify/notification.c b/fs/notify/notification.c
index 5213685..4ab9243 100644
--- a/fs/notify/notification.c
+++ b/fs/notify/notification.c
@@ -56,7 +56,7 @@ static struct kmem_cache *fsnotify_event_holder_cachep;
  * it is needed.  It's refcnt is set 1 at kernel init time and will never
  * get set to 0 so it will never get 'freed'
  */
-static struct fsnotify_event q_overflow_event;
+static struct fsnotify_event *q_overflow_event;
 static atomic_t fsnotify_sync_cookie = ATOMIC_INIT(0);
 
 /**
@@ -193,7 +193,7 @@ alloc_holder:
 	mutex_lock(&group->notification_mutex);
 
 	if (group->q_len >= group->max_events) {
-		event = &q_overflow_event;
+		event = q_overflow_event;
 		/* sorry, no private data on the overflow event */
 		priv = NULL;
 	}
@@ -409,8 +409,11 @@ __init int fsnotify_notification_init(void)
 	fsnotify_event_cachep = KMEM_CACHE(fsnotify_event, SLAB_PANIC);
 	fsnotify_event_holder_cachep = KMEM_CACHE(fsnotify_event_holder, SLAB_PANIC);
 
-	initialize_event(&q_overflow_event);
-	q_overflow_event.mask = FS_Q_OVERFLOW;
+	q_overflow_event = fsnotify_create_event(NULL, FS_Q_OVERFLOW, NULL,
+						 FSNOTIFY_EVENT_NONE, NULL, 0,
+						 GFP_KERNEL);
+	if (!q_overflow_event)
+		panic("unable to allocate fsnotify q_overflow_event\n");
 
 	return 0;
 }

[PATCH 2/5] inotify: use ZERO_PAGE instead of private static declaration

[Eric Paris]

inotify needs to pad filenames to an inotify event length.  Currently this
is done by copying 0s from an event declared globally.  This patch doesn't
waste global address space just to have some 0s and instead uses
clear_user()

Signed-off-by: Eric Paris <eparis@redhat.com>
---

 fs/notify/inotify/inotify_user.c |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c
index c17d5a3..c3086b0 100644
--- a/fs/notify/inotify/inotify_user.c
+++ b/fs/notify/inotify/inotify_user.c
@@ -47,9 +47,6 @@
 
 static struct vfsmount *inotify_mnt __read_mostly;
 
-/* this just sits here and wastes global memory.  used to just pad userspace messages with zeros */
-static struct inotify_event nul_inotify_event;
-
 /* these are configurable via /proc/sys/fs/inotify/ */
 static int inotify_max_user_instances __read_mostly;
 static int inotify_max_queued_events __read_mostly;
@@ -215,7 +212,7 @@ static ssize_t copy_event_to_user(struct fsnotify_group *group,
 	/*
 	 * fsnotify only stores the pathname, so here we have to send the pathname
 	 * and then pad that pathname out to a multiple of sizeof(inotify_event)
-	 * with zeros.  I get my zeros from the nul_inotify_event.
+	 * with zeros.
 	 */
 	if (name_len) {
 		unsigned int len_to_zero = name_len - event->name_len;
@@ -224,8 +221,8 @@ static ssize_t copy_event_to_user(struct fsnotify_group *group,
 			return -EFAULT;
 		buf += event->name_len;
 
-		/* fill userspace with 0's from nul_inotify_event */
-		if (copy_to_user(buf, &nul_inotify_event, len_to_zero))
+		/* fill userspace with 0's */
+		if (clear_user(buf, len_to_zero))
 			return -EFAULT;
 		buf += len_to_zero;
 		event_size += name_len;

Re: [PATCH 2/5] inotify: use ZERO_PAGE instead of private static declaration

[Eric Paris]

On Sun, Aug 9, 2009 at 5:59 PM, Eric Paris<eparis@redhat.com> wrote:
> inotify needs to pad filenames to an inotify event length.  Currently this
> is done by copying 0s from an event declared globally.  This patch doesn't
> waste global address space just to have some 0s and instead uses
> clear_user()
>
> Signed-off-by: Eric Paris <eparis@redhat.com>

Crap, the subject of this message is wrong.  I'll fix the subject when
I commit, the summary and patch are correct.

-Eric

> ---
>
>  fs/notify/inotify/inotify_user.c |    9 +++------
>  1 files changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c
> index c17d5a3..c3086b0 100644
> --- a/fs/notify/inotify/inotify_user.c
> +++ b/fs/notify/inotify/inotify_user.c
> @@ -47,9 +47,6 @@
>
>  static struct vfsmount *inotify_mnt __read_mostly;
>
> -/* this just sits here and wastes global memory.  used to just pad userspace messages with zeros */
> -static struct inotify_event nul_inotify_event;
> -
>  /* these are configurable via /proc/sys/fs/inotify/ */
>  static int inotify_max_user_instances __read_mostly;
>  static int inotify_max_queued_events __read_mostly;
> @@ -215,7 +212,7 @@ static ssize_t copy_event_to_user(struct fsnotify_group *group,
>        /*
>         * fsnotify only stores the pathname, so here we have to send the pathname
>         * and then pad that pathname out to a multiple of sizeof(inotify_event)
> -        * with zeros.  I get my zeros from the nul_inotify_event.
> +        * with zeros.
>         */
>        if (name_len) {
>                unsigned int len_to_zero = name_len - event->name_len;
> @@ -224,8 +221,8 @@ static ssize_t copy_event_to_user(struct fsnotify_group *group,
>                        return -EFAULT;
>                buf += event->name_len;
>
> -               /* fill userspace with 0's from nul_inotify_event */
> -               if (copy_to_user(buf, &nul_inotify_event, len_to_zero))
> +               /* fill userspace with 0's */
> +               if (clear_user(buf, len_to_zero))
>                        return -EFAULT;
>                buf += len_to_zero;
>                event_size += name_len;
>
> --
> 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/
>

[PATCH 3/5] inotify: use container_of instead of casting

[Eric Paris]

inotify_free_mark casts directly from an fsnotify_mark_entry to an
inotify_inode_mark_entry.  This works, but should use container_of instead
for future proofing.

Signed-off-by: Eric Paris <eparis@redhat.com>
---

 fs/notify/inotify/inotify_user.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c
index c3086b0..fb38e73 100644
--- a/fs/notify/inotify/inotify_user.c
+++ b/fs/notify/inotify/inotify_user.c
@@ -424,7 +424,9 @@ skip_send_ignore:
 /* ding dong the mark is dead */
 static void inotify_free_mark(struct fsnotify_mark_entry *entry)
 {
-	struct inotify_inode_mark_entry *ientry = (struct inotify_inode_mark_entry *)entry;
+	struct inotify_inode_mark_entry *ientry;
+
+	ientry = container_of(entry, struct inotify_inode_mark_entry, fsn_entry);
 
 	kmem_cache_free(inotify_inode_mark_cachep, ientry);
 }

[PATCH 4/5] fsnotify: kzalloc fsnotify groups

[Eric Paris]

Use kzalloc for fsnotify_groups so that none of the fields can leak any
information accidentally.

Signed-off-by: Eric Paris <eparis@redhat.com>
---

 fs/notify/group.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/notify/group.c b/fs/notify/group.c
index 0e16771..777ca82 100644
--- a/fs/notify/group.c
+++ b/fs/notify/group.c
@@ -207,7 +207,7 @@ struct fsnotify_group *fsnotify_obtain_group(unsigned int group_num, __u32 mask,
 	struct fsnotify_group *group, *tgroup;
 
 	/* very low use, simpler locking if we just always alloc */
-	group = kmalloc(sizeof(struct fsnotify_group), GFP_KERNEL);
+	group = kzalloc(sizeof(struct fsnotify_group), GFP_KERNEL);
 	if (!group)
 		return ERR_PTR(-ENOMEM);
 

[PATCH 5/5] fsnotify: use kmem_cache_zalloc to simplify event initialization

[Eric Paris]

fsnotify event initialization is done entry by entry with almost everything
set to either 0 or NULL.  Use kmem_cache_zalloc and only initialize things
that need non-zero initialization.  Also means we don't have to change
initialization entries based on the config options.

Signed-off-by: Eric Paris <eparis@redhat.com>
---

 fs/notify/notification.c |   13 +------------
 1 files changed, 1 insertions(+), 12 deletions(-)

diff --git a/fs/notify/notification.c b/fs/notify/notification.c
index 4ab9243..96ffbd0 100644
--- a/fs/notify/notification.c
+++ b/fs/notify/notification.c
@@ -311,25 +311,14 @@ void fsnotify_flush_notify(struct fsnotify_group *group)
 
 static void initialize_event(struct fsnotify_event *event)
 {
-	event->holder.event = NULL;
 	INIT_LIST_HEAD(&event->holder.event_list);
 	atomic_set(&event->refcnt, 1);
 
 	spin_lock_init(&event->lock);
 
-	event->path.dentry = NULL;
-	event->path.mnt = NULL;
-	event->inode = NULL;
 	event->data_type = FSNOTIFY_EVENT_NONE;
 
 	INIT_LIST_HEAD(&event->private_data_list);
-
-	event->to_tell = NULL;
-
-	event->file_name = NULL;
-	event->name_len = 0;
-
-	event->sync_cookie = 0;
 }
 
 /*
@@ -350,7 +339,7 @@ struct fsnotify_event *fsnotify_create_event(struct inode *to_tell, __u32 mask,
 {
 	struct fsnotify_event *event;
 
-	event = kmem_cache_alloc(fsnotify_event_cachep, gfp);
+	event = kmem_cache_zalloc(fsnotify_event_cachep, gfp);
 	if (!event)
 		return NULL;
 

[PATCH 1/5] fsnotify: use fsnotify_create_event to allocate the q_overflow event

[Eric Paris]

Currently fsnotify defines a static fsnotify event which is sent when a
group overflows its allotted queue length.  This patch just allocates that
event from the event cache rather than defining it statically.  There is no
known reason that the current implementation is wrong, but this makes sure the
event is initialized and created like any other.

Signed-off-by: Eric Paris <eparis@redhat.com>
---

 fs/notify/notification.c |   11 +++++++----
 1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/fs/notify/notification.c b/fs/notify/notification.c
index 5213685..4ab9243 100644
--- a/fs/notify/notification.c
+++ b/fs/notify/notification.c
@@ -56,7 +56,7 @@ static struct kmem_cache *fsnotify_event_holder_cachep;
  * it is needed.  It's refcnt is set 1 at kernel init time and will never
  * get set to 0 so it will never get 'freed'
  */
-static struct fsnotify_event q_overflow_event;
+static struct fsnotify_event *q_overflow_event;
 static atomic_t fsnotify_sync_cookie = ATOMIC_INIT(0);
 
 /**
@@ -193,7 +193,7 @@ alloc_holder:
 	mutex_lock(&group->notification_mutex);
 
 	if (group->q_len >= group->max_events) {
-		event = &q_overflow_event;
+		event = q_overflow_event;
 		/* sorry, no private data on the overflow event */
 		priv = NULL;
 	}
@@ -409,8 +409,11 @@ __init int fsnotify_notification_init(void)
 	fsnotify_event_cachep = KMEM_CACHE(fsnotify_event, SLAB_PANIC);
 	fsnotify_event_holder_cachep = KMEM_CACHE(fsnotify_event_holder, SLAB_PANIC);
 
-	initialize_event(&q_overflow_event);
-	q_overflow_event.mask = FS_Q_OVERFLOW;
+	q_overflow_event = fsnotify_create_event(NULL, FS_Q_OVERFLOW, NULL,
+						 FSNOTIFY_EVENT_NONE, NULL, 0,
+						 GFP_KERNEL);
+	if (!q_overflow_event)
+		panic("unable to allocate fsnotify q_overflow_event\n");
 
 	return 0;
 }