linux-kernel - Re: [PATCH 04/10] block: initial patch for on-stack per-task plugging

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 24 Jan 2011 14:36:30 -0500
From:	Jeff Moyer <jmoyer@...hat.com>
To:	Jens Axboe <jaxboe@...ionio.com>
Cc:	linux-kernel@...r.kernel.org, hch@...radead.org
Subject: Re: [PATCH 04/10] block: initial patch for on-stack per-task plugging

Jens Axboe <jaxboe@...ionio.com> writes:

This looks mostly good.  I just have a couple of questions, listed below.

> +/*
> + * Attempts to merge with the plugged list in the current process. Returns
> + * true if merge was succesful, otherwise false.
> + */
> +static bool check_plug_merge(struct task_struct *tsk, struct request_queue *q,
> +			     struct bio *bio)
> +{

Would a better name for this function be attempt_plug_merge?

> +	plug = current->plug;
> +	if (plug && !sync) {
> +		if (!plug->should_sort && !list_empty(&plug->list)) {
> +			struct request *__rq;
> +
> +			__rq = list_entry_rq(plug->list.prev);
> +			if (__rq->q != q)
> +				plug->should_sort = 1;

[snip]

> +static int plug_rq_cmp(void *priv, struct list_head *a, struct list_head *b)
> +{
> +	struct request *rqa = container_of(a, struct request, queuelist);
> +	struct request *rqb = container_of(b, struct request, queuelist);
> +
> +	return !(rqa->q == rqb->q);
> +}


> +static void __blk_finish_plug(struct task_struct *tsk, struct blk_plug *plug)
> +{

[snip]

> +	if (plug->should_sort)
> +		list_sort(NULL, &plug->list, plug_rq_cmp);

The other way to do this is to just keep track of which queues you need
to run after exhausting the plug list.  Is it safe to assume you've done
things this way to keep each request queue's data structures cache hot
while working on it?

> +static inline void blk_flush_plug(struct task_struct *tsk)
> +{
> +	struct blk_plug *plug = tsk->plug;
> +
> +	if (unlikely(plug))
> +		__blk_flush_plug(tsk, plug);
> +}

Why is that unlikely?

Cheers,
Jeff
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/