Re: Questions about scsi.c
Randy Dunlap wrote: On Thu, 25 Oct 2007 16:40:35 -0500 Rob Landley wrote: On Thursday 25 October 2007 12:32:41 pm Randy Dunlap wrote: Entirely possible I'm doing something wrong: sect1 id=scsi_device.h titleinclude/scsi/scsi_device.h/title para /para !Einclude/scsi/scsi_device.h /sect1 !E is for exported symbols and that file has none. USe !I instead. So how do I handle a case like drivers/ata/libata-core.c which has EXPORT_SYMBOL() calls for functions that live in (and are documented in) other files, such as ata_scsi_ioctl() in drivers/ata/libata-scsi.c? I don't see ata_scsi_ioctl() documented at all. Are you looking at a newer tree than I am? (i'm using 2.6.24-rc1) Long-term answer is that we prefer EXPORT_SYMBOL() to be used just under the function that is being exported. In this case, the maintainer may be disagreeing with that. [cc-ed] Short-term answer is to use !Isource_filename_where_kernel_doc_is as though it's not EXPORTed. I think. Yeah I tended to prefer that all exports be in one place, rather than scattered around and difficult to evaluate en masse :) Jeff - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Questions about scsi.c
On Tue, 2007-10-30 at 08:34 -0400, Jeff Garzik wrote: Randy Dunlap wrote: On Thu, 25 Oct 2007 16:40:35 -0500 Rob Landley wrote: On Thursday 25 October 2007 12:32:41 pm Randy Dunlap wrote: Entirely possible I'm doing something wrong: sect1 id=scsi_device.h titleinclude/scsi/scsi_device.h/title para /para !Einclude/scsi/scsi_device.h /sect1 !E is for exported symbols and that file has none. USe !I instead. So how do I handle a case like drivers/ata/libata-core.c which has EXPORT_SYMBOL() calls for functions that live in (and are documented in) other files, such as ata_scsi_ioctl() in drivers/ata/libata-scsi.c? I don't see ata_scsi_ioctl() documented at all. Are you looking at a newer tree than I am? (i'm using 2.6.24-rc1) Long-term answer is that we prefer EXPORT_SYMBOL() to be used just under the function that is being exported. In this case, the maintainer may be disagreeing with that. [cc-ed] Short-term answer is to use !Isource_filename_where_kernel_doc_is as though it's not EXPORTed. I think. Yeah I tended to prefer that all exports be in one place, rather than scattered around and difficult to evaluate en masse :) My personal preference (and how I code) is export at the bottom of the function. However, it's one of those stylistic things that I'm happy to have people code however they want (either everything at the bottom of the file or all exports at the bottom of the exported function) as long as they follow the current style of whatever file they're patching. James - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Questions about scsi.c
On Tuesday 30 October 2007 9:43:29 am James Bottomley wrote:
On Tue, 2007-10-30 at 08:34 -0400, Jeff Garzik wrote:
Randy Dunlap wrote:
On Thu, 25 Oct 2007 16:40:35 -0500 Rob Landley wrote:
On Thursday 25 October 2007 12:32:41 pm Randy Dunlap wrote:
!E is for exported symbols and that file has none.
USe !I instead.
So how do I handle a case like drivers/ata/libata-core.c which has
EXPORT_SYMBOL() calls for functions that live in (and are documented
in) other files, such as ata_scsi_ioctl() in
drivers/ata/libata-scsi.c?
...
Yeah I tended to prefer that all exports be in one place, rather than
scattered around and difficult to evaluate en masse :)
My personal preference (and how I code) is export at the bottom of the
function. However, it's one of those stylistic things that I'm happy to
have people code however they want (either everything at the bottom of
the file or all exports at the bottom of the exported function) as long
as they follow the current style of whatever file they're patching.
Actually, this one is written up in CodingStyle:
In source files, separate functions with one blank line. If the function
is exported, the EXPORT* macro for it should follow immediately after the
closing function brace line. E.g.:
int system_is_up(void)
{
return system_state == SYSTEM_RUNNING;
}
EXPORT_SYMBOL(system_is_up);
The functional problem is that when the EXPORT_SYMBOL() is in a different file
entirely, the documentation infrastructure doesn't pick up that it's an
exported function.
James
Rob
--
One of my most productive days was throwing away 1000 lines of code.
- Ken Thompson.
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Questions about scsi.c
Ok, I'm reading the code and trying to update the kerneldoc comments (first
stab at it attached, a Documentation/DocBook/scsi_midlayer.tmpl is in the
works), and I have a few random questions from the read-through:
Why does __scsi_put_command() pass in both a Scsi_Host * and struct device *
when scsi_get_command() uses dev-host? Is dev-host not guaranteed to be
set?
scsi_put_command() notes the command must not belong to any lists but then
inside the function it says serious error if the command hasn't come form a
device list and does a BUG_ON(list_empty(cmd-list));
scsi_setup_command_freelist() is protection against OOM locking the system
solid, right? Makes sure there's always preallocated memory to send at least
one SCSI command at a time to each host so we can swap enough to free more?
scsi_dispatch_command() I _think_ a nonzero return means the command was
rejected and the device queue needs to be plugged, but I'm not 100% certain
what that means.
The rest sort of seems to make sense, although the kerneldoc comments in
include/scsi/scsi_device.h are before #defines instead of before function
definitions so the make xmldocs infrastructure (something in either
scripts/basic/docproc.c or scripts/kernel-doc) skips right over it
because it can't figure out the argument types. Separate issue, todo item for
later...
Rob
--
One of my most productive days was throwing away 1000 lines of code.
- Ken Thompson.
diff -r a868e8217782 drivers/scsi/scsi.c
--- a/drivers/scsi/scsi.c Mon Oct 22 19:40:02 2007 -0700
+++ b/drivers/scsi/scsi.c Thu Oct 25 06:01:43 2007 -0500
@@ -122,6 +122,11 @@ static const char *const scsi_device_typ
Automation/Drive ,
};
+/**
+ * scsi_device_type - Return 17 char string indicating device type.
+ * @type: type number to look up
+ */
+
const char * scsi_device_type(unsigned type)
{
if (type == 0x1e)
@@ -156,6 +161,14 @@ static struct scsi_host_cmd_pool scsi_cm
static DEFINE_MUTEX(host_cmd_pool_mutex);
+/**
+ * __scsi_get_command - Allocate a struct scsi_cmnd
+ * @shost: host to transmit command
+ * @gfp_mask: allocation mask
+ *
+ * Description: allocate a struct scsi_cmd from host's slab, recycling from the
+ * host's free_list if necessary.
+ */
struct scsi_cmnd *__scsi_get_command(struct Scsi_Host *shost, gfp_t gfp_mask)
{
struct scsi_cmnd *cmd;
@@ -179,13 +192,10 @@ struct scsi_cmnd *__scsi_get_command(str
}
EXPORT_SYMBOL_GPL(__scsi_get_command);
-/*
- * Function: scsi_get_command()
- *
- * Purpose: Allocate and setup a scsi command block
- *
- * Arguments: dev - parent scsi device
- * gfp_mask- allocator flags
+/**
+ * scsi_get_command - Allocate and setup a scsi command block
+ * @dev: parent scsi device
+ * @gfp_mask: allocator flags
*
* Returns: The allocated scsi command structure.
*/
@@ -217,6 +227,12 @@ struct scsi_cmnd *scsi_get_command(struc
}
EXPORT_SYMBOL(scsi_get_command);
+/**
+ * __scsi_put_command - Free a struct scsi_cmnd
+ * @shost: dev-host
+ * @cmd: Command to free
+ * @dev: parent scsi device
+ */
void __scsi_put_command(struct Scsi_Host *shost, struct scsi_cmnd *cmd,
struct device *dev)
{
@@ -237,12 +253,9 @@ void __scsi_put_command(struct Scsi_Host
}
EXPORT_SYMBOL(__scsi_put_command);
-/*
- * Function: scsi_put_command()
- *
- * Purpose: Free a scsi command block
- *
- * Arguments: cmd - command block to free
+/**
+ * scsi_put_command - Free a scsi command block
+ * @cmd: command block to free
*
* Returns: Nothing.
*
@@ -263,12 +276,13 @@ void scsi_put_command(struct scsi_cmnd *
}
EXPORT_SYMBOL(scsi_put_command);
-/*
- * Function: scsi_setup_command_freelist()
- *
- * Purpose: Setup the command freelist for a scsi host.
- *
- * Arguments: shost - host to allocate the freelist for.
+/**
+ * scsi_setup_command_freelist - Setup the command freelist for a scsi host.
+ * @shost: host to allocate the freelist for.
+ *
+ * Description: The command freelist protects against system-wide out of memory
+ * deadlock by preallocating one SCSI command structure for each host, so the
+ * system can always write to a swap file on a device associated with that host.
*
* Returns: Nothing.
*/
@@ -318,12 +332,9 @@ int scsi_setup_command_freelist(struct S
}
-/*
- * Function: scsi_destroy_command_freelist()
- *
- * Purpose: Release the command freelist for a scsi host.
- *
- * Arguments: shost - host that's freelist is going to be destroyed
+/**
+ * scsi_destroy_command_freelist - Release the command freelist for a scsi host.
+ * @shost: host that's freelist is going to be destroyed
*/
void scsi_destroy_command_freelist(struct Scsi_Host *shost)
{
@@ -441,8 +452,12 @@ void scsi_log_completion(struct scsi_cmn
}
#endif
-/*
- * Assign a serial number to the request for error recovery
+/**
+ * scsi_cmd_get_serial - Assign a serial number to a command
+ * @host: the scsi host
+ * @cmd: command to assign serial number to
+ *
+ * Description: a serial number identifies a request for
Re: Questions about scsi.c
On Thu, 25 Oct 2007 06:06:03 -0500 Rob Landley wrote: The rest sort of seems to make sense, although the kerneldoc comments in include/scsi/scsi_device.h are before #defines instead of before function definitions so the make xmldocs infrastructure (something in either scripts/basic/docproc.c or scripts/kernel-doc) skips right over it because it can't figure out the argument types. Separate issue, todo item for later... scripts/kernel-doc is supposed to (and usually does) handle kernel-doc notation of a #define macro. Are these 2 not working? --- ~Randy - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Questions about scsi.c
On Thursday 25 October 2007 10:40:39 am Randy Dunlap wrote: On Thu, 25 Oct 2007 06:06:03 -0500 Rob Landley wrote: The rest sort of seems to make sense, although the kerneldoc comments in include/scsi/scsi_device.h are before #defines instead of before function definitions so the make xmldocs infrastructure (something in either scripts/basic/docproc.c or scripts/kernel-doc) skips right over it because it can't figure out the argument types. Separate issue, todo item for later... scripts/kernel-doc is supposed to (and usually does) handle kernel-doc notation of a #define macro. Are these 2 not working? Not when I tried it. $ make xmldocs make -C /home/landley/linux/hg O=/home/landley/linux/temp xmldocs DOCPROC Documentation/DocBook/scsi_midlayer.xml Warning(/home/landley/linux/hg//include/scsi/scsi_device.h): no structured comments found Entirely possible I'm doing something wrong: sect1 id=scsi_device.h titleinclude/scsi/scsi_device.h/title para /para !Einclude/scsi/scsi_device.h /sect1 Rob -- One of my most productive days was throwing away 1000 lines of code. - Ken Thompson. - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Questions about scsi.c
On Thu, 25 Oct 2007 13:25:37 -0500 Rob Landley wrote: On Thursday 25 October 2007 10:40:39 am Randy Dunlap wrote: On Thu, 25 Oct 2007 06:06:03 -0500 Rob Landley wrote: The rest sort of seems to make sense, although the kerneldoc comments in include/scsi/scsi_device.h are before #defines instead of before function definitions so the make xmldocs infrastructure (something in either scripts/basic/docproc.c or scripts/kernel-doc) skips right over it because it can't figure out the argument types. Separate issue, todo item for later... scripts/kernel-doc is supposed to (and usually does) handle kernel-doc notation of a #define macro. Are these 2 not working? Not when I tried it. $ make xmldocs make -C /home/landley/linux/hg O=/home/landley/linux/temp xmldocs DOCPROC Documentation/DocBook/scsi_midlayer.xml Warning(/home/landley/linux/hg//include/scsi/scsi_device.h): no structured comments found Entirely possible I'm doing something wrong: sect1 id=scsi_device.h titleinclude/scsi/scsi_device.h/title para /para !Einclude/scsi/scsi_device.h /sect1 !E is for exported symbols and that file has none. USe !I instead. --- ~Randy - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Questions about scsi.c
On Thursday 25 October 2007 12:32:41 pm Randy Dunlap wrote: Entirely possible I'm doing something wrong: sect1 id=scsi_device.h titleinclude/scsi/scsi_device.h/title para /para !Einclude/scsi/scsi_device.h /sect1 !E is for exported symbols and that file has none. USe !I instead. Ah, that did it. Thanks. Rob -- One of my most productive days was throwing away 1000 lines of code. - Ken Thompson. - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Questions about scsi.c
On Thursday 25 October 2007 12:32:41 pm Randy Dunlap wrote: Entirely possible I'm doing something wrong: sect1 id=scsi_device.h titleinclude/scsi/scsi_device.h/title para /para !Einclude/scsi/scsi_device.h /sect1 !E is for exported symbols and that file has none. USe !I instead. So how do I handle a case like drivers/ata/libata-core.c which has EXPORT_SYMBOL() calls for functions that live in (and are documented in) other files, such as ata_scsi_ioctl() in drivers/ata/libata-scsi.c? --- ~Randy Rob -- One of my most productive days was throwing away 1000 lines of code. - Ken Thompson. - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Questions about scsi.c
On Thu, 25 Oct 2007 16:40:35 -0500 Rob Landley wrote: On Thursday 25 October 2007 12:32:41 pm Randy Dunlap wrote: Entirely possible I'm doing something wrong: sect1 id=scsi_device.h titleinclude/scsi/scsi_device.h/title para /para !Einclude/scsi/scsi_device.h /sect1 !E is for exported symbols and that file has none. USe !I instead. So how do I handle a case like drivers/ata/libata-core.c which has EXPORT_SYMBOL() calls for functions that live in (and are documented in) other files, such as ata_scsi_ioctl() in drivers/ata/libata-scsi.c? I don't see ata_scsi_ioctl() documented at all. Are you looking at a newer tree than I am? (i'm using 2.6.24-rc1) Long-term answer is that we prefer EXPORT_SYMBOL() to be used just under the function that is being exported. In this case, the maintainer may be disagreeing with that. [cc-ed] Short-term answer is to use !Isource_filename_where_kernel_doc_is as though it's not EXPORTed. I think. --- ~Randy - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
