Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 17 Nov 2020 09:26:08 -0800
From:      John Baldwin <jhb@FreeBSD.org>
To:        Mateusz Guzik <mjg@FreeBSD.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   Re: svn commit: r367695 - in head/sys: kern sys
Message-ID:  <d3624785-1bc4-43d1-37b6-12a989444505@FreeBSD.org>
In-Reply-To: <202011141922.0AEJM2ld055995@repo.freebsd.org>
References:  <202011141922.0AEJM2ld055995@repo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On 11/14/20 11:22 AM, Mateusz Guzik wrote:
> Author: mjg
> Date: Sat Nov 14 19:22:02 2020
> New Revision: 367695
> URL: https://svnweb.freebsd.org/changeset/base/367695
> 
> Log:
>   thread: batch credential freeing
> 
> Modified:
>   head/sys/kern/kern_prot.c
>   head/sys/kern/kern_thread.c
>   head/sys/sys/ucred.h
> 
> Modified: head/sys/kern/kern_prot.c
> ==============================================================================
> --- head/sys/kern/kern_prot.c	Sat Nov 14 19:21:46 2020	(r367694)
> +++ head/sys/kern/kern_prot.c	Sat Nov 14 19:22:02 2020	(r367695)
> @@ -2007,6 +2071,17 @@ crfree(struct ucred *cr)
>  		mtx_unlock(&cr->cr_mtx);
>  		return;
>  	}
> +	crfree_final(cr);
> +}
> +
> +static void
> +crfree_final(struct ucred *cr)
> +{
> +
> +	KASSERT(cr->cr_users == 0, ("%s: users %d not == 0 on cred %p",
> +	    __func__, cr->cr_users, cr));
> +	KASSERT(cr->cr_ref == 0, ("%s: ref %d not == 0 on cred %p",
> +	    __func__, cr->cr_ref, cr));
>  	/*

Please add blank lines before comments.  It's in style(9) and I've noticed
a pattern in your changes of not including them.

> Modified: head/sys/sys/ucred.h
> ==============================================================================
> --- head/sys/sys/ucred.h	Sat Nov 14 19:21:46 2020	(r367694)
> +++ head/sys/sys/ucred.h	Sat Nov 14 19:22:02 2020	(r367695)
> @@ -114,6 +114,28 @@ struct xucred {
>  struct proc;
>  struct thread;
>  
> +struct credbatch {
> +	struct ucred *cred;
> +	int users;
> +	int ref;
> +};
> +
> +static inline void
> +credbatch_prep(struct credbatch *crb)
> +{
> +	crb->cred = NULL;
> +	crb->users = 0;
> +	crb->ref = 0;
> +}
> +void	credbatch_add(struct credbatch *crb, struct thread *td);
> +static inline void
> +credbatch_process(struct credbatch *crb)
> +{
> +
> +}
> +void	credbatch_add(struct credbatch *crb, struct thread *td);
> +void	credbatch_final(struct credbatch *crb);
> +

Do not mix prototypes and inlines, especially without spaces
around the prototype in the middle.  Also, the kernel uses __inline
rather than inline (for better or for worse).  Better would be:

static __inline void
credbatch_prep()
{
      ...
}

static __inline void
credbatch_process()
{
      ...
}

void credbatch_add();
void credbatch_final();

It seems you just have a duplicate credbatch_add() in fact.

Also, why have an empty inline function?

These changes would benefit from review.

-- 
John Baldwin



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?d3624785-1bc4-43d1-37b6-12a989444505>