Re: svn commit: r243311 - in head/sys: fs/ext2fs fs/msdosfs fs/nfsclient fs/nullfs fs/unionfs gnu/fs/reiserfs nfsclient ufs/ffs

2012-11-20 Thread Attilio Rao
On Tue, Nov 20, 2012 at 6:20 AM, Bruce Evans  wrote:
> On Mon, 19 Nov 2012, Attilio Rao wrote:
>
>> Log:
>>  r16312 is not any longer real since many years (likely since when VFS
>>  received granular locking) but the comment present in UFS has been
>>  copied all over other filesystems code incorrectly for several times.
>>
>>  Removes comments that makes no sense now.
>
>
> It still made sense (except for bitrot in the function name), but might not
> be true).  The code made sense with it.  Now the code makes no sense.
>
>
>> Modified: head/sys/ufs/ffs/ffs_vfsops.c
>>
>> ==
>> --- head/sys/ufs/ffs/ffs_vfsops.c   Mon Nov 19 21:58:14 2012
>> (r243310)
>> +++ head/sys/ufs/ffs/ffs_vfsops.c   Mon Nov 19 22:43:45 2012
>> (r243311)
>> @@ -1676,14 +1676,6 @@ ffs_vgetf(mp, ino, flags, vpp, ffs_flags
>> ump = VFSTOUFS(mp);
>> dev = ump->um_dev;
>> fs = ump->um_fs;
>> -
>> -   /*
>> -* If this malloc() is performed after the getnewvnode()
>
>
> This malloc() didn't match the code, which uses uma_zalloc().  Old
> versions used MALLOC() in both the comment and the code.  ffs's comment
> was updated to say malloc() when the code was changed to use malloc(),
> then rotted when the code was changed to use uma_zalloc().  In some
> other file systems, the comment still said MALLOC().
>
>
>> -* it might block, leaving a vnode with a NULL v_data to be
>> -* found by ffs_sync() if a sync happens to fire right then,
>> -* which will cause a panic because ffs_sync() blindly
>> -* dereferences vp->v_data (as well it should).
>> -*/
>> ip = uma_zalloc(uma_inode, M_WAITOK | M_ZERO);
>>
>> /* Allocate a new vnode/inode. */
>>
>
> The code makes no sense now.  The comment explains why ip is allocated
> before vp, instead of in the natural, opposite order like it used to
> be.  Allocating things in an unnatural  order requires extra code to
> free ip when the allocation of vp fails.

"Used to be" is very arguably. The code has been like its current form
many more years than the opposite (16 against 3 I think).
And the code makes perfectly sense if you know the history. So I don't
agree with you.

Thanks for the other comments, I will try to squeeze a patch about it.

Attilio


-- 
Peace can only be achieved by understanding - A. Einstein
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


Re: svn commit: r243320 - head/usr.bin/cut

2012-11-20 Thread Alexey Dokuchaev
On Tue, Nov 20, 2012 at 01:57:21AM +, Eitan Adler wrote:
> Author: eadler
> Date: Tue Nov 20 01:57:21 2012
> New Revision: 243320
> URL: http://svnweb.freebsd.org/changeset/base/243320
> 
> Log:
>   Add 'w' flag to:
>   
>   Use whitespace (spaces and tabs) as the delimiter.
>   Consecutive spaces and tabs count as one single field
>   separator.

Awesome, long desired.  Thanks!

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


Re: svn commit: r243324 - head/etc

2012-11-20 Thread Mike Telahun
On 11/20/2012 10:14 AM, Hiroki Sato wrote:
>  rc_fast, rc_force, and rc_quiet are not intended to be used in
>  /etc/rc.conf or other config files.  These variables have no default
>  value and will be defined forcibly via {fast,force,quiet}start.  If
>  we use checkyesno() here, it checks whether a variable is defined or
>  not and then put an warning message when not defined.  It is a bad
>  side-effect for them.

Ok. Thanks for the explanation.

Cheers,
Mike.
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


Re: svn commit: r243228 - head/etc

2012-11-20 Thread John Hay
Hi Chris,

On Sun, Nov 18, 2012 at 02:21:05PM +, Chris Rees wrote:
> Author: crees (ports committer)
> Date: Sun Nov 18 14:21:05 2012
> New Revision: 243228
> URL: http://svnweb.freebsd.org/changeset/base/243228
> 
> Log:
>   cp -R misses out dotfiles; use pax instead to copy file hierarchies
>   
>   PR: conf/99721 (based on)
>   Submitted by:   Florian Zavatzki 
>   Approved by:hrs
>   MFC after:  1 month
> 
> Modified:
>   head/etc/rc.initdiskless
> 
> Modified: head/etc/rc.initdiskless
> ==
> --- head/etc/rc.initdiskless  Sun Nov 18 14:05:28 2012(r243227)
> +++ head/etc/rc.initdiskless  Sun Nov 18 14:21:05 2012(r243228)
> @@ -354,7 +354,7 @@ for i in ${templates} ; do
>   subdir=${j##*/}
>   if [ -d $j -a ! -f $j.cpio.gz  ]; then
>   create_md $subdir
> - cp -Rp $j/ /$subdir
> + (cd $j && pax -rw . /$subdir)
>   fi
>  done
>  for j in /conf/$i/*.cpio.gz ; do

Have you tested this on a diskless and readonly system? It looks like pax
need to write something in /tmp and it might not be writeable yet. I got
an error, after the first of /bin/pax not found and having to add that to
the list of files needed.

John
-- 
John Hay -- j...@meraka.csir.co.za / j...@freebsd.org
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


svn commit: r243330 - head/usr.sbin/pw

2012-11-20 Thread Baptiste Daroussin
Author: bapt
Date: Tue Nov 20 10:59:41 2012
New Revision: 243330
URL: http://svnweb.freebsd.org/changeset/base/243330

Log:
  Correctly set the password file mode after renaming in NIS mode

Modified:
  head/usr.sbin/pw/pw_nis.c

Modified: head/usr.sbin/pw/pw_nis.c
==
--- head/usr.sbin/pw/pw_nis.c   Tue Nov 20 09:27:56 2012(r243329)
+++ head/usr.sbin/pw/pw_nis.c   Tue Nov 20 10:59:41 2012(r243330)
@@ -68,6 +68,8 @@ pw_nisupdate(const char * path, struct p
}
if (rename(pw_tempname(), path) == -1)
err(1, "rename()");
+   if (chmod(path, 0644) == -1)
+   err(1, "chmod()");
 
free(pw);
pw_fini();
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


Re: svn commit: r243311 - in head/sys: fs/ext2fs fs/msdosfs fs/nfsclient fs/nullfs fs/unionfs gnu/fs/reiserfs nfsclient ufs/ffs

2012-11-20 Thread Bruce Evans

On Tue, 20 Nov 2012, Attilio Rao wrote:


On Tue, Nov 20, 2012 at 6:20 AM, Bruce Evans  wrote:

On Mon, 19 Nov 2012, Attilio Rao wrote:


Log:
 r16312 is not any longer real since many years (likely since when VFS
 received granular locking) but the comment present in UFS has been
 copied all over other filesystems code incorrectly for several times.

 Removes comments that makes no sense now.



It still made sense (except for bitrot in the function name), but might not
be true).  The code made sense with it.  Now the code makes no sense.



Modified: head/sys/ufs/ffs/ffs_vfsops.c

==
--- head/sys/ufs/ffs/ffs_vfsops.c   Mon Nov 19 21:58:14 2012
(r243310)
+++ head/sys/ufs/ffs/ffs_vfsops.c   Mon Nov 19 22:43:45 2012
(r243311)
@@ -1676,14 +1676,6 @@ ffs_vgetf(mp, ino, flags, vpp, ffs_flags
ump = VFSTOUFS(mp);
dev = ump->um_dev;
fs = ump->um_fs;
-
-   /*
-* If this malloc() is performed after the getnewvnode()



This malloc() didn't match the code, which uses uma_zalloc().  Old
versions used MALLOC() in both the comment and the code.  ffs's comment
was updated to say malloc() when the code was changed to use malloc(),
then rotted when the code was changed to use uma_zalloc().  In some
other file systems, the comment still said MALLOC().



-* it might block, leaving a vnode with a NULL v_data to be
-* found by ffs_sync() if a sync happens to fire right then,
-* which will cause a panic because ffs_sync() blindly
-* dereferences vp->v_data (as well it should).
-*/
ip = uma_zalloc(uma_inode, M_WAITOK | M_ZERO);

/* Allocate a new vnode/inode. */



The code makes no sense now.  The comment explains why ip is allocated
before vp, instead of in the natural, opposite order like it used to
be.  Allocating things in an unnatural  order requires extra code to
free ip when the allocation of vp fails.


"Used to be" is very arguably. The code has been like its current form
many more years than the opposite (16 against 3 I think).
And the code makes perfectly sense if you know the history. So I don't
agree with you.


But it shouldn't be necessary to know the history of the code to
understand it.  The code only makes sense if its comment is not removed,
or if you know the history of the code so that you can restore the
removed comment.  However, if the comment makes no sense as you claim,
then the code that it it describes makes no sense.

I didn't point out before that the comment "/* Allocate a new vnode/inode. */"
does less than echo the code, since the code obviously allocates a new
vnode/inode and only the extent of the part which does that is unclear,
and the comment is disorganized so as to make the scope unclear.

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


Re: svn commit: r243311 - in head/sys: fs/ext2fs fs/msdosfs fs/nfsclient fs/nullfs fs/unionfs gnu/fs/reiserfs nfsclient ufs/ffs

2012-11-20 Thread Attilio Rao
On 11/20/12, Bruce Evans  wrote:
> On Tue, 20 Nov 2012, Attilio Rao wrote:
>
>> On Tue, Nov 20, 2012 at 6:20 AM, Bruce Evans 
>> wrote:
>>> On Mon, 19 Nov 2012, Attilio Rao wrote:
>>>
 Log:
  r16312 is not any longer real since many years (likely since when VFS
  received granular locking) but the comment present in UFS has been
  copied all over other filesystems code incorrectly for several times.

  Removes comments that makes no sense now.
>>>
>>>
>>> It still made sense (except for bitrot in the function name), but might
>>> not
>>> be true).  The code made sense with it.  Now the code makes no sense.
>>>
>>>
 Modified: head/sys/ufs/ffs/ffs_vfsops.c

 ==
 --- head/sys/ufs/ffs/ffs_vfsops.c   Mon Nov 19 21:58:14 2012
 (r243310)
 +++ head/sys/ufs/ffs/ffs_vfsops.c   Mon Nov 19 22:43:45 2012
 (r243311)
 @@ -1676,14 +1676,6 @@ ffs_vgetf(mp, ino, flags, vpp, ffs_flags
 ump = VFSTOUFS(mp);
 dev = ump->um_dev;
 fs = ump->um_fs;
 -
 -   /*
 -* If this malloc() is performed after the getnewvnode()
>>>
>>>
>>> This malloc() didn't match the code, which uses uma_zalloc().  Old
>>> versions used MALLOC() in both the comment and the code.  ffs's comment
>>> was updated to say malloc() when the code was changed to use malloc(),
>>> then rotted when the code was changed to use uma_zalloc().  In some
>>> other file systems, the comment still said MALLOC().
>>>
>>>
 -* it might block, leaving a vnode with a NULL v_data to be
 -* found by ffs_sync() if a sync happens to fire right then,
 -* which will cause a panic because ffs_sync() blindly
 -* dereferences vp->v_data (as well it should).
 -*/
 ip = uma_zalloc(uma_inode, M_WAITOK | M_ZERO);

 /* Allocate a new vnode/inode. */

>>>
>>> The code makes no sense now.  The comment explains why ip is allocated
>>> before vp, instead of in the natural, opposite order like it used to
>>> be.  Allocating things in an unnatural  order requires extra code to
>>> free ip when the allocation of vp fails.
>>
>> "Used to be" is very arguably. The code has been like its current form
>> many more years than the opposite (16 against 3 I think).
>> And the code makes perfectly sense if you know the history. So I don't
>> agree with you.
>
> But it shouldn't be necessary to know the history of the code to
> understand it.  The code only makes sense if its comment is not removed,
> or if you know the history of the code so that you can restore the
> removed comment.  However, if the comment makes no sense as you claim,
> then the code that it it describes makes no sense.

The "code that makes no sense" is basically the justification to have
the allocation before the getnewvnode(). It makes no sense because the
order makes no sense (you can allocate before or after getnewvnode(),
you won't have v_data corruption as the comment claims).

Hence the code makes no sense.

I don't understand what is the point you are trying to make honestly.

Attilio


-- 
Peace can only be achieved by understanding - A. Einstein
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


Re: svn commit: r243311 - in head/sys: fs/ext2fs fs/msdosfs fs/nfsclient fs/nullfs fs/unionfs gnu/fs/reiserfs nfsclient ufs/ffs

2012-11-20 Thread Attilio Rao
On 11/20/12, Attilio Rao  wrote:
> On 11/20/12, Bruce Evans  wrote:
>> On Tue, 20 Nov 2012, Attilio Rao wrote:
>>
>>> On Tue, Nov 20, 2012 at 6:20 AM, Bruce Evans 
>>> wrote:
 On Mon, 19 Nov 2012, Attilio Rao wrote:

> Log:
>  r16312 is not any longer real since many years (likely since when VFS
>  received granular locking) but the comment present in UFS has been
>  copied all over other filesystems code incorrectly for several times.
>
>  Removes comments that makes no sense now.


 It still made sense (except for bitrot in the function name), but might
 not
 be true).  The code made sense with it.  Now the code makes no sense.


> Modified: head/sys/ufs/ffs/ffs_vfsops.c
>
> ==
> --- head/sys/ufs/ffs/ffs_vfsops.c   Mon Nov 19 21:58:14 2012
> (r243310)
> +++ head/sys/ufs/ffs/ffs_vfsops.c   Mon Nov 19 22:43:45 2012
> (r243311)
> @@ -1676,14 +1676,6 @@ ffs_vgetf(mp, ino, flags, vpp, ffs_flags
> ump = VFSTOUFS(mp);
> dev = ump->um_dev;
> fs = ump->um_fs;
> -
> -   /*
> -* If this malloc() is performed after the getnewvnode()


 This malloc() didn't match the code, which uses uma_zalloc().  Old
 versions used MALLOC() in both the comment and the code.  ffs's comment
 was updated to say malloc() when the code was changed to use malloc(),
 then rotted when the code was changed to use uma_zalloc().  In some
 other file systems, the comment still said MALLOC().


> -* it might block, leaving a vnode with a NULL v_data to be
> -* found by ffs_sync() if a sync happens to fire right then,
> -* which will cause a panic because ffs_sync() blindly
> -* dereferences vp->v_data (as well it should).
> -*/
> ip = uma_zalloc(uma_inode, M_WAITOK | M_ZERO);
>
> /* Allocate a new vnode/inode. */
>

 The code makes no sense now.  The comment explains why ip is allocated
 before vp, instead of in the natural, opposite order like it used to
 be.  Allocating things in an unnatural  order requires extra code to
 free ip when the allocation of vp fails.
>>>
>>> "Used to be" is very arguably. The code has been like its current form
>>> many more years than the opposite (16 against 3 I think).
>>> And the code makes perfectly sense if you know the history. So I don't
>>> agree with you.
>>
>> But it shouldn't be necessary to know the history of the code to
>> understand it.  The code only makes sense if its comment is not removed,
>> or if you know the history of the code so that you can restore the
>> removed comment.  However, if the comment makes no sense as you claim,
>> then the code that it it describes makes no sense.
>
> The "code that makes no sense" is basically the justification to have
> the allocation before the getnewvnode(). It makes no sense because the
> order makes no sense (you can allocate before or after getnewvnode(),
> you won't have v_data corruption as the comment claims).
>
> Hence the code makes no sense.

Herm, s/code/comment.

Attilio


-- 
Peace can only be achieved by understanding - A. Einstein
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


Re: svn commit: r243328 - head/lib/libutil

2012-11-20 Thread Jaakko Heinonen

Hi!

On 2012-11-20, Baptiste Daroussin wrote:
>   change mode the group file to 0644 after a successfull rename(2)
> 
>  int
>  gr_mkdb(void)
>  {
> - return (rename(tempname, group_file));
> + int ret;
> +
> + ret = rename(tempname, group_file);
> +
> + if (ret == 0)
> + chmod(group_file, 0644);
> +
> + return (ret);
>  }

Rename+chmod is not an atomic operation. There is a window when the file
has wrong permissions. Also, you don't check the return value of
chmod(). Maybe chmod first and then rename?

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


svn commit: r243333 - in head/sys: dev/fdc geom geom/bde geom/cache geom/label geom/mountver geom/multipath geom/nop geom/sched vm

2012-11-20 Thread Jaakko Heinonen
Author: jh
Date: Tue Nov 20 12:32:18 2012
New Revision: 24
URL: http://svnweb.freebsd.org/changeset/base/24

Log:
  - Don't pass geom and provider names as format strings.
  - Add __printflike() attributes.
  - Remove an extra argument for the g_new_geomf() call in swapongeom_ev().
  
  Reviewed by:  pjd

Modified:
  head/sys/dev/fdc/fdc.c
  head/sys/geom/bde/g_bde.c
  head/sys/geom/cache/g_cache.c
  head/sys/geom/geom.h
  head/sys/geom/geom_aes.c
  head/sys/geom/geom_dev.c
  head/sys/geom/geom_mbr.c
  head/sys/geom/geom_slice.c
  head/sys/geom/geom_slice.h
  head/sys/geom/label/g_label.c
  head/sys/geom/mountver/g_mountver.c
  head/sys/geom/multipath/g_multipath.c
  head/sys/geom/nop/g_nop.c
  head/sys/geom/sched/g_sched.c
  head/sys/vm/swap_pager.c

Modified: head/sys/dev/fdc/fdc.c
==
--- head/sys/dev/fdc/fdc.c  Tue Nov 20 12:21:02 2012(r243332)
+++ head/sys/dev/fdc/fdc.c  Tue Nov 20 12:32:18 2012(r24)
@@ -865,7 +865,7 @@ fdc_worker(struct fdc_data *fdc)
g_orphan_provider(fd->fd_provider, ENXIO);
fd->fd_provider->flags |= G_PF_WITHER;
fd->fd_provider =
-   g_new_providerf(fd->fd_geom, fd->fd_geom->name);
+   g_new_providerf(fd->fd_geom, "%s", fd->fd_geom->name);
g_error_provider(fd->fd_provider, 0);
g_topology_unlock();
return (fdc_biodone(fdc, ENXIO));
@@ -2011,7 +2011,7 @@ fd_attach2(void *arg, int flag)
 
fd->fd_geom = g_new_geomf(&g_fd_class,
"fd%d", device_get_unit(fd->dev));
-   fd->fd_provider = g_new_providerf(fd->fd_geom, fd->fd_geom->name);
+   fd->fd_provider = g_new_providerf(fd->fd_geom, "%s", fd->fd_geom->name);
fd->fd_geom->softc = fd;
g_error_provider(fd->fd_provider, 0);
 }

Modified: head/sys/geom/bde/g_bde.c
==
--- head/sys/geom/bde/g_bde.c   Tue Nov 20 12:21:02 2012(r243332)
+++ head/sys/geom/bde/g_bde.c   Tue Nov 20 12:32:18 2012(r24)
@@ -184,7 +184,7 @@ g_bde_create_geom(struct gctl_req *req, 
/* XXX: error check */
kproc_create(g_bde_worker, gp, &sc->thread, 0, 0,
"g_bde %s", gp->name);
-   pp = g_new_providerf(gp, gp->name);
+   pp = g_new_providerf(gp, "%s", gp->name);
pp->stripesize = kp->zone_cont;
pp->stripeoffset = 0;
pp->mediasize = sc->mediasize;

Modified: head/sys/geom/cache/g_cache.c
==
--- head/sys/geom/cache/g_cache.c   Tue Nov 20 12:21:02 2012
(r243332)
+++ head/sys/geom/cache/g_cache.c   Tue Nov 20 12:32:18 2012
(r24)
@@ -502,7 +502,7 @@ g_cache_create(struct g_class *mp, struc
return (NULL);
}
 
-   gp = g_new_geomf(mp, md->md_name);
+   gp = g_new_geomf(mp, "%s", md->md_name);
sc = g_malloc(sizeof(*sc), M_WAITOK | M_ZERO);
sc->sc_type = type;
sc->sc_bshift = bshift;

Modified: head/sys/geom/geom.h
==
--- head/sys/geom/geom.hTue Nov 20 12:21:02 2012(r243332)
+++ head/sys/geom/geom.hTue Nov 20 12:32:18 2012(r24)
@@ -271,8 +271,10 @@ int g_handleattr_int(struct bio *bp, con
 int g_handleattr_off_t(struct bio *bp, const char *attribute, off_t val);
 int g_handleattr_str(struct bio *bp, const char *attribute, const char *str);
 struct g_consumer * g_new_consumer(struct g_geom *gp);
-struct g_geom * g_new_geomf(struct g_class *mp, const char *fmt, ...);
-struct g_provider * g_new_providerf(struct g_geom *gp, const char *fmt, ...);
+struct g_geom * g_new_geomf(struct g_class *mp, const char *fmt, ...)
+__printflike(2, 3);
+struct g_provider * g_new_providerf(struct g_geom *gp, const char *fmt, ...)
+__printflike(2, 3);
 void g_resize_provider(struct g_provider *pp, off_t size);
 int g_retaste(struct g_class *mp);
 void g_spoil(struct g_provider *pp, struct g_consumer *cp);

Modified: head/sys/geom/geom_aes.c
==
--- head/sys/geom/geom_aes.cTue Nov 20 12:21:02 2012(r243332)
+++ head/sys/geom/geom_aes.cTue Nov 20 12:32:18 2012(r24)
@@ -342,7 +342,7 @@ g_aes_taste(struct g_class *mp, struct g
}
}
g_topology_lock();
-   pp = g_new_providerf(gp, gp->name);
+   pp = g_new_providerf(gp, "%s", gp->name);
pp->mediasize = mediasize - sectorsize;
pp->sectorsize = sectorsize;
g_error_provider(pp, 0);

Modified: head/sys/geom/geom_dev.c
=

Re: svn commit: r243328 - head/lib/libutil

2012-11-20 Thread Baptiste Daroussin
On Tue, Nov 20, 2012 at 02:22:26PM +0200, Jaakko Heinonen wrote:
> 
> Hi!
> 
> On 2012-11-20, Baptiste Daroussin wrote:
> >   change mode the group file to 0644 after a successfull rename(2)
> > 
> >  int
> >  gr_mkdb(void)
> >  {
> > -   return (rename(tempname, group_file));
> > +   int ret;
> > +
> > +   ret = rename(tempname, group_file);
> > +
> > +   if (ret == 0)
> > +   chmod(group_file, 0644);
> > +
> > +   return (ret);
> >  }
> 
> Rename+chmod is not an atomic operation. There is a window when the file
> has wrong permissions. Also, you don't check the return value of
> chmod(). Maybe chmod first and then rename?
> 
> -- 
> Jaakko

Yes you are right chmod should probably be first.

regards,
Bapt


pgp2j1SIOZgIB.pgp
Description: PGP signature


Re: svn commit: r243311 - in head/sys: fs/ext2fs fs/msdosfs fs/nfsclient fs/nullfs fs/unionfs gnu/fs/reiserfs nfsclient ufs/ffs

2012-11-20 Thread Bruce Evans

On Tue, 20 Nov 2012, Attilio Rao wrote:


On 11/20/12, Attilio Rao  wrote:

On 11/20/12, Bruce Evans  wrote:

On Tue, 20 Nov 2012, Attilio Rao wrote:


On Tue, Nov 20, 2012 at 6:20 AM, Bruce Evans 
wrote:

On Mon, 19 Nov 2012, Attilio Rao wrote:


Log:
 r16312 is not any longer real since many years (likely since when VFS
 received granular locking) but the comment present in UFS has been
 copied all over other filesystems code incorrectly for several times.

 Removes comments that makes no sense now.



It still made sense (except for bitrot in the function name), but might
not
be true).  The code made sense with it.  Now the code makes no sense.



Modified: head/sys/ufs/ffs/ffs_vfsops.c

==
--- head/sys/ufs/ffs/ffs_vfsops.c   Mon Nov 19 21:58:14 2012
(r243310)
+++ head/sys/ufs/ffs/ffs_vfsops.c   Mon Nov 19 22:43:45 2012
(r243311)
@@ -1676,14 +1676,6 @@ ffs_vgetf(mp, ino, flags, vpp, ffs_flags
ump = VFSTOUFS(mp);
dev = ump->um_dev;
fs = ump->um_fs;
-
-   /*
-* If this malloc() is performed after the getnewvnode()



This malloc() didn't match the code, which uses uma_zalloc().  Old
versions used MALLOC() in both the comment and the code.  ffs's comment
was updated to say malloc() when the code was changed to use malloc(),
then rotted when the code was changed to use uma_zalloc().  In some
other file systems, the comment still said MALLOC().



-* it might block, leaving a vnode with a NULL v_data to be
-* found by ffs_sync() if a sync happens to fire right then,
-* which will cause a panic because ffs_sync() blindly
-* dereferences vp->v_data (as well it should).
-*/
ip = uma_zalloc(uma_inode, M_WAITOK | M_ZERO);

/* Allocate a new vnode/inode. */



The code makes no sense now.  The comment explains why ip is allocated
before vp, instead of in the natural, opposite order like it used to
be.  Allocating things in an unnatural  order requires extra code to
free ip when the allocation of vp fails.


"Used to be" is very arguably. The code has been like its current form
many more years than the opposite (16 against 3 I think).
And the code makes perfectly sense if you know the history. So I don't
agree with you.


But it shouldn't be necessary to know the history of the code to
understand it.  The code only makes sense if its comment is not removed,
or if you know the history of the code so that you can restore the
removed comment.  However, if the comment makes no sense as you claim,
then the code that it it describes makes no sense.


The "code that makes no sense" is basically the justification to have
the allocation before the getnewvnode(). It makes no sense because the
order makes no sense (you can allocate before or after getnewvnode(),
you won't have v_data corruption as the comment claims).

Hence the code makes no sense.


Herm, s/code/comment.


I think you are right that the comment makes no sense.  A preemptible
kernel may be preempted without it calling malloc() where it may block.
Thus ffs_fsync() and anything else that looks at the inode must be
doing something to avoid dereferencing v_data if the vnode is not fully
constructed.  This seems to be done by iterating over vnodes using
MNT_VNODE_FOREACH_ACTIVE*() and not making incomplete vnodes active.
ffs_fsync() still just blindly dereferences the inode.

I think I am right that the code makes no sense.  It is ordered like
it is because placing the allocation of ip before the allocation of
vp used to be enough to prevent v_data being dereferenced.  This makes
no sense when it isn't enough.

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


Re: svn commit: r243311 - in head/sys: fs/ext2fs fs/msdosfs fs/nfsclient fs/nullfs fs/unionfs gnu/fs/reiserfs nfsclient ufs/ffs

2012-11-20 Thread Attilio Rao
On 11/20/12, Bruce Evans  wrote:
> On Tue, 20 Nov 2012, Attilio Rao wrote:
>
>> On 11/20/12, Attilio Rao  wrote:
>>> On 11/20/12, Bruce Evans  wrote:
 On Tue, 20 Nov 2012, Attilio Rao wrote:

> On Tue, Nov 20, 2012 at 6:20 AM, Bruce Evans 
> wrote:
>> On Mon, 19 Nov 2012, Attilio Rao wrote:
>>
>>> Log:
>>>  r16312 is not any longer real since many years (likely since when
>>> VFS
>>>  received granular locking) but the comment present in UFS has been
>>>  copied all over other filesystems code incorrectly for several
>>> times.
>>>
>>>  Removes comments that makes no sense now.
>>
>>
>> It still made sense (except for bitrot in the function name), but
>> might
>> not
>> be true).  The code made sense with it.  Now the code makes no sense.
>>
>>
>>> Modified: head/sys/ufs/ffs/ffs_vfsops.c
>>>
>>> ==
>>> --- head/sys/ufs/ffs/ffs_vfsops.c   Mon Nov 19 21:58:14 2012
>>> (r243310)
>>> +++ head/sys/ufs/ffs/ffs_vfsops.c   Mon Nov 19 22:43:45 2012
>>> (r243311)
>>> @@ -1676,14 +1676,6 @@ ffs_vgetf(mp, ino, flags, vpp, ffs_flags
>>> ump = VFSTOUFS(mp);
>>> dev = ump->um_dev;
>>> fs = ump->um_fs;
>>> -
>>> -   /*
>>> -* If this malloc() is performed after the getnewvnode()
>>
>>
>> This malloc() didn't match the code, which uses uma_zalloc().  Old
>> versions used MALLOC() in both the comment and the code.  ffs's
>> comment
>> was updated to say malloc() when the code was changed to use
>> malloc(),
>> then rotted when the code was changed to use uma_zalloc().  In some
>> other file systems, the comment still said MALLOC().
>>
>>
>>> -* it might block, leaving a vnode with a NULL v_data to be
>>> -* found by ffs_sync() if a sync happens to fire right then,
>>> -* which will cause a panic because ffs_sync() blindly
>>> -* dereferences vp->v_data (as well it should).
>>> -*/
>>> ip = uma_zalloc(uma_inode, M_WAITOK | M_ZERO);
>>>
>>> /* Allocate a new vnode/inode. */
>>>
>>
>> The code makes no sense now.  The comment explains why ip is
>> allocated
>> before vp, instead of in the natural, opposite order like it used to
>> be.  Allocating things in an unnatural  order requires extra code to
>> free ip when the allocation of vp fails.
>
> "Used to be" is very arguably. The code has been like its current form
> many more years than the opposite (16 against 3 I think).
> And the code makes perfectly sense if you know the history. So I don't
> agree with you.

 But it shouldn't be necessary to know the history of the code to
 understand it.  The code only makes sense if its comment is not
 removed,
 or if you know the history of the code so that you can restore the
 removed comment.  However, if the comment makes no sense as you claim,
 then the code that it it describes makes no sense.
>>>
>>> The "code that makes no sense" is basically the justification to have
>>> the allocation before the getnewvnode(). It makes no sense because the
>>> order makes no sense (you can allocate before or after getnewvnode(),
>>> you won't have v_data corruption as the comment claims).
>>>
>>> Hence the code makes no sense.
>>
>> Herm, s/code/comment.
>
> I think you are right that the comment makes no sense.  A preemptible
> kernel may be preempted without it calling malloc() where it may block.
> Thus ffs_fsync() and anything else that looks at the inode must be
> doing something to avoid dereferencing v_data if the vnode is not fully
> constructed.  This seems to be done by iterating over vnodes using
> MNT_VNODE_FOREACH_ACTIVE*() and not making incomplete vnodes active.
> ffs_fsync() still just blindly dereferences the inode.
>
> I think I am right that the code makes no sense.  It is ordered like
> it is because placing the allocation of ip before the allocation of
> vp used to be enough to prevent v_data being dereferenced.  This makes
> no sense when it isn't enough.

In the past, before VFS got locking and kernel was single-threaded,
the comment and code arranged in this way were sensitive and
effective.
As now this is not true anymore, there is no strict relationship
between the getnewvnode() and sleeping points.
It is important to remove stale comments because they confuse people,
the porters and as you can see the code/comment has been cut&paste
quite a bit around.

Attilio


-- 
Peace can only be achieved by understanding - A. Einstein
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


Re: svn commit: r243328 - head/lib/libutil

2012-11-20 Thread Baptiste Daroussin
On Tue, Nov 20, 2012 at 02:22:26PM +0200, Jaakko Heinonen wrote:
> 
> Hi!
> 
> On 2012-11-20, Baptiste Daroussin wrote:
> >   change mode the group file to 0644 after a successfull rename(2)
> > 
> >  int
> >  gr_mkdb(void)
> >  {
> > -   return (rename(tempname, group_file));
> > +   int ret;
> > +
> > +   ret = rename(tempname, group_file);
> > +
> > +   if (ret == 0)
> > +   chmod(group_file, 0644);
> > +
> > +   return (ret);
> >  }
> 
> Rename+chmod is not an atomic operation. There is a window when the file
> has wrong permissions. Also, you don't check the return value of
> chmod(). Maybe chmod first and then rename?
> 
> -- 
> Jaakko

Does this looks better to you?
http://people.freebsd.org/~bapt/gr_util.diff

regards,
Bapt


pgpNUVRv360fH.pgp
Description: PGP signature


Re: svn commit: r243228 - head/etc

2012-11-20 Thread Bruce Evans

On Tue, 20 Nov 2012, John Hay wrote:


On Sun, Nov 18, 2012 at 02:21:05PM +, Chris Rees wrote:

Log:
  cp -R misses out dotfiles; use pax instead to copy file hierarchies

  PR:   conf/99721 (based on)
  Submitted by: Florian Zavatzki 
  Approved by:  hrs
  MFC after:1 month

Modified:
  head/etc/rc.initdiskless

Modified: head/etc/rc.initdiskless
==
--- head/etc/rc.initdisklessSun Nov 18 14:05:28 2012(r243227)
+++ head/etc/rc.initdisklessSun Nov 18 14:21:05 2012(r243228)
@@ -354,7 +354,7 @@ for i in ${templates} ; do
subdir=${j##*/}
if [ -d $j -a ! -f $j.cpio.gz  ]; then
create_md $subdir
-   cp -Rp $j/ /$subdir
+   (cd $j && pax -rw . /$subdir)
fi
 done
 for j in /conf/$i/*.cpio.gz ; do


Have you tested this on a diskless and readonly system? It looks like pax
need to write something in /tmp and it might not be writeable yet. I got
an error, after the first of /bin/pax not found and having to add that to
the list of files needed.


It uses mkstemp(3), normally in /tmp but it honors $TMPDIR.  It seems to
always create 1 temporary file (even for copying a single regular file),
and sometimes 2 temporary files.  Both of the temporary files seem to be
to hold metadata for file times and hashes, in case it is too large for
memory.  cp -Rp probably needs to do the same (except it is imperfect to
unnecessarily assume that /tmp is writable), to fix its link and timestamp
handling.

BTW, I think it is a large bug that ed and vi create temporary files even
before you change anything.  Even view(1) (vi -R) wants to scribble on
/var/tmp/vi.recover.  At least it doesn't refuse to start if this is not
writeable.  ed(1) is considerably more broken.  It
- hard codes /tmp and doesn't use _PATH_TMP or honor $TMPDIR
- always scribbles in /tmp
- refuses to start if /tmp is not writeable.
This makes ed(1) wlays broken in single user shells until '/' is mounted
rw, although ed is the only editor that is sure to be there and the
reason for using a single user shell is often that there is a problem
with mounting '/' rw.

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


Re: svn commit: r243328 - head/lib/libutil

2012-11-20 Thread Jaakko Heinonen
On 2012-11-20, Baptiste Daroussin wrote:
> Does this looks better to you?
> http://people.freebsd.org/~bapt/gr_util.diff

Yes. Thank you.

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


svn commit: r243334 - head/lib/libutil

2012-11-20 Thread Baptiste Daroussin
Author: bapt
Date: Tue Nov 20 14:03:09 2012
New Revision: 243334
URL: http://svnweb.freebsd.org/changeset/base/243334

Log:
  only rename(2) after chmod(2) has succeed
  report error if chmod(2) fails
  
  Reported by:  jh

Modified:
  head/lib/libutil/gr_util.c

Modified: head/lib/libutil/gr_util.c
==
--- head/lib/libutil/gr_util.c  Tue Nov 20 12:32:18 2012(r24)
+++ head/lib/libutil/gr_util.c  Tue Nov 20 14:03:09 2012(r243334)
@@ -318,14 +318,10 @@ gr_copy(int ffd, int tfd, const struct g
 int
 gr_mkdb(void)
 {
-   int ret;
+   if (chmod(tempname, 0644) != 0)
+   return (-1);
 
-   ret = rename(tempname, group_file);
-
-   if (ret == 0)
-   chmod(group_file, 0644);
-
-   return (ret);
+   return (rename(tempname, group_file));
 }
 
 /*
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


svn commit: r243335 - head/usr.sbin/pw

2012-11-20 Thread Baptiste Daroussin
Author: bapt
Date: Tue Nov 20 14:05:46 2012
New Revision: 243335
URL: http://svnweb.freebsd.org/changeset/base/243335

Log:
  In NIS mode first chmod(2) the temporary file  and is succeed then rename(2)

Modified:
  head/usr.sbin/pw/pw_nis.c

Modified: head/usr.sbin/pw/pw_nis.c
==
--- head/usr.sbin/pw/pw_nis.c   Tue Nov 20 14:03:09 2012(r243334)
+++ head/usr.sbin/pw/pw_nis.c   Tue Nov 20 14:05:46 2012(r243335)
@@ -66,10 +66,10 @@ pw_nisupdate(const char * path, struct p
pw_fini();
err(1, "pw_copy()");
}
+   if (chmod(pw_tempname(), 0644) == -1)
+   err(1, "chmod()");
if (rename(pw_tempname(), path) == -1)
err(1, "rename()");
-   if (chmod(path, 0644) == -1)
-   err(1, "chmod()");
 
free(pw);
pw_fini();
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


svn commit: r243336 - head/sys/netinet6

2012-11-20 Thread Andrey V. Elsukov
Author: ae
Date: Tue Nov 20 14:09:37 2012
New Revision: 243336
URL: http://svnweb.freebsd.org/changeset/base/243336

Log:
  Remove opt_inet.h, it isn't required here.
  
  MFC after:1 week

Modified:
  head/sys/netinet6/ip6_mroute.c

Modified: head/sys/netinet6/ip6_mroute.c
==
--- head/sys/netinet6/ip6_mroute.c  Tue Nov 20 14:05:46 2012
(r243335)
+++ head/sys/netinet6/ip6_mroute.c  Tue Nov 20 14:09:37 2012
(r243336)
@@ -81,7 +81,6 @@
 #include 
 __FBSDID("$FreeBSD$");
 
-#include "opt_inet.h"
 #include "opt_inet6.h"
 
 #include 
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


svn commit: r243337 - head/sys/modules

2012-11-20 Thread Andrey V. Elsukov
Author: ae
Date: Tue Nov 20 14:11:27 2012
New Revision: 243337
URL: http://svnweb.freebsd.org/changeset/base/243337

Log:
  Connect ip6_mroute kernel module to the build.
  
  MFC after:1 week

Modified:
  head/sys/modules/Makefile

Modified: head/sys/modules/Makefile
==
--- head/sys/modules/Makefile   Tue Nov 20 14:09:37 2012(r243336)
+++ head/sys/modules/Makefile   Tue Nov 20 14:11:27 2012(r243337)
@@ -150,6 +150,7 @@ SUBDIR= \
${_ipfw} \
ipfw_nat \
${_ipmi} \
+   ip6_mroute_mod \
ip_mroute_mod \
${_ips} \
${_ipw} \
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


Re: svn commit: r243311 - in head/sys: fs/ext2fs fs/msdosfs fs/nfsclient fs/nullfs fs/unionfs gnu/fs/reiserfs nfsclient ufs/ffs

2012-11-20 Thread Konstantin Belousov
On Tue, Nov 20, 2012 at 01:27:07PM +, Attilio Rao wrote:
> On 11/20/12, Bruce Evans  wrote:
> > On Tue, 20 Nov 2012, Attilio Rao wrote:
> >
> >> On 11/20/12, Attilio Rao  wrote:
> >>> On 11/20/12, Bruce Evans  wrote:
>  On Tue, 20 Nov 2012, Attilio Rao wrote:
> 
> > On Tue, Nov 20, 2012 at 6:20 AM, Bruce Evans 
> > wrote:
> >> On Mon, 19 Nov 2012, Attilio Rao wrote:
> >>
> >>> Log:
> >>>  r16312 is not any longer real since many years (likely since when
> >>> VFS
> >>>  received granular locking) but the comment present in UFS has been
> >>>  copied all over other filesystems code incorrectly for several
> >>> times.
> >>>
> >>>  Removes comments that makes no sense now.
> >>
> >>
> >> It still made sense (except for bitrot in the function name), but
> >> might
> >> not
> >> be true).  The code made sense with it.  Now the code makes no sense.
> >>
> >>
> >>> Modified: head/sys/ufs/ffs/ffs_vfsops.c
> >>>
> >>> ==
> >>> --- head/sys/ufs/ffs/ffs_vfsops.c   Mon Nov 19 21:58:14 2012
> >>> (r243310)
> >>> +++ head/sys/ufs/ffs/ffs_vfsops.c   Mon Nov 19 22:43:45 2012
> >>> (r243311)
> >>> @@ -1676,14 +1676,6 @@ ffs_vgetf(mp, ino, flags, vpp, ffs_flags
> >>> ump = VFSTOUFS(mp);
> >>> dev = ump->um_dev;
> >>> fs = ump->um_fs;
> >>> -
> >>> -   /*
> >>> -* If this malloc() is performed after the getnewvnode()
> >>
> >>
> >> This malloc() didn't match the code, which uses uma_zalloc().  Old
> >> versions used MALLOC() in both the comment and the code.  ffs's
> >> comment
> >> was updated to say malloc() when the code was changed to use
> >> malloc(),
> >> then rotted when the code was changed to use uma_zalloc().  In some
> >> other file systems, the comment still said MALLOC().
> >>
> >>
> >>> -* it might block, leaving a vnode with a NULL v_data to be
> >>> -* found by ffs_sync() if a sync happens to fire right then,
> >>> -* which will cause a panic because ffs_sync() blindly
> >>> -* dereferences vp->v_data (as well it should).
> >>> -*/
> >>> ip = uma_zalloc(uma_inode, M_WAITOK | M_ZERO);
> >>>
> >>> /* Allocate a new vnode/inode. */
> >>>
> >>
> >> The code makes no sense now.  The comment explains why ip is
> >> allocated
> >> before vp, instead of in the natural, opposite order like it used to
> >> be.  Allocating things in an unnatural  order requires extra code to
> >> free ip when the allocation of vp fails.
> >
> > "Used to be" is very arguably. The code has been like its current form
> > many more years than the opposite (16 against 3 I think).
> > And the code makes perfectly sense if you know the history. So I don't
> > agree with you.
> 
>  But it shouldn't be necessary to know the history of the code to
>  understand it.  The code only makes sense if its comment is not
>  removed,
>  or if you know the history of the code so that you can restore the
>  removed comment.  However, if the comment makes no sense as you claim,
>  then the code that it it describes makes no sense.
> >>>
> >>> The "code that makes no sense" is basically the justification to have
> >>> the allocation before the getnewvnode(). It makes no sense because the
> >>> order makes no sense (you can allocate before or after getnewvnode(),
> >>> you won't have v_data corruption as the comment claims).
> >>>
> >>> Hence the code makes no sense.
> >>
> >> Herm, s/code/comment.
> >
> > I think you are right that the comment makes no sense.  A preemptible
> > kernel may be preempted without it calling malloc() where it may block.
> > Thus ffs_fsync() and anything else that looks at the inode must be
> > doing something to avoid dereferencing v_data if the vnode is not fully
> > constructed.  This seems to be done by iterating over vnodes using
> > MNT_VNODE_FOREACH_ACTIVE*() and not making incomplete vnodes active.
> > ffs_fsync() still just blindly dereferences the inode.
> >
> > I think I am right that the code makes no sense.  It is ordered like
> > it is because placing the allocation of ip before the allocation of
> > vp used to be enough to prevent v_data being dereferenced.  This makes
> > no sense when it isn't enough.
> 
> In the past, before VFS got locking and kernel was single-threaded,
> the comment and code arranged in this way were sensitive and
> effective.
> As now this is not true anymore, there is no strict relationship
> between the getnewvnode() and sleeping points.
> It is important to remove stale comments because they confuse people,
> the porters and as you can see the code/comment has been cut&paste
> quite a bit around.
> 

Putting the issue of 

Re: svn commit: r243311 - in head/sys: fs/ext2fs fs/msdosfs fs/nfsclient fs/nullfs fs/unionfs gnu/fs/reiserfs nfsclient ufs/ffs

2012-11-20 Thread Attilio Rao
On 11/20/12, Konstantin Belousov  wrote:
> On Tue, Nov 20, 2012 at 01:27:07PM +, Attilio Rao wrote:
>> On 11/20/12, Bruce Evans  wrote:
>> > On Tue, 20 Nov 2012, Attilio Rao wrote:
>> >
>> >> On 11/20/12, Attilio Rao  wrote:
>> >>> On 11/20/12, Bruce Evans  wrote:
>>  On Tue, 20 Nov 2012, Attilio Rao wrote:
>> 
>> > On Tue, Nov 20, 2012 at 6:20 AM, Bruce Evans 
>> > wrote:
>> >> On Mon, 19 Nov 2012, Attilio Rao wrote:
>> >>
>> >>> Log:
>> >>>  r16312 is not any longer real since many years (likely since
>> >>> when
>> >>> VFS
>> >>>  received granular locking) but the comment present in UFS has
>> >>> been
>> >>>  copied all over other filesystems code incorrectly for several
>> >>> times.
>> >>>
>> >>>  Removes comments that makes no sense now.
>> >>
>> >>
>> >> It still made sense (except for bitrot in the function name), but
>> >> might
>> >> not
>> >> be true).  The code made sense with it.  Now the code makes no
>> >> sense.
>> >>
>> >>
>> >>> Modified: head/sys/ufs/ffs/ffs_vfsops.c
>> >>>
>> >>> ==
>> >>> --- head/sys/ufs/ffs/ffs_vfsops.c   Mon Nov 19 21:58:14 2012
>> >>> (r243310)
>> >>> +++ head/sys/ufs/ffs/ffs_vfsops.c   Mon Nov 19 22:43:45 2012
>> >>> (r243311)
>> >>> @@ -1676,14 +1676,6 @@ ffs_vgetf(mp, ino, flags, vpp, ffs_flags
>> >>> ump = VFSTOUFS(mp);
>> >>> dev = ump->um_dev;
>> >>> fs = ump->um_fs;
>> >>> -
>> >>> -   /*
>> >>> -* If this malloc() is performed after the getnewvnode()
>> >>
>> >>
>> >> This malloc() didn't match the code, which uses uma_zalloc().  Old
>> >> versions used MALLOC() in both the comment and the code.  ffs's
>> >> comment
>> >> was updated to say malloc() when the code was changed to use
>> >> malloc(),
>> >> then rotted when the code was changed to use uma_zalloc().  In
>> >> some
>> >> other file systems, the comment still said MALLOC().
>> >>
>> >>
>> >>> -* it might block, leaving a vnode with a NULL v_data to
>> >>> be
>> >>> -* found by ffs_sync() if a sync happens to fire right
>> >>> then,
>> >>> -* which will cause a panic because ffs_sync() blindly
>> >>> -* dereferences vp->v_data (as well it should).
>> >>> -*/
>> >>> ip = uma_zalloc(uma_inode, M_WAITOK | M_ZERO);
>> >>>
>> >>> /* Allocate a new vnode/inode. */
>> >>>
>> >>
>> >> The code makes no sense now.  The comment explains why ip is
>> >> allocated
>> >> before vp, instead of in the natural, opposite order like it used
>> >> to
>> >> be.  Allocating things in an unnatural  order requires extra code
>> >> to
>> >> free ip when the allocation of vp fails.
>> >
>> > "Used to be" is very arguably. The code has been like its current
>> > form
>> > many more years than the opposite (16 against 3 I think).
>> > And the code makes perfectly sense if you know the history. So I
>> > don't
>> > agree with you.
>> 
>>  But it shouldn't be necessary to know the history of the code to
>>  understand it.  The code only makes sense if its comment is not
>>  removed,
>>  or if you know the history of the code so that you can restore the
>>  removed comment.  However, if the comment makes no sense as you
>>  claim,
>>  then the code that it it describes makes no sense.
>> >>>
>> >>> The "code that makes no sense" is basically the justification to have
>> >>> the allocation before the getnewvnode(). It makes no sense because
>> >>> the
>> >>> order makes no sense (you can allocate before or after getnewvnode(),
>> >>> you won't have v_data corruption as the comment claims).
>> >>>
>> >>> Hence the code makes no sense.
>> >>
>> >> Herm, s/code/comment.
>> >
>> > I think you are right that the comment makes no sense.  A preemptible
>> > kernel may be preempted without it calling malloc() where it may block.
>> > Thus ffs_fsync() and anything else that looks at the inode must be
>> > doing something to avoid dereferencing v_data if the vnode is not fully
>> > constructed.  This seems to be done by iterating over vnodes using
>> > MNT_VNODE_FOREACH_ACTIVE*() and not making incomplete vnodes active.
>> > ffs_fsync() still just blindly dereferences the inode.
>> >
>> > I think I am right that the code makes no sense.  It is ordered like
>> > it is because placing the allocation of ip before the allocation of
>> > vp used to be enough to prevent v_data being dereferenced.  This makes
>> > no sense when it isn't enough.
>>
>> In the past, before VFS got locking and kernel was single-threaded,
>> the comment and code arranged in this way were sensitive and
>> effective.
>> As now this is not true anymore, ther

Re: svn commit: r242847 - in head/sys: i386/include kern

2012-11-20 Thread Andrey Zonov
On 11/18/12 6:13 PM, Andre Oppermann wrote:
> On 18.11.2012 15:05, Andrey Zonov wrote:
>> On 11/11/12 3:04 AM, Andre Oppermann wrote:
>>> On 10.11.2012 23:24, Alfred Perlstein wrote:
 On 11/10/12 11:18 AM, Andre Oppermann wrote:
> On 10.11.2012 19:04, Peter Wemm wrote:
>> This is complicated but we need a simple user visible view of it.  It
>> really needs to be something like "nmbclusters defaults to 6% of
>> physical ram, with machine dependent limits".  The MD limits are bad
>> enough, and using bogo-units like "maxusers" just makes it worse.
>
> Yes, that would be optimal.
>
 No it would not.

 I used to be able to tell people "hey just try increasing maxusers"
 and they would and suddenly the
 box would be OK.

 Now I'll have to remember 3,4,5,10,20x tunable to increase?
>>>
>>> No.  The whole mbuf and cluster stuff isn't allocated or reserved
>>> at boot time.  We simply need a limit to prevent it from exhausting
>>> all available kvm / physical memory whichever is less.
>>>
>>
>> For now, we have limit which does not allow to run even one igb(4) NIC
>> in 9k jumbo configuration.
> 
> My patch for mbuf* zone auto-sizing does fix that, or not?
> 

Are you talking about r242910?  I have not tried it.

-- 
Andrey Zonov



signature.asc
Description: OpenPGP digital signature


Re: svn commit: r243311 - in head/sys: fs/ext2fs fs/msdosfs fs/nfsclient fs/nullfs fs/unionfs gnu/fs/reiserfs nfsclient ufs/ffs

2012-11-20 Thread Bruce Evans

On Tue, 20 Nov 2012, Attilio Rao wrote:


On 11/20/12, Konstantin Belousov  wrote:

On Tue, Nov 20, 2012 at 01:27:07PM +, Attilio Rao wrote:

On 11/20/12, Bruce Evans  wrote:

...
I think I am right that the code makes no sense.  It is ordered like
it is because placing the allocation of ip before the allocation of
vp used to be enough to prevent v_data being dereferenced.  This makes
no sense when it isn't enough.


In the past, before VFS got locking and kernel was single-threaded,
the comment and code arranged in this way were sensitive and
effective.
As now this is not true anymore, there is no strict relationship
between the getnewvnode() and sleeping points.
It is important to remove stale comments because they confuse people,
the porters and as you can see the code/comment has been cut&paste
quite a bit around.


Putting the issue of the comment making no sense at all aside, which is
true but tangential to what I want to note.

Although malloc(9) is not ordered with the vnode locks in the lock
order, it is still good practice to not perform allocations under the
vnode lock. The reason is that pagedaemon might need to clean and write
pages, in particular, belonging to the vnodes. Generally, if you own
vnode lock and do something which might kick pagedaemon, you are creating
troubles for pagedaemon, which would attempt to lock the vnode with
timeout, spending the precious time waiting for lock it cannot obtain.

This is less an issue for the new vnode instantiation locations, because
vnode cannot have resident pages yet. But it is good to follow the same
pattern anywhere.


The comment could have been changed to one about this.  Except it doesn't
really apply here.  Neither of the allocations is onder the vnode lock.


Yes, I completely agree. Infact the code is not changed also to avoid
pagedaemon hosing.
However this is a different situation respect a "if you allocate while
you hold the lock you will get a deadlock/race/corruption".


The code is still nonsense since its order is unrelated to this too :-).

The natural code and order is:

o // no lock held here
o allocate vp
o allocate ip and any other stuff needed to complete vp
o aquire exclusive lock
o check that vp, ip and other stuff didn't go away
o wire ip and other stuff into vp // shouldn't do more allocations here
o release lock on return (?)

but the current order for at least ffs is:

o // no lock held here
o // oops, actually we always (?) hold at least a shared lock here.  Often
  // we need to promote to an exclusive lock.
o allocate ip
o allocate vp
o aquire exclusive lock
   // no check that nothing went away??  A comment earlier still says that
   // we intentionally allow races earlier.  Is that correct when we hold
   // at leasr a shared lock throughout?
o wire ip into vp.  Includes futher initializations of both.  Hopefully
   these can't block.
bread() the inode.  Can block now.  No worse than blocking for write(2) (?).
o further zalloc()s for dinodes.  Breaks kib's rule.  It would be painful
  to have to release all the zalloc()ated data on earlier failures.  At
  this point, we can still fail, but have initialized enough of the vnode
  to just fail without cleaning it up -- ufs_reclaim() will clean it.
o further initializations, mostly not involving operations that can block.
release lock on return (?)

The allocation of ip is clearly special -- it contains state info that
hopefully records the state of the initialization, and it must be wired into
vp first so that desrtructors can see this info.

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


svn commit: r243339 - head/sys/modules/ufs

2012-11-20 Thread Konstantin Belousov
Author: kib
Date: Tue Nov 20 15:23:22 2012
New Revision: 243339
URL: http://svnweb.freebsd.org/changeset/base/243339

Log:
  Fix module build after r243245.

Modified:
  head/sys/modules/ufs/Makefile

Modified: head/sys/modules/ufs/Makefile
==
--- head/sys/modules/ufs/Makefile   Tue Nov 20 15:19:55 2012
(r243338)
+++ head/sys/modules/ufs/Makefile   Tue Nov 20 15:23:22 2012
(r243339)
@@ -7,7 +7,8 @@ SRCS=   opt_ddb.h opt_directio.h opt_ffs.h
vnode_if.h ufs_acl.c ufs_bmap.c ufs_dirhash.c ufs_extattr.c \
ufs_gjournal.c ufs_inode.c ufs_lookup.c ufs_quota.c ufs_vfsops.c \
ufs_vnops.c ffs_alloc.c ffs_balloc.c ffs_inode.c ffs_snapshot.c \
-   ffs_softdep.c ffs_subr.c ffs_tables.c ffs_vfsops.c ffs_vnops.c
+   ffs_softdep.c ffs_subr.c ffs_suspend.c ffs_tables.c ffs_vfsops.c \
+   ffs_vnops.c
 
 .if !defined(KERNBUILDDIR)
 CFLAGS+= -DSOFTUPDATES -DUFS_DIRHASH
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


svn commit: r243340 - head/sys/fs/nullfs

2012-11-20 Thread Konstantin Belousov
Author: kib
Date: Tue Nov 20 15:25:00 2012
New Revision: 243340
URL: http://svnweb.freebsd.org/changeset/base/243340

Log:
  Remove the check and panic for an impossible condition. The NULL
  lowervp vnode v_vnlock would cause panic due to NULL pointer
  dereference much earlier.
  
  MFC after:1 week

Modified:
  head/sys/fs/nullfs/null_subr.c

Modified: head/sys/fs/nullfs/null_subr.c
==
--- head/sys/fs/nullfs/null_subr.c  Tue Nov 20 15:23:22 2012
(r243339)
+++ head/sys/fs/nullfs/null_subr.c  Tue Nov 20 15:25:00 2012
(r243340)
@@ -251,8 +251,6 @@ null_nodeget(mp, lowervp, vpp)
vp->v_type = lowervp->v_type;
vp->v_data = xp;
vp->v_vnlock = lowervp->v_vnlock;
-   if (vp->v_vnlock == NULL)
-   panic("null_nodeget: Passed a NULL vnlock.\n");
error = insmntque1(vp, mp, null_insmntque_dtr, xp);
if (error != 0)
return (error);
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


svn commit: r243341 - head/sys/kern

2012-11-20 Thread Konstantin Belousov
Author: kib
Date: Tue Nov 20 15:33:48 2012
New Revision: 243341
URL: http://svnweb.freebsd.org/changeset/base/243341

Log:
  Add a special meaning to the negative ticks argument for
  taskqueue_enqueue_timeout().  Do not rearm the callout if it is
  already armed and the ticks is negative.  Otherwise rearm it to fire
  in abs(ticks) ticks in the future.
  
  The intended use is to call taskqueue_enqueue_timeout() for the given
  timeout_task with the same negative ticks argument.  As result, the
  task is scheduled to execute not further than abs(ticks) ticks in
  future, and the consequent enqueues are coalesced until the already
  scheduled task is finished.
  
  Reviewed by:  rwatson
  Tested by:Markus Gebert 
  MFC after:2 weeks

Modified:
  head/sys/kern/subr_taskqueue.c

Modified: head/sys/kern/subr_taskqueue.c
==
--- head/sys/kern/subr_taskqueue.c  Tue Nov 20 15:25:00 2012
(r243340)
+++ head/sys/kern/subr_taskqueue.c  Tue Nov 20 15:33:48 2012
(r243341)
@@ -252,9 +252,13 @@ taskqueue_enqueue_timeout(struct taskque
} else {
queue->tq_callouts++;
timeout_task->f |= DT_CALLOUT_ARMED;
+   if (ticks < 0)
+   ticks = -ticks; /* Ignore overflow. */
+   }
+   if (ticks > 0) {
+   callout_reset(&timeout_task->c, ticks,
+   taskqueue_timeout_func, timeout_task);
}
-   callout_reset(&timeout_task->c, ticks, taskqueue_timeout_func,
-   timeout_task);
}
TQ_UNLOCK(queue);
return (res);
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


svn commit: r243342 - head/sys/kern

2012-11-20 Thread Konstantin Belousov
Author: kib
Date: Tue Nov 20 15:45:48 2012
New Revision: 243342
URL: http://svnweb.freebsd.org/changeset/base/243342

Log:
  Schedule garbage collection run for the in-flight rights passed over
  the unix domain sockets to the next tick, coalescing the serial calls
  until the collection fires.  The thought is that more work for the
  collector could arise in the near time, allowing to clean more and not
  spend too much CPU on repeated collection when there is no garbage.
  
  Currently the collection task is fired immediately upon unix domain
  socket close if there are any rights in flight, which caused excessive
  CPU usage and too long blocking of the threads waiting for
  unp_list_lock and unp_link_rwlock in write mode.
  
  Robert noted that it would be nice if we could find some heuristic by
  which we decide whether to run GC a bit more quickly.  E.g., if the
  number of UNIX domain sockets is close to its resource limit, but not
  quite.
  
  Reported and tested by:   Markus Gebert 
  Reviewed by:  rwatson
  MFC after:2 weeks

Modified:
  head/sys/kern/uipc_usrreq.c

Modified: head/sys/kern/uipc_usrreq.c
==
--- head/sys/kern/uipc_usrreq.c Tue Nov 20 15:33:48 2012(r243341)
+++ head/sys/kern/uipc_usrreq.c Tue Nov 20 15:45:48 2012(r243342)
@@ -131,7 +131,7 @@ static const struct sockaddrsun_noname 
  * reentrance in the UNIX domain socket, file descriptor, and socket layer
  * code.  See unp_gc() for a full description.
  */
-static struct task unp_gc_task;
+static struct timeout_task unp_gc_task;
 
 /*
  * The close of unix domain sockets attached as SCM_RIGHTS is
@@ -672,7 +672,7 @@ uipc_detach(struct socket *so)
if (vp)
vrele(vp);
if (local_unp_rights)
-   taskqueue_enqueue(taskqueue_thread, &unp_gc_task);
+   taskqueue_enqueue_timeout(taskqueue_thread, &unp_gc_task, -1);
 }
 
 static int
@@ -1784,7 +1784,7 @@ unp_init(void)
LIST_INIT(&unp_shead);
LIST_INIT(&unp_sphead);
SLIST_INIT(&unp_defers);
-   TASK_INIT(&unp_gc_task, 0, unp_gc, NULL);
+   TIMEOUT_TASK_INIT(taskqueue_thread, &unp_gc_task, 0, unp_gc, NULL);
TASK_INIT(&unp_defer_task, 0, unp_process_defers, NULL);
UNP_LINK_LOCK_INIT();
UNP_LIST_LOCK_INIT();
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


Re: svn commit: r243321 - head/usr.sbin/edquota

2012-11-20 Thread Eitan Adler
On 20 November 2012 02:03, Bruce Evans  wrote:
> On Tue, 20 Nov 2012, Eitan Adler wrote:
>
>> Log:
>>  Remove unneeded includes.
>>
>>  Tested with "make universe"; there are no conditional features.
>
>
> "make universe" can't find such features.  Except inversely -- when it
> doesn't find them, it usually means that there is a bug in the headers.

[ thanks for the info. I'll go through it and parse it fully soon ]

Just to explain.  The above sentence means "I tested this with make
universe"  AND I checked to make sure that there were no #ifdefs or
the like which might rely on these headers.


-- 
Eitan Adler
Source, Ports, Doc committer
Bugmeister, Ports Security teams
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


Re: svn commit: r243311 - in head/sys: fs/ext2fs fs/msdosfs fs/nfsclient fs/nullfs fs/unionfs gnu/fs/reiserfs nfsclient ufs/ffs

2012-11-20 Thread Eitan Adler
On 20 November 2012 03:17, Attilio Rao  wrote:
> And the code makes perfectly sense if you know the history.

One shouldn't have to know the history of the code to understand the
present state.


-- 
Eitan Adler
Source, Ports, Doc committer
Bugmeister, Ports Security teams
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


Re: svn commit: r243328 - head/lib/libutil

2012-11-20 Thread Eitan Adler
On 20 November 2012 08:38, Baptiste Daroussin  wrote:
> On Tue, Nov 20, 2012 at 02:22:26PM +0200, Jaakko Heinonen wrote:
>>
>> Hi!
>>
>> On 2012-11-20, Baptiste Daroussin wrote:
>> >   change mode the group file to 0644 after a successfull rename(2)
>> >
>> >  int
>> >  gr_mkdb(void)
>> >  {
>> > -   return (rename(tempname, group_file));
>> > +   int ret;
>> > +
>> > +   ret = rename(tempname, group_file);
>> > +
>> > +   if (ret == 0)
>> > +   chmod(group_file, 0644);
>> > +
>> > +   return (ret);
>> >  }
>>
>> Rename+chmod is not an atomic operation. There is a window when the file
>> has wrong permissions. Also, you don't check the return value of
>> chmod(). Maybe chmod first and then rename?
>>
>> --
>> Jaakko
>
> Does this looks better to you?
> http://people.freebsd.org/~bapt/gr_util.diff

This makes more sense.



-- 
Eitan Adler
Source, Ports, Doc committer
Bugmeister, Ports Security teams
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


svn commit: r243346 - head/tools/regression/lib/libc/resolv

2012-11-20 Thread Ed Maste
Author: emaste
Date: Tue Nov 20 19:23:44 2012
New Revision: 243346
URL: http://svnweb.freebsd.org/changeset/base/243346

Log:
  Non-void function should return a value.
  
  Found by: clang

Modified:
  head/tools/regression/lib/libc/resolv/resolv.c

Modified: head/tools/regression/lib/libc/resolv/resolv.c
==
--- head/tools/regression/lib/libc/resolv/resolv.c  Tue Nov 20 17:08:37 
2012(r243345)
+++ head/tools/regression/lib/libc/resolv/resolv.c  Tue Nov 20 19:23:44 
2012(r243346)
@@ -226,7 +226,7 @@ resolvloop(void *p)
 {
int *nhosts = (int *)p;
if (*nhosts == 0)
-   return;
+   return NULL;
do
resolvone(*nhosts);
while (--(*nhosts));
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


svn commit: r243347 - in head: sys/conf sys/contrib/dev/acpica sys/contrib/dev/acpica/common sys/contrib/dev/acpica/compiler sys/contrib/dev/acpica/components/debugger sys/contrib/dev/acpica/compon...

2012-11-20 Thread Jung-uk Kim
Author: jkim
Date: Tue Nov 20 21:01:59 2012
New Revision: 243347
URL: http://svnweb.freebsd.org/changeset/base/243347

Log:
  Merge ACPICA 20121114.

Added:
  head/sys/contrib/dev/acpica/components/disassembler/dmdeferred.c
 - copied, changed from r243045, 
vendor-sys/acpica/dist/source/components/disassembler/dmdeferred.c
Modified:
  head/sys/conf/files
  head/sys/contrib/dev/acpica/changes.txt   (contents, props changed)
  head/sys/contrib/dev/acpica/common/adfile.c
  head/sys/contrib/dev/acpica/common/adisasm.c
  head/sys/contrib/dev/acpica/common/dmextern.c
  head/sys/contrib/dev/acpica/common/dmrestag.c
  head/sys/contrib/dev/acpica/compiler/aslcompile.c
  head/sys/contrib/dev/acpica/compiler/aslcompiler.h
  head/sys/contrib/dev/acpica/compiler/aslerror.c
  head/sys/contrib/dev/acpica/compiler/aslfiles.c
  head/sys/contrib/dev/acpica/compiler/aslglobal.h
  head/sys/contrib/dev/acpica/compiler/asllisting.c
  head/sys/contrib/dev/acpica/compiler/asllookup.c
  head/sys/contrib/dev/acpica/compiler/aslmain.c
  head/sys/contrib/dev/acpica/compiler/aslstartup.c
  head/sys/contrib/dev/acpica/compiler/dttemplate.c
  head/sys/contrib/dev/acpica/compiler/prutils.c
  head/sys/contrib/dev/acpica/components/debugger/dbfileio.c
  head/sys/contrib/dev/acpica/components/debugger/dbinput.c
  head/sys/contrib/dev/acpica/components/debugger/dbmethod.c
  head/sys/contrib/dev/acpica/components/disassembler/dmopcode.c
  head/sys/contrib/dev/acpica/components/disassembler/dmresrc.c
  head/sys/contrib/dev/acpica/components/disassembler/dmresrcl.c
  head/sys/contrib/dev/acpica/components/disassembler/dmresrcl2.c
  head/sys/contrib/dev/acpica/components/disassembler/dmresrcs.c
  head/sys/contrib/dev/acpica/components/dispatcher/dsmethod.c
  head/sys/contrib/dev/acpica/components/executer/exregion.c
  head/sys/contrib/dev/acpica/components/namespace/nsutils.c
  head/sys/contrib/dev/acpica/components/namespace/nsxfname.c
  head/sys/contrib/dev/acpica/components/resources/rscalc.c
  head/sys/contrib/dev/acpica/components/resources/rscreate.c
  head/sys/contrib/dev/acpica/components/resources/rsdump.c
  head/sys/contrib/dev/acpica/components/resources/rslist.c
  head/sys/contrib/dev/acpica/components/resources/rsmisc.c
  head/sys/contrib/dev/acpica/components/resources/rsxface.c
  head/sys/contrib/dev/acpica/components/utilities/utdelete.c
  head/sys/contrib/dev/acpica/components/utilities/utresrc.c
  head/sys/contrib/dev/acpica/components/utilities/utstate.c
  head/sys/contrib/dev/acpica/components/utilities/uttrack.c
  head/sys/contrib/dev/acpica/include/acdisasm.h
  head/sys/contrib/dev/acpica/include/acmacros.h
  head/sys/contrib/dev/acpica/include/acpixf.h
  head/sys/contrib/dev/acpica/include/acrestyp.h
  head/sys/contrib/dev/acpica/include/acutils.h
  head/sys/modules/acpi/acpi/Makefile
  head/usr.sbin/acpi/acpidb/Makefile
  head/usr.sbin/acpi/iasl/Makefile
Directory Properties:
  head/sys/contrib/dev/acpica/   (props changed)
  head/sys/contrib/dev/acpica/common/   (props changed)
  head/sys/contrib/dev/acpica/compiler/   (props changed)
  head/sys/contrib/dev/acpica/components/debugger/   (props changed)
  head/sys/contrib/dev/acpica/components/disassembler/   (props changed)
  head/sys/contrib/dev/acpica/components/dispatcher/   (props changed)
  head/sys/contrib/dev/acpica/components/executer/   (props changed)
  head/sys/contrib/dev/acpica/components/namespace/   (props changed)
  head/sys/contrib/dev/acpica/components/resources/   (props changed)
  head/sys/contrib/dev/acpica/components/utilities/   (props changed)
  head/sys/contrib/dev/acpica/include/   (props changed)

Modified: head/sys/conf/files
==
--- head/sys/conf/files Tue Nov 20 19:23:44 2012(r243346)
+++ head/sys/conf/files Tue Nov 20 21:01:59 2012(r243347)
@@ -294,6 +294,7 @@ contrib/dev/acpica/components/debugger/d
 contrib/dev/acpica/components/debugger/dbutils.c   optional acpi acpi_debug
 contrib/dev/acpica/components/debugger/dbxface.c   optional acpi acpi_debug
 contrib/dev/acpica/components/disassembler/dmbuffer.c  optional acpi acpi_debug
+contrib/dev/acpica/components/disassembler/dmdeferred.coptional acpi 
acpi_debug
 contrib/dev/acpica/components/disassembler/dmnames.c   optional acpi acpi_debug
 contrib/dev/acpica/components/disassembler/dmopcode.c  optional acpi acpi_debug
 contrib/dev/acpica/components/disassembler/dmobject.c  optional acpi acpi_debug

Modified: head/sys/contrib/dev/acpica/changes.txt
==
--- head/sys/contrib/dev/acpica/changes.txt Tue Nov 20 19:23:44 2012
(r243346)
+++ head/sys/contrib/dev/acpica/changes.txt Tue Nov 20 21:01:59 2012
(r243347)
@@ -1,4 +1,94 @@
 
+14 November 2012. Summary of changes for version 20121114:
+
+This release is available at https://www.acpica.org/downl

svn commit: r243348 - head/share/mk

2012-11-20 Thread Dimitry Andric
Author: dim
Date: Tue Nov 20 21:26:13 2012
New Revision: 243348
URL: http://svnweb.freebsd.org/changeset/base/243348

Log:
  Do not expose LIBCXXRT and LIBCPLUSPLUS in bsd.libnames.mk, if
  WITHOUT_LIBCPLUSPLUS is specified.
  
  Submitted by: Garrett Cooper 
  MFC after:3 days

Modified:
  head/share/mk/bsd.libnames.mk

Modified: head/share/mk/bsd.libnames.mk
==
--- head/share/mk/bsd.libnames.mk   Tue Nov 20 21:01:59 2012
(r243347)
+++ head/share/mk/bsd.libnames.mk   Tue Nov 20 21:26:13 2012
(r243348)
@@ -28,8 +28,10 @@ LIBBSDXML?=  ${DESTDIR}${LIBDIR}/libbsdxm
 LIBBSM?=   ${DESTDIR}${LIBDIR}/libbsm.a
 LIBBSNMP?= ${DESTDIR}${LIBDIR}/libbsnmp.a
 LIBBZ2?=   ${DESTDIR}${LIBDIR}/libbz2.a
+.if ${MK_LIBCPLUSPLUS} != "no"
 LIBCXXRT?= ${DESTDIR}${LIBDIR}/libcxxrt.a
 LIBCPLUSPLUS?= ${DESTDIR}${LIBDIR}/libc++.a
+.endif
 LIBC?= ${DESTDIR}${LIBDIR}/libc.a
 LIBC_PIC?= ${DESTDIR}${LIBDIR}/libc_pic.a
 LIBCALENDAR?=  ${DESTDIR}${LIBDIR}/libcalendar.a
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


svn commit: r243359 - head/sys/arm/arm

2012-11-20 Thread Olivier Houchard
Author: cognet
Date: Wed Nov 21 01:38:40 2012
New Revision: 243359
URL: http://svnweb.freebsd.org/changeset/base/243359

Log:
  Make sure the address starts on a cache line boundary.

Modified:
  head/sys/arm/arm/pl310.c

Modified: head/sys/arm/arm/pl310.c
==
--- head/sys/arm/arm/pl310.cWed Nov 21 01:01:19 2012(r243358)
+++ head/sys/arm/arm/pl310.cWed Nov 21 01:38:40 2012(r243359)
@@ -180,9 +180,13 @@ static void
 pl310_wbinv_range(vm_paddr_t start, vm_size_t size)
 {

+   if (start & g_l2cache_align_mask) {
+   size += start & g_l2cache_align_mask;
+   start &= ~g_l2cache_align_mask;
+   }
if (size & g_l2cache_align_mask) {
size &= ~g_l2cache_align_mask;
-   size += g_l2cache_line_size;
+   size += g_l2cache_line_size;
}
 #if 1
 
@@ -217,6 +221,10 @@ static void
 pl310_wb_range(vm_paddr_t start, vm_size_t size)
 {

+   if (start & g_l2cache_align_mask) {
+   size += start & g_l2cache_align_mask;
+   start &= ~g_l2cache_align_mask;
+   }
if (size & g_l2cache_align_mask) {
size &= ~g_l2cache_align_mask;
size += g_l2cache_line_size;
@@ -235,6 +243,10 @@ static void
 pl310_inv_range(vm_paddr_t start, vm_size_t size)
 {
 
+   if (start & g_l2cache_align_mask) {
+   size += start & g_l2cache_align_mask;
+   start &= ~g_l2cache_align_mask;
+   }
if (size & g_l2cache_align_mask) {
size &= ~g_l2cache_align_mask;
size += g_l2cache_line_size;
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


svn commit: r243366 - head/sys/vm

2012-11-20 Thread Alan Cox
Author: alc
Date: Wed Nov 21 06:26:18 2012
New Revision: 243366
URL: http://svnweb.freebsd.org/changeset/base/243366

Log:
  Correct an error in r230623.  When both VM_ALLOC_NODUMP and VM_ALLOC_ZERO
  were specified to vm_page_alloc(), PG_NODUMP wasn't being set on the
  allocated page when it happened to be pre-zeroed.
  
  MFC after:5 days

Modified:
  head/sys/vm/vm_page.c

Modified: head/sys/vm/vm_page.c
==
--- head/sys/vm/vm_page.c   Wed Nov 21 04:54:02 2012(r243365)
+++ head/sys/vm/vm_page.c   Wed Nov 21 06:26:18 2012(r243366)
@@ -1481,13 +1481,13 @@ vm_page_alloc(vm_object_t object, vm_pin
 * must be cleared before the free page queues lock is released.
 */
flags = 0;
-   if (req & VM_ALLOC_NODUMP)
-   flags |= PG_NODUMP;
if (m->flags & PG_ZERO) {
vm_page_zero_count--;
if (req & VM_ALLOC_ZERO)
flags = PG_ZERO;
}
+   if (req & VM_ALLOC_NODUMP)
+   flags |= PG_NODUMP;
m->flags = flags;
mtx_unlock(&vm_page_queue_free_mtx);
m->aflags = 0;
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"