linux-kernel - Re: [PATCH 9/9] block: remove q->prepare_flush

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20100703101320.GB22329@infradead.org>
Date:	Sat, 3 Jul 2010 06:13:20 -0400
From:	Christoph Hellwig <hch@...radead.org>
To:	FUJITA Tomonori <fujita.tomonori@....ntt.co.jp>
Cc:	axboe@...nel.dk, linux-kernel@...r.kernel.org, knikanth@...e.de,
	jeremy@...source.com, drzeus@...eus.cx
Subject: Re: [PATCH 9/9] block: remove q->prepare_flush_fn completely

Not a comment on the patch itself, but some interesting observations
on the drivers ordered flags:

> diff --git a/drivers/block/loop.c b/drivers/block/loop.c
> index fedfdb7..d285a54 100644
> --- a/drivers/block/loop.c
> +++ b/drivers/block/loop.c
> @@ -831,7 +831,7 @@ static int loop_set_fd(struct loop_device *lo, fmode_t mode,
>  	lo->lo_queue->unplug_fn = loop_unplug;
>  
>  	if (!(lo_flags & LO_FLAGS_READ_ONLY) && file->f_op->fsync)
> -		blk_queue_ordered(lo->lo_queue, QUEUE_ORDERED_DRAIN, NULL);
> +		blk_queue_ordered(lo->lo_queue, QUEUE_ORDERED_DRAIN);

loop actually does flushes.  But implements them by itself because
it's bio based.  Seems like something is not so nice about the API
for implementing barriers with bio based drivers.

Also do we actually get the draining semantics right this way?  Unless
loop only supports one in-flight bio this seems to not be correct.

> diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
> index 495533e..76af65b 100644
> --- a/drivers/block/xen-blkfront.c
> +++ b/drivers/block/xen-blkfront.c
> @@ -373,8 +373,7 @@ static int xlvbd_barrier(struct blkfront_info *info)
>  	int err;
>  
>  	err = blk_queue_ordered(info->rq,
> -				info->feature_barrier ? QUEUE_ORDERED_DRAIN : QUEUE_ORDERED_NONE,
> -				NULL);
> +				info->feature_barrier ? QUEUE_ORDERED_DRAIN : QUEUE_ORDERED_NONE);

Something is really broken with the way Xen implements barriers.
It claims to only require drains, but then actually marks barrier
requests as such.  The qemu backend at least then does the pre and post
drains by itself.  While this gets the correct results for writes marked
as barriers, it can't properly implement empty barriers (aka cache
flushes) which are just as important.

> --- a/drivers/mmc/card/queue.c
> +++ b/drivers/mmc/card/queue.c
> @@ -128,7 +128,7 @@ int mmc_init_queue(struct mmc_queue *mq, struct mmc_card *card, spinlock_t *lock
>  	mq->req = NULL;
>  
>  	blk_queue_prep_rq(mq->queue, mmc_prep_request);
> -	blk_queue_ordered(mq->queue, QUEUE_ORDERED_DRAIN, NULL);
> +	blk_queue_ordered(mq->queue, QUEUE_ORDERED_DRAIN);

So MMC device never have volatile write caches?

--
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/