Re: [PATCH] shrink task_struct by 16 bytes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




Shrink task_struct by replacing the notifier callback with a notifier
list that allows for multiple clients to register notifiers. It also
saves one cacheline on i386.

The patch also renames the un/block_all_signals() functions to more
descriptive names signal_notifier_add/del() and updates the kerneldoc
comments accordingly. The only in-tree user (drm) is converted.

Thanks to Christoph Hellwig for useful comments and review.

Pahole output for task_struct:

i386 before:

   /* size: 2640, cachelines: 42 */
   /* sum members: 2619, holes: 3, sum holes: 9 */
   /* bit holes: 2, sum bit holes: 62 bits */
   /* padding: 12 */
   /* paddings: 1, sum paddings: 8 */
   /* last cacheline: 16 bytes */

i386 after:

   /* size: 2624, cachelines: 41 */
   /* sum members: 2611, holes: 3, sum holes: 9 */
   /* bit holes: 2, sum bit holes: 62 bits */
   /* padding: 4 */
   /* paddings: 1, sum paddings: 8 */

x86_64 before:

   /* size: 3632, cachelines: 57 */
   /* sum members: 3579, holes: 10, sum holes: 45 */
   /* bit holes: 2, sum bit holes: 62 bits */
   /* padding: 8 */
   /* last cacheline: 48 bytes */

x86_64 after:

   /* size: 3616, cachelines: 57 */
   /* sum members: 3563, holes: 10, sum holes: 45 */
   /* bit holes: 2, sum bit holes: 62 bits */
   /* padding: 8 */
   /* last cacheline: 32 bytes */

Signed-off-by: Davi E. M. Arnaut <[email protected]>
Cc: Dave Airlie <[email protected]>

Signed-off-by: Dave Airlie <[email protected]>

No problems from me either..

Dave.

Cc: Christoph Hellwig <[email protected]>

---
 drivers/char/drm/drmP.h     |    4 +-
 drivers/char/drm/drm_lock.c |   16 ++++++----
 include/linux/init_task.h   |    1
 include/linux/sched.h       |   10 ++----
 include/linux/signal.h      |   21 ++++++++++++++
 kernel/fork.c               |    1
 kernel/signal.c             |   66 ++++++++++++++++++++++----------------------
 7 files changed, 71 insertions(+), 48 deletions(-)

Index: linux-2.6/include/linux/sched.h
===================================================================
--- linux-2.6.orig/include/linux/sched.h
+++ linux-2.6/include/linux/sched.h
@@ -963,10 +963,9 @@ struct task_struct {

        unsigned long sas_ss_sp;
        size_t sas_ss_size;
-       int (*notifier)(void *priv);
-       void *notifier_data;
-       sigset_t *notifier_mask;
-
+       /* struct signal_notifier callback list */
+       struct hlist_head signal_notifier_list;
+
        void *security;
        struct audit_context *audit_context;
        seccomp_t seccomp;
@@ -1339,9 +1338,6 @@ static inline int dequeue_signal_lock(st
        return ret;
 }

-extern void block_all_signals(int (*notifier)(void *priv), void *priv,
-                             sigset_t *mask);
-extern void unblock_all_signals(void);
 extern void release_task(struct task_struct * p);
 extern int send_sig_info(int, struct siginfo *, struct task_struct *);
 extern int send_group_sig_info(int, struct siginfo *, struct task_struct *);
Index: linux-2.6/include/linux/init_task.h
===================================================================
--- linux-2.6.orig/include/linux/init_task.h
+++ linux-2.6/include/linux/init_task.h
@@ -157,6 +157,7 @@ extern struct group_info init_groups;
                .list = LIST_HEAD_INIT(tsk.pending.list),               \
                .signal = {{0}}},                                       \
        .blocked        = {{0}},                                        \
+       .signal_notifier_list = HLIST_HEAD_INIT,                        \
        .alloc_lock     = __SPIN_LOCK_UNLOCKED(tsk.alloc_lock),         \
        .journal_info   = NULL,                                         \
        .cpu_timers     = INIT_CPU_TIMERS(tsk.cpu_timers),              \
Index: linux-2.6/include/linux/signal.h
===================================================================
--- linux-2.6.orig/include/linux/signal.h
+++ linux-2.6/include/linux/signal.h
@@ -27,6 +27,27 @@ struct sigpending {
        sigset_t signal;
 };

+/**
+ * struct signal_notifier - the signal notifier struct
+ * @node: hash list node to enqueue into the process notifier list
+ * @func: notifier callback function. if the callback routine returns 0
+ * then then signal will be blocked.
+ * @data: pointer to private data that the notifier routine can use to
+ * determine if the signal should be blocked or not.
+ * @mask: mask of signals to call the callback routine upon
+ */
+struct signal_notifier {
+       struct hlist_node node;
+       int (*func)(void *priv);
+       void *data;
+       sigset_t mask;
+};
+
+/* Add a signal notifier struct to the current process notifier list  */
+extern void signal_notifier_add(struct signal_notifier *);
+/* Remove a signal notifier struct from the current process notifier list */
+extern void signal_notifier_del(struct signal_notifier *);
+
 /*
  * Define some primitives to manipulate sigset_t.
  */
Index: linux-2.6/kernel/fork.c
===================================================================
--- linux-2.6.orig/kernel/fork.c
+++ linux-2.6/kernel/fork.c
@@ -1034,6 +1034,7 @@ static struct task_struct *copy_process(
        p->vfork_done = NULL;
        spin_lock_init(&p->alloc_lock);

+       INIT_HLIST_HEAD(&p->signal_notifier_list);
        clear_tsk_thread_flag(p, TIF_SIGPENDING);
        init_sigpending(&p->pending);

Index: linux-2.6/drivers/char/drm/drm_lock.c
===================================================================
--- linux-2.6.orig/drivers/char/drm/drm_lock.c
+++ linux-2.6/drivers/char/drm/drm_lock.c
@@ -110,14 +110,16 @@ int drm_lock(struct inode *inode, struct
        DRM_DEBUG( "%d %s\n", lock.context, ret ? "interrupted" : "has lock" );
        if (ret) return ret;

-       sigemptyset(&dev->sigmask);
-       sigaddset(&dev->sigmask, SIGSTOP);
-       sigaddset(&dev->sigmask, SIGTSTP);
-       sigaddset(&dev->sigmask, SIGTTIN);
-       sigaddset(&dev->sigmask, SIGTTOU);
+       sigemptyset(&dev->notifier.mask);
+       sigaddset(&dev->notifier.mask, SIGSTOP);
+       sigaddset(&dev->notifier.mask, SIGTSTP);
+       sigaddset(&dev->notifier.mask, SIGTTIN);
+       sigaddset(&dev->notifier.mask, SIGTTOU);
        dev->sigdata.context = lock.context;
        dev->sigdata.lock = dev->lock.hw_lock;
-       block_all_signals(drm_notifier, &dev->sigdata, &dev->sigmask);
+       dev->notifier.data = &dev->sigdata;
+       dev->notifier.func = drm_notifier;
+       signal_notifier_add(&dev->notifier);

        if (dev->driver->dma_ready && (lock.flags & _DRM_LOCK_READY))
                dev->driver->dma_ready(dev);
@@ -189,7 +191,7 @@ int drm_unlock(struct inode *inode, stru
                }
        }

-       unblock_all_signals();
+       signal_notifier_del(&dev->notifier);
        return 0;
 }

Index: linux-2.6/drivers/char/drm/drmP.h
===================================================================
--- linux-2.6.orig/drivers/char/drm/drmP.h
+++ linux-2.6/drivers/char/drm/drmP.h
@@ -750,8 +750,8 @@ typedef struct drm_device {
        drm_sg_mem_t *sg;       /**< Scatter gather memory */
        unsigned long *ctx_bitmap;      /**< context bitmap */
        void *dev_private;              /**< device private data */
-       drm_sigdata_t sigdata;     /**< For block_all_signals */
-       sigset_t sigmask;
+       drm_sigdata_t sigdata;     /**< Signal notifier data */
+       struct signal_notifier notifier;        /**< For block_signals */

        struct drm_driver *driver;
        drm_local_map_t *agp_buffer_map;
Index: linux-2.6/kernel/signal.c
===================================================================
--- linux-2.6.orig/kernel/signal.c
+++ linux-2.6/kernel/signal.c
@@ -237,37 +237,35 @@ flush_signal_handlers(struct task_struct
        }
 }

-
-/* Notify the system that a driver wants to block all signals for this
- * process, and wants to be notified if any signals at all were to be
- * sent/acted upon.  If the notifier routine returns non-zero, then the
- * signal will be acted upon after all.  If the notifier routine returns 0,
- * then then signal will be blocked.  Only one block per process is
- * allowed.  priv is a pointer to private data that the notifier routine
- * can use to determine if the signal should be blocked or not.  */
-
-void
-block_all_signals(int (*notifier)(void *priv), void *priv, sigset_t *mask)
+/**
+ * signal_notifier_add - Add a signal notifier callback to the current process
+ * @notifier: the signal_notifier struct to add
+ *
+ * Notify the system that a driver wants to block a set of signals for this
+ * process and wants to be notified if any signals at all were to be sent or
+ * acted upon. If the notifier routine returns non-zero, then the signal will
+ * be acted upon after all. If the notifier routine returns 0, the signal will
+ * be blocked.
+ */
+void signal_notifier_add(struct signal_notifier *notifier)
 {
        unsigned long flags;

        spin_lock_irqsave(&current->sighand->siglock, flags);
-       current->notifier_mask = mask;
-       current->notifier_data = priv;
-       current->notifier = notifier;
+       hlist_add_head(&notifier->node, &current->signal_notifier_list);
        spin_unlock_irqrestore(&current->sighand->siglock, flags);
 }

-/* Notify the system that blocking has ended. */
-
-void
-unblock_all_signals(void)
+/**
+ * signal_notifier_del - Notify the system that blocking has ended
+ * @notifier: the signal_notifier struct to remove
+ */
+void signal_notifier_del(struct signal_notifier *notifier)
 {
        unsigned long flags;

        spin_lock_irqsave(&current->sighand->siglock, flags);
-       current->notifier = NULL;
-       current->notifier_data = NULL;
+       hlist_del(&notifier->node);
        recalc_sigpending();
        spin_unlock_irqrestore(&current->sighand->siglock, flags);
 }
@@ -318,22 +316,26 @@ static int collect_signal(int sig, struc
 static int __dequeue_signal(struct sigpending *pending, sigset_t *mask,
                        siginfo_t *info)
 {
+       struct hlist_node *entry, *next;
+       struct signal_notifier *notifier;
        int sig = next_signal(pending, mask);
+       struct hlist_head *hash_list = &current->signal_notifier_list;

-       if (sig) {
-               if (current->notifier) {
-                       if (sigismember(current->notifier_mask, sig)) {
-                               if (!(current->notifier)(current->notifier_data)) {
-                                       clear_thread_flag(TIF_SIGPENDING);
-                                       return 0;
-                               }
+       if (unlikely(!sig))
+               return 0;
+
+       hlist_for_each_entry_safe(notifier, entry, next, hash_list, node) {
+               if (sigismember(&notifier->mask, sig)) {
+                       if (!notifier->func(notifier->data)) {
+                               clear_thread_flag(TIF_SIGPENDING);
+                               return 0;
                        }
                }
-
-               if (!collect_signal(sig, pending, info))
-                       sig = 0;
        }

+       if (!collect_signal(sig, pending, info))
+               sig = 0;
+
        return sig;
 }

@@ -1860,8 +1862,8 @@ EXPORT_SYMBOL(ptrace_notify);
 EXPORT_SYMBOL(send_sig);
 EXPORT_SYMBOL(send_sig_info);
 EXPORT_SYMBOL(sigprocmask);
-EXPORT_SYMBOL(block_all_signals);
-EXPORT_SYMBOL(unblock_all_signals);
+EXPORT_SYMBOL(signal_notifier_add);
+EXPORT_SYMBOL(signal_notifier_del);


 /*
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
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-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux