Re: EDEADLK from fcntl(F_SETFL) ?

--ZJM0Iyk369AH1/Ez
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Thu, Jul 03, 2014 at 07:15:51PM -0700, Adrian Chadd wrote:
> Hi,
>=20
> I'm currently testing this out. It seems to be working out alright.
>=20
> adrian@test3:~/work/freebsd % svn diff stable/10/src/sys/kern/
>=20
> Index: stable/10/src/sys/kern/kern_lockf.c
>=20
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>=20
> --- stable/10/src/sys/kern/kern_lockf.c (revision 267627)
>=20
> +++ stable/10/src/sys/kern/kern_lockf.c (working copy)
>=20
> @@ -1425,6 +1425,14 @@
>=20
>                         if (lockf_debug & 1)
>=20
>                                 lf_print("lf_setlock: deadlock", lock);
>=20
>  #endif
>=20
> +
>=20
> +                       /*
>=20
> +                        * If the lock isn't waiting, return EAGAIN
>=20
> +                        * rather than EDEADLK.
>=20
> +                        */
>=20
> +                       if (((lock->lf_flags & F_WAIT) =3D=3D 0) &&
>=20
> +                           (error =3D=3D EDEADLK))
>=20
> +                               error =3D EAGAIN;
>=20
>                         lf_free_lock(lock);
>=20
>                         goto out;
>=20
>                 }
>=20
> On 3 July 2014 17:45, Adrian Chadd <[e-mail]> wrote:
> > Hi!
> >
> > I've seen sqlite3 crap out due to "disk IO error". It looks like the
> > F_SETFL path is returning EDEADLK when it shouldn't be - only the
> > "wait" version of this should be.
> >
> > The kernel code looks to be:
> >
> > lf_setlock() -> lf_add_outgoing() -> lf_add_edge() -> graph_add_edge()
> > -> EDEADLK
> >
> > .. and lf_setlock() will return an error from lf_add_outgoing()
> > without checking if it's (a) EDEADLK, and (b) whether we're going to
> > sleep or not.
> >
> > So, sqlite3 trips up on this. I'm sure other things do. What should
> > the correct thing be? It looks like EWOULDBLOCK is the correct value
> > to return for F_SETFL failing, not EDEADLK.
> >
> > What do those-who-know-POSIX-standards-better-than-I think?

I doubt that the patch is correct. If there is an issue in kernel, the
patch only hides it.

Note that lf_setlock() first calls lf_getblock() to verify that there
is no contending lock on the range, and if there is a conflicting lock,
the very first statement inside the if() checks for F_WAIT.

Either you get a real deadlock, or there is a bug elsewere.

--ZJM0Iyk369AH1/Ez
Content-Type: application/pgp-signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBAgAGBQJTtnTCAAoJEJDCuSvBvK1B74IP/is4vn246sDcNmV3K1UdObpM
WwEz7QUMdP5y4mssxFYtgquxqa+ynNqAxcDk8Dq5H9BKobjV6A5UT69rZcxQAsyC
8Py5zx5rXPEPuHuh+20hSe3h/hhoZaygnUOz1nNAAHxpamTctxwqZmPvzYEWoS0Q
P6Ztoc6z1JxZsUcXERLECKMLnbSqpRa45mfdB59NwlJSei2tGvaN9zJSe8BbJGat
p23s0ObsxFUVX7izDyaJFsZpmHA+o1z6YW48+Gs+eyE9wkEsyuPfkN+uTO9ZIiGL
tES/jrZcNWUzMd+6Mdq99dRMHrzCAEU5BpqoDX9sUUN45BErtI/Ul18+7w1Yjb9e
dBv1NnpO0Gy2emJS+05ecNMJArbKcvLoEjrleHyH6kob0t9l4G2HbR08Oy9CwxL0
uk6ugNgyw1l5TTgkJLP3ewbo+9ngSWyh/USsoOCTj7rMWIQBP/2iPykc/6+HW1Vw
9swbq822tIJI9HeuIZUp0Ssa9yz6TCokiA9Wv1AqXhJ/PaFb2I4I9j5Z47NbxpYG
ofCHyPM2RBsRat6oJQF0dGksag36Pqrw37Twt38yu7fxuKpJS3GIs2RDrY1BgeZQ
7FywjcJzgx6TgRjSRoRK5APQnNC81Qo1fqdr0Btw40Jk/FgNw4+4+9g1CO5ARhui
EHRXS4DEX/0svZmTar9D
=ucwR
-----END PGP SIGNATURE-----

--ZJM0Iyk369AH1/Ez--