RE: [PATCH v2 -mm 6/7] DCA: Add Direct Cache Access driver

2007-08-24 Thread Nelson, Shannon
Randy Dunlap [mailto:[EMAIL PROTECTED] 
>
>On Thu, 23 Aug 2007 17:15:22 -0700 Shannon Nelson wrote:
>
>> Signed-off-by: Shannon Nelson <[EMAIL PROTECTED]>
>> Acked-by: David S. Miller <[EMAIL PROTECTED]>
>> ---
>> 
>>  drivers/Kconfig |2 +
>>  drivers/Makefile|1 
>>  drivers/dca/Kconfig |   11 +++
>>  drivers/dca/Makefile|2 +
>>  drivers/dca/dca-core.c  |  168 
>+++
>>  drivers/dca/dca-sysfs.c |   88 +
>>  include/linux/dca.h |   47 +
>>  7 files changed, 319 insertions(+), 0 deletions(-)
>> 
>> diff --git a/drivers/dca/Kconfig b/drivers/dca/Kconfig
>> new file mode 100644
>> index 000..d901615
>> --- /dev/null
>> +++ b/drivers/dca/Kconfig
>> @@ -0,0 +1,11 @@
>> +#
>> +# DCA server configuration
>> +#
>> +
>> +config DCA
>> +tristate "DCA support for clients and providers"
>> +---help---
>> +  This is a server to help modules that want to use 
>Direct Cache
>> +  Access to find DCA providers that will supply correct 
>CPU tags.
>> +default m
>
>We conventionally put help text last in each config entry.
>& Help text should be indented by 1 tab + 2 spaces.
>
>> diff --git a/drivers/dca/dca-core.c b/drivers/dca/dca-core.c
>> new file mode 100644
>> index 000..c0ff9bd
>> --- /dev/null
>> +++ b/drivers/dca/dca-core.c
>> @@ -0,0 +1,168 @@
>> +/*
>> + * Copyright(c) 2007 Intel Corporation. All rights reserved.
>> + *
>> +/*
>> + * This driver supports an interface for DCA clients and 
>providers to meet.
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +MODULE_LICENSE("GPL");
>> +
>> +/* For now we're assuming a single, global, DCA provider 
>for the system. */
>> +
>> +static DEFINE_SPINLOCK(dca_lock);
>> +
>> +struct dca_provider *global_dca = NULL;
>
>Can global_dca be static, or is it used in other source files?

Yes, this should be static.  I'll fix this.

>
>It would be good to have all of these global/exported interfaces
>documented somewhere.  Did I miss it in another file?
>If not, you could use kernel-doc to add inline function docs.
>See Documentation/kernel-doc-nano-HOWTO.txt.

I'll add the block comments.

>
>> +u8 dca_get_tag(int cpu)
>> +{
>> +if (!global_dca)
>> +return -ENODEV;
>> +return global_dca->ops->get_tag(global_dca, cpu);
>> +}
>> +EXPORT_SYMBOL(dca_get_tag);
>> +
>
>> +
>> +static BLOCKING_NOTIFIER_HEAD(dca_provider_chain);
>> +
>> +
>> +static int __init dca_init(void)
>> +{
>> +int err;
>> +
>> +err = dca_sysfs_init();
>> +if (err)
>> +return err;
>> +return 0;
>
>or just (in all cases):
>
>   return err;
>
>> +}

Yep, will do.

Thanks for the comments,
sln
--
==
Mr. Shannon Nelson LAN Access Division, Intel Corp.
[EMAIL PROTECTED]I don't speak for Intel
(503) 712-7659Parents can't afford to be squeamish.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2 -mm 6/7] DCA: Add Direct Cache Access driver

2007-08-23 Thread Randy Dunlap
On Thu, 23 Aug 2007 17:15:22 -0700 Shannon Nelson wrote:

> Signed-off-by: Shannon Nelson <[EMAIL PROTECTED]>
> Acked-by: David S. Miller <[EMAIL PROTECTED]>
> ---
> 
>  drivers/Kconfig |2 +
>  drivers/Makefile|1 
>  drivers/dca/Kconfig |   11 +++
>  drivers/dca/Makefile|2 +
>  drivers/dca/dca-core.c  |  168 
> +++
>  drivers/dca/dca-sysfs.c |   88 +
>  include/linux/dca.h |   47 +
>  7 files changed, 319 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/dca/Kconfig b/drivers/dca/Kconfig
> new file mode 100644
> index 000..d901615
> --- /dev/null
> +++ b/drivers/dca/Kconfig
> @@ -0,0 +1,11 @@
> +#
> +# DCA server configuration
> +#
> +
> +config DCA
> + tristate "DCA support for clients and providers"
> + ---help---
> +  This is a server to help modules that want to use Direct Cache
> +   Access to find DCA providers that will supply correct CPU tags.
> + default m

We conventionally put help text last in each config entry.
& Help text should be indented by 1 tab + 2 spaces.

> diff --git a/drivers/dca/dca-core.c b/drivers/dca/dca-core.c
> new file mode 100644
> index 000..c0ff9bd
> --- /dev/null
> +++ b/drivers/dca/dca-core.c
> @@ -0,0 +1,168 @@
> +/*
> + * Copyright(c) 2007 Intel Corporation. All rights reserved.
> + *
> +/*
> + * This driver supports an interface for DCA clients and providers to meet.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +MODULE_LICENSE("GPL");
> +
> +/* For now we're assuming a single, global, DCA provider for the system. */
> +
> +static DEFINE_SPINLOCK(dca_lock);
> +
> +struct dca_provider *global_dca = NULL;

Can global_dca be static, or is it used in other source files?

It would be good to have all of these global/exported interfaces
documented somewhere.  Did I miss it in another file?
If not, you could use kernel-doc to add inline function docs.
See Documentation/kernel-doc-nano-HOWTO.txt.

> +u8 dca_get_tag(int cpu)
> +{
> + if (!global_dca)
> + return -ENODEV;
> + return global_dca->ops->get_tag(global_dca, cpu);
> +}
> +EXPORT_SYMBOL(dca_get_tag);
> +

> +
> +static BLOCKING_NOTIFIER_HEAD(dca_provider_chain);
> +
> +
> +static int __init dca_init(void)
> +{
> + int err;
> +
> + err = dca_sysfs_init();
> + if (err)
> + return err;
> + return 0;

or just (in all cases):

return err;

> +}


---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/