linux-kernel - Re: [PATCH 1/3] block: Add blk_queue_copy

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: <20090918190720.GA30063@redhat.com>
Date:	Fri, 18 Sep 2009 15:07:20 -0400
From:	Mike Snitzer <snitzer@...hat.com>
To:	"Jun'ichi Nomura" <j-nomura@...jp.nec.com>
Cc:	Jens Axboe <jens.axboe@...cle.com>,
	"Martin K. Petersen" <martin.petersen@...cle.com>,
	Alasdair G Kergon <agk@...hat.com>,
	David Strand <dpstrand@...il.com>,
	device-mapper development <dm-devel@...hat.com>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/3] block: Add blk_queue_copy_limits()

On Fri, Sep 18 2009 at 12:24pm -0400,
Jun'ichi Nomura <j-nomura@...jp.nec.com> wrote:

> This patch is a preparation for the last patch in this patchset
> which changes blk_set_default_limits() to set 0 to max_sectors.

should read: changes blk_set_default_limits() to set max_sectors to 0.

> dm uses blk_stack_limits() to merge limits of underlying devices
> and copy the end result to the queue.
> But if there's no underlying device (like 'zero' target),
> max_sectors/max_hw_sectors are left unchanged from the default 0
> and just copying it to the queue causes problems.
> 
> Provide blk_queue_copy_limits() to get a safe copy with
> invalid values fixed-up.
> 
> Signed-off-by: Kiyoshi Ueda <k-ueda@...jp.nec.com>
> Signed-off-by: Jun'ichi Nomura <j-nomura@...jp.nec.com>
> Cc: David Strand <dpstrand@...il.com>
> Cc: Mike Snitzer <snitzer@...hat.com>
> Cc: Alasdair G Kergon <agk@...hat.com>
> Cc: Martin K. Petersen <martin.petersen@...cle.com>
> Cc: Jens Axboe <jens.axboe@...cle.com>
> ---
>  block/blk-settings.c   |   28 ++++++++++++++++++++++++++++
>  include/linux/blkdev.h |    1 +
>  2 files changed, 29 insertions(+)
> 
> Index: linux-2.6.31.work/block/blk-settings.c
> ===================================================================
> --- linux-2.6.31.work.orig/block/blk-settings.c
> +++ linux-2.6.31.work/block/blk-settings.c
> @@ -122,6 +122,34 @@ void blk_set_default_limits(struct queue
>  EXPORT_SYMBOL(blk_set_default_limits);
> 
>  /**
> + * blk_queue_copy_limits - copy limits to queue
> + * @q:  the request queue whose limits as a copy destination
> + * @lim:  the queue_limits structure as a copy source
> + *
> + * Description:
> + *   Copies a queue_limit struct contents to @q with fix-ups to
> + *   invalid values.
> + */
> +void blk_queue_copy_limits(struct request_queue *q, struct queue_limits
> *lim)
> +{
> +	q->limits = *lim;
> +
> +	/*
> +	 * blk_set_default_limits() sets 0 to max_sectors/max_hw_sectors
> +	 * so that blk_stack_limits() appropriately propagate the values
> +	 * of lower-stack by min_not_zero().
> +	 * However, if the default value 0 is unchanged (e.g. the stacking
> +	 * device is virtual and has no underlying device), it results
> +	 * in unusable device.

Likewise:
blk_set_default_limits() sets max_sectors/max_hw_sectors to 0?

> +	 * Check if max_sectors/max_hw_sectors have non-zero values,
> +	 * and set SAFE_MAX_SECTORS if they do.
> +	 */
> +	if (q->limits.max_sectors == 0 || q->limits.max_hw_sectors == 0)
> +		blk_queue_max_sectors(q, SAFE_MAX_SECTORS);

Shouldn't this check (and the entire comment above it) get added in the
3rd patch once max_sectors/max_hw_sectors sre actually set to 0?  I'm
being really pedantic here but...
--
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/