linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Mark Peloquin" <peloquin@us.ibm.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: torvalds@transmeta.com, linux-kernel@vger.kernel.org,
	evms-devel@lists.sourceforge.net
Subject: Re: [Evms-devel] Re: [PATCH] EVMS core 2/4: evms.h
Date: Thu, 3 Oct 2002 13:59:47 -0500	[thread overview]
Message-ID: <OF25D731E3.0E245DDC-ON85256C47.00645AA7@pok.ibm.com> (raw)


On 10/03/2002 at 9:50 AM, Christoph Hellwig wrote:

>> +/**
>> + * helpful PROCFS macro
>> + **/
>> +#ifdef CONFIG_PROC_FS
>> +#define PROCPRINT(msg, args...) (sz += sprintf(page + sz, msg, ##
args));\
>> +              if (sz < off)\
>> +                    off -= sz, sz = 0;\
>> +              else if (sz >= off + count)\
>> +                    goto out
>> +#endif

> I think this really wants to be converted to the seq_file interface..

We plan to.

>> +
>> +/**
>> + * PluginID convenience macros
>> + *
>> + * An EVMS PluginID is a 32-bit number with the following bit
positions:
>> + * Top 16 bits: OEM identifier. See IBM_OEM_ID.
>> + * Next 4 bits: Plugin type identifier. See evms_plugin_code.
>> + * Lowest 12 bits: Individual plugin identifier within a given plugin
type.
>> + **/
>> +#define SetPluginID(oem, type, id) ((oem << 16) | (type << 12) | id)
>> +#define GetPluginOEM(pluginid) (pluginid >> 16)
>> +#define GetPluginType(pluginid) ((pluginid >> 12) & 0xf)
>> +#define GetPluginID(pluginid) (pluginid & 0xfff)

> What is the prupose of OEM IDs?

To allow unique identification of plugins. If you wrote
plugin, you might give it an ID of 1. Someone else
may do the same thing. However, by letting you add
something specific which identifies you (i.e. like
your initials), then possibility of collisions is
reduced.

>> +struct evms_plugin_header {
>> +  u32 id;
>> +  struct evms_version version;
>> +  struct evms_version required_services_version;
>> +  struct evms_plugin_fops *fops;
>> +  struct list_head headers;
>> +};

> What is the required services version?

The common services are a set of functions exported
by the core code. We have major,minor,patchlevel
versions for them. Plugin writers specify the
version of the interface they are coded to comply
with. Mismatching core services and plugin
version expectations are caught at plugin registration
(load) time, and prevented from being usable.

>> +struct evms_plugin_fops {

> What about evms_plugin_ops?

>> +  int (*ioctl) (struct evms_logical_node *, struct inode *,
>> +              struct file *, u32, unsigned long);
>> +  int (*direct_ioctl) (struct inode *, struct file *,
>> +                   u32, unsigned long);

> Umm, why do you need two ioctl handlers?

the IOCTL entry point is used to send to volumes.
the DIRECT_IOCTL entry point is used for point-
to-point ioctls between corresponding user space
and kernel space plugins.

>> +/**
>> + * convenience macros to use plugin's fops entry points
>> + **/
>> +#define DISCOVER(node, list) ((plugin)->fops->discover(list))
>> +#define END_DISCOVER(node, list) ((plugin)->fops->end_discover(list))
>> +#define DELETE(node) ((node)->plugin->fops->delete(node))
>> +#define SUBMIT_IO(node, bio) ((node)->plugin->fops->submit_io(node,
bio))
>> +#define INIT_IO(node, rw_flag, start_sec, num_secs, buf_addr) >((node)
->plugin->fops->init_io(node, rw_flag, start_sec, num_secs, buf_addr))
>> +#define IOCTL(node, inode, file, cmd, arg)    ((node)
->plugin->fops->ioctl(node, >inode, file, cmd, arg))
>> +#define DIRECT_IOCTL(plugin, inode, file, cmd, arg)   >((plugin)
->fops->direct_ioctl(inode, file, cmd, arg))

> Do you really need those wrapper?

No the wrappers aren't really needed. However they do make the
code a great deal more readable.

> They just obsfucate the code

The same argument could be made about *all* macros then.
Its simply a tradeoff between readability and potential
hiding.

>> +/**
>> + * struct evms_pool_mgmt - anchor block for private pool management
>> + * @cachep:         kmem_cache_t variable
>> + * @member_size:    size of each element in the pool
>> + * @head:
>> + * @waiters:        count of waiters
>> + * @wait_queue:     list of waiters
>> + * @name:           name of the pool (must be less than 20 chars)
>> + *
>> + * anchor block for private pool management
>> + **/
>> +struct evms_pool_mgmt {
>> +  kmem_cache_t *cachep;
>> +  int member_size;
>> +  void *head;
>> +  atomic_t waiters;
>> +  wait_queue_head_t wait_queue;
>> +  u8 *name;
>> +};

> What's the pruipose of this?  Is it really evms-specific?

Its a reminent of 2.4 stuff before mempool was available.
Its gone.

>> +
>> +/*
>> + * Notes:
>> + *      All of the following kernel thread functions belong to EVMS
base.
>> + *      These functions were copied from md_core.c
>> + */

> What about moving them to the core kernel code so everyone
> can use them?

I've got no problem with doing that.

>> +/* Have to include this at the end, since it depends
>> + * on structures and definitions in this file.
>> + */
>> +#include <linux/evms/evms_ioctl.h>

> Just remove this and make the individual sources include it

Sure, its easy to do. Having nested includes allowed us
to enforce include ordering.

Mark



             reply	other threads:[~2002-10-03 18:50 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2002-10-03 18:59 Mark Peloquin [this message]
2002-10-04 14:14 ` [Evms-devel] Re: [PATCH] EVMS core 2/4: evms.h Christoph Hellwig
2002-10-04 14:34   ` Andreas Dilger
2002-10-04 17:10     ` Christoph Hellwig
  -- strict thread matches above, loose matches on Subject: below --
2002-10-04 18:45 Steve Pratt
2002-10-07 17:25 ` Christoph Hellwig
2002-10-04 14:59 Mark Peloquin
2002-10-04 13:59 Mark Peloquin
2002-10-03 17:42 Mark Peloquin
2002-10-04 12:28 ` Robert Varga
2002-10-04 14:08 ` Christoph Hellwig
2002-10-04 14:36   ` Richard B. Johnson
2002-10-03 12:36 Kevin Corry
2002-10-03 14:50 ` Christoph Hellwig
2002-10-03 15:10   ` [Evms-devel] " Michael Clark
2002-10-03 15:14     ` Christoph Hellwig
2002-10-03 16:22       ` Linus Torvalds

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=OF25D731E3.0E245DDC-ON85256C47.00645AA7@pok.ibm.com \
    --to=peloquin@us.ibm.com \
    --cc=evms-devel@lists.sourceforge.net \
    --cc=hch@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@transmeta.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).