David Howells <dhowells@xxxxxxxxxx> wrote:
>
> ...
>
> --- /dev/null
> +++ b/fs/fscache/cookie.c
> @@ -0,0 +1,1065 @@
> +/* cookie.c: general filesystem cache cookie management
> + *
> + * Copyright (C) 2004-5 Red Hat, Inc. All Rights Reserved.
> + * Written by David Howells (dhowells@xxxxxxxxxx)
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version
> + * 2 of the License, or (at your option) any later version.
> + */
> +
> +#include <linux/module.h>
> +#include "fscache-int.h"
> +
> +static LIST_HEAD(fscache_cache_tag_list);
> +static LIST_HEAD(fscache_cache_list);
> +static LIST_HEAD(fscache_netfs_list);
> +static DECLARE_RWSEM(fscache_addremove_sem);
> +static struct fscache_cache_tag fscache_nomem_tag;
> +
> +kmem_cache_t *fscache_cookie_jar;
> +
> +static void fscache_withdraw_object(struct fscache_cache *cache,
> + struct fscache_object *object);
> +
> +static void __fscache_cookie_put(struct fscache_cookie *cookie);
> +
> +static inline void fscache_cookie_put(struct fscache_cookie *cookie)
> +{
> +#ifdef CONFIG_DEBUG_SLAB
> + BUG_ON((atomic_read(&cookie->usage) & 0xffff0000) == 0x6b6b0000);
> +#endif
Interesting. Perhaps a comment in there?
> +/*****************************************************************************/
> +/*
> + * look up a cache tag
> + */
> +struct fscache_cache_tag *__fscache_lookup_cache_tag(const char *name)
> +{
> + struct fscache_cache_tag *tag, *xtag;
> +
> + /* firstly check for the existence of the tag under read lock */
> + down_read(&fscache_addremove_sem);
> +
> + list_for_each_entry(tag, &fscache_cache_tag_list, link) {
> + if (strcmp(tag->name, name) == 0) {
> + atomic_inc(&tag->usage);
> + up_read(&fscache_addremove_sem);
> + return tag;
> + }
> + }
> +
> + up_read(&fscache_addremove_sem);
> +
> + /* the tag does not exist - create a candidate */
> + xtag = kmalloc(sizeof(*tag) + strlen(name) + 1, GFP_KERNEL);
> + if (!xtag) {
> + /* return a dummy tag if out of memory */
> + up_read(&fscache_addremove_sem);
We already did an up_read().
> +/*****************************************************************************/
> +/*
> + * register a network filesystem for caching
> + */
> +int __fscache_register_netfs(struct fscache_netfs *netfs)
> +{
> + struct fscache_netfs *ptr;
> + int ret;
> +
> + _enter("{%s}", netfs->name);
> +
> + INIT_LIST_HEAD(&netfs->link);
> +
> + /* allocate a cookie for the primary index */
> + netfs->primary_index =
> + kmem_cache_alloc(fscache_cookie_jar, SLAB_KERNEL);
> +
> + if (!netfs->primary_index) {
> + _leave(" = -ENOMEM");
> + return -ENOMEM;
> + }
> +
> + /* initialise the primary index cookie */
> + memset(netfs->primary_index, 0, sizeof(*netfs->primary_index));
We have kmem_cache_zalloc() now.
> +EXPORT_SYMBOL(__fscache_register_netfs);
This code exports a huge number of symbols. Should they be non-GPL?
> +void fscache_withdraw_cache(struct fscache_cache *cache)
> +{
> + struct fscache_object *object;
> +
> + _enter("");
> +
> + printk(KERN_NOTICE
> + "FS-Cache: Withdrawing cache \"%s\"\n",
> + cache->tag->name);
> +
> + /* make the cache unavailable for cookie acquisition */
> + down_write(&cache->withdrawal_sem);
> +
> + down_write(&fscache_addremove_sem);
> + list_del_init(&cache->link);
> + cache->tag->cache = NULL;
> + up_write(&fscache_addremove_sem);
> +
> + /* mark all objects as being withdrawn */
> + spin_lock(&cache->object_list_lock);
> + list_for_each_entry(object, &cache->object_list, cache_link) {
> + set_bit(FSCACHE_OBJECT_WITHDRAWN, &object->flags);
> + }
> + spin_unlock(&cache->object_list_lock);
> +
> + /* make sure all pages pinned by operations on behalf of the netfs are
> + * written to disc */
> + cache->ops->sync_cache(cache);
> +
> + /* dissociate all the netfs pages backed by this cache from the block
> + * mappings in the cache */
> + cache->ops->dissociate_pages(cache);
> +
> + /* we now have to destroy all the active objects pertaining to this
> + * cache */
> + spin_lock(&cache->object_list_lock);
> +
> + while (!list_empty(&cache->object_list)) {
> + object = list_entry(cache->object_list.next,
> + struct fscache_object, cache_link);
> + list_del_init(&object->cache_link);
> + spin_unlock(&cache->object_list_lock);
> +
> + _debug("withdraw %p", object->cookie);
> +
> + /* we've extracted an active object from the tree - now dispose
> + * of it */
> + fscache_withdraw_object(cache, object);
> +
> + spin_lock(&cache->object_list_lock);
> + }
This looks livelockable.
> + spin_unlock(&cache->object_list_lock);
> +
> + fscache_release_cache_tag(cache->tag);
> + cache->tag = NULL;
> +
> + _leave("");
> +
> +} /* end fscache_withdraw_cache() */
> +
>
> ...
>
> +static struct fscache_object *fscache_lookup_object(struct fscache_cookie *cookie,
> + struct fscache_cache *cache)
> +{
> + struct fscache_cookie *parent = cookie->parent;
> + struct fscache_object *pobject, *object;
> + struct hlist_node *_p;
> +
> + _enter("{%s/%s},",
> + parent && parent->def ? parent->def->name : "",
> + cookie->def ? (char *) cookie->def->name : "<file>");
> +
> + if (test_bit(FSCACHE_IOERROR, &cache->flags))
> + return NULL;
> +
> + /* see if we have the backing object for this cookie + cache immediately
> + * to hand
> + */
> + object = NULL;
> + hlist_for_each_entry(object, _p,
> + &cookie->backing_objects, cookie_link
> + ) {
That's really weird-looking. How about
hlist_for_each_entry(object, _p, &cookie->backing_objects, cookie_link){
> + hlist_for_each_entry(pobject, _p,
> + &parent->backing_objects, cookie_link
> + ) {
> + if (pobject->cache == cache)
> + break;
> + }
Ditto.
> + if (!pobject) {
> + /* we don't know about the parent object */
> + up_read(&parent->sem);
> + down_write(&parent->sem);
> +
> + pobject = fscache_lookup_object(parent, cache);
> + if (IS_ERR(pobject)) {
> + up_write(&parent->sem);
> + _leave(" = %ld [no ipobj]", PTR_ERR(pobject));
> + return pobject;
> + }
> +
> + _debug("pobject=%p", pobject);
> +
> + BUG_ON(pobject->cookie != parent);
> +
> + downgrade_write(&parent->sem);
> + }
> +
> + /* now we can attempt to look up this object in the parent, possibly
> + * creating a representation on disc when we do so
> + */
> + object = cache->ops->lookup_object(cache, pobject, cookie);
> + up_read(&parent->sem);
> +
> + if (IS_ERR(object)) {
> + _leave(" = %ld [no obj]", PTR_ERR(object));
> + return object;
> + }
> +
> + /* keep track of it */
> + cache->ops->lock_object(object);
> +
> + BUG_ON(!hlist_unhashed(&object->cookie_link));
> +
> + /* attach to the cache's object list */
> + if (list_empty(&object->cache_link)) {
> + spin_lock(&cache->object_list_lock);
> + list_add(&object->cache_link, &cache->object_list);
> + spin_unlock(&cache->object_list_lock);
> + }
> +
> + /* attach to the cookie */
> + object->cookie = cookie;
> + atomic_inc(&cookie->usage);
> + hlist_add_head(&object->cookie_link, &cookie->backing_objects);
> +
> + /* done */
> + cache->ops->unlock_object(object);
I assume ->lock_object() provides the locking for the hlist_add_head()?
> + _leave(" = %p [new]", object);
> + return object;
> +
> +} /* end fscache_lookup_object() */
>
> ...
>
> +/*
> + * update the index entries backing a cookie
> + */
> +void __fscache_update_cookie(struct fscache_cookie *cookie)
> +{
> + struct fscache_object *object;
> + struct hlist_node *_p;
> +
> + if (cookie == FSCACHE_NEGATIVE_COOKIE) {
> + _leave(" [no cookie]");
> + return;
> + }
> +
> + _enter("{%s}", cookie->def->name);
> +
> + BUG_ON(!cookie->def->get_aux);
> +
> + down_write(&cookie->sem);
> + down_read(&cookie->parent->sem);
That's a surprising locking order. Normally things are parent-first.
> + /* update the index entry on disc in each cache backing this cookie */
> + hlist_for_each_entry(object, _p,
> + &cookie->backing_objects, cookie_link
> + ) {
> + if (!test_bit(FSCACHE_IOERROR, &object->cache->flags))
> + object->cache->ops->update_object(object);
> + }
> +
> + up_read(&cookie->parent->sem);
> + up_write(&cookie->sem);
> + _leave("");
> +
> +} /* end __fscache_update_cookie() */
> +
>
>...
>
> +int __fscache_pin_cookie(struct fscache_cookie *cookie)
> +{
> + struct fscache_object *object;
> + int ret;
> +
> + _enter("%p", cookie);
> +
> + if (hlist_empty(&cookie->backing_objects)) {
> + _leave(" = -ENOBUFS");
> + return -ENOBUFS;
> + }
> +
> + /* not supposed to use this for indexes */
> + BUG_ON(cookie->def->type == FSCACHE_COOKIE_TYPE_INDEX);
> +
> + /* prevent the file from being uncached whilst we access it and exclude
> + * read and write attempts on pages
> + */
> + down_write(&cookie->sem);
> +
> + ret = -ENOBUFS;
> + if (!hlist_empty(&cookie->backing_objects)) {
> + /* get and pin the backing object */
> + object = hlist_entry(cookie->backing_objects.first,
> + struct fscache_object, cookie_link);
> +
> + if (test_bit(FSCACHE_IOERROR, &object->cache->flags))
> + goto out;
> +
> + if (!object->cache->ops->pin_object) {
> + ret = -EOPNOTSUPP;
> + goto out;
> + }
> +
> + /* prevent the cache from being withdrawn */
> + if (down_read_trylock(&object->cache->withdrawal_sem)) {
trylock is often a sign that something is mucked up. A comment describing
why it is needed is always appropriate.
> + if (object->cache->ops->grab_object(object)) {
> + /* ask the cache to honour the operation */
> + ret = object->cache->ops->pin_object(object);
> +
> + object->cache->ops->put_object(object);
> + }
> +
> + up_read(&object->cache->withdrawal_sem);
> + }
> + }
> +
> +out:
> + up_write(&cookie->sem);
> + _leave(" = %d", ret);
> + return ret;
> +
> +} /* end __fscache_pin_cookie() */
> +
> +EXPORT_SYMBOL(__fscache_pin_cookie);
> +
> +/*****************************************************************************/
> +/*
> + * unpin an object into the cache
> + */
> +void __fscache_unpin_cookie(struct fscache_cookie *cookie)
> +{
> + struct fscache_object *object;
> + int ret;
> +
> + _enter("%p", cookie);
> +
> + if (hlist_empty(&cookie->backing_objects)) {
> + _leave(" [no obj]");
> + return;
> + }
> +
> + /* not supposed to use this for indexes */
> + BUG_ON(cookie->def->type == FSCACHE_COOKIE_TYPE_INDEX);
> +
> + /* prevent the file from being uncached whilst we access it and exclude
> + * read and write attempts on pages
> + */
> + down_write(&cookie->sem);
> +
> + ret = -ENOBUFS;
> + if (!hlist_empty(&cookie->backing_objects)) {
> + /* get and unpin the backing object */
> + object = hlist_entry(cookie->backing_objects.first,
> + struct fscache_object, cookie_link);
> +
> + if (test_bit(FSCACHE_IOERROR, &object->cache->flags))
> + goto out;
> +
> + if (!object->cache->ops->unpin_object)
> + goto out;
> +
> + /* prevent the cache from being withdrawn */
> + if (down_read_trylock(&object->cache->withdrawal_sem)) {
Ditto.
> + if (object->cache->ops->grab_object(object)) {
> + /* ask the cache to honour the operation */
> + object->cache->ops->unpin_object(object);
> +
> + object->cache->ops->put_object(object);
> + }
> +
> + up_read(&object->cache->withdrawal_sem);
> + }
> + }
> +
> +out:
> + up_write(&cookie->sem);
> + _leave("");
> +
> +} /* end __fscache_unpin_cookie() */
> +
>
>...
>
> +/*
> + * debug tracing
> + */
> +#define dbgprintk(FMT,...) \
> + printk("[%-6.6s] "FMT"\n",current->comm ,##__VA_ARGS__)
> +#define _dbprintk(FMT,...) do { } while(0)
> +
> +#define kenter(FMT,...) dbgprintk("==> %s("FMT")",__FUNCTION__ ,##__VA_ARGS__)
> +#define kleave(FMT,...) dbgprintk("<== %s()"FMT"",__FUNCTION__ ,##__VA_ARGS__)
> +#define kdebug(FMT,...) dbgprintk(FMT ,##__VA_ARGS__)
> +
> +#define kjournal(FMT,...) _dbprintk(FMT ,##__VA_ARGS__)
> +
> +#define dbgfree(ADDR) _dbprintk("%p:%d: FREEING %p",__FILE__,__LINE__,ADDR)
That's your fourth implementation of kenter(). Maybe we
need <linux/dhowells.h>?
> --- /dev/null
> +++ b/fs/fscache/fsdef.c
> @@ -0,0 +1,113 @@
> +/* fsdef.c: filesystem index definition
> + *
> + * Copyright (C) 2004 Red Hat, Inc. All Rights Reserved.
> + * Written by David Howells (dhowells@xxxxxxxxxx)
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version
> + * 2 of the License, or (at your option) any later version.
> + */
> +
> +#include <linux/module.h>
> +#include "fscache-int.h"
> +
> +static uint16_t fscache_fsdef_netfs_get_key(const void *cookie_netfs_data,
> + void *buffer, uint16_t bufmax);
> +
> +static uint16_t fscache_fsdef_netfs_get_aux(const void *cookie_netfs_data,
> + void *buffer, uint16_t bufmax);
u16 would be preferred.
> new file mode 100644
> index 0000000..613c3b1
> --- /dev/null
> +++ b/fs/fscache/main.c
> @@ -0,0 +1,150 @@
> +/* main.c: general filesystem caching manager
> + *
> + * Copyright (C) 2004 Red Hat, Inc. All Rights Reserved.
> + * Written by David Howells (dhowells@xxxxxxxxxx)
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version
> + * 2 of the License, or (at your option) any later version.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/sched.h>
> +#include <linux/completion.h>
> +#include <linux/slab.h>
> +#include "fscache-int.h"
> +
> +int fscache_debug = 0;
unneeded initialisation
> +/*****************************************************************************/
> +/*
> + * initialise the fs caching module
> + */
> +static int fscache_init(void)
__init?
> +/*****************************************************************************/
> +/*
> + * release the ktype
> + */
> +static void fscache_ktype_release(struct kobject *kobject)
> +{
> +} /* end fscache_ktype_release() */
Don't empty kobject release() functions upset Greg?
> +#if 0
> +void __cyg_profile_func_enter (void *this_fn, void *call_site)
> +__attribute__((no_instrument_function));
> +
> +void __cyg_profile_func_enter (void *this_fn, void *call_site)
> +{
> + asm volatile(" movl %%esp,%%edi \n"
> + " andl %0,%%edi \n"
> + " addl %1,%%edi \n"
> + " movl %%esp,%%ecx \n"
> + " subl %%edi,%%ecx \n"
> + " shrl $2,%%ecx \n"
> + " movl $0xedededed,%%eax \n"
> + " rep stosl \n"
> + :
> + : "i"(~(THREAD_SIZE-1)), "i"(sizeof(struct thread_info))
> + : "eax", "ecx", "edi", "memory", "cc"
> + );
> +}
> +
> +void __cyg_profile_func_exit(void *this_fn, void *call_site)
> +__attribute__((no_instrument_function));
> +
> +void __cyg_profile_func_exit(void *this_fn, void *call_site)
> +{
> + asm volatile(" movl %%esp,%%edi \n"
> + " andl %0,%%edi \n"
> + " addl %1,%%edi \n"
> + " movl %%esp,%%ecx \n"
> + " subl %%edi,%%ecx \n"
> + " shrl $2,%%ecx \n"
> + " movl $0xdadadada,%%eax \n"
> + " rep stosl \n"
> + :
> + : "i"(~(THREAD_SIZE-1)), "i"(sizeof(struct thread_info))
> + : "eax", "ecx", "edi", "memory", "cc"
> + );
> +}
> +#endif
Removeable?
> diff --git a/fs/fscache/page.c b/fs/fscache/page.c
> new file mode 100644
> index 0000000..b197be7
> --- /dev/null
> +++ b/fs/fscache/page.c
> @@ -0,0 +1,548 @@
> +/* page.c: general filesystem cache cookie management
> + *
> + * Copyright (C) 2004-5 Red Hat, Inc. All Rights Reserved.
> + * Written by David Howells (dhowells@xxxxxxxxxx)
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version
> + * 2 of the License, or (at your option) any later version.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/fscache-cache.h>
> +#include <linux/buffer_head.h>
> +#include <linux/pagevec.h>
> +#include "fscache-int.h"
> +
> +/*****************************************************************************/
> +/*
> + * set the data file size on an object in the cache
> + */
> +int __fscache_set_i_size(struct fscache_cookie *cookie, loff_t i_size)
> +{
> + struct fscache_object *object;
> + int ret;
> +
> + _enter("%p,%llu,", cookie, i_size);
> +
> + if (hlist_empty(&cookie->backing_objects)) {
> + _leave(" = -ENOBUFS");
> + return -ENOBUFS;
> + }
> +
> + /* not supposed to use this for indexes */
> + BUG_ON(cookie->def->type == FSCACHE_COOKIE_TYPE_INDEX);
> +
> + /* prevent the file from being uncached whilst we access it and exclude
> + * read and write attempts on pages
> + */
> + down_write(&cookie->sem);
> +
> + ret = -ENOBUFS;
> + if (!hlist_empty(&cookie->backing_objects)) {
> + /* get and pin the backing object */
> + object = hlist_entry(cookie->backing_objects.first,
> + struct fscache_object, cookie_link);
> +
> + if (test_bit(FSCACHE_IOERROR, &object->cache->flags))
> + goto out;
> +
> + /* prevent the cache from being withdrawn */
> + if (object->cache->ops->set_i_size &&
> + down_read_trylock(&object->cache->withdrawal_sem)
another trylock.
> + ) {
> + if (object->cache->ops->grab_object(object)) {
> + /* ask the cache to honour the operation */
> + ret = object->cache->ops->set_i_size(object,
> + i_size);
> +
> + object->cache->ops->put_object(object);
> + }
> +
> + up_read(&object->cache->withdrawal_sem);
> + }
> + }
> +
> +out:
> + up_write(&cookie->sem);
> + _leave(" = %d", ret);
> + return ret;
> +
> +} /* end __fscache_set_i_size() */
> +
> +EXPORT_SYMBOL(__fscache_set_i_size);
> +
> +/*****************************************************************************/
> +/*
> + * reserve space for an object
> + */
> +int __fscache_reserve_space(struct fscache_cookie *cookie, loff_t size)
> +{
> + struct fscache_object *object;
> + int ret;
> +
> + _enter("%p,%llu,", cookie, size);
> +
> + if (hlist_empty(&cookie->backing_objects)) {
> + _leave(" = -ENOBUFS");
> + return -ENOBUFS;
> + }
> +
> + /* not supposed to use this for indexes */
> + BUG_ON(cookie->def->type == FSCACHE_COOKIE_TYPE_INDEX);
> +
> + /* prevent the file from being uncached whilst we access it and exclude
> + * read and write attempts on pages
> + */
> + down_write(&cookie->sem);
> +
> + ret = -ENOBUFS;
> + if (!hlist_empty(&cookie->backing_objects)) {
> + /* get and pin the backing object */
> + object = hlist_entry(cookie->backing_objects.first,
> + struct fscache_object, cookie_link);
> +
> + if (test_bit(FSCACHE_IOERROR, &object->cache->flags))
> + goto out;
> +
> + if (!object->cache->ops->reserve_space) {
> + ret = -EOPNOTSUPP;
> + goto out;
> + }
> +
> + /* prevent the cache from being withdrawn */
> + if (down_read_trylock(&object->cache->withdrawal_sem)) {
and another.
> + if (object->cache->ops->grab_object(object)) {
> + /* ask the cache to honour the operation */
> + ret = object->cache->ops->reserve_space(object,
> + size);
> +
> + object->cache->ops->put_object(object);
> + }
> +
> + up_read(&object->cache->withdrawal_sem);
> + }
> + }
> +
> +out:
> + up_write(&cookie->sem);
> + _leave(" = %d", ret);
> + return ret;
> +
> +} /* end __fscache_reserve_space() */
> +
>
> ...
>
> + if (down_read_trylock(&object->cache->withdrawal_sem)) {
> + if (down_read_trylock(&object->cache->withdrawal_sem)) {
> + if (down_read_trylock(&object->cache->withdrawal_sem)) {
> + if (down_read_trylock(&object->cache->withdrawal_sem)) {
> + if (down_read_trylock(&object->cache->withdrawal_sem)) {
> + if (down_read_trylock(&object->cache->withdrawal_sem)) {
> + if (down_read_trylock(&object->cache->withdrawal_sem)) {
more.
> +
> +/* find the parent index object for a object */
> +static inline
> +struct fscache_object *fscache_find_parent_object(struct fscache_object *object)
> +{
> + struct fscache_object *parent;
> + struct fscache_cookie *cookie = object->cookie;
> + struct fscache_cache *cache = object->cache;
> + struct hlist_node *_p;
> +
> + hlist_for_each_entry(parent, _p,
> + &cookie->parent->backing_objects,
> + cookie_link
> + ) {
> + if (parent->cache == cache)
> + return parent;
> + }
> +
> + return NULL;
> +}
Large?
> +#endif /* _LINUX_FSCACHE_CACHE_H */
> diff --git a/include/linux/fscache.h b/include/linux/fscache.h
> new file mode 100644
> index 0000000..8aa464b
> --- /dev/null
> +++ b/include/linux/fscache.h
> @@ -0,0 +1,484 @@
> +/* fscache.h: general filesystem caching interface
> + *
> + * Copyright (C) 2004-5 Red Hat, Inc. All Rights Reserved.
> + * Written by David Howells (dhowells@xxxxxxxxxx)
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version
> + * 2 of the License, or (at your option) any later version.
> + */
> +
> +#ifndef _LINUX_FSCACHE_H
> +#define _LINUX_FSCACHE_H
> +
> +#include <linux/config.h>
> +#include <linux/fs.h>
> +#include <linux/list.h>
> +#include <linux/pagemap.h>
> +#include <linux/pagevec.h>
> +
> +#ifdef CONFIG_FSCACHE_MODULE
> +#define CONFIG_FSCACHE
> +#endif
Defining symbols which are owned by the Kconfig system isn't very nice.
> +
> +/*
> + * request a page be stored in the cache
> + * - this request may be ignored if no cache block is currently allocated, in
> + * which case it:
> + * - returns -ENOBUFS
> + * - if a cache block was already allocated:
> + * - a BIO will be dispatched to write the page (end_io_func will be called
> + * from the completion function)
> + * - returns 0
> + */
> +#ifdef CONFIG_FSCACHE
> +extern int __fscache_write_page(struct fscache_cookie *cookie,
> + struct page *page,
> + fscache_rw_complete_t end_io_func,
> + void *end_io_data,
> + gfp_t gfp);
hm. I tend to find it better for functions to be documented at the
definition site, not at the declaration site. I don't expect people even
think to go looking in a header file to learn about the function which
they're presently looking at.
