Re: Core Dump / panic sleeping thread

2013-03-20 Thread Rick Macklem
Konstantin Belousov wrote:
> On Wed, Mar 20, 2013 at 11:37:56AM -0400, Rick Macklem wrote:
> > Konstantin Belousov wrote:
> > > On Wed, Mar 20, 2013 at 12:13:05PM +0100, Michael Landin Hostbaek
> > > wrote:
> > > >
> > > > On Mar 20, 2013, at 10:49 AM, Konstantin Belousov
> > > >  wrote:
> > > > >
> > > > > I do not like it. As I said in the previous response to
> > > > > Andrey,
> > > > > I think that moving the vnode_pager_setsize() after the unlock
> > > > > is
> > > > > better, since it reduces races with other thread seeing
> > > > > half-done
> > > > > attribute update or making attribute change simultaneously.
> > > >
> > > > OK - so should I wait for another patch - or?
> > >
> > > I think the following is what I mean. As an additional note, why
> > > nfs
> > > client does not trim the buffers when server reported node size
> > > change
> > > ?
> > >
> > > diff --git a/sys/fs/nfsclient/nfs_clport.c
> > > b/sys/fs/nfsclient/nfs_clport.c
> > > index a07a67f..4fe2e35 100644
> > > --- a/sys/fs/nfsclient/nfs_clport.c
> > > +++ b/sys/fs/nfsclient/nfs_clport.c
> > > @@ -361,6 +361,8 @@ nfscl_loadattrcache(struct vnode **vpp, struct
> > > nfsvattr *nap, void *nvaper,
> > > struct nfsnode *np;
> > > struct nfsmount *nmp;
> > > struct timespec mtime_save;
> > > + u_quad_t nsize;
> > > + int setnsize;
> > >
> > > /*
> > > * If v_type == VNON it is a new node, so fill in the v_type,
> > > @@ -418,6 +420,7 @@ nfscl_loadattrcache(struct vnode **vpp, struct
> > > nfsvattr *nap, void *nvaper,
> > > } else
> > > vap->va_fsid = vp->v_mount->mnt_stat.f_fsid.val[0];
> > > np->n_attrstamp = time_second;
> > > + setnsize = 0;
> > > if (vap->va_size != np->n_size) {
> > > if (vap->va_type == VREG) {
> > > if (dontshrink && vap->va_size < np->n_size) {
> > > @@ -444,10 +447,13 @@ nfscl_loadattrcache(struct vnode **vpp,
> > > struct
> > > nfsvattr *nap, void *nvaper,
> > > np->n_size = vap->va_size;
> > > np->n_flag |= NSIZECHANGED;
> > > }
> > > - vnode_pager_setsize(vp, np->n_size);
> > > } else {
> > > np->n_size = vap->va_size;
> > > }
> > > + if (vap->va_type == VREG || vap->va_type == VDIR) {
> > > + setnsize = 1;
> > > + nsize = vap->va_size;
> > I might have used np->n_size here, since that is what is given
> > as the argument for the pre-patched version, but since
> > np->n_size should equal vap->va_size (it is set the same for
> > all cases in the code at this point), it doesn't really matter.
> >
> > I have no idea what the implications of doing vnode_pager_setsize()
> > for VDIR is, but Kostik would be much more conversant that I on
> > this,
> > so if he thinks it's ok, that's fine with me.
> >
> > > + }
> > > }
> > > /*
> > > * The following checks are added to prevent a race between (say)
> > > @@ -480,6 +486,8 @@ nfscl_loadattrcache(struct vnode **vpp, struct
> > > nfsvattr *nap, void *nvaper,
> > > KDTRACE_NFS_ATTRCACHE_LOAD_DONE(vp, vap, 0);
> > > #endif
> > > NFSUNLOCKNODE(np);
> > > + if (setnsize)
> > > + vnode_pager_setsize(vp, nsize);
> > > return (0);
> > > }
> > Yes, I think Kostik's version of the patch is good. I had thought
> > of doing it that way, but want for the "minimal change" version.
> > I agree that avoiding unlocking/relocking the mutex is a good idea,
> > although I didn't see anything after the relock that I thought
> > might be an issue if something changed while unlocked.
> If the parallel calls to nfscl_loadattrcache() are possible, then
> IMHO at least the n_attrstamp could be cleared needlessly.
> 
> >
> > Kostik, thanks for posting this version, rick
> > ps: Michael, I'd suggest you try this patch instead of mine.
> Still, my patch has the issue I noted for the head as well: the
> buffers
> are not destroyed if the size of the vnode is decreased. I would be
> inclined to suggest the following change on top of my patch, but I am
> sure that it does not work, since vnode is generally not locked in
> the nfs_loadattrcache(), I think:
> 
Oh, and I think jhb@ was mentioning, if this client is only reading
the file, it will invalidate the buffers when it sees the mtime
change on a subsequent read.

rick

> diff --git a/sys/fs/nfsclient/nfs_clport.c
> b/sys/fs/nfsclient/nfs_clport.c
> index 4fe2e35..3a08424 100644
> --- a/sys/fs/nfsclient/nfs_clport.c
> +++ b/sys/fs/nfsclient/nfs_clport.c
> @@ -487,7 +487,7 @@ nfscl_loadattrcache(struct vnode **vpp, struct
> nfsvattr *nap, void *nvaper,
> #endif
> NFSUNLOCKNODE(np);
> if (setnsize)
> - vnode_pager_setsize(vp, nsize);
> + vtruncbuf(vp, NOCRED, nsize, vp->v_bufobj.bo_bsize);
> return (0);
> }
___
freebsd-stable@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to "freebsd-stable-unsubscr...@freebsd.org"


Re: Core Dump / panic sleeping thread

2013-03-20 Thread Rick Macklem
Konstantin Belousov wrote:
> On Wed, Mar 20, 2013 at 11:37:56AM -0400, Rick Macklem wrote:
> > Konstantin Belousov wrote:
> > > On Wed, Mar 20, 2013 at 12:13:05PM +0100, Michael Landin Hostbaek
> > > wrote:
> > > >
> > > > On Mar 20, 2013, at 10:49 AM, Konstantin Belousov
> > > >  wrote:
> > > > >
> > > > > I do not like it. As I said in the previous response to
> > > > > Andrey,
> > > > > I think that moving the vnode_pager_setsize() after the unlock
> > > > > is
> > > > > better, since it reduces races with other thread seeing
> > > > > half-done
> > > > > attribute update or making attribute change simultaneously.
> > > >
> > > > OK - so should I wait for another patch - or?
> > >
> > > I think the following is what I mean. As an additional note, why
> > > nfs
> > > client does not trim the buffers when server reported node size
> > > change
> > > ?
> > >
> > > diff --git a/sys/fs/nfsclient/nfs_clport.c
> > > b/sys/fs/nfsclient/nfs_clport.c
> > > index a07a67f..4fe2e35 100644
> > > --- a/sys/fs/nfsclient/nfs_clport.c
> > > +++ b/sys/fs/nfsclient/nfs_clport.c
> > > @@ -361,6 +361,8 @@ nfscl_loadattrcache(struct vnode **vpp, struct
> > > nfsvattr *nap, void *nvaper,
> > > struct nfsnode *np;
> > > struct nfsmount *nmp;
> > > struct timespec mtime_save;
> > > + u_quad_t nsize;
> > > + int setnsize;
> > >
> > > /*
> > > * If v_type == VNON it is a new node, so fill in the v_type,
> > > @@ -418,6 +420,7 @@ nfscl_loadattrcache(struct vnode **vpp, struct
> > > nfsvattr *nap, void *nvaper,
> > > } else
> > > vap->va_fsid = vp->v_mount->mnt_stat.f_fsid.val[0];
> > > np->n_attrstamp = time_second;
> > > + setnsize = 0;
> > > if (vap->va_size != np->n_size) {
> > > if (vap->va_type == VREG) {
> > > if (dontshrink && vap->va_size < np->n_size) {
> > > @@ -444,10 +447,13 @@ nfscl_loadattrcache(struct vnode **vpp,
> > > struct
> > > nfsvattr *nap, void *nvaper,
> > > np->n_size = vap->va_size;
> > > np->n_flag |= NSIZECHANGED;
> > > }
> > > - vnode_pager_setsize(vp, np->n_size);
> > > } else {
> > > np->n_size = vap->va_size;
> > > }
> > > + if (vap->va_type == VREG || vap->va_type == VDIR) {
> > > + setnsize = 1;
> > > + nsize = vap->va_size;
> > I might have used np->n_size here, since that is what is given
> > as the argument for the pre-patched version, but since
> > np->n_size should equal vap->va_size (it is set the same for
> > all cases in the code at this point), it doesn't really matter.
> >
> > I have no idea what the implications of doing vnode_pager_setsize()
> > for VDIR is, but Kostik would be much more conversant that I on
> > this,
> > so if he thinks it's ok, that's fine with me.
> >
> > > + }
> > > }
> > > /*
> > > * The following checks are added to prevent a race between (say)
> > > @@ -480,6 +486,8 @@ nfscl_loadattrcache(struct vnode **vpp, struct
> > > nfsvattr *nap, void *nvaper,
> > > KDTRACE_NFS_ATTRCACHE_LOAD_DONE(vp, vap, 0);
> > > #endif
> > > NFSUNLOCKNODE(np);
> > > + if (setnsize)
> > > + vnode_pager_setsize(vp, nsize);
> > > return (0);
> > > }
> > Yes, I think Kostik's version of the patch is good. I had thought
> > of doing it that way, but want for the "minimal change" version.
> > I agree that avoiding unlocking/relocking the mutex is a good idea,
> > although I didn't see anything after the relock that I thought
> > might be an issue if something changed while unlocked.
> If the parallel calls to nfscl_loadattrcache() are possible, then
> IMHO at least the n_attrstamp could be cleared needlessly.
> 
And that could result in an attribute cache miss, since setting
n_attrstamp = 0 invalidates the cached attributes.

I agree that your patch is preferred. I'll admit I was mostly lazy
(and a little afraid of moving the vnode_pager_setsize()) when I
did the patch.

> >
> > Kostik, thanks for posting this version, rick
> > ps: Michael, I'd suggest you try this patch instead of mine.
> Still, my patch has the issue I noted for the head as well: the
> buffers
> are not destroyed if the size of the vnode is decreased. I would be
> inclined to suggest the following change on top of my patch, but I am
> sure that it does not work, since vnode is generally not locked in
> the nfs_loadattrcache(), I think:
> 
Well, read/write sharing of files over NFS is pretty rare, so I suspect
a truncation of a file by another client (or locally in the NFS server)
is a rare event. As such, not invalidating the buffers here doesn't seem
like a big issue? (The client uses np->n_size to determine EOF.)

Also, I think close-to-open consistency will typically throw away the
buffers on the next open when it sees the mtime changed. (Yes, there
won't necessarily be another open, but...)

As you point out, I don't think the vnode will be locked when
nfscl_loadattrcache() is called for read/writes being done by the
nfsiod threads and will only be a shared vnode lock for other reads.

I think your patch without the below addition is just fine, rick

> diff --git a/sys/fs/nfsclient/nfs_clport.c
> b/sys/fs/nfsclient/nfs_

Re: Core Dump / panic sleeping thread

2013-03-20 Thread Konstantin Belousov
On Wed, Mar 20, 2013 at 08:58:08PM +0200, Konstantin Belousov wrote:
> On Wed, Mar 20, 2013 at 09:43:20AM -0400, John Baldwin wrote:
> > On Wednesday, March 20, 2013 9:22:22 am Konstantin Belousov wrote:
> > > On Wed, Mar 20, 2013 at 12:13:05PM +0100, Michael Landin Hostbaek wrote:
> > > > 
> > > > On Mar 20, 2013, at 10:49 AM, Konstantin Belousov  
> > wrote:
> > > > > 
> > > > > I do not like it. As I said in the previous response to Andrey,
> > > > > I think that moving the vnode_pager_setsize() after the unlock is
> > > > > better, since it reduces races with other thread seeing half-done
> > > > > attribute update or making attribute change simultaneously.
> > > > 
> > > > OK - so should I wait for another patch - or? 
> > > 
> > > I think the following is what I mean. As an additional note, why nfs
> > > client does not trim the buffers when server reported node size change ?
> > 
> > Will changing the size always result in an mtime change forcing the client 
> > to
> > throw away the data on the next read or fault anyway (or does it only affect
> > ctime)?
> 
> UFS only modifies ctime on truncation, it seems.

No, I was wrong. ffs_truncate() indeed only sets both IN_CHANGE | IN_UPDATE
flags for the inode, and IN_UPDATE causes mtime update in ufs_itimes(),
called from UFS_UPDATE().



pgp1vaxm89T7D.pgp
Description: PGP signature


Re: Panic : bad pte

2013-03-20 Thread David Demelier
2013/3/19 Jeremy Chadwick :
> On Tue, Mar 19, 2013 at 06:34:24PM +0100, David Demelier wrote:
>> Hello,
>>
>> There it is, all my computers on FreeBSD 9.1-RELEASE had panic. I can
>> just say there is a problem in the 9.1-RELEASE because I had no panic
>> before. What afraid me is that my production server also panic'ed a
>> few days ago, fortunately it does not appears so often.
>>
>> This is a panic that happened on my desktop computer, with a graphic
>> card. The crash usually appears when X starts.
>>
>> GNU gdb 6.1.1 [FreeBSD]
>> Copyright 2004 Free Software Foundation, Inc.
>> GDB is free software, covered by the GNU General Public License, and you are
>> welcome to change it and/or distribute copies of it under certain conditions.
>> Type "show copying" to see the conditions.
>> There is absolutely no warranty for GDB.  Type "show warranty" for details.
>> This GDB was configured as "amd64-marcel-freebsd"...
>>
>> Unread portion of the kernel message buffer:
>> panic: bad pte
>> cpuid = 3
>> KDB: stack backtrace:
>> Uptime: 2m31s
>> Dumping 183 out of 1950 MB:..9%..18%..27%..35%..44%..53%..62%..79%..88%..96%
>>
>> Reading symbols from /boot/modules/nvidia.ko...done.
>> Loaded symbols for /boot/modules/nvidia.ko
>> #0  doadump (textdump=Variable "textdump" is not available.
>> ) at pcpu.h:224
>> 224 pcpu.h: No such file or directory.
>> in pcpu.h
>> (kgdb) bt
>> #0  doadump (textdump=Variable "textdump" is not available.
>> ) at pcpu.h:224
>> #1  0x0004 in ?? ()
>> #2  0x8048c156 in kern_reboot (howto=260) at
>> /usr/src/sys/kern/kern_shutdown.c:448
>> #3  0x8048c619 in panic (fmt=0x1 )
>> at /usr/src/sys/kern/kern_shutdown.c:636
>> #4  0x8065f88a in pmap_remove_pages (pmap=0xfe0005a2fa60)
>> at /usr/src/sys/amd64/amd64/pmap.c:4156
>> #5  0x8063d26b in vmspace_exit (td=0xfe0005a05470) at
>> /usr/src/sys/vm/vm_map.c:422
>> #6  0x8045d725 in exit1 (td=0xfe0005a05470, rv=Variable
>> "rv" is not available.
>> ) at /usr/src/sys/kern/kern_exit.c:315
>> #7  0x8045e5ce in sys_sys_exit (td=Variable "td" is not available.
>> ) at /usr/src/sys/kern/kern_exit.c:122
>> #8  0x8066737f in amd64_syscall (td=0xfe0005a05470,
>> traced=0) at subr_syscall.c:135
>> #9  0x80652d97 in Xfast_syscall () at
>> /usr/src/sys/amd64/amd64/exception.S:387
>> #10 0x000800d51c1c in ?? ()
>> Previous frame inner to this frame (corrupt stack?)
>> (kgdb)
>>
>> Of course I may do something wrong, and I hope so but unfortunately I
>> can't find any solution. May the nvidia driver be the problem?
>
> Interesting timing.  Semi-recently (February) src/sys/amd64/amd64/pmap.c
> in 9.1-STABLE (not -RELEASE) was modified to increase the information
> shown for this specific type of panic.  See revision 247079:
>
> http://svnweb.freebsd.org/base/stable/9/sys/amd64/amd64/pmap.c?view=log
>
> I've CC'd Konstantin Belousov (kib@), who should be able to help step
> you through getting information out of the crash dump, to help track
> down the root cause.
>
> --
> | Jeremy Chadwick   j...@koitsu.org |
> | UNIX Systems Administratorhttp://jdc.koitsu.org/ |
> | Mountain View, CA, US|
> | Making life hard for others since 1977. PGP 4BD6C0CB |

You will not believe that, when I leave the desktop. I completely
detach the AC adaptor (usually at evening). And everyday when I plug
it and start the machine it panics. But when it reboots and start
again no panic anymore. I just can't believe it.

The panic is completely different from yesterday's one :

Fatal trap 12: page fault while in kernel mode
cpuid = 0; apic id = 00
fault virtual address   = 0x8010
fault code  = supervisor read data, page not present
instruction pointer = 0x20:0x8049db4e
stack pointer   = 0x28:0xff8000225a90
frame pointer   = 0x28:0xfe000247c8e0
code segment= base 0x0, limit 0xf, type 0x1b
= DPL 0, pres 1, long 1, def32 0, gran 1
processor eflags= resume, IOPL = 0
current process = 10 (idle: cpu0)
trap number = 12
panic: page fault
cpuid = 0
KDB: stack backtrace:
Uptime: 1m3s
Dumping 324 out of 1950 MB:..5%..15%..25%..35%..45%..55%..65%..74%..84%..94%

Reading symbols from /boot/modules/nvidia.ko...done.
Loaded symbols for /boot/modules/nvidia.ko
#0  doadump (textdump=Variable "textdump" is not available.
) at pcpu.h:224
224 pcpu.h: No such file or directory.
in pcpu.h
(kgdb) bt
#0  doadump (textdump=Variable "textdump" is not available.
) at pcpu.h:224
#1  0x0004 in ?? ()
#2  0x80489506 in kern_reboot (howto=260) at
/usr/src/sys/kern/kern_shutdown.c:448
#3  0x804899c9 in panic (fmt=0x1 )
at /usr/src/sys/kern/kern_shutdown.c:636
#4  0x80664e39 in trap_fatal (frame=0xc, eva=Variable "eva" is
not available.
) at /usr/sr

Re: Core Dump / panic sleeping thread

2013-03-20 Thread Konstantin Belousov
On Wed, Mar 20, 2013 at 09:43:20AM -0400, John Baldwin wrote:
> On Wednesday, March 20, 2013 9:22:22 am Konstantin Belousov wrote:
> > On Wed, Mar 20, 2013 at 12:13:05PM +0100, Michael Landin Hostbaek wrote:
> > > 
> > > On Mar 20, 2013, at 10:49 AM, Konstantin Belousov  
> wrote:
> > > > 
> > > > I do not like it. As I said in the previous response to Andrey,
> > > > I think that moving the vnode_pager_setsize() after the unlock is
> > > > better, since it reduces races with other thread seeing half-done
> > > > attribute update or making attribute change simultaneously.
> > > 
> > > OK - so should I wait for another patch - or? 
> > 
> > I think the following is what I mean. As an additional note, why nfs
> > client does not trim the buffers when server reported node size change ?
> 
> Will changing the size always result in an mtime change forcing the client to
> throw away the data on the next read or fault anyway (or does it only affect
> ctime)?

UFS only modifies ctime on truncation, it seems.


pgpOCtPQobYQ4.pgp
Description: PGP signature


Re: Core Dump / panic sleeping thread

2013-03-20 Thread John Baldwin
On Wednesday, March 20, 2013 9:22:22 am Konstantin Belousov wrote:
> On Wed, Mar 20, 2013 at 12:13:05PM +0100, Michael Landin Hostbaek wrote:
> > 
> > On Mar 20, 2013, at 10:49 AM, Konstantin Belousov  
wrote:
> > > 
> > > I do not like it. As I said in the previous response to Andrey,
> > > I think that moving the vnode_pager_setsize() after the unlock is
> > > better, since it reduces races with other thread seeing half-done
> > > attribute update or making attribute change simultaneously.
> > 
> > OK - so should I wait for another patch - or? 
> 
> I think the following is what I mean. As an additional note, why nfs
> client does not trim the buffers when server reported node size change ?

Will changing the size always result in an mtime change forcing the client to
throw away the data on the next read or fault anyway (or does it only affect
ctime)?

-- 
John Baldwin
___
freebsd-stable@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to "freebsd-stable-unsubscr...@freebsd.org"


Re: Core Dump / panic sleeping thread

2013-03-20 Thread Konstantin Belousov
On Wed, Mar 20, 2013 at 11:37:56AM -0400, Rick Macklem wrote:
> Konstantin Belousov wrote:
> > On Wed, Mar 20, 2013 at 12:13:05PM +0100, Michael Landin Hostbaek
> > wrote:
> > >
> > > On Mar 20, 2013, at 10:49 AM, Konstantin Belousov
> > >  wrote:
> > > >
> > > > I do not like it. As I said in the previous response to Andrey,
> > > > I think that moving the vnode_pager_setsize() after the unlock is
> > > > better, since it reduces races with other thread seeing half-done
> > > > attribute update or making attribute change simultaneously.
> > >
> > > OK - so should I wait for another patch - or?
> > 
> > I think the following is what I mean. As an additional note, why nfs
> > client does not trim the buffers when server reported node size change
> > ?
> > 
> > diff --git a/sys/fs/nfsclient/nfs_clport.c
> > b/sys/fs/nfsclient/nfs_clport.c
> > index a07a67f..4fe2e35 100644
> > --- a/sys/fs/nfsclient/nfs_clport.c
> > +++ b/sys/fs/nfsclient/nfs_clport.c
> > @@ -361,6 +361,8 @@ nfscl_loadattrcache(struct vnode **vpp, struct
> > nfsvattr *nap, void *nvaper,
> > struct nfsnode *np;
> > struct nfsmount *nmp;
> > struct timespec mtime_save;
> > + u_quad_t nsize;
> > + int setnsize;
> > 
> > /*
> > * If v_type == VNON it is a new node, so fill in the v_type,
> > @@ -418,6 +420,7 @@ nfscl_loadattrcache(struct vnode **vpp, struct
> > nfsvattr *nap, void *nvaper,
> > } else
> > vap->va_fsid = vp->v_mount->mnt_stat.f_fsid.val[0];
> > np->n_attrstamp = time_second;
> > + setnsize = 0;
> > if (vap->va_size != np->n_size) {
> > if (vap->va_type == VREG) {
> > if (dontshrink && vap->va_size < np->n_size) {
> > @@ -444,10 +447,13 @@ nfscl_loadattrcache(struct vnode **vpp, struct
> > nfsvattr *nap, void *nvaper,
> > np->n_size = vap->va_size;
> > np->n_flag |= NSIZECHANGED;
> > }
> > - vnode_pager_setsize(vp, np->n_size);
> > } else {
> > np->n_size = vap->va_size;
> > }
> > + if (vap->va_type == VREG || vap->va_type == VDIR) {
> > + setnsize = 1;
> > + nsize = vap->va_size;
> I might have used np->n_size here, since that is what is given
> as the argument for the pre-patched version, but since
> np->n_size should equal vap->va_size (it is set the same for
> all cases in the code at this point), it doesn't really matter.
> 
> I have no idea what the implications of doing vnode_pager_setsize()
> for VDIR is, but Kostik would be much more conversant that I on this,
> so if he thinks it's ok, that's fine with me.
> 
> > + }
> > }
> > /*
> > * The following checks are added to prevent a race between (say)
> > @@ -480,6 +486,8 @@ nfscl_loadattrcache(struct vnode **vpp, struct
> > nfsvattr *nap, void *nvaper,
> > KDTRACE_NFS_ATTRCACHE_LOAD_DONE(vp, vap, 0);
> > #endif
> > NFSUNLOCKNODE(np);
> > + if (setnsize)
> > + vnode_pager_setsize(vp, nsize);
> > return (0);
> > }
> Yes, I think Kostik's version of the patch is good. I had thought
> of doing it that way, but want for the "minimal change" version.
> I agree that avoiding unlocking/relocking the mutex is a good idea,
> although I didn't see anything after the relock that I thought
> might be an issue if something changed while unlocked.
If the parallel calls to nfscl_loadattrcache() are possible, then
IMHO at least the n_attrstamp could be cleared needlessly.

> 
> Kostik, thanks for posting this version, rick
> ps: Michael, I'd suggest you try this patch instead of mine.
Still, my patch has the issue I noted for the head as well: the buffers
are not destroyed if the size of the vnode is decreased. I would be
inclined to suggest the following change on top of my patch, but I am
sure that it does not work, since vnode is generally not locked in
the nfs_loadattrcache(), I think:

diff --git a/sys/fs/nfsclient/nfs_clport.c b/sys/fs/nfsclient/nfs_clport.c
index 4fe2e35..3a08424 100644
--- a/sys/fs/nfsclient/nfs_clport.c
+++ b/sys/fs/nfsclient/nfs_clport.c
@@ -487,7 +487,7 @@ nfscl_loadattrcache(struct vnode **vpp, struct nfsvattr 
*nap, void *nvaper,
 #endif
NFSUNLOCKNODE(np);
if (setnsize)
-   vnode_pager_setsize(vp, nsize);
+   vtruncbuf(vp, NOCRED, nsize, vp->v_bufobj.bo_bsize);
return (0);
 }
 


pgpXjtJ_eVr_v.pgp
Description: PGP signature


Re: [HEADS UP] pkgng binary packages regression in 1.0.9. Fixed in 1.0.9_1

2013-03-20 Thread Jeremy Chadwick
On Wed, Mar 20, 2013 at 04:20:02PM +0100, Matthias Gamsjager wrote:
> >  Due to the security incident, there are still no official FreeBSD
> > packages.
> 
> Do you know what the status is on that issue?

I'd also like to find out what the status of this is.

The packages at:

ftp://ftp.freebsd.org/pub/FreeBSD/ports/amd64/packages-9-stable/

Are still circa October 2012 -- that's 4-5 months ago.

While I truly and deeply understand that proper engineering design and
infrastructure changes take time, there has been absolutely no
communication presented to the community as to what has (or hasn't)
transpired, if there is (or isn't) a plan, or if people are simply
waiting until future in-person BSD* events to work things out.
freebsd-ops-announce has been silent on this matter as well:

http://lists.freebsd.org/mailman/listinfo/freebsd-ops-announce

At this point users and administrators do not know if newer packages
will be made available or if they should stick to building purely from
source.

Deep down I'm worried that this will solicit a response of "switch to
ports-mgmt/pkg and ports-mgmt/poudriere".  While I'm not opposed to the
tools themselves, I'm strongly opposed to that kind of response as I'm
tired of seeing the security incident being used as a opportunistic
crutch (as it was for the sudden cvsup/csup deprecation).

-- 
| Jeremy Chadwick   j...@koitsu.org |
| UNIX Systems Administratorhttp://jdc.koitsu.org/ |
| Mountain View, CA, US|
| Making life hard for others since 1977. PGP 4BD6C0CB |
___
freebsd-stable@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to "freebsd-stable-unsubscr...@freebsd.org"


Re: Core Dump / panic sleeping thread

2013-03-20 Thread Rick Macklem
Konstantin Belousov wrote:
> On Wed, Mar 20, 2013 at 12:13:05PM +0100, Michael Landin Hostbaek
> wrote:
> >
> > On Mar 20, 2013, at 10:49 AM, Konstantin Belousov
> >  wrote:
> > >
> > > I do not like it. As I said in the previous response to Andrey,
> > > I think that moving the vnode_pager_setsize() after the unlock is
> > > better, since it reduces races with other thread seeing half-done
> > > attribute update or making attribute change simultaneously.
> >
> > OK - so should I wait for another patch - or?
> 
> I think the following is what I mean. As an additional note, why nfs
> client does not trim the buffers when server reported node size change
> ?
> 
> diff --git a/sys/fs/nfsclient/nfs_clport.c
> b/sys/fs/nfsclient/nfs_clport.c
> index a07a67f..4fe2e35 100644
> --- a/sys/fs/nfsclient/nfs_clport.c
> +++ b/sys/fs/nfsclient/nfs_clport.c
> @@ -361,6 +361,8 @@ nfscl_loadattrcache(struct vnode **vpp, struct
> nfsvattr *nap, void *nvaper,
> struct nfsnode *np;
> struct nfsmount *nmp;
> struct timespec mtime_save;
> + u_quad_t nsize;
> + int setnsize;
> 
> /*
> * If v_type == VNON it is a new node, so fill in the v_type,
> @@ -418,6 +420,7 @@ nfscl_loadattrcache(struct vnode **vpp, struct
> nfsvattr *nap, void *nvaper,
> } else
> vap->va_fsid = vp->v_mount->mnt_stat.f_fsid.val[0];
> np->n_attrstamp = time_second;
> + setnsize = 0;
> if (vap->va_size != np->n_size) {
> if (vap->va_type == VREG) {
> if (dontshrink && vap->va_size < np->n_size) {
> @@ -444,10 +447,13 @@ nfscl_loadattrcache(struct vnode **vpp, struct
> nfsvattr *nap, void *nvaper,
> np->n_size = vap->va_size;
> np->n_flag |= NSIZECHANGED;
> }
> - vnode_pager_setsize(vp, np->n_size);
> } else {
> np->n_size = vap->va_size;
> }
> + if (vap->va_type == VREG || vap->va_type == VDIR) {
> + setnsize = 1;
> + nsize = vap->va_size;
I might have used np->n_size here, since that is what is given
as the argument for the pre-patched version, but since
np->n_size should equal vap->va_size (it is set the same for
all cases in the code at this point), it doesn't really matter.

I have no idea what the implications of doing vnode_pager_setsize()
for VDIR is, but Kostik would be much more conversant that I on this,
so if he thinks it's ok, that's fine with me.

> + }
> }
> /*
> * The following checks are added to prevent a race between (say)
> @@ -480,6 +486,8 @@ nfscl_loadattrcache(struct vnode **vpp, struct
> nfsvattr *nap, void *nvaper,
> KDTRACE_NFS_ATTRCACHE_LOAD_DONE(vp, vap, 0);
> #endif
> NFSUNLOCKNODE(np);
> + if (setnsize)
> + vnode_pager_setsize(vp, nsize);
> return (0);
> }
Yes, I think Kostik's version of the patch is good. I had thought
of doing it that way, but want for the "minimal change" version.
I agree that avoiding unlocking/relocking the mutex is a good idea,
although I didn't see anything after the relock that I thought
might be an issue if something changed while unlocked.

Kostik, thanks for posting this version, rick
ps: Michael, I'd suggest you try this patch instead of mine.
___
freebsd-stable@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to "freebsd-stable-unsubscr...@freebsd.org"


Re: [HEADS UP] pkgng binary packages regression in 1.0.9. Fixed in 1.0.9_1

2013-03-20 Thread Matthias Gamsjager
>  Due to the security incident, there are still no official FreeBSD
> packages.
>


Do you know what the status is on that issue?
___
freebsd-stable@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to "freebsd-stable-unsubscr...@freebsd.org"


Re: Core Dump / panic sleeping thread

2013-03-20 Thread Konstantin Belousov
On Wed, Mar 20, 2013 at 12:13:05PM +0100, Michael Landin Hostbaek wrote:
> 
> On Mar 20, 2013, at 10:49 AM, Konstantin Belousov  wrote:
> > 
> > I do not like it. As I said in the previous response to Andrey,
> > I think that moving the vnode_pager_setsize() after the unlock is
> > better, since it reduces races with other thread seeing half-done
> > attribute update or making attribute change simultaneously.
> 
> OK - so should I wait for another patch - or? 

I think the following is what I mean. As an additional note, why nfs
client does not trim the buffers when server reported node size change ?

diff --git a/sys/fs/nfsclient/nfs_clport.c b/sys/fs/nfsclient/nfs_clport.c
index a07a67f..4fe2e35 100644
--- a/sys/fs/nfsclient/nfs_clport.c
+++ b/sys/fs/nfsclient/nfs_clport.c
@@ -361,6 +361,8 @@ nfscl_loadattrcache(struct vnode **vpp, struct nfsvattr 
*nap, void *nvaper,
struct nfsnode *np;
struct nfsmount *nmp;
struct timespec mtime_save;
+   u_quad_t nsize;
+   int setnsize;
 
/*
 * If v_type == VNON it is a new node, so fill in the v_type,
@@ -418,6 +420,7 @@ nfscl_loadattrcache(struct vnode **vpp, struct nfsvattr 
*nap, void *nvaper,
} else
vap->va_fsid = vp->v_mount->mnt_stat.f_fsid.val[0];
np->n_attrstamp = time_second;
+   setnsize = 0;
if (vap->va_size != np->n_size) {
if (vap->va_type == VREG) {
if (dontshrink && vap->va_size < np->n_size) {
@@ -444,10 +447,13 @@ nfscl_loadattrcache(struct vnode **vpp, struct nfsvattr 
*nap, void *nvaper,
np->n_size = vap->va_size;
np->n_flag |= NSIZECHANGED;
}
-   vnode_pager_setsize(vp, np->n_size);
} else {
np->n_size = vap->va_size;
}
+   if (vap->va_type == VREG || vap->va_type == VDIR) {
+   setnsize = 1;
+   nsize = vap->va_size;
+   }
}
/*
 * The following checks are added to prevent a race between (say)
@@ -480,6 +486,8 @@ nfscl_loadattrcache(struct vnode **vpp, struct nfsvattr 
*nap, void *nvaper,
KDTRACE_NFS_ATTRCACHE_LOAD_DONE(vp, vap, 0);
 #endif
NFSUNLOCKNODE(np);
+   if (setnsize)
+   vnode_pager_setsize(vp, nsize);
return (0);
 }
 


pgpa8lhv_88qt.pgp
Description: PGP signature


Re: Core Dump / panic sleeping thread

2013-03-20 Thread Mark Saad

Comments in line .
---


On Mar 19, 2013, at 1:45 PM, Michael Landin Hostbaek  wrote:

> 
> On Mar 19, 2013, at 6:35 PM, Jeremy Chadwick  wrote:
> 
>> On Tue, Mar 19, 2013 at 06:18:06PM +0100, Michael Landin Hostbaek wrote:
>> The kernel panic is happening in NFS-related code.  Rick Macklem (and/or
>> John Baldwin) should be able to help with this; I've CC'd both here.
> 
> OK, thanks. 
> 
> 
>> 
>> You're going to need to provide the following details:
>> 
>> 1. Contents of /etc/rc.conf
> 
> sshd_enable="YES"
> ntpdate_enable="YES"
> ntpdate_hosts="xx.xx.xx.xx"
> fsck_y_enable="YES"
> named_enable="YES"
> dumpdev="AUTO"
> nfs_client_enable="YES"
> rpc_lockd_enable="YES"
> rpc_statd_enable="YES"
> ifconfig_em0="inet xx.xx.xx.xx netmask 255.255.255.0 broadcast xx.xx.xx.xx"
> defaultrouter="xx.xx.xx.xx"
> hostname=""
> cloned_interfaces="vlan"
> ifconfig_vlan="inet xx.xx.xx.xx netmask 255.240.0.0 broadcast xx.xx.xx.xx 
> vlan  vlandev em0"
> apache22_enable="YES"
> pureftpd_enable="YES"
> revealcloud_enable=YES
> 
> 
>> 2. Contents of /etc/sysctl.conf (if modified)
> 
> vm.pmap.shpgperproc=250
> 

Small side note. This sysctl is no longer valid . It's had no effect after 7.2 
iirc . 



>> 3. Contents of /etc/fstab
> 
> # DeviceMountpoint  FStype  Options DumpPass#
> /dev/mirror/gm0s1a/ufsrw11
> /dev/mirror/gm0s1bnoneswapsw00
> /dev/mirror/gm0s1d/varufsrw22
> /dev/mirror/gm0s1e/logsufsrw22
> /dev/mirror/gm0s1f/extraufsrw22
> /dev/mirror/gm0s1g/usrufsrw22
> proc/proc   procfs  rw  0   0
> xx.xx.xx.xx:/zpool-000xxx/www/mnt/wwwnfsrw00
> xx.xx.xx.xx:/zpool-000xxx/data/mnt/datanfsrw,tcp00
> linproc/compat/linux/proclinprocfsrw00
> 
> 
>> 4. ifconfig -a
> 
> em0: flags=8843 metric 0 mtu 1500
>
> options=4219b
>ether 00:25:90:79:a5:ac
>inet xx.xx.xx.xx netmask 0xff00 broadcast xx.xx.xx.xx
>inet6 xx::a5ac%em0 prefixlen 64 scopeid 0x1 
>nd6 options=29
>media: Ethernet autoselect (1000baseT )
>status: active
> em1: flags=8c02 metric 0 mtu 1500
>
> options=4219b
>ether 00:25:90:79:a5:ad
>nd6 options=29
>media: Ethernet autoselect
>status: no carrier
> lo0: flags=8049 metric 0 mtu 16384
>options=63
>inet6 ::1 prefixlen 128 
>inet6 fe80::1%lo0 prefixlen 64 scopeid 0xb 
>inet 127.0.0.1 netmask 0xff00 
>nd6 options=21
> vlan: flags=8843 metric 0 mtu 1500
>options=103
>ether 00:25:90:79:a5:ac
>inet xx.xx.xx.xx netmask 0xfff0 broadcast xx.xx.xx.xx
>inet6 x:::5ac%vlan prefixlen 64 scopeid 0xc 
>nd6 options=29
>media: Ethernet autoselect (1000baseT )
>status: active
>vlan:  parent interface: em0
> 
> 
>> 5. OS used by the NFS server, and all configuration details pertaining
>> to that system
> 
> This is a hosted service, so I do not have access to this - though I believe 
> this is a ZFS fs.
> Here's more info about the product: http://help.ovh.co.uk/Nas
> 
> 
>> 
>> You may also be asked to upgrade to 9.1-STABLE, as there may be fixes
>> for whatever this is in base/stable/9 that are not in -RELEASE, but this
>> is speculative on my part.
> 
> That is not a problem. I would simply like to confirm the issue, before 
> upgrading. 
> 
> 
> Thanks, 
> 
> /mich
> 
> 
> ___
> freebsd-stable@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-stable
> To unsubscribe, send any mail to "freebsd-stable-unsubscr...@freebsd.org"


Mark saad | mark.s...@longcount.org

___
freebsd-stable@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to "freebsd-stable-unsubscr...@freebsd.org"


Re: Empty @cwd in some ports

2013-03-20 Thread Daniel O'Connor

On 20/03/2013, at 22:06, Baptiste Daroussin  wrote:
> Empty cwd is normal, it is equivalent to @cwd %%PREFIX%%


OK thanks.

--
Daniel O'Connor software and network engineer
for Genesis Software - http://www.gsoft.com.au
"The nice thing about standards is that there
are so many of them to choose from."
  -- Andrew Tanenbaum
GPG Fingerprint - 5596 B766 97C0 0E94 4347 295E E593 DC20 7B3F CE8C






___
freebsd-stable@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to "freebsd-stable-unsubscr...@freebsd.org"


Re: Empty @cwd in some ports

2013-03-20 Thread Baptiste Daroussin
On Wed, Mar 20, 2013 at 09:13:08PM +1030, Daniel O'Connor wrote:
> I noticed I have a lot of ports which have an empty @cwd line in the 
> +CONTENTS file (even after reinstalling).
> 
> For example a2ps..
> @comment PKG_FORMAT_REVISION:1.1
> @name a2ps-a4-4.13b_4
> @comment ORIGIN:print/a2ps-a4
> @cwd /usr/local
> @pkgdep xineramaproto-1.2.1
> @comment DEPORIGIN:x11/xineramaproto
> 
> @exec /sbin/ldconfig -m /usr/local/lib
> @unexec /sbin/ldconfig -R
> @comment OPTIONS:+NLS -I18N +EMACS
> @cwd 
> @dirrm share/licenses/a2ps-a4-4.13b_4
> 
> And curl..
> @comment PKG_FORMAT_REVISION:1.1
> @name curl-7.24.0_2
> @comment ORIGIN:ftp/curl
> @cwd /usr/local
> @pkgdep ca_root_nss-3.14.3
> @comment DEPORIGIN:security/ca_root_nss
> 
> @comment OPTIONS:-CARES -CURL_DEBUG -GNUTLS +IPV6 -KERBEROS4 -LDAP -LDAPS 
> -LIBIDN -LIBSSH2 -NTLM +OPENSSL +CA_BUNDLE +PROXY -RTMP
> @comment OPTIONS:-TRACKMEMORY
> @cwd 
> @dirrm share/licenses/curl-7.24.0_2
> @unexec rmdir %D/share/licenses 2>/dev/null || true
> 
> This seems pretty broken but I am not sure what causes it :(
> [midget 21:09] /var/db/pkg >egrep -l '@cwd $' */+CONTENTS| wc -l
>  300
> 
> --
> Daniel O'Connor software and network engineer
> for Genesis Software - http://www.gsoft.com.au
> "The nice thing about standards is that there
> are so many of them to choose from."
>   -- Andrew Tanenbaum
> GPG Fingerprint - 5596 B766 97C0 0E94 4347 295E E593 DC20 7B3F CE8C
> 
> 
> 
> 
> 
> 
> ___
> freebsd-stable@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-stable
> To unsubscribe, send any mail to "freebsd-stable-unsubscr...@freebsd.org"

Empty cwd is normal, it is equivalent to @cwd %%PREFIX%%

regards,
Bapt


pgpBq5HRWAoxm.pgp
Description: PGP signature


Re: Core Dump / panic sleeping thread

2013-03-20 Thread Michael Landin Hostbaek

On Mar 20, 2013, at 10:49 AM, Konstantin Belousov  wrote:
> 
> I do not like it. As I said in the previous response to Andrey,
> I think that moving the vnode_pager_setsize() after the unlock is
> better, since it reduces races with other thread seeing half-done
> attribute update or making attribute change simultaneously.

OK - so should I wait for another patch - or? 

Thanks,


/mich

___
freebsd-stable@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to "freebsd-stable-unsubscr...@freebsd.org"


Empty @cwd in some ports

2013-03-20 Thread Daniel O'Connor
I noticed I have a lot of ports which have an empty @cwd line in the +CONTENTS 
file (even after reinstalling).

For example a2ps..
@comment PKG_FORMAT_REVISION:1.1
@name a2ps-a4-4.13b_4
@comment ORIGIN:print/a2ps-a4
@cwd /usr/local
@pkgdep xineramaproto-1.2.1
@comment DEPORIGIN:x11/xineramaproto

@exec /sbin/ldconfig -m /usr/local/lib
@unexec /sbin/ldconfig -R
@comment OPTIONS:+NLS -I18N +EMACS
@cwd 
@dirrm share/licenses/a2ps-a4-4.13b_4

And curl..
@comment PKG_FORMAT_REVISION:1.1
@name curl-7.24.0_2
@comment ORIGIN:ftp/curl
@cwd /usr/local
@pkgdep ca_root_nss-3.14.3
@comment DEPORIGIN:security/ca_root_nss

@comment OPTIONS:-CARES -CURL_DEBUG -GNUTLS +IPV6 -KERBEROS4 -LDAP -LDAPS 
-LIBIDN -LIBSSH2 -NTLM +OPENSSL +CA_BUNDLE +PROXY -RTMP
@comment OPTIONS:-TRACKMEMORY
@cwd 
@dirrm share/licenses/curl-7.24.0_2
@unexec rmdir %D/share/licenses 2>/dev/null || true

This seems pretty broken but I am not sure what causes it :(
[midget 21:09] /var/db/pkg >egrep -l '@cwd $' */+CONTENTS| wc -l
 300

--
Daniel O'Connor software and network engineer
for Genesis Software - http://www.gsoft.com.au
"The nice thing about standards is that there
are so many of them to choose from."
  -- Andrew Tanenbaum
GPG Fingerprint - 5596 B766 97C0 0E94 4347 295E E593 DC20 7B3F CE8C






___
freebsd-stable@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to "freebsd-stable-unsubscr...@freebsd.org"


Re: Core Dump / panic sleeping thread

2013-03-20 Thread Konstantin Belousov
On Tue, Mar 19, 2013 at 07:37:43PM -0400, Rick Macklem wrote:
> Andriy Gapon wrote:
> > on 19/03/2013 19:35 Jeremy Chadwick said the following:
> > > On Tue, Mar 19, 2013 at 06:18:06PM +0100, Michael Landin Hostbaek
> > > wrote:
> > [snip]
> > >> Unread portion of the kernel message buffer:
> > >> Sleeping thread (tid 100256, pid 85641) owns a non-sleepable lock
> > >> KDB: stack backtrace of thread 100256:
> > >> #0 0x808f2d46 at mi_switch+0x186
> > >> #1 0x8092bb52 at sleepq_wait+0x42
> > >> #2 0x808f34d6 at _sleep+0x376
> > >> #3 0x80b4f3ae at vm_object_page_remove+0x2ce
> > >> #4 0x80b5ac7d at vnode_pager_setsize+0x17d
> > >> #5 0x8082102c at nfscl_loadattrcache+0x2cc
> > >> #6 0x80818d37 at nfs_getattr+0x287
> > >> #7 0x8098f1c0 at vn_stat+0xb0
> > >> #8 0x809869d9 at kern_statat_vnhook+0xf9
> > >> #9 0x80986b55 at kern_statat+0x15
> > >> #10 0x80986c1a at sys_lstat+0x2a
> > >> #11 0x80bd7ae6 at amd64_syscall+0x546
> > >> #12 0x80bc3447 at Xfast_syscall+0xf7
> > >> panic: sleeping thread
> > >> cpuid = 0
> > >> KDB: stack backtrace:
> > >> #0 0x809208a6 at kdb_backtrace+0x66
> > >> #1 0x808ea8be at panic+0x1ce
> > >> #2 0x8092ed22 at propagate_priority+0x1d2
> > >> #3 0x8092fa4e at turnstile_wait+0x1be
> > >> #4 0x808d8d48 at _mtx_lock_sleep+0xd8
> > >> #5 0x80820fa4 at nfscl_loadattrcache+0x244
> > >> #6 0x8081758c at ncl_readrpc+0xac
> > >> #7 0x80824c45 at ncl_getpages+0x485
> > >> #8 0x80b5aa0c at vnode_pager_getpages+0x9c
> > >> #9 0x80b3fc93 at vm_fault_hold+0x673
> > >> #10 0x80b41cc3 at vm_fault+0x73
> > >> #11 0x80bd84b4 at trap_pfault+0x124
> > >> #12 0x80bd8c6c at trap+0x49c
> > >> #13 0x80bc315f at calltrap+0x8
> > [snip]
> > 
> > I think that the regular mutex which is acquired via NFSLOCKNODE() in
> > nfscl_loadattrcache() can not be held across vnode_pager_setsize.
> > I am not sure though when vap->va_size != np->n_size case is
> > triggered.
> > 
> Yep, I'd agree to that. The same bug is in the old NFS client and
> the new NFS client cribbed the code from there.
> 
> I have attached a simple patch that unlocks the mutex for the
> vnode_pager_setsize() call. Maybe you could test it?
> 
> Thanks for reporting this, rick
> ps: Hopefully "patch" can apply this patch (there have been
> recent changes to this file, so the line#s could be off).
> It should be easy to do manually if not. The change is
> in nfscl_loadattrcache() in sys/fs/nfsclient/nfs_clport.c.
> 
> 
> > > You're going to need to provide the following details:
> > >
> > > 1. Contents of /etc/rc.conf
> > > 2. Contents of /etc/sysctl.conf (if modified)
> > > 3. Contents of /etc/fstab
> > > 4. ifconfig -a
> > > 5. OS used by the NFS server, and all configuration details
> > > pertaining
> > > to that system
> > >
> > > You may also be asked to upgrade to 9.1-STABLE, as there may be
> > > fixes
> > > for whatever this is in base/stable/9 that are not in -RELEASE, but
> > > this
> > > is speculative on my part.
> > >
> > I do not see a need for any of these.
> > 
> > --
> > Andriy Gapon
> > ___
> > freebsd-stable@freebsd.org mailing list
> > http://lists.freebsd.org/mailman/listinfo/freebsd-stable
> > To unsubscribe, send any mail to
> > "freebsd-stable-unsubscr...@freebsd.org"

> --- fs/nfsclient/nfs_clport.c.savit   2013-03-19 18:37:33.0 -0400
> +++ fs/nfsclient/nfs_clport.c 2013-03-19 18:44:21.0 -0400
> @@ -444,7 +444,9 @@ nfscl_loadattrcache(struct vnode **vpp, 
>   np->n_size = vap->va_size;
>   np->n_flag |= NSIZECHANGED;
>   }
> + NFSUNLOCKNODE(np);
>   vnode_pager_setsize(vp, np->n_size);
> + NFSLOCKNODE(np);
>   } else {
>   np->n_size = vap->va_size;
>   }

I do not like it. As I said in the previous response to Andrey,
I think that moving the vnode_pager_setsize() after the unlock is
better, since it reduces races with other thread seeing half-done
attribute update or making attribute change simultaneously.


pgpZb2TvHmqTm.pgp
Description: PGP signature


Re: Core Dump / panic sleeping thread

2013-03-20 Thread Michael Landin Hostbaek

On Mar 20, 2013, at 12:37 AM, Rick Macklem  wrote:

>> 
> Yep, I'd agree to that. The same bug is in the old NFS client and
> the new NFS client cribbed the code from there.
> 
> I have attached a simple patch that unlocks the mutex for the
> vnode_pager_setsize() call. Maybe you could test it?
> 
> Thanks for reporting this, rick
> ps: Hopefully "patch" can apply this patch (there have been
>recent changes to this file, so the line#s could be off).
>It should be easy to do manually if not. The change is
>in nfscl_loadattrcache() in sys/fs/nfsclient/nfs_clport.c.

Thanks Rick, it's compiling right now.
I will let you know if the problem persists. 

/mich

ps. the patch worked perfectly up against REL9.1

___
freebsd-stable@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to "freebsd-stable-unsubscr...@freebsd.org"