On Wed, 12 Mar 2014 13:36:38 +1100 NeilBrown <neilb@xxxxxxx> wrote:
>
> The md driver currently supports 'poll' on /proc/mdstat.
> This is unsafe as if the md-mod module is removed while a 'poll'
> or 'select' is outstanding on /proc/mdstat, an oops occurs
> when the syscall completes.
> poll_freewait() will call remove_wait_queue() on a wait_queue_head_t
> which was local to the module which no-longer exists.
>
> This problem is particular to /proc. Most filesystems do not
> allow the module to be unloaded while any files are open on it.
> /proc only blocks module unloading while a file_operations
> call is currently active into the module, not while the file is open.
> kernfs has this property too but kernfs allocates a wait_queue_head_t
> in its internal data structures so the module doesn't need to provide
> one.
> (A previous patch to add a similar allocation to procfs was not
> accepted).
By who, me? I was hoping we could somehow keep the implementation
contained within md. I don't think I actually looked at it to any
significant extent!
Was hoping that viro would pipe up.
> This patch takes a different approach and allows a module to
> disconnect the wait_queue_head_t that was passed to poll_wait()
> from all the clients which are waiting on it. Thus after calling
> proc_remove_entry("mdstat", NULL);
> we simply call
> wait_queue_purge(&md_event_waiters);
>
> and then know that it is safe to remove the module.
>
> rcu infrastructure is used to avoid races.
> poll_freewait() checks if the purge has happened under rcu_read_lock()
> to ensure that it never touches any freed memory. wait_queue_purge()
> uses synchronize_rcu() to ensure no poll_freewait() could still be
> looking at the wait_queue_head_t.
>
> ...
>
> +/**
> + * wait_queue_purge - remove all waiter from a wait_queue
> + * @q: The queue to be purged
> + *
> + * Unlink all pending waiters from the queue.
> + * This can be used prior to freeing a queue providing all waiters are
> + * prepared for queue purging.
> + * Waiters must call remove_wait_queue_puregeable() rather than
> + * remove_wait_queue().
> + *
> + */
> +void wait_queue_purge(wait_queue_head_t *q)
> +{
> + spin_lock(&q->lock);
> + while (!list_empty(&q->task_list))
> + list_del_init(q->task_list.next);
> + spin_unlock(&q->lock);
> + synchronize_rcu();
> +}
> +EXPORT_SYMBOL(wait_queue_purge);
I don't get this. If a task is waiting on that wait_queue_head_t, how
does it get woken?
> +/**
> + * remove_wait_queue_puregeable - remove_wait_queue if wait_queue_purge might be used.
> + * @q: the queue, which may already be purged, to remove from
> + * @wait: the waiter to remove
> + *
> + * Remove a waiter from a queue if it hasn't already been purged.
> + * If the queue has already been purged then task_list will be empty.
> + * If it isn't then it is still safe to lock the queue and remove
> + * the task.
> + */
> +void remove_wait_queue_purgeable(wait_queue_head_t *q, wait_queue_t *wait)
> +{
> + unsigned long flags;
> +
> + rcu_read_lock();
> + if (!list_empty(&wait->task_list)) {
> + spin_lock_irqsave(&q->lock, flags);
Mixture of spin_lock_irqsave() here and spin_lock() in
wait_queue_purge() looks odd.
> + list_del_init(&wait->task_list);
> + spin_unlock_irqrestore(&q->lock, flags);
> + }
> + rcu_read_unlock();
> +}
> +EXPORT_SYMBOL(remove_wait_queue_purgeable);
--
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
