On Wed, Feb 05, 2014 at 03:51:35PM -0800, James Bottomley wrote:
> On Wed, 2014-02-05 at 04:39 -0800, Christoph Hellwig wrote:
>
> > Just have one level of alloc/free functions that take a host instead
> > of two levels for the allocation and different calling conventions
> > for the free.
> >
> > Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> > ---
> > drivers/scsi/scsi.c | 77 +++++++++++++++------------------------------------
> > 1 file changed, 22 insertions(+), 55 deletions(-)
> >
> > diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
> > index 4139486..5347f7d 100644
> > --- a/drivers/scsi/scsi.c
> > +++ b/drivers/scsi/scsi.c
> > @@ -160,79 +160,46 @@ static struct scsi_host_cmd_pool scsi_cmd_dma_pool = {
> >
> > static DEFINE_MUTEX(host_cmd_pool_mutex);
> >
> > -/**
> > - * scsi_pool_alloc_command - internal function to get a fully allocated command
> > - * @pool: slab pool to allocate the command from
> > - * @gfp_mask: mask for the allocation
> > - *
> > - * Returns a fully allocated command (with the allied sense buffer) or
> > - * NULL on failure
> > - */
> > -static struct scsi_cmnd *
> > -scsi_pool_alloc_command(struct scsi_host_cmd_pool *pool, gfp_t gfp_mask)
> > -{
> > - struct scsi_cmnd *cmd;
> > -
> > - cmd = kmem_cache_zalloc(pool->cmd_slab, gfp_mask | pool->gfp_mask);
> > - if (!cmd)
> > - return NULL;
> > -
> > - cmd->sense_buffer = kmem_cache_alloc(pool->sense_slab,
> > - gfp_mask | pool->gfp_mask);
> > - if (!cmd->sense_buffer) {
> > - kmem_cache_free(pool->cmd_slab, cmd);
> > - return NULL;
> > - }
> > -
> > - return cmd;
> > -}
> > -
> > -/**
> > - * scsi_pool_free_command - internal function to release a command
> > - * @pool: slab pool to allocate the command from
> > - * @cmd: command to release
> > - *
> > - * the command must previously have been allocated by
> > - * scsi_pool_alloc_command.
> > - */
> > static void
> > -scsi_pool_free_command(struct scsi_host_cmd_pool *pool,
> > - struct scsi_cmnd *cmd)
> > +scsi_host_free_command(struct Scsi_Host *shost, struct scsi_cmnd *cmd)
>
> You lost the docbook function description here when you changed the name.
It's a static function with two callers, and it's more obvious from the
function than the comment what it does.
I'm probably one of the people with the highest comment to code ratios
in the kernel, but I'd rather explain in long comments when something
is non-obvious than putting useless boilerplates in. If you insist on
the comment I'll put it back, but it seems utterly useless.
> This docbook elimination looks spurious; why do it?
Same as above.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
