[pnfs] pnfsd export/callback API [was Re: [PATCH] pnfsd: Notify device ID changes]
Marc Eshel
eshel at almaden.ibm.com
Fri Jun 13 17:11:28 EDT 2008
"J. Bruce Fields" <bfields at fieldses.org> wrote on 06/13/2008 01:25:07 PM:
> "J. Bruce Fields" <bfields at fieldses.org>
> 06/13/2008 01:25 PM
>
> To
>
> Marc Eshel <eshel at almaden.ibm.com>
>
> cc
>
> Benny Halevy <bhalevy at panasas.com>, pNFS Mailing List
<pnfs at linux-nfs.org>
>
> Subject
>
> Re: [pnfs] pnfsd export/callback API [was Re: [PATCH] pnfsd: Notify
> device ID changes]
>
> On Thu, Jun 12, 2008 at 01:37:41PM -0700, Marc Eshel wrote:
> > "J. Bruce Fields" <bfields at fieldses.org> wrote on 06/12/2008 11:32:05
AM:
> >
> > > Re: [pnfs] pnfsd export/callback API [was Re: [PATCH] pnfsd: Notify
> > > device ID changes]
> > >
> > > On Thu, Jun 12, 2008 at 08:34:46PM +0300, Benny Halevy wrote:
> > > > Bruce, here's the thread on which we last discussed the
> > > > pnfsd API issues. I'd appreciate your input on it.
> > >
> > > I agree that something like your struct pnfsd_callback_operations,
that
> > > keeps calls into nfsd separate from calls into the filessytem, makes
> > > sense.
> > >
> > > There need only be one struct pnfsd_callback_operations on the whole
> > > system (because there's only one nfsd), though there may be multiple
> > > pointers to it.
> > >
> > > One problem you don't address below is: how does the filesystem find
> > > that struct pnfsd_callback_operations? It must be created and
filled in
> > > by nfsd. We have to guarantee that nobody will try to call one of
its
> > > functions when nfsd isn't loaded. Filesystems shouldn't just refer
to
> > > it by name, since, as Marc says, we probably don't want to force
nfsd to
> > > be loaded before any filesystem that supports pnfs is loaded.
> > >
> > > I'm also not sure where to store the pointer to it. Is the
superblock
> > > really the right place?
> >
> > I think it needs to be the superblock, can you think of a better
place?
>
> Might be OK. Or maybe it could just be a global?
Then you need an exported symbol which also create a dependency on nfsd by
the fs.
>
> Or if you know you only need to use it while you hold a layout (for
> example) then it might make sense to put it there.
>
Maybe we can pass it to the fs when it calls the export operations that
checks if the fs supports pNFS but we still don't know when we can not use
it anymore.
> >
> > >
> > > What are the natural places in the filesystem code where the
filesystem
> > > a) knows it may need to do pnfs callbacks, and b) knows it no longer
> > > needs to do callbacks?
> >
> > I prefer not to have to keep track of the need for the calls,
>
> So there's not some object (layout, or something else?) whose lifetime
> naturally bounds the time when you'd need the callbacks?
no
>
> > maybe we can
> > just zero the pointer in the superblock when nfsd is unloaded.
>
> If the pointer is something owned by the filesystem (e.g. it's in the
> superblock) then how will nfsd know to zero it?
>
> I guess you could something like:
>
> superblock->sb_pnfs_callbacks = symbol_get(nfsd_pnfs_callbacks)
>
> and let that hold the reference on nfsd until the filesystem's unmounted
> (at which point you do a symbol_put())? That'd keep nfsd referenced
> longer than necessary, but maybe nobody cares.
That sounds like a possibility.
>
> > >
> > > E.g. does it only do callbacks while someone's got a layout?
> > >
> > > By the way, a miscellaneous question:
> > >
> > > > Maybe it's just weird to set up the pointer in struct
> > export_operations
> > > > since the calls into the pnfsd go in the opposite direction.
> > > >
> > > > I'd rather separate the two APIs, for example:
> > > >
> > > > in include/linux/nfsd/nfsd4_pnfs.h:
> > > >
> > > > enum pnfs_export_capabilities {
> > > > PNFS_EXPORT_CAN_MERGE_LAYOUTS = 1 << 0,
> > > > };
> > > >
> > > > /*
> > > > * pNFS filesystem export API
> > > > */
> > > > struct pnfs_export {
> > > > u32 layout_type;
> > > > u32 policy_flags;
> > > >
> > > > int (* layout_get) (struct inode *inode,
> > > > struct pnfs_layoutget_arg *arg);
> > > > int (* layout_commit) (struct inode *inode,
> > > > struct nfsd4_pnfs_layoutcommit * *p);
> > > > int (* layout_return) (struct inode *inode,
> > > > struct nfsd4_pnfs_layoutreturn *p);
> > > >
> > > > void (* get_verifier) (struct super_block *sb, u32 *p);
> > > >
> > > > int (* get_device_info) (struct super_block *sb,
> > > > struct pnfs_devinfo_arg *arg);
> > > > int (* get_device_iter) (struct super_block *sb,
> > > > struct pnfs_deviter_arg *arg);
> > >
> > > How does the caller free a pnfs_devinfo_arg, and when do they know
it's
> > > safe to do so?
> >
> > When the call returns.
>
> So they free it just with a kfree()?
Who are they? the nfsd doesn't free anything. The fs puts the information
into the xdr reply.
For layout_get nfsd->fs->encode-xdr-reply.
>
> --b.
>
> > If nfsd need to keep any information it needs to
> > make a copy.
> >
> > >
> > > --b.
> > >
> > > > };
> > > >
> > > > /*
> > > > * pnfsd callback methods called by the filesystem
> > > > */
> > > > struct pnfsd_callback_operations {
> > > > int (* cb_layout_recall) (struct super_block *sb, struct inode
> > *inode,
> > > > struct nfsd4_pnfs_cb_layout *p);
> > > >
> > > > int (* cb_device_notify) (struct super_block *sb,
> > > > struct nfsd4_pnfs_cb_device *p);
> > > >
> > > > /* callbacks from fs on MDS only */
> > > > int (* mds_get_state) (struct super_block *sb,
> > > > struct pnfs_get_state *state);
> > > >
> > > > /* callbacks from fs on DS only */
> > > > int (* ds_get_state) (struct inode *inode, void *fh,
> > > > struct pnfs_get_state *state);
> > > >
> > > > /// we haven't implemented this yet */
> > > > int (* ds_change_state) (struct pnfs_get_state *state);
> > > > };
> > > >
> > > > in include/linux/fs.h:
> > > >
> > > > struct pnfs_export;
> > > > struct pnfsd_callback_operations;
> > > >
> > > > struct super_block {
> > > > ...
> > > > const struct export_operations *s_export_op;
> > > > #if defined(CONFIG_PNFSD)
> > > > const struct pnfs_export *s_pnfs_export;
> > > > const struct pnfsd_callback_operations *s_pnfsd_cb_op;
> > > > #endif /* CONFIG_PNFSD */
> > > > };
> > > >
> > > > Benny
> > > >
> > > >
> > > _______________________________________________
> > > pNFS mailing list
> > > pNFS at linux-nfs.org
> > > http://linux-nfs.org/cgi-bin/mailman/listinfo/pnfs
> >
More information about the pNFS
mailing list