2003-01-10 13:00:26

by Andries E. Brouwer

[permalink] [raw]
Subject: sd_read_cache_type

Last year I wrote half a dozen drivers for various USB card readers.
Some don't work anymore with 2.5.recent.
I just investigated one. The reason it stopped working is the
sd_read_cache_type()
call added in 2.5.41. (With that call removed it works again.)

Will look a bit more at the details later.
For now a question: this call does a MODE_SENSE with the DBD
(disable block descriptors) bit set. Is there a reason for that?
Wouldn't the same code work in the same way without that bit?

And the reason I ask is that we already have sd_do_mode_sense6(),
so part of sd_read_cache_type() can be simply replaced by a call
of sd_do_mode_sense6(), but the latter needs an extra parameter
if DBD is really needed.

And a second question: sd_read_cache_type() is called also
when no medium is present. Objections against only calling
when media are present?

Andries


2003-01-10 15:05:35

by James Bottomley

[permalink] [raw]
Subject: Re: sd_read_cache_type

[email protected] said:
> Will look a bit more at the details later. For now a question: this
> call does a MODE_SENSE with the DBD (disable block descriptors) bit
> set. Is there a reason for that? Wouldn't the same code work in the
> same way without that bit?

The reason was just least line of resistance. The code doesn't use the block
descriptors, so there's no need to ask for them. The code is written to skip
over the block descriptors, just in case the device returns them, so setting
DBD to zero should have no practical effect.

[email protected] said:
> And the reason I ask is that we already have sd_do_mode_sense6(), so
> part of sd_read_cache_type() can be simply replaced by a call of
> sd_do_mode_sense6(), but the latter needs an extra parameter if DBD is
> really needed.

I agree with replacing the functionality with this call. On general
principle, the DBD bit (and any other missing pieces) should be added to
mode_sense6 just to make it as useful as possible (and, I suppose, the call
should be moved into scsi_lib since more than just sd could make use of it).

> And a second question: sd_read_cache_type() is called also when no
> medium is present. Objections against only calling when media are
> present?

Well, the cache is pretty often part of the permanent assembly, not part of
the removable medium, so I think it should still be called for removable
media. That begs the question, of course, what should the cache type be---it
strikes me as rather unsafe to have a removable RW medium with a write back
cache? I suppose at the very least we should to a SYNCHRONIZE on ejection if
it's write back?

James


2003-01-10 17:41:46

by Cress, Andrew R

[permalink] [raw]
Subject: RE: sd_read_cache_type


RE: cache
Yes, at least a synchronize, but can we always know (in time) if the medium
has been removed? We may not always get an eject request, right?
I think write-back cache is inherently unsafe in general, but I guess we
have to allow unsafe things like that for non-production use.

Andy

-----Original Message-----
From: James Bottomley
Sent: Friday, January 10, 2003 10:14 AM
[...]

Well, the cache is pretty often part of the permanent assembly, not part of
the removable medium, so I think it should still be called for removable
media. That begs the question, of course, what should the cache type be---it

strikes me as rather unsafe to have a removable RW medium with a write back
cache? I suppose at the very least we should to a SYNCHRONIZE on ejection
if
it's write back?

James
[...]