Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 5 Aug 2011 15:41:01 +0400
From:      Eygene Ryabinkin <rea@freebsd.org>
To:        Steven Hartland <killing@multiplay.co.uk>
Cc:        freebsd-hackers@freebsd.org, mav@freebsd.org
Subject:   Re: cam / ata timeout limited to 2147 due to overflow bug?
Message-ID:  <IZymNZtOnyjv6wqqoirLBsUvqyw@H1uwQtuDamiOTxQ5dYkc3ncI/0w>
In-Reply-To: <FEC6035595894C6CBD490CCEDEA977FB@multiplay.co.uk>
References:  <4CAD348034DD463E80C89DD5A0BDD71B@multiplay.co.uk> <L31KSlcfsHEIWijui9oQC3siWnE@H1uwQtuDamiOTxQ5dYkc3ncI/0w> <FEC6035595894C6CBD490CCEDEA977FB@multiplay.co.uk>

next in thread | previous in thread | raw e-mail | index | archive | help

--cvVnyQ+4j833TQvp
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Fri, Aug 05, 2011 at 10:59:43AM +0100, Steven Hartland wrote:
> I've tried the patch and it a few cut and paste errors, which I've fixed,

Thanks for spotting that!

> and confirmed it works as expected, so thanks for that :)
>=20
> There's also a load more drivers with the same issue so I've gone through
> and fixed all the occurances I can find. Here's the updated patch:-
> http://blog.multiplay.co.uk/dropzone/freebsd/ccb_timeout.patch

I had found a couple of missed drivers, fixed overlong lines and fixed
the missing 10 in the sys/dev/hptrr/hptrr_os_bsd.c.  Also changed ciss
to have u_int32_t timeouts instead of int ones: this should not harm
anything, because all passed timeouts are explicit numbers that are
not larger than 100000.  And I had also renamed
CAM_HDR_TIMEOUT_TO_TICKS to the base CAM_TIMEOUT_TO_TICKS, because it
seems that every CAM timeout is 32-bit long.  The new patch lives at
  http://codelabs.ru/fbsd/patches/cam/CAM-properly-convert-timeout-to-ticks=
=2Ediff

But there are some cases where the argument to the
CAM_TIMEOUT_TO_TICKS is int and not u_int32_t.  It should be mostly
harmless for now, since the values do not exceed 2^32, but my current
feeling about timeouts that are counted in milliseconds that there
should be an in-kernel type for this stuff.  Seems like 32-bit wide
unsigned value is good for it: maximal value is around 46 days that
should be fine for the millisecond-precision timeout.

Through my grep session for the kernel sources I had seen other
(t * hz / 1000) constructs, so I feel that the fix should be
extended to cover these cases as well.

I am interested in the other's opinions on this.

Thanks again!
--=20
Eygene Ryabinkin                                        ,,,^..^,,,
[ Life's unfair - but root password helps!           | codelabs.ru ]
[ 82FE 06BC D497 C0DE 49EC  4FF0 16AF 9EAE 8152 ECFB | freebsd.org ]

--cvVnyQ+4j833TQvp
Content-Type: application/pgp-signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.17 (FreeBSD)

iF4EAREIAAYFAk471s0ACgkQFq+eroFS7PvdvgD/Y8vcBd31rE8RQgX3HAjf76Z/
hubFqM3PkZknlM63XEIA/25zx2hjvBuzzgbC3QftshBTHZpf155idkJZgFjLFZAx
=Kpxm
-----END PGP SIGNATURE-----

--cvVnyQ+4j833TQvp--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?IZymNZtOnyjv6wqqoirLBsUvqyw>