Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 15 Oct 2003 13:36:56 -0700 (PDT)
From:      Nate Lawson <nate@root.org>
To:        Poul-Henning Kamp <phk@FreeBSD.org>
Cc:        cvs-all@FreeBSD.org
Subject:   Re: cvs commit: src/sys/sys conf.h src/sys/fs/specfs spec_vnops.c
Message-ID:  <20031015133353.W35236@root.org>
In-Reply-To: <20031015200134.3404116A558@hub.freebsd.org>
References:  <20031015200134.3404116A558@hub.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, 15 Oct 2003, Poul-Henning Kamp wrote:
>   Modified files:
>     sys/sys              conf.h
>     sys/fs/specfs        spec_vnops.c
>   Log:
>   Introduce a new optional memberfunction for cdevsw, fdopen() which
>   passes the fdidx from VOP_OPEN down.
>
>   This is for all I know the final API for this functionality, but
>   the locking semantics for messing with the filedescriptor from
>   the device driver are not settled at this time.
>
>   Revision  Changes    Path
>   1.211     +7 -2      src/sys/fs/specfs/spec_vnops.c
>   1.178     +2 -0      src/sys/sys/conf.h
>
>
> Index: src/sys/fs/specfs/spec_vnops.c
> diff -u src/sys/fs/specfs/spec_vnops.c:1.210 src/sys/fs/specfs/spec_vnops.c:1.211
> --- src/sys/fs/specfs/spec_vnops.c:1.210	Sat Oct  4 02:20:00 2003
> +++ src/sys/fs/specfs/spec_vnops.c	Wed Oct 15 13:00:59 2003
> @@ -196,9 +196,14 @@
>  	VOP_UNLOCK(vp, 0, td);
>  	if(dsw->d_flags & D_NOGIANT) {
>  		DROP_GIANT();
> -		error = dsw->d_open(dev, ap->a_mode, S_IFCHR, td);
> +		if (dsw->d_fdopen != NULL)
> +			error = dsw->d_fdopen(dev, ap->a_mode, td, ap->a_fdidx);
> +		else
> +			error = dsw->d_open(dev, ap->a_mode, S_IFCHR, td);
>  		PICKUP_GIANT();
> -	} else
> +	} else if (dsw->d_fdopen != NULL)
> +		error = dsw->d_fdopen(dev, ap->a_mode, td, ap->a_fdidx);
> +	else
>  		error = dsw->d_open(dev, ap->a_mode, S_IFCHR, td);
>  	vn_lock(vp, LK_EXCLUSIVE | LK_RETRY, td);

I assume this is to avoid a trip through a vnode when doing IO to a
device?  Can you point me at the analysis of this approach?  I've heard
you talking about it before but don't have a reference.

> Index: src/sys/sys/conf.h
> diff -u src/sys/sys/conf.h:1.177 src/sys/sys/conf.h:1.178
> --- src/sys/sys/conf.h:1.177	Sun Sep 28 13:50:36 2003
> +++ src/sys/sys/conf.h	Wed Oct 15 13:00:58 2003
> @@ -145,6 +145,7 @@
>  typedef struct thread d_thread_t;
>
>  typedef int d_open_t(dev_t dev, int oflags, int devtype, struct thread *td);
> +typedef int d_fdopen_t(dev_t dev, int oflags, struct thread *td, int fdidx);
>  typedef int d_close_t(dev_t dev, int fflag, int devtype, struct thread *td);
>  typedef void d_strategy_t(struct bio *bp);
>  typedef int d_ioctl_t(dev_t dev, u_long cmd, caddr_t data,
> @@ -223,6 +224,7 @@
>  	u_int		d_flags;
>  	const char	*d_name;
>  	d_open_t	*d_open;
> +	d_fdopen_t	*d_fdopen;
>  	d_close_t	*d_close;
>  	d_read_t	*d_read;
>  	d_write_t	*d_write;

Sure we have C99 now but for binary compatibility with third party
drivers, shouldn't this be added at the end of the structure?  Especially
since this is an optional function.

-Nate



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20031015133353.W35236>