Re: [Scst-devel] Upstream submission of scst_local and Kconfig

[Richard S. <rea...@gm...>]

On Mon, Sep 27, 2010 at 3:41 AM, Bart Van Assche <bva...@ac...> wrote:
> On Sun, Sep 26, 2010 at 10:00 PM, Richard Sharpe
> <rea...@gm...> wrote:
>>
>> On Sun, Sep 26, 2010 at 3:14 AM, Bart Van Assche <bva...@ac...> wrote:
>> > Hello Vlad and Richard,
>> >
>> > A shortcoming of the recently posted SCST patch series is that a Kconfig
>> > entry for scst_local is missing - users have to edit .config with a text
>> > editor in order to enable scst_local instead of enabling it via make xconfig
>> > / make menuconfig. A proper include statement should be added in
>> > scst/kernel/in-tree/Kconfig.scst.
>>
>> Something like this would seem to be needed:
>>
>> Index: scst/kernel/in-tree/Kconfig.scst
>> ===================================================================
>> --- scst/kernel/in-tree/Kconfig.scst    (revision 2284)
>> +++ scst/kernel/in-tree/Kconfig.scst    (working copy)
>> @@ -251,5 +251,6 @@
>>  source "drivers/scst/fcst/Kconfig"
>>  source "drivers/scst/iscsi-scst/Kconfig"
>>  source "drivers/scst/srpt/Kconfig"
>> +source "drivers/scst/scst_local/Kconfig"
>>
>>  endmenu
>
> Looks good to me. After having applied that patch locally, rerunning the
> regression tests revealed that scst_local is not yet sparse clean
> (scripts/run-regression-tests -d /tmp/bart 2.6.35.5). Can you please
> review the patch below and apply it if you agree with it ? The patch below
> shouldn't modify the behavior of scst_local.

It looks good to me ... I will apply it later today.

> Signed-off-by: Bart Van Assche <bva...@ac...>
>
> Index: scst_local/scst_local.c
> ===================================================================
> --- scst_local/scst_local.c     (revision 2285)
> +++ scst_local/scst_local.c     (working copy)
> @@ -798,7 +798,7 @@
>  static int scst_local_device_reset(struct scsi_cmnd *SCpnt)
>  {
>        struct scst_local_sess *sess;
> -       uint16_t lun;
> +       __be16 lun;
>        int ret;
>        DECLARE_COMPLETION_ONSTACK(dev_reset_completion);
>
> @@ -806,8 +806,7 @@
>
>        sess = to_scst_lcl_sess(scsi_get_device(SCpnt->device->host));
>
> -       lun = SCpnt->device->lun;
> -       lun = cpu_to_be16(lun);
> +       lun = cpu_to_be16(SCpnt->device->lun);
>
>        ret = scst_rx_mgmt_fn_lun(sess->scst_sess, SCST_LUN_RESET,
>                        (const uint8_t *)&lun, sizeof(lun), FALSE,
> @@ -829,7 +828,7 @@
>  static int scst_local_target_reset(struct scsi_cmnd *SCpnt)
>  {
>        struct scst_local_sess *sess;
> -       uint16_t lun;
> +       __be16 lun;
>        int ret;
>        DECLARE_COMPLETION_ONSTACK(dev_reset_completion);
>
> @@ -837,8 +836,7 @@
>
>        sess = to_scst_lcl_sess(scsi_get_device(SCpnt->device->host));
>
> -       lun = SCpnt->device->lun;
> -       lun = cpu_to_be16(lun);
> +       lun = cpu_to_be16(SCpnt->device->lun);
>
>        ret = scst_rx_mgmt_fn_lun(sess->scst_sess, SCST_TARGET_RESET,
>                        (const uint8_t *)&lun, sizeof(lun), FALSE,
> @@ -919,7 +917,7 @@
>        struct scst_local_sess *sess;
>        struct scatterlist *sgl = NULL;
>        int sgl_count = 0;
> -       uint16_t lun;
> +       __be16 lun;
>        struct scst_cmd *scst_cmd = NULL;
>        scst_data_direction dir;
>
> @@ -956,8 +954,7 @@
>         * Tell the target that we have a command ... but first we need
>         * to get the LUN into a format that SCST understand
>         */
> -       lun = SCpnt->device->lun;
> -       lun = cpu_to_be16(lun);
> +       lun = cpu_to_be16(SCpnt->device->lun);
>        scst_cmd = scst_rx_cmd(sess->scst_sess, (const uint8_t *)&lun,
>                               sizeof(lun), SCpnt->cmnd, SCpnt->cmd_len, TRUE);
>        if (!scst_cmd) {
> @@ -1090,6 +1087,8 @@
>  /* Must be called under sess->aen_lock. Drops then reacquires it inside. */
>  static void scst_process_aens(struct scst_local_sess *sess,
>        bool cleanup_only)
> +       __releases(&sess->aen_lock)
> +       __acquires(&sess->aen_lock)
>  {
>        struct scst_aen_work_item *work_item = NULL;
>



-- 
Regards,
Richard Sharpe