CVS commit: src/sys/fs/tmpfs
Module Name:src Committed By: riastradh Date: Sat Apr 29 08:15:13 UTC 2023 Modified Files: src/sys/fs/tmpfs: tmpfs_subr.c Log Message: tmpfs: Assert no arithmetic overflow in directory node tn_size. Need >2^57 directory entries before this is a problem. If we created a million per second, this would take over 4000 years. To generate a diff of this commit: cvs rdiff -u -r1.116 -r1.117 src/sys/fs/tmpfs/tmpfs_subr.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/fs/tmpfs/tmpfs_subr.c diff -u src/sys/fs/tmpfs/tmpfs_subr.c:1.116 src/sys/fs/tmpfs/tmpfs_subr.c:1.117 --- src/sys/fs/tmpfs/tmpfs_subr.c:1.116 Sat Apr 29 08:13:27 2023 +++ src/sys/fs/tmpfs/tmpfs_subr.c Sat Apr 29 08:15:13 2023 @@ -1,4 +1,4 @@ -/* $NetBSD: tmpfs_subr.c,v 1.116 2023/04/29 08:13:27 riastradh Exp $ */ +/* $NetBSD: tmpfs_subr.c,v 1.117 2023/04/29 08:15:13 riastradh Exp $ */ /* * Copyright (c) 2005-2020 The NetBSD Foundation, Inc. @@ -73,7 +73,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: tmpfs_subr.c,v 1.116 2023/04/29 08:13:27 riastradh Exp $"); +__KERNEL_RCSID(0, "$NetBSD: tmpfs_subr.c,v 1.117 2023/04/29 08:15:13 riastradh Exp $"); #include #include @@ -522,6 +522,7 @@ tmpfs_dir_attach(tmpfs_node_t *dnode, tm /* Insert the entry to the directory (parent of inode). */ TAILQ_INSERT_TAIL(>tn_spec.tn_dir.tn_dir, de, td_entries); + KASSERT(dnode->tn_size <= __type_max(off_t) - sizeof(tmpfs_dirent_t)); dnode->tn_size += sizeof(tmpfs_dirent_t); uvm_vnp_setsize(dvp, dnode->tn_size); @@ -580,6 +581,7 @@ tmpfs_dir_detach(tmpfs_node_t *dnode, tm dnode->tn_spec.tn_dir.tn_readdir_lastp = NULL; } TAILQ_REMOVE(>tn_spec.tn_dir.tn_dir, de, td_entries); + KASSERT(dnode->tn_size >= sizeof(tmpfs_dirent_t)); dnode->tn_size -= sizeof(tmpfs_dirent_t); tmpfs_dir_putseq(dnode, de);
CVS commit: src/sys/fs/tmpfs
Module Name:src Committed By: riastradh Date: Sat Apr 29 08:15:13 UTC 2023 Modified Files: src/sys/fs/tmpfs: tmpfs_subr.c Log Message: tmpfs: Assert no arithmetic overflow in directory node tn_size. Need >2^57 directory entries before this is a problem. If we created a million per second, this would take over 4000 years. To generate a diff of this commit: cvs rdiff -u -r1.116 -r1.117 src/sys/fs/tmpfs/tmpfs_subr.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/fs/tmpfs
Module Name:src Committed By: riastradh Date: Sat Apr 29 08:13:27 UTC 2023 Modified Files: src/sys/fs/tmpfs: tmpfs_subr.c Log Message: tmpfs: Refuse sizes that overflow round_page. Reported-by: syzbot+8dbeee84de15f86df...@syzkaller.appspotmail.com https://syzkaller.appspot.com/bug?id=4a27b9fe074f8d4b0afbe22969339b8dfdb157e8 To generate a diff of this commit: cvs rdiff -u -r1.115 -r1.116 src/sys/fs/tmpfs/tmpfs_subr.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/fs/tmpfs/tmpfs_subr.c diff -u src/sys/fs/tmpfs/tmpfs_subr.c:1.115 src/sys/fs/tmpfs/tmpfs_subr.c:1.116 --- src/sys/fs/tmpfs/tmpfs_subr.c:1.115 Sat Apr 29 06:29:55 2023 +++ src/sys/fs/tmpfs/tmpfs_subr.c Sat Apr 29 08:13:27 2023 @@ -1,4 +1,4 @@ -/* $NetBSD: tmpfs_subr.c,v 1.115 2023/04/29 06:29:55 riastradh Exp $ */ +/* $NetBSD: tmpfs_subr.c,v 1.116 2023/04/29 08:13:27 riastradh Exp $ */ /* * Copyright (c) 2005-2020 The NetBSD Foundation, Inc. @@ -73,7 +73,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: tmpfs_subr.c,v 1.115 2023/04/29 06:29:55 riastradh Exp $"); +__KERNEL_RCSID(0, "$NetBSD: tmpfs_subr.c,v 1.116 2023/04/29 08:13:27 riastradh Exp $"); #include #include @@ -907,6 +907,9 @@ tmpfs_reg_resize(struct vnode *vp, off_t KASSERT(vp->v_type == VREG); KASSERT(newsize >= 0); + if (newsize > __type_max(off_t) - PAGE_SIZE + 1) + return EFBIG; + oldsize = node->tn_size; oldpages = round_page(oldsize) >> PAGE_SHIFT; newpages = round_page(newsize) >> PAGE_SHIFT;
CVS commit: src/sys/fs/tmpfs
Module Name:src Committed By: riastradh Date: Sat Apr 29 08:13:27 UTC 2023 Modified Files: src/sys/fs/tmpfs: tmpfs_subr.c Log Message: tmpfs: Refuse sizes that overflow round_page. Reported-by: syzbot+8dbeee84de15f86df...@syzkaller.appspotmail.com https://syzkaller.appspot.com/bug?id=4a27b9fe074f8d4b0afbe22969339b8dfdb157e8 To generate a diff of this commit: cvs rdiff -u -r1.115 -r1.116 src/sys/fs/tmpfs/tmpfs_subr.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/fs/tmpfs
Module Name:src Committed By: riastradh Date: Sat Apr 29 06:29:55 UTC 2023 Modified Files: src/sys/fs/tmpfs: tmpfs_mem.c tmpfs_subr.c Log Message: tmpfs: Nix trailing whitespace. No functional change intended. To generate a diff of this commit: cvs rdiff -u -r1.13 -r1.14 src/sys/fs/tmpfs/tmpfs_mem.c cvs rdiff -u -r1.114 -r1.115 src/sys/fs/tmpfs/tmpfs_subr.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/fs/tmpfs/tmpfs_mem.c diff -u src/sys/fs/tmpfs/tmpfs_mem.c:1.13 src/sys/fs/tmpfs/tmpfs_mem.c:1.14 --- src/sys/fs/tmpfs/tmpfs_mem.c:1.13 Thu Jun 11 19:20:46 2020 +++ src/sys/fs/tmpfs/tmpfs_mem.c Sat Apr 29 06:29:55 2023 @@ -1,4 +1,4 @@ -/* $NetBSD: tmpfs_mem.c,v 1.13 2020/06/11 19:20:46 ad Exp $ */ +/* $NetBSD: tmpfs_mem.c,v 1.14 2023/04/29 06:29:55 riastradh Exp $ */ /* * Copyright (c) 2010, 2011, 2020 The NetBSD Foundation, Inc. @@ -35,7 +35,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: tmpfs_mem.c,v 1.13 2020/06/11 19:20:46 ad Exp $"); +__KERNEL_RCSID(0, "$NetBSD: tmpfs_mem.c,v 1.14 2023/04/29 06:29:55 riastradh Exp $"); #include #include @@ -81,8 +81,6 @@ tmpfs_mntmem_set(struct tmpfs_mount *mp, return error; } - - /* * tmpfs_mem_info: return the number of available memory pages. * Index: src/sys/fs/tmpfs/tmpfs_subr.c diff -u src/sys/fs/tmpfs/tmpfs_subr.c:1.114 src/sys/fs/tmpfs/tmpfs_subr.c:1.115 --- src/sys/fs/tmpfs/tmpfs_subr.c:1.114 Wed Oct 20 03:08:17 2021 +++ src/sys/fs/tmpfs/tmpfs_subr.c Sat Apr 29 06:29:55 2023 @@ -1,4 +1,4 @@ -/* $NetBSD: tmpfs_subr.c,v 1.114 2021/10/20 03:08:17 thorpej Exp $ */ +/* $NetBSD: tmpfs_subr.c,v 1.115 2023/04/29 06:29:55 riastradh Exp $ */ /* * Copyright (c) 2005-2020 The NetBSD Foundation, Inc. @@ -73,7 +73,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: tmpfs_subr.c,v 1.114 2021/10/20 03:08:17 thorpej Exp $"); +__KERNEL_RCSID(0, "$NetBSD: tmpfs_subr.c,v 1.115 2023/04/29 06:29:55 riastradh Exp $"); #include #include @@ -892,7 +892,7 @@ done: } /* - * tmpfs_reg_resize: resize the underlying UVM object associated with the + * tmpfs_reg_resize: resize the underlying UVM object associated with the * specified regular file. */ int
CVS commit: src/sys/fs/tmpfs
Module Name:src Committed By: riastradh Date: Sat Apr 29 06:29:55 UTC 2023 Modified Files: src/sys/fs/tmpfs: tmpfs_mem.c tmpfs_subr.c Log Message: tmpfs: Nix trailing whitespace. No functional change intended. To generate a diff of this commit: cvs rdiff -u -r1.13 -r1.14 src/sys/fs/tmpfs/tmpfs_mem.c cvs rdiff -u -r1.114 -r1.115 src/sys/fs/tmpfs/tmpfs_subr.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/fs/tmpfs
Module Name:src
Committed By: hannken
Date: Thu Nov 10 10:54:14 UTC 2022
Modified Files:
src/sys/fs/tmpfs: tmpfs_vfsops.c
Log Message:
Tmpfs_mount() uses tmpfs_unmount() for cleanup if set_statvfs_info() fails.
This will not work as tmpfs_unmount() needs a suspended file system.
Just call set_statvfs_info() before allocating the root vnode and add
and use a common error exit label.
Reported-by: syzbot+343f2bfea65a32ab4...@syzkaller.appspotmail.com
To generate a diff of this commit:
cvs rdiff -u -r1.77 -r1.78 src/sys/fs/tmpfs/tmpfs_vfsops.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/fs/tmpfs/tmpfs_vfsops.c
diff -u src/sys/fs/tmpfs/tmpfs_vfsops.c:1.77 src/sys/fs/tmpfs/tmpfs_vfsops.c:1.78
--- src/sys/fs/tmpfs/tmpfs_vfsops.c:1.77 Sat Apr 4 20:49:30 2020
+++ src/sys/fs/tmpfs/tmpfs_vfsops.c Thu Nov 10 10:54:14 2022
@@ -1,4 +1,4 @@
-/* $NetBSD: tmpfs_vfsops.c,v 1.77 2020/04/04 20:49:30 ad Exp $ */
+/* $NetBSD: tmpfs_vfsops.c,v 1.78 2022/11/10 10:54:14 hannken Exp $ */
/*
* Copyright (c) 2005, 2006, 2007 The NetBSD Foundation, Inc.
@@ -42,7 +42,7 @@
*/
#include
-__KERNEL_RCSID(0, "$NetBSD: tmpfs_vfsops.c,v 1.77 2020/04/04 20:49:30 ad Exp $");
+__KERNEL_RCSID(0, "$NetBSD: tmpfs_vfsops.c,v 1.78 2022/11/10 10:54:14 hannken Exp $");
#include
#include
@@ -196,6 +196,11 @@ tmpfs_mount(struct mount *mp, const char
tmpfs_mntmem_init(tmp, memlimit);
mp->mnt_data = tmp;
+ error = set_statvfs_info(path, UIO_USERSPACE, "tmpfs", UIO_SYSSPACE,
+ mp->mnt_op->vfs_name, mp, curlwp);
+ if (error)
+ goto errout;
+
/* Allocate the root node. */
vattr_null();
va.va_type = VDIR;
@@ -203,13 +208,8 @@ tmpfs_mount(struct mount *mp, const char
va.va_uid = args->ta_root_uid;
va.va_gid = args->ta_root_gid;
error = vcache_new(mp, NULL, , NOCRED, NULL, );
- if (error) {
- mp->mnt_data = NULL;
- tmpfs_mntmem_destroy(tmp);
- mutex_destroy(>tm_lock);
- kmem_free(tmp, sizeof(*tmp));
- return error;
- }
+ if (error)
+ goto errout;
KASSERT(vp != NULL);
root = VP_TO_TMPFS_NODE(vp);
KASSERT(root != NULL);
@@ -224,11 +224,14 @@ tmpfs_mount(struct mount *mp, const char
tmp->tm_root = root;
vrele(vp);
- error = set_statvfs_info(path, UIO_USERSPACE, "tmpfs", UIO_SYSSPACE,
- mp->mnt_op->vfs_name, mp, curlwp);
- if (error) {
- (void)tmpfs_unmount(mp, MNT_FORCE);
- }
+ return 0;
+
+errout:
+ mp->mnt_data = NULL;
+ tmpfs_mntmem_destroy(tmp);
+ mutex_destroy(>tm_lock);
+ kmem_free(tmp, sizeof(*tmp));
+
return error;
}
CVS commit: src/sys/fs/tmpfs
Module Name:src Committed By: hannken Date: Thu Nov 10 10:54:14 UTC 2022 Modified Files: src/sys/fs/tmpfs: tmpfs_vfsops.c Log Message: Tmpfs_mount() uses tmpfs_unmount() for cleanup if set_statvfs_info() fails. This will not work as tmpfs_unmount() needs a suspended file system. Just call set_statvfs_info() before allocating the root vnode and add and use a common error exit label. Reported-by: syzbot+343f2bfea65a32ab4...@syzkaller.appspotmail.com To generate a diff of this commit: cvs rdiff -u -r1.77 -r1.78 src/sys/fs/tmpfs/tmpfs_vfsops.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/fs/tmpfs
Module Name:src Committed By: hannken Date: Wed Jun 1 08:42:38 UTC 2022 Modified Files: src/sys/fs/tmpfs: tmpfs_vnops.c Log Message: tmpfs_read: respect MNT_NOATIME. To generate a diff of this commit: cvs rdiff -u -r1.149 -r1.150 src/sys/fs/tmpfs/tmpfs_vnops.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/fs/tmpfs/tmpfs_vnops.c diff -u src/sys/fs/tmpfs/tmpfs_vnops.c:1.149 src/sys/fs/tmpfs/tmpfs_vnops.c:1.150 --- src/sys/fs/tmpfs/tmpfs_vnops.c:1.149 Sun Mar 27 16:24:57 2022 +++ src/sys/fs/tmpfs/tmpfs_vnops.c Wed Jun 1 08:42:38 2022 @@ -1,4 +1,4 @@ -/* $NetBSD: tmpfs_vnops.c,v 1.149 2022/03/27 16:24:57 christos Exp $ */ +/* $NetBSD: tmpfs_vnops.c,v 1.150 2022/06/01 08:42:38 hannken Exp $ */ /* * Copyright (c) 2005, 2006, 2007, 2020 The NetBSD Foundation, Inc. @@ -35,7 +35,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: tmpfs_vnops.c,v 1.149 2022/03/27 16:24:57 christos Exp $"); +__KERNEL_RCSID(0, "$NetBSD: tmpfs_vnops.c,v 1.150 2022/06/01 08:42:38 hannken Exp $"); #include #include @@ -555,7 +555,9 @@ tmpfs_read(void *v) UBC_READ | UBC_PARTIALOK | UBC_VNODE_FLAGS(vp)); } - tmpfs_update(vp, TMPFS_UPDATE_ATIME); + if ((vp->v_mount->mnt_flag & MNT_NOATIME) == 0) + tmpfs_update(vp, TMPFS_UPDATE_ATIME); + return error; }
CVS commit: src/sys/fs/tmpfs
Module Name:src Committed By: hannken Date: Wed Jun 1 08:42:38 UTC 2022 Modified Files: src/sys/fs/tmpfs: tmpfs_vnops.c Log Message: tmpfs_read: respect MNT_NOATIME. To generate a diff of this commit: cvs rdiff -u -r1.149 -r1.150 src/sys/fs/tmpfs/tmpfs_vnops.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/fs/tmpfs
Module Name:src Committed By: thorpej Date: Wed Oct 20 14:28:21 UTC 2021 Modified Files: src/sys/fs/tmpfs: tmpfs_rename.c Log Message: Move a mis-placed KASSERT(). To generate a diff of this commit: cvs rdiff -u -r1.11 -r1.12 src/sys/fs/tmpfs/tmpfs_rename.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/fs/tmpfs/tmpfs_rename.c diff -u src/sys/fs/tmpfs/tmpfs_rename.c:1.11 src/sys/fs/tmpfs/tmpfs_rename.c:1.12 --- src/sys/fs/tmpfs/tmpfs_rename.c:1.11 Wed Oct 20 03:08:17 2021 +++ src/sys/fs/tmpfs/tmpfs_rename.c Wed Oct 20 14:28:21 2021 @@ -1,4 +1,4 @@ -/* $NetBSD: tmpfs_rename.c,v 1.11 2021/10/20 03:08:17 thorpej Exp $ */ +/* $NetBSD: tmpfs_rename.c,v 1.12 2021/10/20 14:28:21 thorpej Exp $ */ /*- * Copyright (c) 2012 The NetBSD Foundation, Inc. @@ -34,7 +34,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: tmpfs_rename.c,v 1.11 2021/10/20 03:08:17 thorpej Exp $"); +__KERNEL_RCSID(0, "$NetBSD: tmpfs_rename.c,v 1.12 2021/10/20 14:28:21 thorpej Exp $"); #include #include @@ -402,11 +402,12 @@ tmpfs_gro_remove(struct mount *mp, kauth KASSERT(VOP_ISLOCKED(dvp) == LK_EXCLUSIVE); KASSERT(VOP_ISLOCKED(vp) == LK_EXCLUSIVE); + KASSERT((*dep)->td_node == VP_TO_TMPFS_NODE(vp)); + tmpfs_dir_detach(dnode, *dep); tmpfs_free_dirent(VFS_TO_TMPFS(mp), *dep); tmpfs_update(dvp, TMPFS_UPDATE_MTIME | TMPFS_UPDATE_CTIME); - KASSERT((*dep)->td_node == VP_TO_TMPFS_NODE(vp)); *tvp_nlinkp = VP_TO_TMPFS_NODE(vp)->tn_links; return 0;
CVS commit: src/sys/fs/tmpfs
Module Name:src Committed By: thorpej Date: Wed Oct 20 14:28:21 UTC 2021 Modified Files: src/sys/fs/tmpfs: tmpfs_rename.c Log Message: Move a mis-placed KASSERT(). To generate a diff of this commit: cvs rdiff -u -r1.11 -r1.12 src/sys/fs/tmpfs/tmpfs_rename.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
Re: CVS commit: src/sys/fs/tmpfs
On Tue, May 19, 2020 at 11:09:07PM +, Andrew Doran wrote: > vnode locks are not held during getpages/putpages. ^ for fault handling, anyway. for read/write they are held by the caller to ubc_uiomove(). Andrew
Re: CVS commit: src/sys/fs/tmpfs
On Sun, May 17, 2020 at 11:49:52PM +, m...@netbsd.org wrote: > On Sun, May 17, 2020 at 09:47:50PM +, m...@netbsd.org wrote: > > On Sun, May 17, 2020 at 07:39:15PM +, Andrew Doran wrote: > > > Module Name: src > > > Committed By: ad > > > Date: Sun May 17 19:39:15 UTC 2020 > > > > > > Modified Files: > > > src/sys/fs/tmpfs: tmpfs.h tmpfs_subr.c tmpfs_vnops.c > > > > > > Log Message: > > > PR kern/55268: tmpfs is slow > > > > > > tmpfs_getpages(): ...implement lazy update of atime/mtime. > > > > > > > I'm confused about how this makes sense. Can you elaborate? > > Presumably RAM is as fast as other RAM. > > riastradh responded to this elsewhere: avoid atomic ops Right, also to: - avoid taking a mutex to serialize the update to the time fields in the tmpfs node. vnode locks are not held during getpages/putpages. the vmobjlock is held here and often read-held so there can be lots of concurrent access. - avoid doing a writeback on the tmpfs_node, which if it's heavily shared (consider for example ld.so or libc.so) involves lots of cache coherency overhead. - avoid querying the clock. Andrew
Re: CVS commit: src/sys/fs/tmpfs
On Sun, May 17, 2020 at 09:47:50PM +, m...@netbsd.org wrote: > On Sun, May 17, 2020 at 07:39:15PM +, Andrew Doran wrote: > > Module Name:src > > Committed By: ad > > Date: Sun May 17 19:39:15 UTC 2020 > > > > Modified Files: > > src/sys/fs/tmpfs: tmpfs.h tmpfs_subr.c tmpfs_vnops.c > > > > Log Message: > > PR kern/55268: tmpfs is slow > > > > tmpfs_getpages(): ...implement lazy update of atime/mtime. > > > > I'm confused about how this makes sense. Can you elaborate? > Presumably RAM is as fast as other RAM. riastradh responded to this elsewhere: avoid atomic ops thanks
Re: CVS commit: src/sys/fs/tmpfs
On Sun, May 17, 2020 at 07:39:15PM +, Andrew Doran wrote: > Module Name: src > Committed By: ad > Date: Sun May 17 19:39:15 UTC 2020 > > Modified Files: > src/sys/fs/tmpfs: tmpfs.h tmpfs_subr.c tmpfs_vnops.c > > Log Message: > PR kern/55268: tmpfs is slow > > tmpfs_getpages(): ...implement lazy update of atime/mtime. > I'm confused about how this makes sense. Can you elaborate? Presumably RAM is as fast as other RAM.
CVS commit: src/sys/fs/tmpfs
Module Name:src
Committed By: mrg
Date: Fri Oct 4 12:34:40 UTC 2019
Modified Files:
src/sys/fs/tmpfs: tmpfs_vfsops.c
Log Message:
remove an always false check and its' "This can never happen?" comment.
To generate a diff of this commit:
cvs rdiff -u -r1.74 -r1.75 src/sys/fs/tmpfs/tmpfs_vfsops.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/fs/tmpfs/tmpfs_vfsops.c
diff -u src/sys/fs/tmpfs/tmpfs_vfsops.c:1.74 src/sys/fs/tmpfs/tmpfs_vfsops.c:1.75
--- src/sys/fs/tmpfs/tmpfs_vfsops.c:1.74 Tue Jan 1 10:06:54 2019
+++ src/sys/fs/tmpfs/tmpfs_vfsops.c Fri Oct 4 12:34:40 2019
@@ -1,4 +1,4 @@
-/* $NetBSD: tmpfs_vfsops.c,v 1.74 2019/01/01 10:06:54 hannken Exp $ */
+/* $NetBSD: tmpfs_vfsops.c,v 1.75 2019/10/04 12:34:40 mrg Exp $ */
/*
* Copyright (c) 2005, 2006, 2007 The NetBSD Foundation, Inc.
@@ -42,7 +42,7 @@
*/
#include
-__KERNEL_RCSID(0, "$NetBSD: tmpfs_vfsops.c,v 1.74 2019/01/01 10:06:54 hannken Exp $");
+__KERNEL_RCSID(0, "$NetBSD: tmpfs_vfsops.c,v 1.75 2019/10/04 12:34:40 mrg Exp $");
#include
#include
@@ -132,10 +132,6 @@ tmpfs_mount(struct mount *mp, const char
if (args->ta_root_uid == VNOVAL || args->ta_root_gid == VNOVAL)
return EINVAL;
- /* This can never happen? */
- if ((args->ta_root_mode & ALLPERMS) == VNOVAL)
- return EINVAL;
-
/* Get the memory usage limit for this file-system. */
if (args->ta_size_max < PAGE_SIZE) {
memlimit = UINT64_MAX;
CVS commit: src/sys/fs/tmpfs
Module Name:src Committed By: mrg Date: Fri Oct 4 12:34:40 UTC 2019 Modified Files: src/sys/fs/tmpfs: tmpfs_vfsops.c Log Message: remove an always false check and its' "This can never happen?" comment. To generate a diff of this commit: cvs rdiff -u -r1.74 -r1.75 src/sys/fs/tmpfs/tmpfs_vfsops.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/fs/tmpfs
Module Name:src Committed By: maxv Date: Sun Jul 14 05:58:44 UTC 2019 Modified Files: src/sys/fs/tmpfs: tmpfs_rename.c Log Message: Fix uninitialized variable: if 'tvp' is NULL, '*tdep' is not initialized. This could have caused the KASSERT to wrongfully fire. ok riastradh@ To generate a diff of this commit: cvs rdiff -u -r1.8 -r1.9 src/sys/fs/tmpfs/tmpfs_rename.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/fs/tmpfs
Module Name:src Committed By: maxv Date: Sun Jul 14 05:58:44 UTC 2019 Modified Files: src/sys/fs/tmpfs: tmpfs_rename.c Log Message: Fix uninitialized variable: if 'tvp' is NULL, '*tdep' is not initialized. This could have caused the KASSERT to wrongfully fire. ok riastradh@ To generate a diff of this commit: cvs rdiff -u -r1.8 -r1.9 src/sys/fs/tmpfs/tmpfs_rename.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/fs/tmpfs/tmpfs_rename.c diff -u src/sys/fs/tmpfs/tmpfs_rename.c:1.8 src/sys/fs/tmpfs/tmpfs_rename.c:1.9 --- src/sys/fs/tmpfs/tmpfs_rename.c:1.8 Mon Jul 6 10:24:59 2015 +++ src/sys/fs/tmpfs/tmpfs_rename.c Sun Jul 14 05:58:44 2019 @@ -1,4 +1,4 @@ -/* $NetBSD: tmpfs_rename.c,v 1.8 2015/07/06 10:24:59 wiz Exp $ */ +/* $NetBSD: tmpfs_rename.c,v 1.9 2019/07/14 05:58:44 maxv Exp $ */ /*- * Copyright (c) 2012 The NetBSD Foundation, Inc. @@ -34,7 +34,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: tmpfs_rename.c,v 1.8 2015/07/06 10:24:59 wiz Exp $"); +__KERNEL_RCSID(0, "$NetBSD: tmpfs_rename.c,v 1.9 2019/07/14 05:58:44 maxv Exp $"); #include #include @@ -282,7 +282,7 @@ tmpfs_gro_rename(struct mount *mp, kauth KASSERT(tcnp != NULL); KASSERT(tdep != NULL); KASSERT(fdep != tdep); - KASSERT((*fdep) != (*tdep)); + KASSERT((tvp == NULL) || (*fdep) != (*tdep)); KASSERT((*fdep) != NULL); KASSERT((*fdep)->td_node == VP_TO_TMPFS_NODE(fvp)); KASSERT((tvp == NULL) || ((*tdep) != NULL));
CVS commit: src/sys/fs/tmpfs
Module Name:src Committed By: maxv Date: Sat Jul 13 14:24:37 UTC 2019 Modified Files: src/sys/fs/tmpfs: tmpfs_mem.c Log Message: Remove the roundups, they are incorrect and cause memcmp to wrongfully fail because of uninitialized bytes at the end of the buffers. ok rmind@ To generate a diff of this commit: cvs rdiff -u -r1.9 -r1.10 src/sys/fs/tmpfs/tmpfs_mem.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/fs/tmpfs
Module Name:src
Committed By: maxv
Date: Sat Jul 13 14:24:37 UTC 2019
Modified Files:
src/sys/fs/tmpfs: tmpfs_mem.c
Log Message:
Remove the roundups, they are incorrect and cause memcmp to wrongfully fail
because of uninitialized bytes at the end of the buffers.
ok rmind@
To generate a diff of this commit:
cvs rdiff -u -r1.9 -r1.10 src/sys/fs/tmpfs/tmpfs_mem.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/fs/tmpfs/tmpfs_mem.c
diff -u src/sys/fs/tmpfs/tmpfs_mem.c:1.9 src/sys/fs/tmpfs/tmpfs_mem.c:1.10
--- src/sys/fs/tmpfs/tmpfs_mem.c:1.9 Mon Aug 22 23:07:36 2016
+++ src/sys/fs/tmpfs/tmpfs_mem.c Sat Jul 13 14:24:37 2019
@@ -1,4 +1,4 @@
-/* $NetBSD: tmpfs_mem.c,v 1.9 2016/08/22 23:07:36 skrll Exp $ */
+/* $NetBSD: tmpfs_mem.c,v 1.10 2019/07/13 14:24:37 maxv Exp $ */
/*
* Copyright (c) 2010, 2011 The NetBSD Foundation, Inc.
@@ -35,7 +35,7 @@
*/
#include
-__KERNEL_RCSID(0, "$NetBSD: tmpfs_mem.c,v 1.9 2016/08/22 23:07:36 skrll Exp $");
+__KERNEL_RCSID(0, "$NetBSD: tmpfs_mem.c,v 1.10 2019/07/13 14:24:37 maxv Exp $");
#include
#include
@@ -234,8 +234,8 @@ tmpfs_strname_free(struct tmpfs_mount *m
bool
tmpfs_strname_neqlen(struct componentname *fcnp, struct componentname *tcnp)
{
- const size_t fln = roundup2(fcnp->cn_namelen, TMPFS_NAME_QUANTUM);
- const size_t tln = roundup2(tcnp->cn_namelen, TMPFS_NAME_QUANTUM);
+ const size_t fln = fcnp->cn_namelen;
+ const size_t tln = tcnp->cn_namelen;
return (fln != tln) || memcmp(fcnp->cn_nameptr, tcnp->cn_nameptr, fln);
}
Re: CVS commit: src/sys/fs/tmpfs
On Wed, Jan 11, 2017 at 09:12:33PM +, David Holland wrote: > On Wed, Jan 11, 2017 at 12:12:33PM +, Joerg Sonnenberger wrote: > > Modified Files: > >src/sys/fs/tmpfs: tmpfs_vnops.c > > > > Log Message: > > Remove RO check in tmpfs_putpages for now, the syncer doesn't like the > > error code. > > Either removing it is wrong or it should be changed to KASSERT :-) So the problem is that the syncer will unconditionally call putpages e.g. on umount. It might need a two stage approach for dealing with dirty mmapped pages to implement properly, but for the use cases of read-only tmpfs I have (and likely others), it doesn't really matter. E.g. if you want to build multiple build chroots without paying for the extreme locking penalty of nullfs. Joerg
Re: CVS commit: src/sys/fs/tmpfs
On Wed, Jan 11, 2017 at 12:12:33PM +, Joerg Sonnenberger wrote: > Modified Files: > src/sys/fs/tmpfs: tmpfs_vnops.c > > Log Message: > Remove RO check in tmpfs_putpages for now, the syncer doesn't like the > error code. Either removing it is wrong or it should be changed to KASSERT :-) -- David A. Holland dholl...@netbsd.org
Re: CVS commit: src/sys/fs/tmpfs
J. Hannken-Illjes hann...@eis.cs.tu-bs.de wrote: On Jan 8, 2014, at 5:11 PM, pedro martelletto pe...@netbsd.org wrote: Module Name:src Committed By: pedro Date: Wed Jan 8 16:11:04 UTC 2014 Modified Files: src/sys/fs/tmpfs: tmpfs_subr.c Log Message: Allocate direntp on the stack in tmpfs_dir_getdents(), thus saving calls to kmem_zalloc() and kmem_free(); OK rmind@. From OpenBSD. Is it really a good idea to allocate 528 bytes on the kernel stack? File systems nest and already use much stack space. It is harmless in this case since we get a few or more pages for the stack. Looks better to use a pool_cache. It is worth to create a separate pool_cache(9) only if the allocations can potentially be very intensive. -- Mindaugas
Re: CVS commit: src/sys/fs/tmpfs
On Jan 8, 2014, at 5:11 PM, pedro martelletto pe...@netbsd.org wrote: Module Name: src Committed By: pedro Date: Wed Jan 8 16:11:04 UTC 2014 Modified Files: src/sys/fs/tmpfs: tmpfs_subr.c Log Message: Allocate direntp on the stack in tmpfs_dir_getdents(), thus saving calls to kmem_zalloc() and kmem_free(); OK rmind@. From OpenBSD. Is it really a good idea to allocate 528 bytes on the kernel stack? File systems nest and already use much stack space. Looks better to use a pool_cache. -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/sys/fs/tmpfs
On Jan 3, 2014, at 10:18 PM, J. Hannken-Illjes hann...@eis.cs.tu-bs.de wrote: On Jan 3, 2014, at 10:13 PM, Mindaugas Rasiukevicius rm...@netbsd.org wrote: Juergen Hannken-Illjes hann...@netbsd.org wrote: Module Name:src Committed By: hannken Date: Fri Jan 3 09:53:12 UTC 2014 Modified Files: src/sys/fs/tmpfs: tmpfs_subr.c tmpfs_vnops.c Log Message: Fix a race where thread1 runs VOP_REMOVE() and gets preempted in tmpfs_reclaim() before the call to tmpfs_free_node(). Thread2 runs VFS_FHTOVP() and gets a new vnode attached to the node thread1 is about to destroy. Change tmpfs_alloc_node() to always assign non-zero generation number and tmpfs_inactive() to set the generation number of unlinked nodes to zero. Can you explain how does this help? It still seems racy to me. Please describe the race in more detail. Tmpfs_fhtovp() will fail as soon as an unlinked tmpfs node drops its last vnode reference. Ok -- got it. We check the generation number too early in tmpfs_fhtovp(). Should be fixed with tmpfs_vfsops.c Rev. 1.55 -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/sys/fs/tmpfs
Juergen Hannken-Illjes hann...@netbsd.org wrote: Module Name: src Committed By: hannken Date: Fri Jan 3 09:53:12 UTC 2014 Modified Files: src/sys/fs/tmpfs: tmpfs_subr.c tmpfs_vnops.c Log Message: Fix a race where thread1 runs VOP_REMOVE() and gets preempted in tmpfs_reclaim() before the call to tmpfs_free_node(). Thread2 runs VFS_FHTOVP() and gets a new vnode attached to the node thread1 is about to destroy. Change tmpfs_alloc_node() to always assign non-zero generation number and tmpfs_inactive() to set the generation number of unlinked nodes to zero. Can you explain how does this help? It still seems racy to me. Why not just check for tn_links == 0 in tmpfs_fhtovp()? -- Mindaugas
Re: CVS commit: src/sys/fs/tmpfs
On Jan 3, 2014, at 10:13 PM, Mindaugas Rasiukevicius rm...@netbsd.org wrote: Juergen Hannken-Illjes hann...@netbsd.org wrote: Module Name: src Committed By:hannken Date:Fri Jan 3 09:53:12 UTC 2014 Modified Files: src/sys/fs/tmpfs: tmpfs_subr.c tmpfs_vnops.c Log Message: Fix a race where thread1 runs VOP_REMOVE() and gets preempted in tmpfs_reclaim() before the call to tmpfs_free_node(). Thread2 runs VFS_FHTOVP() and gets a new vnode attached to the node thread1 is about to destroy. Change tmpfs_alloc_node() to always assign non-zero generation number and tmpfs_inactive() to set the generation number of unlinked nodes to zero. Can you explain how does this help? It still seems racy to me. Please describe the race in more detail. Tmpfs_fhtovp() will fail as soon as an unlinked tmpfs node drops its last vnode reference. Why not just check for tn_links == 0 in tmpfs_fhtovp()? Because it is ok as long as the corresponding vnode is open/referenced. -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/sys/fs/tmpfs
On Nov 8, 2013, at 4:44 PM, Mindaugas Rasiukevicius rm...@netbsd.org wrote: Module Name: src Committed By: rmind Date: Fri Nov 8 15:44:23 UTC 2013 Modified Files: src/sys/fs/tmpfs: tmpfs.h tmpfs_rename.c tmpfs_subr.c tmpfs_vfsops.c tmpfs_vnops.c Log Message: tmpfs: replace the broken tmpfs_dircookie() logic which uses the node address truncated to 31 bits (required for 32-bit readdir compatibility, e.g. linux32). Instead, assign 2^31 range using the following logic: - The first half of the 2^31 is assigned incrementally (the fast path). - When exceeded, use the second half of 2^31, but manage with vmem(9). It will require 2 billion files per-directory to trigger vmem(9) usage. Also, while here, add some fixes for tmpfs_unmount(). Should fix PR/47739, PR/47480, PR/46088 and PR/41068. Thanks to wiz@ for stress testing. The tests fs/vfs/t_union/tmpfs_basic and fs/vfs/t_union/tmpfs_whiteout start failing after this commit. -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/sys/fs/tmpfs
J. Hannken-Illjes hann...@eis.cs.tu-bs.de wrote: Module Name:src Committed By: rmind Date: Fri Nov 8 15:44:23 UTC 2013 ... The tests fs/vfs/t_union/tmpfs_basic and fs/vfs/t_union/tmpfs_whiteout start failing after this commit. Fixed. -- Mindaugas
Re: CVS commit: src/sys/fs/tmpfs
Forgot to add that this also fixes a space leak in tmpfs_rename, introduced a couple months ago, which nobody reported as far as I know. The leak sometimes caused tmpfs_renamerace_dirs to fail with ENOSPC. The problem was that renaming a directory over an empty directory would fail to decrement the target's link count enough, so that the target would never be released.
Re: CVS commit: src/sys/fs/tmpfs
On Thu, Aug 18, 2011 at 09:51:50PM +, Taylor R Campbell wrote: Forgot to add that this also fixes a space leak in tmpfs_rename, introduced a couple months ago, which nobody reported as far as I know. The leak sometimes caused tmpfs_renamerace_dirs to fail with ENOSPC. The problem was that renaming a directory over an empty directory would fail to decrement the target's link count enough, so that the target would never be released. There was a thread somewhere a while back (in tech-kern I think) about refcount leaks in tmpfs. As I recall what changed a couple months ago was a semi-compensating error somewhere else... or maybe that was earlier. -- David A. Holland dholl...@netbsd.org
Re: CVS commit: src/sys/fs/tmpfs
On May 14, 2011, at 1:43 PM, Martin Husemann wrote: On Sat, May 14, 2011 at 10:34:05AM +0200, Marc Balmer wrote: What is the current state of C99 vs. older Cs? Do all arches / compilers we have support C99? We have lost the playstation2 port, because we don't have a working C99 compiler for it (so a -current kernel can not be compiled). No, we lost the playstation2 port because it required a special compiler and needs lots of one-off changes in the mips code.
Re: CVS commit: src/sys/fs/tmpfs
On Fri, May 13, 2011 at 06:37:53PM +0100, Mindaugas Rasiukevicius wrote: Generally - C99 is encouraged. However, I disagree that variables should be declared in the middle of context (for a minimum scope), unless there is a *clear* benefit. Otherwise, it makes code harder to read, especially if code fragment is long and/or complex. The i in for (int i=0; in; i++) isn't in the middle of a context; it's at the beginning of the for loop's context. so it should really be considered as a different case from other mid-block decls. -- David A. Holland dholl...@netbsd.org
Re: CVS commit: src/sys/fs/tmpfs
On Sat, May 14, 2011 at 2:37 AM, Mindaugas Rasiukevicius rm...@netbsd.org wrote: Matt Thomas m...@3am-software.com wrote: On May 7, 2011, at 5:03 PM, Christos Zoulas wrote: Module Name: src Committed By: christos Date: Sun May 8 00:03:35 UTC 2011 Modified Files: src/sys/fs/tmpfs: tmpfs_vnops.c Log Message: no c99 please. The kernel explicitly allows C99 and actually C99 is encouraged. So that should reverted :) Generally - C99 is encouraged. However, I disagree that variables should be declared in the middle of context (for a minimum scope), unless there is a *clear* benefit. Otherwise, it makes code harder to read, especially if code fragment is long and/or complex. I disagree. If variables are declared in the middle of context, those variables have narrower contexts. Narrowing context is always a win IMO. I'd like to hear the benefit not doing this (== the old convention). Benefits could be, e.g. use of const or limitation of the variable scope for performance sensitive code, also avoiding of #ifdefs, etc. In this case, I used for (int i = 0; ...) because the loop was in the beginning of context and #ifdef DEBUG-only. -- Mindaugas
Re: CVS commit: src/sys/fs/tmpfs
Am 10.05.11 02:34, schrieb Matt Thomas:
Module Name: src
Committed By: matt
Date: Tue May 10 00:34:26 UTC 2011
Modified Files:
src/sys/fs/tmpfs: tmpfs_vnops.c
Log Message:
yes, more C99 please (back out previous change).
After this committ/back-out/back-out-pf-the-back-out fest I have a
technical question:
What is the current state of C99 vs. older Cs? Do all arches /
compilers we have support C99? I assume gcc, llvm/clang are safe, but
what about pcc wrt C99?
I'd like a short clarification here, since this might influence my
coding... tnx.
To generate a diff of this commit:
cvs rdiff -u -r1.79 -r1.80 src/sys/fs/tmpfs/tmpfs_vnops.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/fs/tmpfs/tmpfs_vnops.c
diff -u src/sys/fs/tmpfs/tmpfs_vnops.c:1.79
src/sys/fs/tmpfs/tmpfs_vnops.c:1.80
--- src/sys/fs/tmpfs/tmpfs_vnops.c:1.79 Sun May 8 00:03:35 2011
+++ src/sys/fs/tmpfs/tmpfs_vnops.cTue May 10 00:34:26 2011
@@ -1,4 +1,4 @@
-/* $NetBSD: tmpfs_vnops.c,v 1.79 2011/05/08 00:03:35 christos Exp $
*/
+/* $NetBSD: tmpfs_vnops.c,v 1.80 2011/05/10 00:34:26 matt Exp $*/
/*
* Copyright (c) 2005, 2006, 2007 The NetBSD Foundation, Inc.
@@ -35,7 +35,7 @@
*/
#include sys/cdefs.h
-__KERNEL_RCSID(0, $NetBSD: tmpfs_vnops.c,v 1.79 2011/05/08 00:03:35
christos Exp $);
+__KERNEL_RCSID(0, $NetBSD: tmpfs_vnops.c,v 1.80 2011/05/10 00:34:26 matt
Exp $);
#include sys/param.h
#include sys/dirent.h
@@ -1466,8 +1466,7 @@
#if defined(DEBUG)
if (!error pgs) {
- int i;
- for (i = 0; i npages; i++) {
+ for (int i = 0; i npages; i++) {
KASSERT(pgs[i] != NULL);
}
}
Re: CVS commit: src/sys/fs/tmpfs
On 05/14/2011 10:34 AM, Marc Balmer wrote:
Am 10.05.11 02:34, schrieb Matt Thomas:
Module Name:src
Committed By: matt
Date: Tue May 10 00:34:26 UTC 2011
Modified Files:
src/sys/fs/tmpfs: tmpfs_vnops.c
Log Message:
yes, more C99 please (back out previous change).
After this committ/back-out/back-out-pf-the-back-out fest I have a
technical question:
What is the current state of C99 vs. older Cs? Do all arches /
compilers we have support C99? I assume gcc, llvm/clang are safe, but
what about pcc wrt C99?
pcc should be C99 compliant. If something do not work as expected, I'll
fix it.
-- Ragge
I'd like a short clarification here, since this might influence my
coding... tnx.
To generate a diff of this commit:
cvs rdiff -u -r1.79 -r1.80 src/sys/fs/tmpfs/tmpfs_vnops.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/fs/tmpfs/tmpfs_vnops.c
diff -u src/sys/fs/tmpfs/tmpfs_vnops.c:1.79 src/sys/fs/tmpfs/tmpfs_vnops.c:1.80
--- src/sys/fs/tmpfs/tmpfs_vnops.c:1.79 Sun May 8 00:03:35 2011
+++ src/sys/fs/tmpfs/tmpfs_vnops.c Tue May 10 00:34:26 2011
@@ -1,4 +1,4 @@
-/* $NetBSD: tmpfs_vnops.c,v 1.79 2011/05/08 00:03:35 christos Exp $
*/
+/* $NetBSD: tmpfs_vnops.c,v 1.80 2011/05/10 00:34:26 matt Exp $*/
/*
* Copyright (c) 2005, 2006, 2007 The NetBSD Foundation, Inc.
@@ -35,7 +35,7 @@
*/
#includesys/cdefs.h
-__KERNEL_RCSID(0, $NetBSD: tmpfs_vnops.c,v 1.79 2011/05/08 00:03:35 christos Exp
$);
+__KERNEL_RCSID(0, $NetBSD: tmpfs_vnops.c,v 1.80 2011/05/10 00:34:26 matt Exp
$);
#includesys/param.h
#includesys/dirent.h
@@ -1466,8 +1466,7 @@
#if defined(DEBUG)
if (!error pgs) {
- int i;
- for (i = 0; i npages; i++) {
+ for (int i = 0; i npages; i++) {
KASSERT(pgs[i] != NULL);
}
}
Re: CVS commit: src/sys/fs/tmpfs
Am 14.05.11 10:45, schrieb Anders Magnusson: [...] What is the current state of C99 vs. older Cs? Do all arches / compilers we have support C99? I assume gcc, llvm/clang are safe, but what about pcc wrt C99? pcc should be C99 compliant. If something do not work as expected, I'll fix it. cool! thanks for the update. -- Ragge [...]
Re: CVS commit: src/sys/fs/tmpfs
On Sat, 14 May 2011, Marc Balmer wrote: What is the current state of C99 vs. older Cs? Do all arches / compilers we have support C99? I assume gcc, llvm/clang are safe, but what about pcc wrt C99? I'd like a short clarification here, since this might influence my coding... tnx. pcc is a C99 compiler (with some gcc compatibility) which is still under development, though C99 feature support is complete. pcc is capable of building large parts of userland (I am running with /bin, /sbin and /usr/bin currently, and am going to install /usr/sbin soon), plus i386 kernels though there are still bugs to track down (eg no system crash but a build.sh failed, I think due to some corrupted files..) I'm thinking that though we have some support for C99 in tree, the 'official' position is that llvm/clang and pcc are not yet supported (eg there has been no such announcement of support, llvm/clang source is not yet in tree and the in-tree pcc is a year out of date). So IMO, apart from style issues (which it would be nice to update the share/misc/style document with), it should be safe to use any C99 features and, excepting some of the build tools which may be needed for bootstrapping, I don't think its useful to restrict ourselves to an older standard.. iain
Re: CVS commit: src/sys/fs/tmpfs
Masao Uebayashi uebay...@gmail.com wrote:
The kernel explicitly allows C99 and actually C99 is encouraged.
So that should reverted :)
Generally - C99 is encouraged. However, I disagree that variables
should be declared in the middle of context (for a minimum scope),
unless there is a *clear* benefit. Otherwise, it makes code harder
to read, especially if code fragment is long and/or complex.
I disagree. If variables are declared in the middle of context, those
variables have narrower contexts. Narrowing context is always a win
IMO.
I'd like to hear the benefit not doing this (== the old convention).
Benefit is code readability. It is easier to track the variables when
they are defined and initialised in the beginning of context.
If code is longer and/or complex - it likely has loops or conditional
statements, and variables can be defined in the beginning of *their*
context (basically, after { bracket). This is very encouraged.
For other cases, compilers do a good job anyway (if you do not believe
me - check with objdump) and there is no need to hurt code readability.
Benefits could be, e.g. use of const or limitation of the variable
scope for performance sensitive code, also avoiding of #ifdefs, etc.
In this case, I used for (int i = 0; ...) because the loop was in
the beginning of context and #ifdef DEBUG-only.
--
Mindaugas
--
Mindaugas
Re: CVS commit: src/sys/fs/tmpfs
On May 14, 12:00pm, rm...@netbsd.org (Mindaugas Rasiukevicius) wrote:
-- Subject: Re: CVS commit: src/sys/fs/tmpfs
| Benefit is code readability. It is easier to track the variables when
| they are defined and initialised in the beginning of context.
|
| If code is longer and/or complex - it likely has loops or conditional
| statements, and variables can be defined in the beginning of *their*
| context (basically, after { bracket). This is very encouraged.
|
| For other cases, compilers do a good job anyway (if you do not believe
| me - check with objdump) and there is no need to hurt code readability.
This whole discussion misses the point. The reason I changed the
code back to regular c from c99 is because the code did not compile.
This happened because rump did not pass -std=gnu99 in the compiler
flags. Now I did 'for (size_t i = 0;' in sys/crypto and the regression
tests failed. If we are going to be compiling the kernel in c99
mode, then I suggest that we do the same for userland instead of
turning it on for userland piecemeal.
christos
Re: CVS commit: src/sys/fs/tmpfs
On Sat, May 14, 2011 at 12:07:20PM -0400, Christos Zoulas wrote: If we are going to be compiling the kernel in c99 mode, then I suggest that we do the same for userland instead of turning it on for userland piecemeal. +1 is there anything we expect to break? -- David A. Holland dholl...@netbsd.org
Re: CVS commit: src/sys/fs/tmpfs
14.05.2011, 10:38, Masao Uebayashi uebay...@gmail.com: I disagree. If variables are declared in the middle of context, those variables have narrower contexts. Narrowing context is always a win IMO. That's true only if you don't use gotos. Otherwise, you might jump past an initialization point with obvious consequences. If I remember correctly, a compiler reports an error only for variable length arrays in this case. -- Alex
Re: CVS commit: src/sys/fs/tmpfs
On Sat, May 14, 2011 at 10:34:05AM +0200, Marc Balmer wrote: What is the current state of C99 vs. older Cs? Do all arches / compilers we have support C99? We have lost the playstation2 port, because we don't have a working C99 compiler for it (so a -current kernel can not be compiled). Martin
Re: CVS commit: src/sys/fs/tmpfs
On Tue, 10 May 2011, Takahiro Kambe wrote: In message 20110509170006.GA15831@marx.bitnet on Mon, 9 May 2011 20:00:06 +0300, Jukka Ruohonen jruoho...@iki.fi wrote: On Mon, May 09, 2011 at 06:50:08PM +0200, Adam Hoka wrote: So can we use for (int i = 0; ... ? :p lint(1) support them? Yes, lint(1) knows about it, see lint(7): 325 variable declaration in for loop and lint -S (use C9X mode) disables this warning.. iain
Re: CVS commit: src/sys/fs/tmpfs
In message alpine.neb.2.00.1105100716550.29...@galant.ukfsn.org on Tue, 10 May 2011 07:19:51 +0100 (BST), Iain Hibbert plu...@rya-online.net wrote: On Tue, 10 May 2011, Takahiro Kambe wrote: In message 20110509170006.GA15831@marx.bitnet on Mon, 9 May 2011 20:00:06 +0300, Jukka Ruohonen jruoho...@iki.fi wrote: On Mon, May 09, 2011 at 06:50:08PM +0200, Adam Hoka wrote: So can we use for (int i = 0; ... ? :p lint(1) support them? Yes, lint(1) knows about it, see lint(7): 325 variable declaration in for loop Wow, thanks much! -- Takahiro Kambe t...@back-street.net
Re: CVS commit: src/sys/fs/tmpfs
On May 8, 9:21pm, m...@3am-software.com (Matt Thomas) wrote: -- Subject: Re: CVS commit: src/sys/fs/tmpfs | The kernel explicitly allows C99 and actually C99 is encouraged. | So that should reverted :) Test it. Build a DEBUG kernel and when it works, you can or I will revert it. In this case it thinks that i is used out of the loop. christos
Re: CVS commit: src/sys/fs/tmpfs
On Mon, May 09, 2011 at 06:50:08PM +0200, Adam Hoka wrote: So can we use for (int i = 0; ... ? :p Hopefully not... - Jukka.
Re: CVS commit: src/sys/fs/tmpfs
On May 9, 2011, at 5:25 AM, Christos Zoulas wrote: On May 8, 9:21pm, m...@3am-software.com (Matt Thomas) wrote: -- Subject: Re: CVS commit: src/sys/fs/tmpfs | The kernel explicitly allows C99 and actually C99 is encouraged. | So that should reverted :) Test it. Build a DEBUG kernel and when it works, you can or I will revert it. Been doing DEBUG builds with tmpfs for a long time with no problems. In this case it thinks that i is used out of the loop. Where? After the for loop, it returns.
Re: CVS commit: src/sys/fs/tmpfs
On May 9, 10:23am, m...@3am-software.com (Matt Thomas) wrote: -- Subject: Re: CVS commit: src/sys/fs/tmpfs | | On May 9, 2011, at 5:25 AM, Christos Zoulas wrote: | | On May 8, 9:21pm, m...@3am-software.com (Matt Thomas) wrote: | -- Subject: Re: CVS commit: src/sys/fs/tmpfs | =20 | | The kernel explicitly allows C99 and actually C99 is encouraged. | | So that should reverted :) | =20 | Test it. Build a DEBUG kernel and when it works, you can or I will revert= | it. | | Been doing DEBUG builds with tmpfs for a long time with no problems. | | In this case it thinks that i is used out of the loop. | | Where? After the for loop, it returns. Well, just add back the commented out #CPPFLAGS+=-DDEBUG in /usr/src/sys/rump/Makefile.rump and build again. My MKDEBUG build causes that to happen too, and so it fails. christos
Re: CVS commit: src/sys/fs/tmpfs
In message 20110509170006.GA15831@marx.bitnet on Mon, 9 May 2011 20:00:06 +0300, Jukka Ruohonen jruoho...@iki.fi wrote: On Mon, May 09, 2011 at 06:50:08PM +0200, Adam Hoka wrote: So can we use for (int i = 0; ... ? :p lint(1) support them? Hopefully not... Me, too. -- Takahiro Kambe t...@back-street.net
Re: CVS commit: src/sys/fs/tmpfs
On May 9, 2011, at 4:35 PM, Christos Zoulas wrote: | Been doing DEBUG builds with tmpfs for a long time with no problems. | | In this case it thinks that i is used out of the loop. | | Where? After the for loop, it returns. Well, just add back the commented out #CPPFLAGS+=-DDEBUG in /usr/src/sys/rump/Makefile.rump and build again. My MKDEBUG build causes that to happen too, and so it fails. Oh. rump. Rump incorrectly didn't use -std=gnu99. That has now been fixed.
Re: CVS commit: src/sys/fs/tmpfs
On May 9, 2011, at 10:00 AM, Jukka Ruohonen wrote: On Mon, May 09, 2011 at 06:50:08PM +0200, Adam Hoka wrote: So can we use for (int i = 0; ... ? :p Hopefully not... For the kernel, for (int ... is allowed.
Re: CVS commit: src/sys/fs/tmpfs
On May 7, 2011, at 5:03 PM, Christos Zoulas wrote: Module Name: src Committed By: christos Date: Sun May 8 00:03:35 UTC 2011 Modified Files: src/sys/fs/tmpfs: tmpfs_vnops.c Log Message: no c99 please. The kernel explicitly allows C99 and actually C99 is encouraged. So that should reverted :)
Re: CVS commit: src/sys/fs/tmpfs
Simon Burge sim...@netbsd.org wrote: Simplify tmpfs_itimes() and use vfs_timestamp(). [ ... ] Was changing from getnanotime() to effectively nanotime() (via vfs_timestamp()) deliberate? The original intention of using getnanotime() for filesystem timestamps was that having a perfect timestamp was less important than the time taken to obtain the timestamp itself. Yes, deliberate. It is consistent with other *_itimes() routines. I doubt that such performance difference matters in this place. However, if we want to move to getnanotime(), then it should be changed in vfs_timestamp(). -- Mindaugas
Re: CVS commit: src/sys/fs/tmpfs
hi, Mindaugas Rasiukevicius wrote: Module Name: src Committed By:rmind Date:Wed Nov 11 09:59:42 UTC 2009 Modified Files: src/sys/fs/tmpfs: tmpfs_subr.c Log Message: Simplify tmpfs_itimes() and use vfs_timestamp(). [ ... ] Was changing from getnanotime() to effectively nanotime() (via vfs_timestamp()) deliberate? The original intention of using getnanotime() for filesystem timestamps was that having a perfect timestamp was less important than the time taken to obtain the timestamp itself. Cheers, Simon. perfect timestamp is important for nfs. freebsd has a knob to switch getnanotime/nanotime for vfs_timestamp for the tradeoff. YAMAMOTO Takashi
Re: CVS commit: src/sys/fs/tmpfs
On Tue Apr 28 2009 at 00:29:05 +, YAMAMOTO Takashi wrote: Hmm, does this work correctly if you find the component via the cache_lookup() path? Ok, I dug into this a little. Short answer: no, but ... It seems that cache_lookup() always returns false if MAKEENTRY is not set. However, it first does the lookup and removes the entry. Does anyone know why it then returns false and forces a relookup? Now in the case of tmpfs we always get 1 cache lookup and 2 full lookups for each remove/rename operation. because it's what ufs expects. :-) Oh right, it wants the offset. i introduced cache_lookup_raw for nfs, which doesn't want these behaviours. i wanted to replace cache_lookup but some people prefered the current one. Based on a quick look it seems like tmpfs could've switched to cache_lookup_raw() when directory caching was removed.
Re: CVS commit: src/sys/fs/tmpfs
hi, On Fri Apr 24 2009 at 18:00:45 +0300, Antti Kantee wrote: On Sat Apr 11 2009 at 20:42:59 +, Andrew Doran wrote: On Sat, Apr 11, 2009 at 12:21:57AM +, Perry E. Metzger wrote: Modified Files: src/sys/fs/tmpfs: tmpfs_vnops.c Log Message: SAVENAME was not set for rename and delete as required Patch from christos, fixes pr 41183 Now it leaks pathname buffers. Who reviewed this change and who okayed it? Hmm, does this work correctly if you find the component via the cache_lookup() path? Ok, I dug into this a little. Short answer: no, but ... It seems that cache_lookup() always returns false if MAKEENTRY is not set. However, it first does the lookup and removes the entry. Does anyone know why it then returns false and forces a relookup? Now in the case of tmpfs we always get 1 cache lookup and 2 full lookups for each remove/rename operation. because it's what ufs expects. :-) i introduced cache_lookup_raw for nfs, which doesn't want these behaviours. i wanted to replace cache_lookup but some people prefered the current one. YAMAMOTO Takashi
Re: CVS commit: src/sys/fs/tmpfs
On Sat Apr 11 2009 at 20:42:59 +, Andrew Doran wrote: On Sat, Apr 11, 2009 at 12:21:57AM +, Perry E. Metzger wrote: Modified Files: src/sys/fs/tmpfs: tmpfs_vnops.c Log Message: SAVENAME was not set for rename and delete as required Patch from christos, fixes pr 41183 Now it leaks pathname buffers. Who reviewed this change and who okayed it? Hmm, does this work correctly if you find the component via the cache_lookup() path?
Re: CVS commit: src/sys/fs/tmpfs
On Fri Apr 10 2009 at 22:36:38 +, Andrew Doran wrote: On Sat, Apr 11, 2009 at 01:32:09AM +0300, Antti Kantee wrote: On Fri Apr 10 2009 at 21:34:10 +, Andrew Doran wrote: On Fri, Apr 10, 2009 at 06:57:45PM +0200, Frank Kardel wrote: It may be related: I am now seeing a tmpfs uvm_fault(): hand copied bt: uvm_fault() tmpfs_do_detach() tmpfs_remove() VOP_REMOVE() do_sys_unlink() syscall() It may be this: http://gnats.netbsd.org/41183 Might be. I filed a PR for that ages ago and had forgotten all about it by now. See kern/38188. On the face of it what do you think of: - preserve pnbuf across entirety of operations that use it - retire SAVENAME, VOP_ABORTOP() - encapsulate operations with namei_init(), fini() or whatnot. I thought the general agreement was to move to deferring final lookup until the actual operation. I don't see the difference between namei_fini() and ABORTOP, although maybe the first is more explicit and less confusing for people not familiar with vfs. - copy pathname to persistent storage where required (NFS?) Or make pnbuf an object with a reference counter? But is that optimizing for the worst case? The two questions I'm always too tired to answer to myself are: 1) how does this work with nfs(d) 2) how does this work with layered file systems (ok, i peeked at unionfs. unionfs has a creative abortop... *punt*)
