Re: [PATCH 00/11] st: remove scsi_execute_async usage (the first half)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 4 Dec 2008 22:23:06 +0200 (EET)
Kai Makisara <Kai.Makisara@xxxxxxxxxxx> wrote:

> On Thu, 4 Dec 2008, FUJITA Tomonori wrote:
> 
> > On Wed, 3 Dec 2008 21:40:53 +0200 (EET)
> > Kai Makisara <Kai.Makisara@xxxxxxxxxxx> wrote:
> > 
> > > > If so, can you try this patch to convert st_int_ioctl?
> > > > 
> > > I replaced my patch with yours. Now the tests pass. I did some tests with 
> > > debugging enabled and those showed that the sense data is returned 
> > > correctly.
> > 
> > Great, then can I get your ACK on this patchset?
> > 
> Yes, you can add
> Acked-by: Kai Makisara <Kai.Makisara@xxxxxxxxxxx>

Thanks!


> > > > Do you prefer me to change st_scsi_kern_execute to set -EBUSY to
> > > > syscall_result if st_scsi_kern_execute internally fails (that is,
> > > > blk_rq_map_kern or blk_get_request fails)?
> > > > 
> > > I am not sure that -EBUSY is a valid error return any more. Should the 
> > > error be -ENOMEM if blk_get_request fails and otherwise return what 
> > > blk_rq_map_kern returns?
> > > 
> > > > Note that scsi_execute_async (which st_do_scsi uses) could fail
> > > > internally. scsi_execute_async and st_scsi_kern_execute uses the same
> > > > functions that could fail, blk_rq_map_kern or blk_get_request
> > > > 
> > > Yes. I am not sure that -EBUSY is correct return value there.
> > 
> > I think that it's the right thing to return an error value to user
> > space that scsi_execute_async returns but st_do_scsi uses has returned
> > -EBUSY for any failure for long time so it might be safer to keep the
> > current behavior.
> > 
> Retaining the old behaviour is always a safe solution but sometimes it is 
> time to improve code. Note that scsi_execute_async() only returns 
> (DRIVER_ERROR << 24) in case of error.

Ah, right, I overlooked it.


> This does not allow using different 
> error codes for different error reasons. With st_scsi_kern_execute it is 
> possible.
> 
> > I'll change st_scsi_kern_execute to set -EBUSY in case of internal
> > failure in the next submission if you prefer.
> > 
> I don't like returning -EBUSY always now that we have a choice. However, 
> if you think the conservative solution of returning -EBUSY is better, I 
> don't object.

Thanks, I see.

I simplified st_scsi_kern_execute by using scsi_execute. scsi_execute
returns DRIVER_ERROR << 24 for internal failures like
scsi_execute_async does. I changed st_scsi_kern_execute to return
-EBUSY for now as st_do_scsi does.

st_scsi_kern_execute would be changed again in the second half so this
might be changed. But I think if we change return code, it would be
better to do it in a separate patch (explicitly).
--
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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]
  Powered by Linux