linux-kernel - Re: dm: check max_sectors in dm_merge_bvec (was: Re: dm: max_segments=1 if merge_bvec

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] [day] [month] [year] [list]

Date:	Sat, 4 Dec 2010 14:21:29 -0500
From:	Mike Snitzer <snitzer@...hat.com>
To:	Lars Ellenberg <lars.ellenberg@...bit.com>
Cc:	device-mapper development <dm-devel@...hat.com>,
	Mikulas Patocka <mpatocka@...hat.com>,
	linux-kernel@...r.kernel.org, Alasdair G Kergon <agk@...hat.com>,
	jaxboe@...ionio.com
Subject: Re: dm: check max_sectors in dm_merge_bvec  (was: Re: dm:
 max_segments=1 if merge_bvec_fn is not supported)

On Sat, Dec 04 2010 at 11:03am -0500,
Lars Ellenberg <lars.ellenberg@...bit.com> wrote:

> On Sat, Dec 04, 2010 at 01:43:08AM -0500, Mike Snitzer wrote:
>
> > Given dm_set_device_limits() sets q->limits->max_sectors,
> > shouldn't dm_merge_bvec() be using queue_max_sectors rather than
> > queue_max_hw_sectors?
> > 
> > blk_queue_max_hw_sectors() establishes that max_hw_sectors is the hard
> > limit and max_sectors the soft.  But AFAICT no relation is maintained
> > between the two over time (even though max_sectors <= max_hw_sectors
> > _should_ be enforced; in practice there is no blk_queue_max_sectors
> > setter that uniformly enforces as much).
> 
> Just for the record, in case someone finds this in the archives,
> and wants to backport or base his own work on this:
> 
>  A long time ago, there was no .max_hw_sectors.  Then max_hw_sectors got
>  introduced, but without accessor function.
> 
>  Before 2.6.31, there was no blk_queue_max_hw_sectors(),
>  only blk_queue_max_sectors(), which set both.
> 
>  2.6.31 introduced some blk_queue_max_hw_sectors(), which _only_ set
>  max_hw_sectors, and enforced a lower limit of BLK_DEF_MAX_SECTORS, so
>  using that only, you have not been able to actually set lower limits
>  than 512 kB. With 2.6.31 to 2.6.33, inclusive, you still need to use
>  blk_queue_max_sectors() to set your limits.
> 
>  2.6.34 finally dropped the newly introduced function again,
>  but renamed the other, so starting with 2.6.34 you need to use
>  blk_queue_max_hw_sectors(), which now basically has the function body
>  blk_queue_max_sectors() had up until 2.6.33.
> 
> > dm_set_device_limits() will set q->limits->max_sectors to <= PAGE_SIZE
> > if an underlying device has a merge_bvec_fn.  Therefore, dm_merge_bvec()
> > must use queue_max_sectors() rather than queue_max_hw_sectors() to check
> > the appropriate limit.
> 
> IMO, you should not do this.
> max_sectors is a user tunable, capped by max_hw_sectors.
> max_hw_sectors is the driver limit.
>
> Please set max_hw_sectors in dm_set_device_limits instead.

Right, good point.. will do (unless I happen upon a reason not to or
someone else shouts).

Thanks,
Mike
--
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/