Hello,
Holidays are over, I'm getting back to this diff, anyone is ok with it?
Martin Natano wrote:
> Hi,
>
> the diff reads fine to me, except for following nit-picks: It would be
> nice if the hashfree() function would check for negative/zero element
> counts, like hashinit() does. Also, new functionality should be
> documented in the manual.
>
> Please find below a regenerated diff including both changes.
>
> cheers,
> natano
>
>
> Index: sys//isofs/udf/udf_vfsops.c
> ===================================================================
> RCS file: /cvs/src/sys/isofs/udf/udf_vfsops.c,v
> retrieving revision 1.46
> diff -u -r1.46 udf_vfsops.c
> --- sys//isofs/udf/udf_vfsops.c 31 Aug 2015 06:56:25 -0000 1.46
> +++ sys//isofs/udf/udf_vfsops.c 24 Dec 2015 13:25:48 -0000
> @@ -440,8 +440,7 @@
> return (0);
>
> bail:
> - if (ump->um_hashtbl != NULL)
> - free(ump->um_hashtbl, M_UDFMOUNT, 0);
> + hashfree(ump->um_hashtbl, UDF_HASHTBLSIZE, M_UDFMOUNT);
>
> if (ump != NULL) {
> free(ump, M_UDFMOUNT, 0);
> @@ -490,9 +489,7 @@
> if (ump->um_stbl != NULL)
> free(ump->um_stbl, M_UDFMOUNT, 0);
>
> - if (ump->um_hashtbl != NULL)
> - free(ump->um_hashtbl, M_UDFMOUNT, 0);
> -
> + hashfree(ump->um_hashtbl, UDF_HASHTBLSIZE, M_UDFMOUNT);
> free(ump, M_UDFMOUNT, 0);
>
> mp->mnt_data = (qaddr_t)0;
> Index: sys//kern/kern_descrip.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/kern_descrip.c,v
> retrieving revision 1.127
> diff -u -r1.127 kern_descrip.c
> --- sys//kern/kern_descrip.c 17 Dec 2015 16:59:26 -0000 1.127
> +++ sys//kern/kern_descrip.c 24 Dec 2015 13:25:48 -0000
> @@ -1093,7 +1093,7 @@
> if (fdp->fd_rdir)
> vrele(fdp->fd_rdir);
> free(fdp->fd_knlist, M_TEMP, fdp->fd_knlistsize * sizeof(struct klist));
> - free(fdp->fd_knhash, M_TEMP, 0);
> + hashfree(fdp->fd_knhash, KN_HASHSIZE, M_TEMP);
> pool_put(&fdesc_pool, fdp);
> }
>
> Index: sys//kern/kern_event.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/kern_event.c,v
> retrieving revision 1.70
> diff -u -r1.70 kern_event.c
> --- sys//kern/kern_event.c 20 Dec 2015 17:56:18 -0000 1.70
> +++ sys//kern/kern_event.c 24 Dec 2015 13:25:49 -0000
> @@ -118,7 +118,6 @@
> knote_enqueue(kn); \
> } while(0)
>
> -#define KN_HASHSIZE 64 /* XXX should be
> tunable */
> #define KN_HASH(val, mask) (((val) ^ (val >> 8)) & (mask))
>
> extern struct filterops sig_filtops;
> Index: sys//kern/kern_subr.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/kern_subr.c,v
> retrieving revision 1.45
> diff -u -r1.45 kern_subr.c
> --- sys//kern/kern_subr.c 11 Dec 2015 16:07:02 -0000 1.45
> +++ sys//kern/kern_subr.c 24 Dec 2015 13:25:49 -0000
> @@ -183,6 +183,20 @@
> return (hashtbl);
> }
>
> +void
> +hashfree(void *hash, int elements, int type)
> +{
> + u_long hashsize;
> + LIST_HEAD(generic, generic) *hashtbl = hash;
> +
> + if (elements <= 0)
> + panic("hashfree: bad cnt");
> + for (hashsize = 1; hashsize < elements; hashsize <<= 1)
> + continue;
> +
> + free(hashtbl, type, sizeof(*hashtbl) * hashsize);
> +}
> +
> /*
> * "startup hook" types, functions, and variables.
> */
> Index: sys//netinet/ip_mroute.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/ip_mroute.c,v
> retrieving revision 1.89
> diff -u -r1.89 ip_mroute.c
> --- sys//netinet/ip_mroute.c 14 Nov 2015 15:54:27 -0000 1.89
> +++ sys//netinet/ip_mroute.c 24 Dec 2015 13:25:49 -0000
> @@ -611,7 +611,7 @@
> }
>
> memset(nexpire, 0, sizeof(nexpire));
> - free(mfchashtbl, M_MRTABLE, 0);
> + hashfree(mfchashtbl, MFCTBLSIZ, M_MRTABLE);
> mfchashtbl = NULL;
>
> ip_mrouter = NULL;
> Index: sys//ntfs/ntfs_ihash.c
> ===================================================================
> RCS file: /cvs/src/sys/ntfs/ntfs_ihash.c,v
> retrieving revision 1.19
> diff -u -r1.19 ntfs_ihash.c
> --- sys//ntfs/ntfs_ihash.c 14 Mar 2015 03:38:52 -0000 1.19
> +++ sys//ntfs/ntfs_ihash.c 24 Dec 2015 13:25:49 -0000
> @@ -70,7 +70,7 @@
>
> nthashtbl = hashinit(initialvnodes, M_NTFSNTHASH, M_WAITOK, &nthash);
> if (ntfs_nthashtbl) {
> - free(nthashtbl, M_NTFSNTHASH, 0);
> + hashfree(nthashtbl, initialvnodes, M_NTFSNTHASH);
> return;
> }
> ntfs_nthashtbl = nthashtbl;
> Index: sys//sys/event.h
> ===================================================================
> RCS file: /cvs/src/sys/sys/event.h,v
> retrieving revision 1.21
> diff -u -r1.21 event.h
> --- sys//sys/event.h 6 Oct 2015 03:29:35 -0000 1.21
> +++ sys//sys/event.h 24 Dec 2015 13:25:50 -0000
> @@ -123,6 +123,8 @@
> knote((list), (hint)); \
> } while (0)
>
> +#define KN_HASHSIZE 64 /* XXX should be
> tunable */
> +
> /*
> * Flag indicating hint is a signal. Used by EVFILT_SIGNAL, and also
> * shared by EVFILT_PROC (all knotes attached to p->p_klist)
> Index: sys//sys/systm.h
> ===================================================================
> RCS file: /cvs/src/sys/sys/systm.h,v
> retrieving revision 1.109
> diff -u -r1.109 systm.h
> --- sys//sys/systm.h 11 Dec 2015 16:07:02 -0000 1.109
> +++ sys//sys/systm.h 24 Dec 2015 13:25:50 -0000
> @@ -158,6 +158,7 @@
> int seltrue(dev_t dev, int which, struct proc *);
> int selfalse(dev_t dev, int which, struct proc *);
> void *hashinit(int, int, int, u_long *);
> +void hashfree(void *, int, int);
> int sys_nosys(struct proc *, void *, register_t *);
>
> void panic(const char *, ...)
> Index: sys//uvm/uvm_aobj.c
> ===================================================================
> RCS file: /cvs/src/sys/uvm/uvm_aobj.c,v
> retrieving revision 1.80
> diff -u -r1.80 uvm_aobj.c
> --- sys//uvm/uvm_aobj.c 21 Aug 2015 16:04:35 -0000 1.80
> +++ sys//uvm/uvm_aobj.c 24 Dec 2015 13:25:50 -0000
> @@ -388,7 +388,8 @@
> pool_put(&uao_swhash_elt_pool, elt);
> }
> }
> - free(aobj->u_swhash, M_UVMAOBJ, 0);
> +
> + hashfree(aobj->u_swhash, UAO_SWHASH_BUCKETS(aobj->u_pages),
> M_UVMAOBJ);
> } else {
> int i;
>
> @@ -470,7 +471,7 @@
> }
> }
>
> - free(aobj->u_swhash, M_UVMAOBJ, 0);
> + hashfree(aobj->u_swhash, UAO_SWHASH_BUCKETS(aobj->u_pages), M_UVMAOBJ);
>
> aobj->u_swhash = new_swhash;
> aobj->u_pages = pages;
> @@ -507,7 +508,7 @@
> }
> }
>
> - free(aobj->u_swhash, M_UVMAOBJ, 0);
> + hashfree(aobj->u_swhash, UAO_SWHASH_BUCKETS(aobj->u_pages), M_UVMAOBJ);
>
> aobj->u_swslots = new_swslots;
> aobj->u_pages = pages;
> @@ -627,7 +628,7 @@
> }
> }
>
> - free(aobj->u_swhash, M_UVMAOBJ, 0);
> + hashfree(aobj->u_swhash, UAO_SWHASH_BUCKETS(aobj->u_pages), M_UVMAOBJ);
>
> aobj->u_swhash = new_swhash;
> aobj->u_pages = pages;
> Index: share/man/man9/Makefile
> ===================================================================
> RCS file: /cvs/src/share/man/man9/Makefile,v
> retrieving revision 1.271
> diff -u -r1.271 Makefile
> --- share/man/man9/Makefile 20 Dec 2015 08:10:36 -0000 1.271
> +++ share/man/man9/Makefile 24 Dec 2015 13:28:09 -0000
> @@ -167,6 +167,7 @@
> MLINKS+=file.9 falloc.9 file.9 fdrelease.9 file.9 FREF.9 file.9 FRELE.9 \
> file.9 fd_getfile.9 file.9 getvnode.9 file.9 getsock.9
> MLINKS+=getdevvp.9 bdevvp.9 getdevvp.9 cdevvp.9
> +MLINKS+=hashinit.9 hashfree.9
> MLINKS+=hook_establish.9 hook_disestablish.9
> MLINKS+=hz.9 tick.9 hz.9 tickadj.9 hz.9 stathz.9 hz.9 profhz.9
> MLINKS+=idgen32.9 idgen32_init.9
> Index: share/man/man9/hashinit.9
> ===================================================================
> RCS file: /cvs/src/share/man/man9/hashinit.9,v
> retrieving revision 1.5
> diff -u -r1.5 hashinit.9
> --- share/man/man9/hashinit.9 4 Jun 2013 19:27:07 -0000 1.5
> +++ share/man/man9/hashinit.9 24 Dec 2015 13:28:09 -0000
> @@ -29,14 +29,15 @@
> .Dt HASHINIT 9
> .Os
> .Sh NAME
> -.Nm hashinit
> -.\" This should be ported from netbsd as well...
> -.\" .Nm hashdone
> +.Nm hashinit ,
> +.Nm hashfree
> .Nd kernel hashtable functions
> .Sh SYNOPSIS
> .In sys/systm.h
> .Ft void *
> .Fn hashinit "int num" "int type" "int flags" "u_long *mask"
> +.Ft void
> +.Fn hashfree "void *hash" "int num" "int type"
> .Sh DESCRIPTION
> The
> .Fn hashinit
> @@ -60,6 +61,17 @@
> hashing table.
> For an example of its use, see
> .Xr hash 9 .
> +.Pp
> +The
> +.Fn hashfree
> +function causes memory allocated by the
> +.Fn hashinit
> +function to be released.
> +The
> +.Fa num
> +and
> +.Fa type
> +arguments of related calls must match.
> .Sh RETURN VALUES
> The
> .Fn hashinit
>