Roman Zippel wrote:
> I have two small bug fixes for the sg device.
>
> - sg might need to restart the sd queue (this is done in scsi_ioctl.c but
> not in sg.c)
> - initializing my_cmdp in a new Sg_request with 0 marks it free and until
> it's really initialized, sg_write might sleep and sg_read removes this as
> finished command (this is a problem for mutlithreaded applications, which
> use a seperate threads for reading/writing scsi commands).
>
> This patch is for 2.2.15, but I think the first problem also exists in
> 2.3.x.
Roman,
Sorry about the delayed response.
With regards to the scsi_request_fn() call: it is called from
ioctl_internal_command() [with a 'was_reset' guard] and from
scsi_ioctl_send_command() [without a 'was_reset' guard]. Both these
functions are in the scsi_ioctl.c file. It is also worth noting
that it is called from the user context in both cases (not the
bottom half handle as shown in your patch). However neither
sd, sr nor st upper level drivers call this function, so if
nothing else sg is being consistent. Perhaps Eric Y. would
like to comment.
With regard to the sg_read() race: there is a window for a race
if a read is done on a fd at the same time as a write on the same
fd. [There is also a remote chance of a race if a read starts
_before_ the write and the read gets interrupted by a signal at
the same time as the write is happening.] As your post implies,
this is now guarded against in the 2.3 sg driver (which runs
a slightly more complex state machine). All these scenarios
revolve around running read during or before the write they
are logically associated with. These are arguably protocol
violations of sg's write command followed by read response
pattern.
At least one other bad race I know about is multiple readers
on the same fd. Interestingly this in not a problem if they
are trying to fetch distinct 'pack_id's which is the mode
used by the sg multi-threading dd variant called "sgp_dd".
My latest patch (not yet accepted) in lk 2.3 guards against
this. Then there are the issues raised by Manfred Spraul's
changes to poll(). Question is, how far do you go with this
sort of stuff?
Back to the reported bug fixes and a question. Was there
some observed problems that these fixes solved?
Doug Gilbert
P.S. With regard to the patch
.....
- resp->my_cmdp = NULL;
+ resp->my_cmdp = (Scsi_Cmnd *)-1;
.....
This trick will cause sg_debug() to crash since it
tries to dereference non-null "my_cmdp" pointers.
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
