Re: [Scst-devel] Fwd: scst_local compilation errors on 2.6.18 kernel

[Bart V. A. <bva...@ac...>]

 On Sat, Sep 18, 2010 at 4:48 AM, Richard Sharpe <
rea...@gm...> wrote:

> [ ... ]
>

Hello Richard,

Thanks for cleaning up the patch -- see below for a few comments.


> @@ -934,7 +939,7 @@ static int scst_local_queuecommand(struc
>          */
>        tgt_specific = kmem_cache_alloc(tgt_specific_pool, GFP_ATOMIC);
>        if (!tgt_specific) {
> -               PRINT_ERROR("Unable to create tgt_specific (size %d)",
> +               PRINT_ERROR("Unable to create tgt_specific (size %ld)",
>                        sizeof(*tgt_specific));
>                return -ENOMEM;
>        }
>

A minor nit: while %ld seems to work fine for printing sizeof() values on
32-bit and 64-bit x86 systems, are you aware that the format specifier %zu
is recommended for printing sizeof() values ? See also
Documentation/printk-formats.txt.

[ ... ]
>
>        TRACE_ENTRY();
>
> -       TRACE_MGMT_DBG("Target work %p)", work);
> +       TRACE_MGMT_DBG("Target work %p)", sess);
> +       printk(KERN_INFO "%s: sess = %p\n", __func__, sess);
> +       msleep(5000);
>
>        spin_lock(&sess->aen_lock);
>        scst_process_aens(sess, false);
>

Please use KERN_DEBUG or TRACE_DEBUG for the printk() statement above. Also,
is the msleep() statement necessary for another purpose than debugging ?


> @@ -1174,6 +1189,7 @@ static int scst_local_report_aen(struct
>
>                spin_unlock(&sess->aen_lock);
>
> +               printk(KERN_INFO "%s: sess = %p\n", __func__, sess);
>                schedule_work(&sess->aen_work);
>                break;
>
>
Same remark here -- this also looks like a debugging statement to me ?

Bart.