On 3 Jul, [EMAIL PROTECTED] wrote:
>>From: [EMAIL PROTECTED]
>
>>> I am still not sure if would be better not to allow to raise the DMA
>>> limit in pre-2.4 kernels.
>
>>My understanding of the scsi-generic.txt is that a reserved buffer is
>>indeed available, in which case it would be useful to use
>>SG_SET_RESERVED_SIZE:
>
>>| The size of this reserved buffer can
>>| subsequently be modified with the SG_SET_RESERVED_SIZE ioctl(). In both
>>| cases these are requests subject to various dynamic constraints. The actual
>>| amount of memory obtained can be found by the SG_GET_RESERVED_SIZE ioctl().
>>| The reserved buffer will be used if:
>>| - it is not already in use (eg when command queuing is in use)
>>| - a write() does not call for a buffer size larger than the
>>| reserved size.
>
>>Where did you find that the buffer might not be available later when
>>actually trying to use it? Neither can a libscg application use command
>>queuing, nor should it expect to succeed when transmitting more data
>>than allowed by scsi_maxdma(), so neither of these exceptions applies.
>
>>From reading the source:
>
> case SG_SET_RESERVED_SIZE:
> /* currently ignored, future extension */
> if (O_RDWR != (filp->f_flags & O_ACCMODE))
> return -EACCES;
> result = verify_area(VERIFY_READ, (const void *)arg, sizeof(int));
> if (result) return result;
> return 0;
> case SG_GET_RESERVED_SIZE:
> result = verify_area(VERIFY_WRITE, (void *) arg, sizeof(int));
> if (result) return result;
> put_user(sfp->fb_size, (int *)arg);
> return 0;
>
> As you see the March 99 version if sg.c implemented SG_SET_RESERVED_SIZE as NO-OP.
I don't know which sg version this file corresponds to. I only have the
kernel sources of 2.2.13, which contains sg.c 2.1.34 and the
corresponding linux/Documentation/scsi-generic.txt. This documentation
carries the date 990606 and the sg.c 2.1.34 does implement SG_SET_RESERVED_SIZE,
so I suspect that the sg.c that you have quoted above simply wasn't
meant to actually implement the reliable buffer reservation yet. It only
provided the new interface (correctly, as pointed out by Douglas in his
mail on this subject).
> For this reason, I mailed to the maintainer but he was either unwilling or unable
> to understand that this is unreliable code which I cannot support.
Well, it was changed later. I don't know enough about the history, but
the comment quoted above makes me think that this change was already
planed in advance.
> He believed that it would be perfect to hope that the driver will be able to
> allocate memory when it is needed. His intension was to define an interface
> that works like: "Trust me, just walk throu the wall - it will work".
>
> While this works perfectly for stepping into automounted directories,
> it will not work for the sg driver as the "space behind the wall" would have to
> be allocated from a limited resource - just in time.
Please forgive me for commenting on a conversation that I know nothing
about, but to me it seems like there never was a fundamental difference
about what had to be changed in the interface and its implementation.
Please let me summarize the controversial issues:
- Joerg wants the driver to guarantee that a command won't fail
because of insufficient memory - definitely a must for secure
CD writing. This is provided if the application plays by the rules and
does not send more than one command and only with less data than
guaranteed to be safe by SG_GET_RESERVED_SIZE.
Please correct me if I misunderstood something here...
- Raising this limit may be attempted with SG_SET_RESERVED_SIZE, but
requires SG_GET_RESERVED_SIZE to get the result of the operation,
which according to Joerg is not "how interfaces in a POSIX enviroment
are implemented". I must admit that I don't understand this criticism:
my impression was that ioctl() could be used to accomplish all kinds
of completely different things.
Even if there is some convention about how errors are reported
(which Joerg without doubt is more familiar with than I am), then to
me the way it is done makes sense:
Assume SG_SET_RESERVED_SIZE would report an error when failing to
allocate the requested buffer instead of using a smaller one. How
would you find out how much smaller the buffer size must be for the
call to succeed? The way it is handled now perfectly matches libscg's
scsi_maxdma() - try to set a certain buffer size, then tell how much
is actually possible.
>>Or does the driver deallocate a buffer although this is not mentioned
>>in the documentation?
>
> I am not sure which version of the docomentation you are referring to. It
> has been changed many times like the interface of the sg driver.
See above for the version. Has the interface really changed that often?
I am only aware of the "old" version, 2.x with SG_GET/SET_RESERVED_SIZE
and now the most recent one in linux 2.4.x.
>>> If you would check my code thoroughly, you would have notived that
>>> this is already the case. scsi_raisedma() only affects drives that
>>> are going to be used.
>
>>Then how do you detect which drives are going to be used? You are right,
>>I haven't checked the current code well enough to answer this myself.
>
> The drive(s) that have been specified by scsi_open()
>
>>However, at some point in the past I got the impression that one SCSI
>>handle may be used to send SCSI commands to all available SCSI drives,
>>because scsibus/target/lun is not marked as private and used to find the
>>file descriptor in scsitransp.c:scsicmd() - scsi_scan.c:select_target()
>>even uses this to find all drives.
>
> You may use this feature if you open a SCSI * handle for scanning.
> This is done by using a NULL pointer for char *scsidev.
> An empty string should work too.
At the moment I call scsi_open() with only bus, target and lun (i.e.
NULL scsidev) even if I only want to use one drive, because that's the
only information I have. The Amiga scsi.device identifies drives this
way, so I obtain a list of available drives and their bus/target/lun
each time the emulator is started.
Do I have to build a scsidev string that repeats the bus/target/lun
values like in the cdrecord "dev=" argument, or wouldn't it be better
to accept the tuple "scsidev NULL, bus/target/lun != -1" as another way
to open a SCSI handle for just one drive?
Should I rather try to obtain the (not necessarily) Linux device name?
If yes, how can this be done without asking the user or making the
application OS specific?
>> I hope you
>>understand that everyone else can only guess about how to use the
>>available functions or try to figure it out from the implementation(s),
>>which is difficult and probably won't result in a complete or correct
>>picture.
>
> People who are interested could ask. Heiko did ask....
Oh, I do ask right now, too ;-) I just don't know if I ask the right
questions and I wouldn't expect you to answer the same questions over
and over again once libscg gets more wide-spread usage...
>>> NO. The current implementation is not clean.
>
>>> It may be that I will have to forbid raisedma() for pre-2.4 kernels
>>> if it turnes out that raising the dma limit is only safe with 2.4
>>> kernels. In this case, I will remove the current hacky code and revert
>>> to the safe implementation that uses the stack variable only.
>
>>> This is possible as the 2.4 kernels introduce an ioctl() implementation
>>> that does not have limitations on the DMA address anymore.
>>> It is not needed to copy data in user space anymore and there is no need
>>> for a copy buffer of sufficient size.
>
>>Which address restriction are you referring to here? As far as I
>>understand it the data has to follow the SCSI header and both must be
>>transfered in one read/write() call, but I don't see why allocating
>>a buffer to prepend the data with the header dynamically should be
>>dangerous.
>
> I don't understand what you mean. If you believe that I should force
> the application to reserve space before the DMA buffer, then you are
> on the wrong way. Why should I force my application to do silly things
> just for the sake of one ill defined OS interface?
No, of course not, and that wasn't what I meant. You said "the 2.4 kernels
introduce an ioctl() implementation that does not have limitations on the
DMA address anymore". What I wanted to know was: are you talking about
a limitation different from the one that the data has to follow the
header directly?
I am asking because I (still) don't understand why you called "the
implementation that uses the stack variable only" "safe". I see that
allocating (and remembering) a buffer dynamically is more complex, but
it should be possible to do it correctly, and it is the only way to
support varying chunk sizes. My comment about increasing the size
of the stack variable wasn't meant to show a solution, but
rather the problem. I agree, one cannot simply raise the limit because
as you said the system might not support very large stack variables,
and this would still impose an additional constraint.
> My imterpretation of the problem is: If Linux people like to have
> the SCSI CDB before the data, then they decided to have a non-portable
> interface or they accept that applications on Linux will be slower
> because the data needs to be copied.
I agree, this interface is cumbersome, but it is so old that you really
cannot blame Douglas for it, and staying compatible with old code is
a reason not to throw it away. IMO even the original author shouldn't be
blamed for trying to improve the kernel, even if the result was
probably not up to the expectations.
Bye, Patrick Ohly
--
[EMAIL PROTECTED] (new permanent address)
[EMAIL PROTECTED] (MakeCD related mails)
http://home.pages.de/~Ohly/
http://makecd.core.de/ (MakeCD home page)
--
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]
