Per-open file private data for the cdevs

[Kostik Belousov]

On Mon, May 12, 2008 at 10:03:14AM -1000, Jeff Roberson wrote:
> 
> On Mon, 12 May 2008, Kostik Belousov wrote:
> 
> >On Sun, May 11, 2008 at 03:40:14PM -1000, Jeff Roberson wrote:
> >>
> >>On Sun, 11 May 2008, Kostik Belousov wrote:
> >>
> >>>On Sun, May 11, 2008 at 02:50:30PM +0300, Kostik Belousov wrote:
> >>>>On Sat, May 10, 2008 at 09:53:12PM -1000, Jeff Roberson wrote:
> >>>>>On Sun, 4 May 2008, Kostik Belousov wrote:
> >>>>>
> >>>>>>Since the review for the clone-at-open patch (fdclone) posted some 
> >>>>>>time
> >>>>>>ago
> >>>>>>mostly says that it would be better to implement per-file private data
> >>>>>>instead, I produced the patch along this line,
> >>>>>>
> >>>>>>The patch does not change the cdevsw ABI, instead, three new functions
> >>>>>>nt	devfs_get_cdevpriv(void **datap);
> >>>>>>int	devfs_set_cdevpriv(void *priv, cdevpriv_dtr_t dtr);
> >>>>>>void	devfs_clear_cdevpriv(void);
> >>>>>>are provided for manipulation of the per-file private data.
> >>>>>>
> >>>>>>devfs_set_cdevpriv assigns the priv as private data for the file
> >>>>>>descriptor
> >>>>>>which is used to initiate currently performed driver operation. dtr
> >>>>>>is the function that will be called when either the last refernce to
> >>>>>>the file goes away or devfs_clear_cdevpriv is called.
> >>>>>>
> >>>>>>devfs_get_cdevpriv is the obvious accessor.
> >>>>>>
> >>>>>>devfs_clear_cdevpriv allows to clear the private data for the still
> >>>>>>open file.
> >>>>>>
> >>>>>>The synchronization of the cdev data and file private data is left
> >>>>>>to the driver code, I did not found any generic helper mechanism that
> >>>>>>could be useful there.
> >>>>>>
> >>>>>>Patch:
> >>>>>>http://people.freebsd.org/~kib/misc/fdpriv.1.patch
> >>>>>>
> >>>>>>Dumb driver that shows the basic usage of the proposed KPI:
> >>>>>>http://people.freebsd.org/~kib/misc/fpclone.c
> >>>>>>
> >>>>>>Previous version of the patch was tested by Peter Holm.
> >>>>>>
> >>>>>
> >>>>>Hi Kostik,
> >>>>>
> >>>>>Are these per-instances structures intended to be used by anything 
> >>>>>other
> >>>>>than devices?  If not can we make them a union with the DTYPE_VNODE
> >>>>>fields to save space?
> >>>>>
> >>>>>Thanks,
> >>>>>Jeff
> >>>>
> >>>>The current version of the patch is at
> >>>>http://people.freebsd.org/~kib/misc/fdpriv.3.patch
> >>>>
> >>>>Per insistence of John Baldwin and request of Eric Anholt, the 
> >>>>destructors
> >>>>are called now when either file is last closed, or the device is
> >>>>destroyed.
> >>>>This versions adds only one pointer to the struct file.
> >>>>
> >>>>Jeff, would you, please, explicitely specify what field you propose to
> >>>>union with the f_cdevpriv ?
> >>
> >>f_nextoff and f_seqcount are only used if vn_read() and vn_write() are
> >>used.  They do not apply to any other descriptors.
> >I use the f_cdevpriv != NULL as an indicator for the necessity to enter
> >the cdevpriv code, in particular, locking the cdevpriv_mtx, that would
> >otherwise needed to be entered at each last close. I think that one
> >pointer for the struct file is not too big cost, do you agree ?
> 
> No, it's not a big cost, however if it is possible to avoid that is best.
> 
> Can you not check the type before checking f_cdevpriv?  Should we not only 
> be checking cdevpriv in contexts where we know that it is not a vnode?

I am sorry, my english may be not enough, so I may interpret your
proposal mistakenly. I read it as a suggestion to check the file type
before accessing the f_cdevpriv.

The problem with the f_cdevpriv exists only at the _fdrop(). There, we
have a file of f_type == DTYPE_VNODE both for devfs and normal files.
I cannot check the f_vnode since the vnode may be reclaimed. The only
differentiator is the f_ops, that is devfs_ops_f for devfs file, and
vnops for the normal file during the file lifetime. Unfortunately,
f_ops is reset to the badfileops by vn_closefile before the _fdrop() is
getting called.

Reserving the flag in the f_flag looks not good due to interaction with
the userspace.

I do not want the callback to be called before the d_close() driver method
gets a chance to clean the data.
> 
> >
> >>
> >>>
> >>>Of course, I forgot to answer the question. Yes, the KPI is supposed to
> >>>be used by the drivers only. Please, note that device open files have
> >>>DTYPE_VNODE.
> >>>
> >
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/freebsd-arch/attachments/20080513/91c68e39/attachment.pgp