Date: Thu, 14 Apr 2011 23:10:26 +0300 From: Alexander Motin <mav@FreeBSD.org> To: Jaakko Heinonen <jh@FreeBSD.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r220618 - head/sys/cam/ata Message-ID: <4DA754B2.6070802@FreeBSD.org> In-Reply-To: <20110414160059.GA1644@a91-153-123-205.elisa-laajakaista.fi> References: <201104140911.p3E9BooX027074@svn.freebsd.org> <20110414160059.GA1644@a91-153-123-205.elisa-laajakaista.fi>
next in thread | previous in thread | raw e-mail | index | archive | help
On 14.04.2011 19:00, Jaakko Heinonen wrote:
> On 2011-04-14, Alexander Motin wrote:
>> Fix typo in write_cache tunable name and move TUNABLE_INT_FETCH() out of
>> the SIM lock to fix WITNESS warning.
>
> Thanks!
>
> Does this analogous patch for cd(4) and da(4) look good?
Looks fine.
> I am not
> completely sure if it's safe to move setting of minimum_cmd_size in
> daregister().
I see no problem. dastart() in NORMAL state, that using it, won't be
called until device created (to be opened) and hold will not be dropped.
> ---
>
> Move TUNABLE_INT_FETCH() out of the SIM lock to avoid sleeping while
> holding the lock. The fix is analogous to r220618 for ada(4).
>
> %%%
> Index: sys/cam/scsi/scsi_cd.c
> ===================================================================
> --- sys/cam/scsi/scsi_cd.c (revision 220619)
> +++ sys/cam/scsi/scsi_cd.c (working copy)
> @@ -687,6 +687,8 @@ cdregister(struct cam_periph *periph, vo
> else
> softc->minimum_command_size = 6;
>
> + (void)cam_periph_hold(periph, PRIBIO);
> + cam_periph_unlock(periph);
> /*
> * Load the user's default, if any.
> */
> @@ -712,7 +714,6 @@ cdregister(struct cam_periph *periph, vo
> * WORM peripheral driver. WORM drives will also have the WORM
> * driver attached to them.
> */
> - cam_periph_unlock(periph);
> softc->disk = disk_alloc();
> softc->disk->d_devstat = devstat_new_entry("cd",
> periph->unit_number, 0,
> @@ -745,6 +746,7 @@ cdregister(struct cam_periph *periph, vo
> softc->disk->d_hba_subdevice = cpi.hba_subdevice;
> disk_create(softc->disk, DISK_VERSION);
> cam_periph_lock(periph);
> + cam_periph_unhold(periph);
>
> /*
> * Add an async callback so that we get
> Index: sys/cam/scsi/scsi_da.c
> ===================================================================
> --- sys/cam/scsi/scsi_da.c (revision 220619)
> +++ sys/cam/scsi/scsi_da.c (working copy)
> @@ -1231,35 +1231,6 @@ daregister(struct cam_periph *periph, vo
> TASK_INIT(&softc->sysctl_task, 0, dasysctlinit, periph);
>
> /*
> - * RBC devices don't have to support READ(6), only READ(10).
> - */
> - if (softc->quirks& DA_Q_NO_6_BYTE || SID_TYPE(&cgd->inq_data) == T_RBC)
> - softc->minimum_cmd_size = 10;
> - else
> - softc->minimum_cmd_size = 6;
> -
> - /*
> - * Load the user's default, if any.
> - */
> - snprintf(tmpstr, sizeof(tmpstr), "kern.cam.da.%d.minimum_cmd_size",
> - periph->unit_number);
> - TUNABLE_INT_FETCH(tmpstr,&softc->minimum_cmd_size);
> -
> - /*
> - * 6, 10, 12 and 16 are the currently permissible values.
> - */
> - if (softc->minimum_cmd_size< 6)
> - softc->minimum_cmd_size = 6;
> - else if ((softc->minimum_cmd_size> 6)
> - && (softc->minimum_cmd_size<= 10))
> - softc->minimum_cmd_size = 10;
> - else if ((softc->minimum_cmd_size> 10)
> - && (softc->minimum_cmd_size<= 12))
> - softc->minimum_cmd_size = 12;
> - else if (softc->minimum_cmd_size> 12)
> - softc->minimum_cmd_size = 16;
> -
> - /*
> * Register this media as a disk
> */
>
> @@ -1291,6 +1262,35 @@ daregister(struct cam_periph *periph, vo
> dasendorderedtag, softc);
>
> mtx_unlock(periph->sim->mtx);
> + /*
> + * RBC devices don't have to support READ(6), only READ(10).
> + */
> + if (softc->quirks& DA_Q_NO_6_BYTE || SID_TYPE(&cgd->inq_data) == T_RBC)
> + softc->minimum_cmd_size = 10;
> + else
> + softc->minimum_cmd_size = 6;
> +
> + /*
> + * Load the user's default, if any.
> + */
> + snprintf(tmpstr, sizeof(tmpstr), "kern.cam.da.%d.minimum_cmd_size",
> + periph->unit_number);
> + TUNABLE_INT_FETCH(tmpstr,&softc->minimum_cmd_size);
> +
> + /*
> + * 6, 10, 12 and 16 are the currently permissible values.
> + */
> + if (softc->minimum_cmd_size< 6)
> + softc->minimum_cmd_size = 6;
> + else if ((softc->minimum_cmd_size> 6)
> + && (softc->minimum_cmd_size<= 10))
> + softc->minimum_cmd_size = 10;
> + else if ((softc->minimum_cmd_size> 10)
> + && (softc->minimum_cmd_size<= 12))
> + softc->minimum_cmd_size = 12;
> + else if (softc->minimum_cmd_size> 12)
> + softc->minimum_cmd_size = 16;
> +
> softc->disk = disk_alloc();
> softc->disk->d_open = daopen;
> softc->disk->d_close = daclose;
> %%%
>
--
Alexander Motin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4DA754B2.6070802>