Re: panic: LK_RETRY set with incompatible flags

On 5 February 2013 04:13, Rick Macklem <rmacklem_at_uoguelph.ca> wrote:
> Sergey Kandaurov wrote:
>> On 4 February 2013 06:32, Rick Macklem <rmacklem_at_uoguelph.ca> wrote:
>> > Konstantin Belousov wrote:
>> >> On Sat, Feb 02, 2013 at 09:30:39PM -0500, Rick Macklem wrote:
>> >> > Andriy Gapon wrote:
>> >> > > on 31/01/2013 15:29 Sergey Kandaurov said the following:
>> >> > > > Hi.
>> >> > > >
>> >> > > > Got this assertion on idle NFS server while `ls -la
>> >> > > > /.zfs/shares/'
>> >> > > > issued on NFS client.
>> > Ok, here's a snippet of zfs_dirlook() from zfs_dir.c:
>> > 388 } else if (name[0] == '.' && name[1] == '.' && name[2] == 0) {
>> > 389 zfsvfs_t *zfsvfs = dzp->z_zfsvfs;
>> > 390
>> > 391 /*
>> > 392 * If we are a snapshot mounted under .zfs, return
>> > 393 * the vp for the snapshot directory.
>> > 394 */
>> > 395 if ((error = sa_lookup(dzp->z_sa_hdl,
>> > 396 SA_ZPL_PARENT(zfsvfs), &parent, sizeof (parent))) != 0)
>> > 397 return (error);
>> > 398 if (parent == dzp->z_id && zfsvfs->z_parent != zfsvfs) {
>> > 399 error = zfsctl_root_lookup(zfsvfs->z_parent->z_ctldir,
>> > 400 "snapshot", vpp, NULL, 0, NULL, kcred,
>> > 401 NULL, NULL, NULL);
>> > 402 return (error);
>> > 403 }
>> >
>> > Just reading the comment, I don't know what it is referring to by
>> > "snapshot directory". Would that be "shares" for Sergey's case?
>> >
>> > It seems too obvious, but maybe the lookup of ".." is returning the
>> > vnode for /.zfs/shares for this case?
>> >
>> > I don't know why this wouldn't have shown up before, but maybe it
>> > usually
>> > replies from its cache (I think zfs_lookup() calls it a "fast
>> > path").
>> >
>> > It might still be interesting to replace zfs_vnops.c line# 1451
>> > with:
>> >       if ((cnp->cn_flags & ISDOTDOT) && *vpp != dvp)
>> > and see what happens?
>> >
>>
>> With this change `ls /home/user1001/.zfs/shares/' lists empty
>> directory
>> (although the relevant dataset has snapshot, but that's a different
>> story :)).
>> Great!
>> Nothing panics/asserts/etc, just seemingly unrelated LOR
>>
> Yes, I think the patch is relatively safe, since lookup() checks for
> same vnode and does a vrele() instead of a vput() when they are the same,
> at least for a plain lookup without wantparent.
>
> So, since I've never used ZFS, what does a "ls -la /home/user1001/.zfs/shares/"
> give you when done locally one the server?

On server (with unmodified kernel):
# ls -la /pool1/user1001/.zfs/share
total 2
dr-xr-xr-x  2 root  wheel  2 Feb  2 20:06 .
dr-xr-xr-x  4 root  wheel  4 Feb  2 20:06 ..

It crashes only when .zfs/share is accessed via NFS
(with and without snapshots), and not when accessed locally.

-- 
wbr,
pluknet